Re: [PATCH v6 13/17] spi: cadence-quadspi: reprogram CS timing on every chip-select switch

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Hardware timing delays are silently truncated and can overflow, leading to near-zero delays and data corruption.
--

--- Patch [13]: [PATCH v6 13/17] spi: cadence-quadspi: reprogram CS timing on every chip-select switch ---
commit cfd5fbcbe64f53f00c899f4f86da36ebc0b596a7
Author: Santhosh Kumar K <s-k6@ti.com>

spi: cadence-quadspi: reprogram CS timing on every chip-select switch

This patch updates the cadence-quadspi driver to reprogram per-device CS
timing and read-capture registers on any chip-select switch, preventing
devices on the same frequency from using stale timing parameters.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 542ad331ff167..8bf3a5a8bf43d 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1570,10 +1570,19 @@ static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
>  		cqspi_chipselect(f_pdata);
>  	}
>  
> -	/* Setup baudrate divisor and delays */
> +	/* Update baudrate only on clock change. */
>  	if (switch_ck) {
>  		cqspi->sclk = sclk;
>  		cqspi_config_baudrate_div(cqspi);
> +	}
> +
> +	/*
> +	 * Reprogram per-device CS timing and non-PHY read capture on any
> +	 * chip-select or clock switch. Without this, two devices at the same
> +	 * frequency would each see the other's timing parameters after a CS
> +	 * switch, since the lazy clock-only check would never trigger.
> +	 */
> +	if (switch_cs || switch_ck) {
>  		cqspi_delay(f_pdata);

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in the
hardware timing calculations when cqspi_delay() is called here.

If a large delay is specified in the device tree (like cdns,tshsl-ns) or
the system operates at a high reference clock, could the calculation in
calculate_ticks_for_ns() overflow a 32-bit integer?

drivers/spi/spi-cadence-quadspi.c:calculate_ticks_for_ns() {
    ...
    ticks = ref_clk_hz / 1000;
    ticks = DIV_ROUND_UP(ticks * ns_val, 1000000);
    ...
}

If the reference clock is high and the requested delay exceeds ~8.5
microseconds, the multiplication might overflow.

Additionally, in cqspi_delay(), the calculated ticks are masked rather
than clamped:

drivers/spi/spi-cadence-quadspi.c:cqspi_delay() {
    ...
    reg = (tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
           << CQSPI_REG_DELAY_TSHSL_LSB;
    reg |= (tchsh & CQSPI_REG_DELAY_TCHSH_MASK)
           << CQSPI_REG_DELAY_TCHSH_LSB;
    ...
}

If the calculated tick count exceeds 255 (the mask limit), wouldn't the
masking cause the value to wrap around to a near-zero delay instead of
clamping to the maximum allowed 255 ticks? This might lead to inadequate
timing delays and silent data corruption.

>  		cqspi_readdata_capture(cqspi, !cqspi->rclk_en, false,
>  				       f_pdata->read_delay);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723110219.3180747-1-s-k6@ti.com?part=13