[PATCH 0/5] spi: spi-sun4i: assorted fixes

[PATCH 0/5] spi: spi-sun4i: assorted fixes

[Jonas Rebmann]

This is a series of patches for the spi-sun4i driver, some of which have
been sitting around for a while.

1-4 are fixes and 5 is a small patch that introduces reporting of the
effective SPI speed.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Marc Kleine-Budde (5):
      spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH
      spi: spi-sun4i: sun4i_spi_fill_fifo(): always apply A10 FIFO fill level workaround
      spi: spi-sun4i: sun4i_spi_transfer_one(): fix setting of clock rate
      spi: spi-sun4i: sun4i_spi_handler(): fix race condition between transfer completion and RX FIFO interrupt
      spi: spi-sun4i: sun4i_spi_transfer_one(): report effectively used speed_hz of transfer

 drivers/spi/spi-sun4i.c | 51 ++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 24 deletions(-)
---
base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
change-id: 20260902-spi-sun4i-fixes-1a50e880dc7b

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH 1/5] spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH

[Jonas Rebmann]

From: Marc Kleine-Budde <mkl@pengutronix.de>

In commit 6d9fe44bd73d ("spi: sun4i: fix FIFO limit"), the TX-FIFO is
filled max to SUN4I_FIFO_DEPTH - 1 (= 63) bytes to work around timeouts
observed on A10s SoCs.

Commit 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO
size") added support for transfers larger than the FIFO size. But this
commit only enabled the TX-FIFO empty interrupt for transfers larger
than the FIFO (= 64) bytes.

This breaks transfers with exactly 64 bytes: the TX-FIFO is only filled
with 63 bytes but the interrupt to refill the FIFO is not triggered. The
problem can be reproduced with the following command:

| spidev_test -D /dev/spidev0.1 -S 64 -s 20000000 -I 1
|
| [ 7797.548745] spi_master spi0: spi0.1: timeout transferring 64 bytes@20000000Hz for 110(100)ms
| [ 7797.557237] spidev spi0.1: SPI transfer failed: -110
| [ 7797.562308] spi_master spi0: failed to transfer one message from queue
| [ 7797.568936] spi_master spi0: noqueue transfer failed

To fix the problem enable the TX-FIFO interrupt if the total TX length
is larger than SUN4I_FIFO_DEPTH - 1 (= 63) bytes.

Fixes: 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO size")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-sun4i.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index d5c16392cd4d..2e2324453905 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -322,7 +322,7 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 	sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
 					 SUN4I_INT_CTL_RF_F34);
 	/* Only enable Tx FIFO interrupt if we really need it */
-	if (tx_len > SUN4I_FIFO_DEPTH)
+	if (tx_len > SUN4I_FIFO_DEPTH - 1)
 		sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);
 
 	/* Start the transfer */

-- 
2.55.0.123.gf60db8d575

Re: [PATCH 1/5] spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If the hardware reports a TX FIFO count greater than SUN4I_FIFO_DEPTH - 1 (e.g., a full FIFO of 64), sun4i_spi_fill_fifo encounters an integer underflow and enters a nearly infinite loop.
--

commit bfe90ee7eb547aed612b50ceef43a3538623eb4d
Author: Marc Kleine-Budde <mkl@pengutronix.de>

spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH

This commit fixes SPI transfers with exactly 64 bytes by enabling the
TX-FIFO empty interrupt when the transfer length is greater than
SUN4I_FIFO_DEPTH - 1. Previously, the interrupt was only enabled for
transfers strictly larger than the 64-byte FIFO, causing timeouts since
only 63 bytes are initially filled.

> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index d5c16392cd4d7..2e23244539053 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -322,7 +322,7 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  	sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
>  					 SUN4I_INT_CTL_RF_F34);
>  	/* Only enable Tx FIFO interrupt if we really need it */
> -	if (tx_len > SUN4I_FIFO_DEPTH)
> +	if (tx_len > SUN4I_FIFO_DEPTH - 1)
>  		sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);

[Severity: High]
This isn't a bug introduced by this patch, but does the hardware reporting a
TX FIFO count greater than SUN4I_FIFO_DEPTH - 1 cause an integer underflow?

If sun4i_spi_get_tx_fifo_count(sspi) returns a value >= 64 (which is
structurally possible since SUN4I_FIFO_DEPTH is 64 and the register mask is
0x7f), the unsigned subtraction in sun4i_spi_fill_fifo() underflows:

