Re: [PATCH v9 14/18] spi: cadence-quadspi: reprogram PHY DLL on runtime resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Disabling 'use_tuned_phy' on DLL resync failure bypasses the PHY DLL while continuing to run at high speed, causing silent data corruption.
- [Medium] Iteratively reprogramming the global PHY DLL for all chip selects on runtime resume causes redundant delays and overwrites state.
--

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

spi: cadence-quadspi: reprogram PHY DLL on runtime resume

The PHY DLL loses its programmed delay values and lock when the OSPI
controller is power-gated during runtime suspend. On resume,
cqspi_controller_init() reinitialises the controller but does not
touch the PHY DLL registers, leaving the DLL in an unlocked state.
Any subsequent PHY read or write would then fail at the cqspi_resync_dll()
call inside cqspi_tune_phy().

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 530335f002fb4..94d67e8bcfbc4 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c

[ ... ]

> @@ -4042,6 +4042,28 @@ static int cqspi_runtime_resume(struct device *dev)
>  
>  	cqspi->current_cs = -1;
>  	cqspi->sclk = 0;
> +
> +	for (cs = 0; cs < CQSPI_MAX_CHIPSELECT; cs++) {
> +		struct cqspi_flash_pdata *f_pdata = &cqspi->f_pdata[cs];
> +
> +		if (!f_pdata->use_tuned_phy)
> +			continue;
> +
> +		cqspi_phy_set_dll_master(cqspi);
> +
> +		cqspi_set_dll(cqspi->iobase, f_pdata->phy_setting.rx,
> +			      f_pdata->phy_setting.tx);
> +		ret = cqspi_resync_dll(cqspi);

[Severity: Medium]
Will this iteratively reprogram the global PHY DLL for all chip selects on
runtime resume, causing redundant delays and overwriting state?

The PHY config register (CQSPI_REG_PHY_CONFIG) is global, yet the resume loop
iterates over all chip selects, writes each CS's RX/TX taps to the global
register, and triggers a full DLL resync each time. Because only the final
CS's taps remain active in hardware, the driver's cqspi_tune_phy() function
will redundantly re-sync the DLL again on the next transfer to any other chip
select, completely negating the purpose of pre-locking them during resume.

Furthermore, master DLL lock is independent of the slave RX/TX taps.

> +		if (ret) {
> +			dev_warn(dev, "cs%d: PHY DLL resync failed on resume: %d, disabling PHY\n",
> +				 cs, ret);
> +			f_pdata->dll_locked = false;
> +			f_pdata->use_tuned_phy = false;

[Severity: Critical]
Does disabling 'use_tuned_phy' here on DLL resync failure bypass the PHY DLL
while continuing to run at high speed, causing silent data corruption?

If the PHY DLL resync fails during resume, setting 'use_tuned_phy = false'
causes subsequent flash operations to bypass the PHY data capture logic.

However, the SPI clock frequency (op->max_freq) is not downgraded and
continues running at the high PHY-tuned speed (e.g., 150-200MHz). The
controller's non-PHY fallback cannot reliably sample data at these high
frequencies, resulting in garbage data being read from or written to the
flash.

> +		} else {
> +			f_pdata->dll_locked = true;
> +		}
> +	}
> +
>  	return 0;
>  }
>

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