Re: [PATCH v9 07/18] spi: cadence-quadspi: add DQS support to read data capture

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a `dqs` parameter to `cqspi_readdata_capture()` but hardcodes it to `false` in `cqspi_configure()`, unconditionally clearing the hardware DQS bit and breaking platforms that rely on the bootloader to enable it.
--

commit e47fa544a8e52f8469fd8ae3d4eeedbed9dc8a9e
Author: Santhosh Kumar K <s-k6@ti.com>

spi: cadence-quadspi: add DQS support to read data capture

This commit adds a DQS (Data Strobe) parameter to cqspi_readdata_capture()
to control data capture timing. DQS mode uses a dedicated strobe signal for
improved timing margins in high-speed SPI modes.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index f8e2663723677..046fa5d69b0f7 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
[ ... ]
> @@ -476,6 +477,11 @@ static void cqspi_readdata_capture(struct cqspi_st *cqspi, const bool bypass,
>  	reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
>  	       << CQSPI_REG_READCAPTURE_DELAY_LSB;
>  
> +	if (dqs)
> +		reg |= BIT(CQSPI_REG_READCAPTURE_DQS_LSB);
> +	else
> +		reg &= ~BIT(CQSPI_REG_READCAPTURE_DQS_LSB);
> +

[Severity: High]
Does explicitly clearing the DQS bit here destroy hardware state established
by the bootloader?

Previously, this function used a read-modify-write operation that preserved
the existing DQS bit. Platforms that don't utilize the new kernel PHY tuning
infrastructure, but rely on the bootloader to enable DQS, might now
experience read failures and data corruption in high-speed SPI modes when
this state is lost.

>  	writel(reg, reg_base + CQSPI_REG_READCAPTURE);
>  }
>  
> @@ -1317,7 +1323,7 @@ static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
>  		cqspi->sclk = sclk;
>  		cqspi_config_baudrate_div(cqspi);
>  		cqspi_delay(f_pdata);
> -		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
> +		cqspi_readdata_capture(cqspi, !cqspi->rclk_en, false,
>  				       f_pdata->read_delay);
>  	}

[Severity: High]
Does hardcoding false here unconditionally disable DQS on every configuration
cycle?

Because cqspi_configure() is executed unconditionally on any chip-select or
clock frequency switch, this passes false to cqspi_readdata_capture(). This
will unconditionally clear the hardware DQS bit across all Cadence QSPI
controllers, leading to regressions for boards that require it to remain
enabled.

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