drivers/spi/spi-sun4i.c:sun4i_spi_fill_fifo() {
	cnt = SUN4I_FIFO_DEPTH - 1 - sun4i_spi_get_tx_fifo_count(sspi);

	len = min_t(int, cnt, sspi->len);

	while (len--) {
		...
}

This underflow produces 0xFFFFFFFF, which min_t(int, cnt, sspi->len) casts
to -1.

This negative length could cause the while (len--) loop to evaluate -1 as
true, turning it into an infinite loop that executes ~4 billion times and
overruns sspi->tx_buf.

This problem appears to be made more likely by the subsequent commit "spi:
spi-sun4i: sun4i_spi_fill_fifo(): always apply A10 FIFO fill level
workaround" which lowered the underflow threshold from > 64 to >= 64.

Can this out-of-bounds memory access occur if the hardware FIFO is completely
full (64 bytes) or reports spurious values?

>  
>  	/* Start the transfer */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-spi-sun4i-fixes-v1-0-19985ef75673@pengutronix.de?part=1

[PATCH 2/5] spi: spi-sun4i: sun4i_spi_fill_fifo(): always apply A10 FIFO fill level workaround

[Jonas Rebmann]

From: Marc Kleine-Budde <mkl@pengutronix.de>

In commit 6d9fe44bd73d ("spi: sun4i: fix FIFO limit"), the TX FIFO is
filled max to SUN4I_FIFO_DEPTH - 1 (= 63) bytes to work around
timeouts observed on A10s SoCs.

Commit 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO
size") added support for transfers larger than the FIFO size. This
commit did not consider the A10 workaround (limit the TX FIFO fill
size to SUN4I_FIFO_DEPTH - 1 (= 63) bytes) for refilling the TX FIFO
in the IRQ handler.

To apply the workaround independent from where sun4i_spi_fill_fifo()
is called, remove the length argument from the function and directly
take the max fill level of SUN4I_FIFO_DEPTH - 1 into account when
calculating the free space in the FIFO.

Due to the lack of HW this patch has not been tested on an A10 SoC,
but on an A20 SoC. It was not possible to reproduce the timeout on the
A20.

Fixes: 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO size")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-sun4i.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 2e2324453905..3649bcabcc9a 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -142,15 +142,21 @@ static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
 	}
 }
 
-static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
+static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi)
 {
 	u32 cnt;
+	int len;
 	u8 byte;
 
-	/* See how much data we can fit */
-	cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi);
+	/*
+	 * See how much data we can fit
+	 *
+	 * Filling the FIFO fully causes timeout for some reason
+	 * at least on spi2 on A10s
+	 */
+	cnt = SUN4I_FIFO_DEPTH - 1 - sun4i_spi_get_tx_fifo_count(sspi);
 
-	len = min3(len, (int)cnt, sspi->len);
+	len = min_t(int, cnt, sspi->len);
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
@@ -311,12 +317,8 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 	sun4i_spi_write(sspi, SUN4I_BURST_CNT_REG, SUN4I_BURST_CNT(tfr->len));
 	sun4i_spi_write(sspi, SUN4I_XMIT_CNT_REG, SUN4I_XMIT_CNT(tx_len));
 
-	/*
-	 * Fill the TX FIFO
-	 * Filling the FIFO fully causes timeout for some reason
-	 * at least on spi2 on A10s
-	 */
-	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
+	/* Fill the TX FIFO */
+	sun4i_spi_fill_fifo(sspi);
 
 	/* Enable the interrupts */
 	sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
