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.

Walk the controller's device tree children before SPI_COMMAND1 is
written and set CS_POL_INACTIVE for each valid chip select without
spi-cs-high. That way the line is already deasserted when the first
child transfers and does not change level when setup runs.

Disabled children are walked too. The core only registers available
children, so tegra_spi_setup() never runs for a disabled one and probe
is the only chance to park its chip select at the level its hardware
needs.

Fixes: f333a331adfa ("spi/tegra114: add spi driver")
Assisted-by: Claude:Opus-5
Signed-off-by: Abraham Zukor <abe@maticrobots.com>
---
v3:
- Drop the cs-gpios check; programming an unused native chip select does
  not affect a device using a GPIO chip select.
- Split the compound condition into separate checks for readability.
- Retest on a Jetson Orin Nano with CS1 disabled. The old module left CS1
  asserted, while the patched module parked both chip selects inactive.

v2:
- Derive the inactive level per chip select from the device tree instead
  of setting the whole CS_POL_INACTIVE field, which is incorrect for an
  active-high or mixed-polarity bus.

drivers/spi/spi-tegra114.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index aa44ffd09e61..463bb7e61b6e 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -1395,6 +1395,32 @@ static int tegra_spi_probe(struct platform_device *pdev)
 	udelay(2);
 	reset_control_deassert(tspi->rst);
 	tspi->def_command1_reg  = SPI_M_S;
+
+	/*
+	 * SPI_COMMAND1 is written before any child is registered, so a chip
+	 * select with CS_POL_INACTIVE clear stays asserted until
+	 * tegra_spi_setup() runs for that device. Program the level the
+	 * device tree asks for up front instead.
+	 *
+	 * Disabled children are included deliberately. The core never
+	 * registers them, so this is the only chance to park their chip
+	 * select at the level their hardware needs.
+	 */
+	for_each_child_of_node_scoped(pdev->dev.of_node, np) {
+		u32 cs;
+
+		if (of_property_read_bool(np, "spi-cs-high"))
+			continue;
+
+		if (of_property_read_u32(np, "reg", &cs))
+			continue;
+
+		if (cs >= MAX_CHIP_SELECT)
+			continue;
+
+		tspi->def_command1_reg |= SPI_CS_POL_INACTIVE(cs);
+	}
+
 	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

On 26/08/2026 01:05, 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.
> 
> Walk the controller's device tree children before SPI_COMMAND1 is
> written and set CS_POL_INACTIVE for each valid chip select without
> spi-cs-high. That way the line is already deasserted when the first
> child transfers and does not change level when setup runs.
> 
> Disabled children are walked too. The core only registers available
> children, so tegra_spi_setup() never runs for a disabled one and probe
> is the only chance to park its chip select at the level its hardware
> needs.
> 
> Fixes: f333a331adfa ("spi/tegra114: add spi driver")
> Assisted-by: Claude:Opus-5
> Signed-off-by: Abraham Zukor <abe@maticrobots.com>
> ---
> v3:
> - Drop the cs-gpios check; programming an unused native chip select does
>    not affect a device using a GPIO chip select.
> - Split the compound condition into separate checks for readability.
> - Retest on a Jetson Orin Nano with CS1 disabled. The old module left CS1
>    asserted, while the patched module parked both chip selects inactive.
> 
> v2:
> - Derive the inactive level per chip select from the device tree instead
>    of setting the whole CS_POL_INACTIVE field, which is incorrect for an
>    active-high or mixed-polarity bus.
> 
> drivers/spi/spi-tegra114.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
> index aa44ffd09e61..463bb7e61b6e 100644
> --- a/drivers/spi/spi-tegra114.c
> +++ b/drivers/spi/spi-tegra114.c
> @@ -1395,6 +1395,32 @@ static int tegra_spi_probe(struct platform_device *pdev)
>   	udelay(2);
>   	reset_control_deassert(tspi->rst);
>   	tspi->def_command1_reg  = SPI_M_S;
> +
> +	/*
> +	 * SPI_COMMAND1 is written before any child is registered, so a chip
> +	 * select with CS_POL_INACTIVE clear stays asserted until
> +	 * tegra_spi_setup() runs for that device. Program the level the
> +	 * device tree asks for up front instead.
> +	 *
> +	 * Disabled children are included deliberately. The core never
> +	 * registers them, so this is the only chance to park their chip
> +	 * select at the level their hardware needs.
> +	 */
> +	for_each_child_of_node_scoped(pdev->dev.of_node, np) {
> +		u32 cs;
> +
> +		if (of_property_read_bool(np, "spi-cs-high"))
> +			continue;
> +
> +		if (of_property_read_u32(np, "reg", &cs))
> +			continue;
> +
> +		if (cs >= MAX_CHIP_SELECT)
> +			continue;
> +
> +		tspi->def_command1_reg |= SPI_CS_POL_INACTIVE(cs);
> +	}
> +

Thanks for updating this. This looks fine to me, the only comment
I have is whether we should check and warn for any badly configured
device-trees where the same CS is used on multiple devices even if
one is disabled? It shouldn't be, but could be good to flag this.

Jon

-- 
nvpublic

On Thu, Aug 27, 2026 at 04:18:23PM +0100, Jon Hunter wrote:

> Thanks for updating this. This looks fine to me, the only comment
> I have is whether we should check and warn for any badly configured
> device-trees where the same CS is used on multiple devices even if
> one is disabled? It shouldn't be, but could be good to flag this.

I can see a system having multiple variants with the bootloader
selecting which device is active in the DT based on runtime
enumeration.  Not sure if anyone is *actually* doing that, but given the
state of the world with overlays...

On Thu, Aug 27, 2026 at 04:38:39PM +0100, Mark Brown wrote:
> I can see a system having multiple variants with the bootloader
> selecting which device is active in the DT based on runtime
> enumeration.  Not sure if anyone is *actually* doing that, but given
> the state of the world with overlays...

I think we should handle this per native chip select:

If exactly one node is enabled, use its polarity, ignoring disabled
alternatives.

If none are enabled, use the polarity specified by the disabled nodes
when they agree, including when there is only one. If they disagree, log
a warning and use the polarity of one of them, since no inactive level
can satisfy both polarities.

Multiple enabled nodes using the same CS are a configuration error that
the SPI core already checks.

My v3 needs adjusting for the first case. I will implement this as a v4
if no one sees an issue.