diff mbox series

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

Message ID 20260902-spi-sun4i-fixes-v1-5-19985ef75673@pengutronix.de (mailing list archive)
State New
Headers
Series spi: spi-sun4i: assorted fixes |

Commit Message

Jonas Rebmann Sept. 2, 2026, 3:35 p.m. UTC 
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(+)

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 3:43 p.m. UTC | #1 
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);
diff mbox series

Patch

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