Re: [PATCH v9 12/18] spi: cadence-quadspi: enable PHY for indirect writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing verification of the current operation frequency (`op->max_freq`) against the calibrated PHY frequency, potentially causing the PHY to be incorrectly enabled for uncalibrated writes.
--

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

spi: cadence-quadspi: enable PHY for indirect writes

This commit enables PHY for indirect writes of at least 1KB, restricting
PHY writes to NAND devices where a write op template was provided. It also
cancels the indirect write before disabling PHY in the error path to avoid
timeouts while the controller is still waiting for AHB data.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index d38a0253e4bda..20ece785c97a4 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c

[ ... ]

> @@ -1370,18 +1372,32 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
>  
>  static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
>  					loff_t to_addr, const u8 *txbuf,
> -					const size_t n_tx)
> +					const size_t n_tx,
> +					u32 post_config_max_speed_hz)
>  {
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	struct device *dev = &cqspi->pdev->dev;
>  	void __iomem *reg_base = cqspi->iobase;
>  	unsigned int remaining = n_tx;
>  	unsigned int write_bytes;
> +	bool use_tuned_phy_write;
>  	int ret;
>  
>  	if (!refcount_read(&cqspi->refcount))
>  		return -ENODEV;
>  
> +	/* Use PHY only for large writes at the calibrated rate */
> +	use_tuned_phy_write = n_tx >= CQSPI_PHY_MIN_INDIRECT_WRITE_LEN &&
> +			      f_pdata->use_tuned_phy &&
> +			      f_pdata->phy_write_op.max_freq ==
> +				      post_config_max_speed_hz;

[Severity: High]
Does this logic correctly handle operations where the command-specific speed
limit (op->max_freq) is lower than post_config_max_speed_hz?

If a large indirect write is executed with an op->max_freq less than
post_config_max_speed_hz, the hardware clock will be lowered to op->max_freq.
However, this condition only checks the controller's post-configuration
maximum speed against the calibrated PHY frequency, completely omitting a
check against the actual frequency of the current SPI operation.

Could this lead to the PHY being incorrectly enabled using high-speed DLL
tuning values while operating at a lower clock speed? Applying DLL delay
values calibrated for a high clock speed to a lower clock speed could cause
the PHY data transmission eye to shift, potentially leading to incorrect
sampling and silent data corruption on flash writes.

The read path explicitly validates op->max_freq via cqspi_use_tuned_phy(),
should the write path perform a similar validation before enabling the PHY?

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