[PATCH v2 0/2] spi: sun6i: Fix chip select handling around autosuspend

[PATCH v2 0/2] spi: sun6i: Fix chip select handling around autosuspend

[Kevin Mehall]

Move the initialization of the SUN6I_TFR_CTL_REG bits out of
sun6i_spi_transfer_one() into earlier callbacks to fix two bugs:

 - With a native chip select, the setup_delay is not correctly applied to
   the first transfer after autosuspend.
 - With a GPIO chip select, the CS is asserted before SCK is driven to the
   correct initial level per the SPI mode. When the mode is set, it can
   cause an extra SCK transition with CS low and corrupt the transfer.
 
Changes since the previous single patch:
 - Move the line that sets `SUN6I_TFR_CTL_CS_MANUAL` into
  `sun6i_spi_set_cs()` rather than into `sun6i_spi_prepare_message()`.
  This change is now a separate commit.

Kevin Mehall (2):
  spi: sun6i: Honor CS setup delay on the first transfer with native CS
  spi: sun6i: Set SPI mode in prepare_message

 drivers/spi/spi-sun6i.c | 74 +++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 29 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.53.0

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

[Kevin Mehall]

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(-)

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) {
-- 
2.53.0

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

[Kevin Mehall]

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/

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 2/2] spi: sun6i: Set SPI mode in prepare_message

[Kevin Mehall]

With a GPIO chip select, CS is asserted before entering transfer_one.
The spi-sun6i driver previously configured the SPI mode (including clock
polarity) and enabled the bus in transfer_one, which can cause an
extraneous SCK transition with CS asserted, corrupting the transferred
data.

This patch moves the SPI mode configuration and bus enable to the
spi_prepare_message callback, ensuring that SCK is driven to the correct
level prior to asserting CS.

A previous fix for a related issue (0d7993b234c9f) was incomplete in
that it only delayed enabling the SCK output drive to prevent it from
being driven at the wrong level when resuming from autosuspend, but
didn't help if switching CPOL modes between chip selects while active,
or if SCK floats to the opposite level when suspended.

Fixes: 0d7993b234c9 ("spi: spi-sun6i: Fix chipselect/clock bug")
Signed-off-by: Kevin Mehall <km@kevinmehall.net>
---
 drivers/spi/spi-sun6i.c | 67 +++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index fc228574ed38..983e791e3396 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -205,6 +205,44 @@ static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
 	return SUN6I_MAX_XFER_SIZE - 1;
 }
 
+static int sun6i_spi_prepare_message(struct spi_controller *ctlr,
+				     struct spi_message *msg)
+{
+	struct sun6i_spi *sspi = spi_controller_get_devdata(ctlr);
+	struct spi_device *spi = msg->spi;
+	u32 reg;
+
+	/* Set the mode bits in the transfer control register */
+	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+
+	if (spi->mode & SPI_CPOL)
+		reg |= SUN6I_TFR_CTL_CPOL;
+	else
+		reg &= ~SUN6I_TFR_CTL_CPOL;
+
+	if (spi->mode & SPI_CPHA)
+		reg |= SUN6I_TFR_CTL_CPHA;
+	else
+		reg &= ~SUN6I_TFR_CTL_CPHA;
+
+	if (spi->mode & SPI_LSB_FIRST)
+		reg |= SUN6I_TFR_CTL_FBS;
+	else
+		reg &= ~SUN6I_TFR_CTL_FBS;
+
+	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+
+	/*
+	 * Now that the clock polarity is configured, enable the bus if the
+	 * controller was previously suspended.
+	 */
+	reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG);
+	reg |= SUN6I_GBL_CTL_BUS_ENABLE;
+	sun6i_spi_write(sspi, SUN6I_GBL_CTL_REG, reg);
+
+	return 0;
+}
+
 static void sun6i_spi_dma_rx_cb(void *param)
 {
 	struct sun6i_spi *sspi = param;
@@ -336,31 +374,12 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
 
 	sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG, reg);
 
-	/*
-	 * Setup the transfer control register: Chip Select,
-	 * polarities, etc.
-	 */
-	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
-
-	if (spi->mode & SPI_CPOL)
-		reg |= SUN6I_TFR_CTL_CPOL;
-	else
-		reg &= ~SUN6I_TFR_CTL_CPOL;
-
-	if (spi->mode & SPI_CPHA)
-		reg |= SUN6I_TFR_CTL_CPHA;
-	else
-		reg &= ~SUN6I_TFR_CTL_CPHA;
-
-	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUN6I_TFR_CTL_FBS;
-	else
-		reg &= ~SUN6I_TFR_CTL_FBS;
-
 	/*
 	 * If it's a TX only transfer, we don't want to fill the RX
 	 * FIFO with bogus data
 	 */
+	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+
 	if (sspi->rx_buf) {
 		reg &= ~SUN6I_TFR_CTL_DHB;
 		rx_len = tfr->len;
@@ -429,11 +448,6 @@ static int sun6i_spi_transfer_one(struct spi_controller *host,
 		sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
 	}
 
-	/* Finally enable the bus - doing so before might raise SCK to HIGH */
-	reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG);
-	reg |= SUN6I_GBL_CTL_BUS_ENABLE;
-	sun6i_spi_write(sspi, SUN6I_GBL_CTL_REG, reg);
-
 	/* Setup the transfer now... */
 	if (sspi->tx_buf) {
 		tx_len = tfr->len;
@@ -668,6 +682,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	host->max_speed_hz = 100 * 1000 * 1000;
 	host->min_speed_hz = 3 * 1000;
 	host->use_gpio_descriptors = true;
+	host->prepare_message = sun6i_spi_prepare_message;
 	host->set_cs = sun6i_spi_set_cs;
 	host->transfer_one = sun6i_spi_transfer_one;
 	host->num_chipselect = 4;
-- 
2.53.0