@@ -373,7 +375,7 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
 
 	/* Transmit FIFO 3/4 empty */
 	if (status & SUN4I_INT_CTL_TF_E34) {
-		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
+		sun4i_spi_fill_fifo(sspi);
 
 		if (!sspi->len)
 			/* nothing left to transmit */

-- 
2.55.0.123.gf60db8d575

[PATCH 3/5] spi: spi-sun4i: sun4i_spi_transfer_one(): fix setting of clock rate

[Jonas Rebmann]

From: Marc Kleine-Budde <mkl@pengutronix.de>

A SPI transfer defines the _maximum_ speed of the SPI transfer. However
the driver doesn't take into account that the clock divider is always
rounded down (due to integer arithmetic). This results in a too high
clock rate for the SPI transfer.

E.g.: with an mclk_rate of 24 MHz and an SPI transfer speed of 10 MHz,
the original code calculates a reg of "0", which results in an effective
divider of "2" and a 12 MHz clock for the SPI transfer.

Use DIV_ROUND_UP() instead of a plain integer division to fix the
problem.

While there simplify the divider calculation for the CDR1 case, use
order_base_2() instead of two ilog2() calculations.

Fixes: b5f6517948cc ("spi: sunxi: Add Allwinner A10 SPI controller driver")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-sun4i.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 3649bcabcc9a..ae009d598450 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -212,7 +212,7 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 				  struct spi_transfer *tfr)
 {
 	struct sun4i_spi *sspi = spi_controller_get_devdata(host);
-	unsigned int mclk_rate, div;
+	unsigned int mclk_rate, div, div_cdr1, div_cdr2;
 	unsigned long time_left;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
@@ -296,14 +296,12 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 	 * First try CDR2, and if we can't reach the expected
 	 * frequency, fall back to CDR1.
 	 */
-	div = mclk_rate / (2 * tfr->speed_hz);
-	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
-		if (div > 0)
-			div--;
-
-		reg = SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
+	div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
+	div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
+	if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+		reg = SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
 	} else {
-		div = ilog2(mclk_rate) - ilog2(tfr->speed_hz);
+		div = min(SUN4I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
 		reg = SUN4I_CLK_CTL_CDR1(div);
 	}
 

-- 
2.55.0.123.gf60db8d575

Re: [PATCH 3/5] spi: spi-sun4i: sun4i_spi_transfer_one(): fix setting of clock rate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Off-by-one in the CDR1 clock divider calculation omits a required '- 1', resulting in an SPI clock that runs at less than half the requested speed, which deterministically causes false transfer timeouts for large payloads.
--

commit e08997f492b7dd1b9df03ec9d36b3a190b6b3325
Author: Marc Kleine-Budde <mkl@pengutronix.de>

spi: spi-sun4i: sun4i_spi_transfer_one(): fix setting of clock rate

This commit modifies sun4i_spi_transfer_one() to use DIV_ROUND_UP() for 
the clock rate calculation to prevent the SPI transfer clock from 
exceeding the requested maximum speed. It also simplifies the CDR1 
divider calculation by using order_base_2().

> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 3649bcabcc9a7..ae009d5984505 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
[ ... ]
> @@ -296,14 +296,12 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  	 * First try CDR2, and if we can't reach the expected
>  	 * frequency, fall back to CDR1.
>  	 */
> -	div = mclk_rate / (2 * tfr->speed_hz);
> -	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> -		if (div > 0)
> -			div--;
> -
> -		reg = SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
> +	div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
> +	div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
> +	if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> +		reg = SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
>  	} else {
> -		div = ilog2(mclk_rate) - ilog2(tfr->speed_hz);
> +		div = min(SUN4I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
>  		reg = SUN4I_CLK_CTL_CDR1(div);

[Severity: High]
Does this calculation in sun4i_spi_transfer_one() set the divider exponent
one higher than required?

The CDR1 formula is MOD_CLK / (2 ^ (div + 1)). By using order_base_2(div_cdr1)
without subtracting 1, the actual divider becomes 2 * 2^ceil(...), dropping
the clock speed below speed_hz / 2.

Could this break the tx_time calculation for large payloads and result in
spurious -ETIMEDOUT failures when a low speed_hz falls back to CDR1? 
Should this subtract 1 from the order_base_2() result?

>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-spi-sun4i-fixes-v1-0-19985ef75673@pengutronix.de?part=3

[PATCH 4/5] spi: spi-sun4i: sun4i_spi_handler(): fix race condition between transfer completion and RX FIFO interrupt

[Jonas Rebmann]

From: Marc Kleine-Budde <mkl@pengutronix.de>

In commit 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO
size"), support for transfers larger than the FIFO size was added.

This commit moves the draining of the RX-FIFO from
sun4i_spi_transfer_one() (after completion of the transfer) to the IRQ
handler when the IRQ "transfer complete" is handled. In addition, the
IRQ "RX-FIFO 3/4 full" is activated for all transfers.

However, this does not take into account that the RX-IRQ for transfers
that exceed 3/4 of the FIFO size is still pending after the IRQ
"transfer complete" has been processed. All interrupt sources are only
deactivated after the wait_for_completion_timeout() in
sun4i_spi_transfer_one().

This opens a race window for "RX-FIFO 3/4 full" interrupts to come.
The sequence is as follows:

| sun4i_spi_transfer_one()
|     sun4i_spi_fill_fifo()       // fill TX-FIFO with 48 bytes
|     // enable RX-FIFO 3/4 full IRQ
|     wait_for_completion_timeout();
|
| // SPI controller transfers 48 bytes
| // SPI controller issues "transfer complete" and "RX-FIFO 3/4 full" IRQ
|
| // IRQ handler start
|     sun4i_spi_handler()
|         // ACK "transfer complete" IRQ
|         sun4i_spi_drain_fifo();
|         complete();                         ----.
|         return IRQ_HANDLED;                      \
| // IRQ handler end                                \__ race
|                                                   /   window
|     // wait_for_completion_timeout() continues   /
|     // disable all IRQ sources              ----'

Avoid the race condition by disabling all interrupts when handling the
"transfer complete" IRQ and before calling complete(). Also move the
draining of the RX-FIFO back into sun4i_spi_transfer_one() where it
was before commit 196737912da5 ("spi: sun4i: Allow transfers larger than
FIFO size").

This has the added benefit of spending a little less time in the IRQ
handler.

Cc: Tobias Schramm <t.schramm@manjaro.org>
Fixes: 196737912da5 ("spi: sun4i: Allow transfers larger than FIFO size")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-sun4i.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index ae009d598450..18951c1972da 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -333,6 +333,9 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 	start = jiffies;
 	time_left = wait_for_completion_timeout(&sspi->done,
 						msecs_to_jiffies(tx_time));
+
+	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+
 	end = jiffies;
 	if (!time_left) {
 		dev_warn(&host->dev,
@@ -340,13 +343,12 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
 			 jiffies_to_msecs(end - start), tx_time);
 		ret = -ETIMEDOUT;
+		sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
 		goto out;
 	}
 
 
 out:
-	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
-
 	return ret;
 }
 
@@ -357,8 +359,7 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
 
 	/* Transfer complete */
 	if (status & SUN4I_INT_CTL_TC) {
-		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC);
-		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+		sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}

-- 
2.55.0.123.gf60db8d575

Re: [PATCH 4/5] spi: spi-sun4i: sun4i_spi_handler(): fix race condition between transfer completion and RX FIFO interrupt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Data race and out-of-bounds write due to unsafe concurrent execution of `sun4i_spi_drain_fifo()` on the timeout path.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing IRQ synchronization on the timeout error path leads to use-after-free and state corruption.
--

commit 2cab2ade096996934bf270db7137df45f943bc9b
Author: Marc Kleine-Budde <mkl@pengutronix.de>

spi: spi-sun4i: sun4i_spi_handler(): fix race condition between transfer completion and RX FIFO interrupt

This commit moves the RX-FIFO draining back to sun4i_spi_transfer_one() and
attempts to close a race window by disabling interrupts early in the IRQ
handler.

> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index ae009d5984505..18951c1972da6 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -333,6 +333,9 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  	start = jiffies;
>  	time_left = wait_for_completion_timeout(&sspi->done,
>  						msecs_to_jiffies(tx_time));
> +
> +	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
> +

[Severity: High]
Can this unconditionally drain the FIFO while interrupts are still enabled?

If wait_for_completion_timeout() times out, the main thread will call
sun4i_spi_drain_fifo(). Because SPI interrupts are not disabled until the
subsequent timeout error block, a SUN4I_INT_CTL_RF_F34 interrupt can
fire concurrently.

This allows sun4i_spi_handler() to also invoke sun4i_spi_drain_fifo()
simultaneously:

CPU1:
sun4i_spi_transfer_one()
    sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
        if (sspi->rx_buf)
            *sspi->rx_buf++ = byte;

CPU2:
sun4i_spi_handler()
    if (status & SUN4I_INT_CTL_RF_F34) {
        sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
            if (sspi->rx_buf)
                *sspi->rx_buf++ = byte;

Could this concurrent execution lead to non-atomic increments of sspi->rx_buf,
pointer corruption, and potential out-of-bounds memory writes?

>  	end = jiffies;
>  	if (!time_left) {
>  		dev_warn(&host->dev,
> @@ -340,13 +343,12 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
>  			 jiffies_to_msecs(end - start), tx_time);
>  		ret = -ETIMEDOUT;
> +		sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
>  		goto out;
>  	}
>  
>  
>  out:
> -	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
> -
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the timeout error path
need synchronization with currently executing IRQ handlers?

If an SPI transfer times out while the IRQ handler is already executing on
another CPU, the transfer function will disable interrupts at the controller
level, return -ETIMEDOUT, and the SPI core will tear down the transfer.

Since the code does not wait for in-flight handlers using synchronize_irq()
or a lock, the still-running sun4i_spi_handler() can subsequently write to
freed buffers in sun4i_spi_drain_fifo(), or call complete(&sspi->done)
which corrupts the state of the next transfer.

[ ... ]

> @@ -357,8 +359,7 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
>  
>  	/* Transfer complete */
>  	if (status & SUN4I_INT_CTL_TC) {
> -		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC);
> -		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
> +		sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
>  		complete(&sspi->done);
>  		return IRQ_HANDLED;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-spi-sun4i-fixes-v1-0-19985ef75673@pengutronix.de?part=4

[PATCH 5/5] spi: spi-sun4i: sun4i_spi_transfer_one(): report effectively used speed_hz of transfer

[Jonas Rebmann]

From: Marc Kleine-Budde <mkl@pengutronix.de>

Report the effectively used speed_hz for the transfer by setting
tfr->effective_speed_hz.

For more information see commit 5d7e2b5ed585 ("spi: core: allow
reporting the effectivly used speed_hz for a transfer"), which adds this
feature to the SPI core.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-sun4i.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 18951c1972da..fff0065ce9af 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -300,9 +300,11 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
 	div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
 	if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
 		reg = SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
+		tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);
 	} else {
 		div = min(SUN4I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
 		reg = SUN4I_CLK_CTL_CDR1(div);
+		tfr->effective_speed_hz = mclk_rate / (1 << div);
 	}
 
 	sun4i_spi_write(sspi, SUN4I_CLK_CTL_REG, reg);

-- 
2.55.0.123.gf60db8d575

Re: [PATCH 5/5] spi: spi-sun4i: sun4i_spi_transfer_one(): report effectively used speed_hz of transfer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Kernel panic due to division by zero when calculating `effective_speed_hz` if `mclk_rate` evaluates to 0.
--

commit 035881eeeccd1fd72487c3c0c263d972c7662e72
Author: Marc Kleine-Budde <mkl@pengutronix.de>

spi: spi-sun4i: sun4i_spi_transfer_one(): report effectively used speed_hz of transfer

This commit updates the sun4i SPI driver to report the effectively used
transfer speed by setting tfr->effective_speed_hz. It calculates this
based on the chosen clock divider (CDR1 or CDR2).

> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 18951c1972da6..fff0065ce9af9 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
[ ... ]
> @@ -300,9 +300,11 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  	div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
>  	if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>  		reg = SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
> +		tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);

[Severity: High]
Could this result in a division by zero kernel panic if the clock rate
fails to initialize or is stubbed?

If clk_get_rate(sspi->mclk) returns 0 (which can happen if the clock is
missing or fails to set a rate), the calculation for div_cdr1 becomes:
div_cdr1 = DIV_ROUND_UP(0, tfr->speed_hz);

This evaluates to 0, which then makes div_cdr2 evaluate to 0:
div_cdr2 = DIV_ROUND_UP(0, 2);

Since 0 is less than or equal to SUN4I_CLK_CTL_CDR2_MASK + 1, this
branch is taken. The new code then evaluates to:
tfr->effective_speed_hz = 0 / (2 * 0);

Would it be safer to check if div_cdr2 is non-zero before performing
this division, or perhaps handle a 0 mclk_rate earlier in the function?

>  	} else {
>  		div = min(SUN4I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
>  		reg = SUN4I_CLK_CTL_CDR1(div);
> +		tfr->effective_speed_hz = mclk_rate / (1 << div);
>  	}
>  
>  	sun4i_spi_write(sspi, SUN4I_CLK_CTL_REG, reg);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-spi-sun4i-fixes-v1-0-19985ef75673@pengutronix.de?part=5