Re: [PATCH] spi: tegra114: assert CS GPIO for every transfer in a message

[Nuno Sá]

On Fri, Oct 02, 2026 at 04:55:40PM +0800, Carlos Jones wrote:
> From: Carlos Jones Jr <carlosjr.jones@xxxxxxxxxx>
> 
> The driver provides its own transfer_one_message() and therefore drives
> the chip-select GPIO itself rather than through the SPI core. The GPIO is
> asserted only in the is_first_of_msg path of
> tegra_spi_setup_transfer_one(), but tegra_spi_transfer_end() deasserts it
> whenever a transfer has cs_change set. For a multi-transfer message whose
> earlier transfers request cs_change -- for example a register read that
> sends the address in one transfer and clocks the data back in the next --
> the CS GPIO is released after the first transfer and never re-asserted,
> so the remaining transfers are clocked with chip-select inactive and the
> peripheral never drives the bus.
> 
> This was observed with an ADIS16470 IMU (adis16475 driver), whose
> register read is a two-transfer, cs_change-framed exchange: a logic
> analyzer showed the data frame clocked with CS high, and the driver read
> 0 while the device drove 0x4056 on the wire.
> 
> Re-assert the CS GPIO for every transfer so that multi-transfer messages
> keep chip-select active across the whole message. gpiod_set_value() is
> idempotent and polarity-aware, so this is a no-op for the first transfer
> and for messages that keep CS asserted throughout.
> 
> Fixes: 63c1440596ff ("spi: tegra114: add support for gpio based CS")
> Signed-off-by: Carlos Jones Jr <carlosjr.jones@xxxxxxxxxx>
> ---
>  drivers/spi/spi-tegra114.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
> index 1234567..89abcde 100644
> --- a/drivers/spi/spi-tegra114.c
> +++ b/drivers/spi/spi-tegra114.c
> @@ -859,6 +859,13 @@ static u32 tegra_spi_setup_transfer_one(struct spi_device *spi,
>  		command1 |= SPI_BIT_LENGTH(bits_per_word - 1);
>  	}
> 
> +	/*
> +	 * Re-assert the CS GPIO for every transfer: transfer_end() drops it
> +	 * on cs_change, and the assert above only covers is_first_of_msg.
> +	 */
> +	if (spi_get_csgpiod(spi, 0))
> +		gpiod_set_value(spi_get_csgpiod(spi, 0), 1);
> +

Makes not much sense to me do do this on top of the `if (is_first_of_msg)`
path. I would remove it. Or just set the gpio (if no hw constrain for
it) right after:

tegra_spi_transfer_end(spi);
spi_transfer_delay_exec(xfer);

in the `else if (xfer->cs_change)`. We know it's not the last xfer.
Would be nice to avoid useless calls to gpiolib.

- Nuno Sá

>  	return command1;
>  }
> 
> --
> 2.43.0