diff mbox series

[v2,1/2] spi: sun6i: Honor CS setup delay on the first transfer with native CS

Message ID 20260423174001.2797797-2-km@kevinmehall.net (mailing list archive)
State New
Headers
Series spi: sun6i: Fix chip select handling around autosuspend |

Commit Message

Kevin Mehall April 23, 2026, 5:40 p.m. UTC 
Move SUN6I_TFR_CTL_CS_MANUAL to sun6i_spi_set_cs.

The CS_MANUAL bit is required for CS_LEVEL to affect the CS pin state.
Set it in the same place as other CS bits to ensure that set_cs takes
effect immediately, and to make it easier to reason about CS behavior.

Previously, this bit was not set until the first transfer's
sun6i_spi_transfer_one. That meant that on the first transfer, set_cs
would have no immediate effect, and the CS falling edge was deferred
until the bit is set in transfer_one. As any configured cs_setup delay
happens between those two steps, the configured delay would have
effectively been ignored on the very first transfer. This change makes
the first transfer work like subsequent ones.

Link: https://lore.kernel.org/linux-spi/d199f72a-093b-41bb-b33e-b6685563f704@app.fastmail.com/
Fixes: 3558fe900e8a ("spi: sunxi: Add Allwinner A31 SPI controller driver")
Signed-off-by: Kevin Mehall <km@kevinmehall.net>
---
 drivers/spi/spi-sun6i.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Kevin Mehall April 23, 2026, 6:40 p.m. UTC | #1 
I realized this is incorrect. With a GPIO chip select, sun6i_spi_set_cs() is not called,
thus CS_MANUAL would not be set, and the hardware would then automatically
assert native CS 0 during the transfer. My testing was with native CS 1 and a second GPIO chip select,
so I didn't see this because native CS 0 is not used on the Orange Pi Zero 3.

Therefore, I think the best place to set CS_MANUAL is in sun6i_spi_prepare_message
as I had in my original patch [1], unless you think it should be duplicated in both places
or have a better suggestion of where to set it. sun6i_spi_prepare_message() is still called
before set_cs(), so it still fixes the skipped delay.

[1]: https://lore.kernel.org/linux-spi/20260420164755.1131645-1-km@kevinmehall.net/
Mark Brown April 23, 2026, 6:50 p.m. UTC | #2 
On Thu, Apr 23, 2026 at 12:40:34PM -0600, Kevin Mehall wrote:
> I realized this is incorrect. With a GPIO chip select, sun6i_spi_set_cs() is not called,
> thus CS_MANUAL would not be set, and the hardware would then automatically
> assert native CS 0 during the transfer. My testing was with native CS 1 and a second GPIO chip select,
> so I didn't see this because native CS 0 is not used on the Orange Pi Zero 3.

> Therefore, I think the best place to set CS_MANUAL is in sun6i_spi_prepare_message
> as I had in my original patch [1], unless you think it should be duplicated in both places
> or have a better suggestion of where to set it. sun6i_spi_prepare_message() is still called
> before set_cs(), so it still fixes the skipped delay.

That sounds about right, the other option would be
SPI_CONTROLLER_GPIO_SS but I don't think this controller needs it.
diff mbox series

Patch

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 240e46f84f7b..fc228574ed38 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -185,6 +185,10 @@  static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
 	u32 reg;
 
 	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+
+	/* SUN6I_TFR_CTL_CS_LEVEL sets CS rather than the controller doing it automatically */
+	reg |= SUN6I_TFR_CTL_CS_MANUAL;
+
 	reg &= ~SUN6I_TFR_CTL_CS_MASK;
 	reg |= SUN6I_TFR_CTL_CS(spi_get_chipselect(spi, 0));
 
@@ -364,9 +368,6 @@  static int sun6i_spi_transfer_one(struct spi_controller *host,
 		reg |= SUN6I_TFR_CTL_DHB;
 	}
 
-	/* We want to control the chip select manually */
-	reg |= SUN6I_TFR_CTL_CS_MANUAL;
-
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
 
 	if (sspi->cfg->has_clk_ctl) {