Re: [PATCH v6 07/17] 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:
- [Medium] The patch unconditionally disables DQS by explicitly clearing bit 8 in CQSPI_REG_READCAPTURE, breaking bisectability and systems that rely on the bootloader or firmware to enable DQS.
--

--- Patch [7]: [PATCH v6 07/17] spi: cadence-quadspi: add DQS support to read data capture ---
commit 67e381a8bfc8dddc89f5ce8ea17f73b3529e597b
Author: Santhosh Kumar K <s-k6@ti.com>

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

Add 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);
> +
>  	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: Medium]
Does unconditionally passing false for the dqs parameter break platforms
that rely on the bootloader or firmware to enable the data strobe?

Previously, cqspi_readdata_capture() passively preserved the data strobe
bit state since it only masked and set the bypass and delay bits. Now that
the function explicitly clears the CQSPI_REG_READCAPTURE_DQS_LSB bit when
the dqs argument is false, this will disable the strobe on systems where it
was already configured prior to booting.

Loss of this signal reduces timing margins in high-speed modes and could
lead to read failures. Is it intended to override the bootloader configuration
here?

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