Re: [PATCH v3 07/13] 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 unconditionally clears the DQS (Data Strobe) bit, introducing a hardware regression for platforms relying on bootloader configuration.
--

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

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

This commit introduces a DQS (Data Strobe) parameter to the
cqspi_readdata_capture() function. It updates the register configuration
to explicitly set or clear the DQS bit based on the provided boolean flag.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 0cc0d648d78db..f4efb92c9446a 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c

[ ... ]

> @@ -472,6 +473,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]
Could this cause a regression for platforms that rely on the bootloader to
configure the DQS setup? Prior to this change, the read-modify-write
operation implicitly preserved this bit.

Because the caller below is updated to pass false unconditionally, explicitly
clearing the bit here might lead to a loss of timing margins or immediate
read failures on platforms depending on the inherited hardware state for
high-speed DTR modes.

> +
>  	writel(reg, reg_base + CQSPI_REG_READCAPTURE);
>  }
>  
> @@ -1313,7 +1319,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,

[Severity: High]
Does hardcoding this argument to false break bisectability? Platforms lacking
a device tree reset line that need DQS might fail here until subsequent
patches in the series restore proper tuning configurations.

>  				       f_pdata->read_delay);
>  	}

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