[PATCH] spi: tegra114: initialize native chip selects inactive

[PATCH] spi: tegra114: initialize native chip selects inactive

[Abraham Zukor]

tegra_spi_probe() initializes SPI_COMMAND1 with only SPI_M_S, leaving
CS_POL_INACTIVE clear for every chip select. This drives every native
active-low chip select low until tegra_spi_setup() runs for that device.

SPI children are registered and probed one at a time. A synchronous
probe of an earlier child can therefore transfer while a later child's
chip select is still asserted. On a Tegra234 system with active-low devices
on CS0 and CS1, this caused both devices to be selected when the CS1
device probed first. Its initialization then intermittently failed. A
logic analyzer showed CS0 remained low throughout the CS1 transfer and
went high only after the CS1 probe failed.

The Tegra234 TRM also documents all CS_POL_INACTIVE bits as one after
reset, so the existing probe code replaces the hardware's safe reset
polarity with zeros on that SoC.

Set CS_POL_INACTIVE for all native chip selects during controller probe.
This matches SPI's default active-low polarity and the documented
Tegra234 reset polarity. tegra_spi_setup() continues to configure the
inactive polarity of SPI_CS_HIGH devices before their drivers probe.

Fixes: f333a331adfa ("spi/tegra114: add spi driver")
Assisted-by: Codex:GPT-5
Signed-off-by: Abraham Zukor <abe@maticrobots.com>
---
 drivers/spi/spi-tegra114.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index aa44ffd09e61..c28aae093d57 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -1394,7 +1394,7 @@ static int tegra_spi_probe(struct platform_device *pdev)
 	reset_control_assert(tspi->rst);
 	udelay(2);
 	reset_control_deassert(tspi->rst);
-	tspi->def_command1_reg  = SPI_M_S;
+	tspi->def_command1_reg  = SPI_M_S | SPI_CS_POL_INACTIVE_MASK;
 	tegra_spi_writel(tspi, tspi->def_command1_reg, SPI_COMMAND1);
 	tspi->spi_cs_timing1 = tegra_spi_readl(tspi, SPI_CS_TIMING1);
 	tspi->spi_cs_timing2 = tegra_spi_readl(tspi, SPI_CS_TIMING2);
-- 
2.54.0

Re: [PATCH] spi: tegra114: initialize native chip selects inactive

[Jon Hunter]

On 25/07/2026 05:29, Abraham Zukor wrote:
> tegra_spi_probe() initializes SPI_COMMAND1 with only SPI_M_S, leaving
> CS_POL_INACTIVE clear for every chip select. This drives every native
> active-low chip select low until tegra_spi_setup() runs for that device.
> 
> SPI children are registered and probed one at a time. A synchronous
> probe of an earlier child can therefore transfer while a later child's
> chip select is still asserted. On a Tegra234 system with active-low devices
> on CS0 and CS1, this caused both devices to be selected when the CS1
> device probed first. Its initialization then intermittently failed. A
> logic analyzer showed CS0 remained low throughout the CS1 transfer and
> went high only after the CS1 probe failed.

What about the case where the SPI devices CS is active high or a mixture 
of the two?

> The Tegra234 TRM also documents all CS_POL_INACTIVE bits as one after
> reset, so the existing probe code replaces the hardware's safe reset
> polarity with zeros on that SoC.
> 
> Set CS_POL_INACTIVE for all native chip selects during controller probe.
> This matches SPI's default active-low polarity and the documented
> Tegra234 reset polarity. tegra_spi_setup() continues to configure the
> inactive polarity of SPI_CS_HIGH devices before their drivers probe.
> 
> Fixes: f333a331adfa ("spi/tegra114: add spi driver")
> Assisted-by: Codex:GPT-5
> Signed-off-by: Abraham Zukor <abe@maticrobots.com>
> ---
>   drivers/spi/spi-tegra114.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
> index aa44ffd09e61..c28aae093d57 100644
> --- a/drivers/spi/spi-tegra114.c
> +++ b/drivers/spi/spi-tegra114.c
> @@ -1394,7 +1394,7 @@ static int tegra_spi_probe(struct platform_device *pdev)
>   	reset_control_assert(tspi->rst);
>   	udelay(2);
>   	reset_control_deassert(tspi->rst);
> -	tspi->def_command1_reg  = SPI_M_S;
> +	tspi->def_command1_reg  = SPI_M_S | SPI_CS_POL_INACTIVE_MASK;
>   	tegra_spi_writel(tspi, tspi->def_command1_reg, SPI_COMMAND1);
>   	tspi->spi_cs_timing1 = tegra_spi_readl(tspi, SPI_CS_TIMING1);
>   	tspi->spi_cs_timing2 = tegra_spi_readl(tspi, SPI_CS_TIMING2);

-- 
nvpublic

Re: [PATCH] spi: tegra114: initialize native chip selects inactive

[Abraham Zukor]

On 27/07/2026 13:07, Jon Hunter wrote:
> What about the case where the SPI devices CS is active high or a
> mixture of the two?

An all-active-high bus is the case the window gets right today: probe
writes zero to the field, so an active-high CS idles deasserted and an
active-low one sits asserted. This patch inverts that. No single
constant covers a mixed board - one polarity or the other is asserted
until tegra_spi_setup() runs for that device - so the question is
which default is the right one, and active low is what the SPI core
assumes when a device says nothing.

Setting that default in probe and correcting it per device later is an
established model for controllers with a programmable CS idle level.
f9c6ef6cfe9c ("spi/xilinx: Support for spi mode CS_HIGH") added both
halves at once, to a driver that does advertise SPI_CS_HIGH:
xilinx_spi_probe() sets cs_inactive to all ones, and
xilinx_spi_setup_transfer() clears the bit for an SPI_CS_HIGH device.
The changelog gives the rationale:

  The core controls the chip select lines individually.

  By default, all the lines are consider active_low. After
  spi_setup_transfer, it has its real value.

tegra_qspi_probe() ends up in the same state by another route:
def_command1_reg = QSPI_M_S | QSPI_CS_SW_HW | QSPI_CS_SW_VAL drives
the line high in software CS mode, so its native chip select idles
deasserted before setup runs, even though its CS_POL_INACTIVE bits are
zero like ours. That is a single line on Tegra234, where cs_count is
1, so it never meets the multi-CS case, but the idle level it picks is
the active-low one.

Others leave the reset polarity alone rather than overwrite it -
sifive_spi_probe() saves and restores CSDEF, bcm63xx_hsspi_probe()
reads GLOBAL_CTRL_CS_POLARITY back into bs->cs_polarity.
tegra_spi_probe() does neither: it writes the field, and writes zeros.
The Tegra234 TRM documents all CS_POL_INACTIVE bits as one after
reset, which is the inactive level for an active-low device, so probe
is discarding a value that suits the common case for one that does
not.

Thanks,
Abe

Re: [PATCH] spi: tegra114: initialize native chip selects inactive

[Jon Hunter]

On 28/07/2026 03:53, Abraham Zukor wrote:
> On 27/07/2026 13:07, Jon Hunter wrote:
>> What about the case where the SPI devices CS is active high or a
>> mixture of the two?
> 
> An all-active-high bus is the case the window gets right today: probe
> writes zero to the field, so an active-high CS idles deasserted and an
> active-low one sits asserted. This patch inverts that. No single
> constant covers a mixed board - one polarity or the other is asserted
> until tegra_spi_setup() runs for that device - so the question is
> which default is the right one, and active low is what the SPI core
> assumes when a device says nothing.

Exactly. That's my point. Which is the right one? The answer is probably 
neither, but whatever is present on the bus.

-- 
nvpublic

Re: [PATCH] spi: tegra114: initialize native chip selects inactive

[Abraham Zukor]

On 28/07/2026 10:34, Jon Hunter wrote:
> Exactly. That's my point. Which is the right one? The answer is
> probably neither, but whatever is present on the bus.

I think that defaulting to active-low is correct. The Tegra234 SPI
controller hardware defaults to active-low (all bits in CS_POL_INACTIVE
are set) so an active-high device is already asserted before this driver
initializes. I checked the controller drivers under drivers/spi/; they
either default to active-low or preserve the hardware default.

A more complex option is to scan the device tree children, check each
item for whether it uses native chip select (as opposed to GPIO CS) and
check for spi-cs-high. I found no other spi drivers that do this at
probe time, and I don't think it's worth the complexity, but I can send
it as a v2 if needed.

Thanks,
Abe