PHY tuning calibrates timing delays for specific read and write commands
at high frequency. Using this frequency for uncalibrated operations
causes timing violations.

Apply PHY frequency only to operations that were tuned. All other
operations use the lower non-PHY frequency.

Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 33 ++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 930ea094f6d8..3669936ae4e1 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1532,13 +1532,44 @@ static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
 	return cqspi_indirect_read_execute(f_pdata, buf, from, len);
 }
 
+/*
+ * Check if operation exactly matches the tuned operations.
+ */
+static bool cqspi_op_matches_tuned(const struct spi_mem_op *op,
+				   const struct spi_mem_op *tuned_op)
+{
+	return op->cmd.opcode == tuned_op->cmd.opcode &&
+	       op->cmd.buswidth == tuned_op->cmd.buswidth &&
+	       op->cmd.dtr == tuned_op->cmd.dtr &&
+	       op->addr.buswidth == tuned_op->addr.buswidth &&
+	       op->addr.dtr == tuned_op->addr.dtr &&
+	       op->data.buswidth == tuned_op->data.buswidth &&
+	       op->data.dtr == tuned_op->data.dtr;
+}
+
 static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct cqspi_st *cqspi = spi_controller_get_devdata(mem->spi->controller);
 	struct cqspi_flash_pdata *f_pdata;
 
 	f_pdata = &cqspi->f_pdata[spi_get_chipselect(mem->spi, 0)];
-	cqspi_configure(f_pdata, op->max_freq);
+
+	/*
+	 * PHY tuning allows high-frequency operation only for calibrated
+	 * commands. Uncalibrated operations use safe non-PHY frequency to
+	 * avoid timing violations.
+	 */
+	if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
+	    (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
+	     cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
+		cqspi_configure(f_pdata, op->max_freq);
+	} else if (cqspi->ddata->execute_tuning) {
+		/* Use safe frequency for untuned operations */
+		cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
+	} else {
+		/* No tuning support, always use requested frequency */
+		cqspi_configure(f_pdata, op->max_freq);
+	}
 
 	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
 	/*
-- 
2.34.1

Hi Santhosh,

> +	/*
> +	 * PHY tuning allows high-frequency operation only for calibrated
> +	 * commands. Uncalibrated operations use safe non-PHY frequency to
> +	 * avoid timing violations.
> +	 */
> +	if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
> +	    (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
> +	     cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
> +		cqspi_configure(f_pdata, op->max_freq);
> +	} else if (cqspi->ddata->execute_tuning) {
> +		/* Use safe frequency for untuned operations */
> +		cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
> +	} else {
> +		/* No tuning support, always use requested frequency */
> +		cqspi_configure(f_pdata, op->max_freq);
> +	}

Shouldn't we handle this at the core level? We know what kind of
operation pattern we provided, so it is easy to set the correct
frequency in the operation structure.

Can you please make this happen? Perhaps you can return the operation
frequency once the calibration is successful (in the read and write op
templates maybe?) so this can be picked up by the core and used for the
following operations. This way the controller driver no longer needs to
check if the operation has been tuned or not, it can just look at the
frequency. When using the highest frequency, PHY tuning must be
used/enabled, otherwise not.

Thanks,
Miquèl

On 05/02/26 23:17, Miquel Raynal wrote:
> Hi Santhosh,
> 
>> +	/*
>> +	 * PHY tuning allows high-frequency operation only for calibrated
>> +	 * commands. Uncalibrated operations use safe non-PHY frequency to
>> +	 * avoid timing violations.
>> +	 */
>> +	if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
>> +	    (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
>> +	     cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
>> +		cqspi_configure(f_pdata, op->max_freq);
>> +	} else if (cqspi->ddata->execute_tuning) {
>> +		/* Use safe frequency for untuned operations */
>> +		cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
>> +	} else {
>> +		/* No tuning support, always use requested frequency */
>> +		cqspi_configure(f_pdata, op->max_freq);
>> +	}
> 
> Shouldn't we handle this at the core level? We know what kind of
> operation pattern we provided, so it is easy to set the correct
> frequency in the operation structure.
> 
> Can you please make this happen? Perhaps you can return the operation
> frequency once the calibration is successful (in the read and write op
> templates maybe?) so this can be picked up by the core and used for the
> following operations. This way the controller driver no longer needs to
> check if the operation has been tuned or not, it can just look at the
> frequency. When using the highest frequency, PHY tuning must be
> used/enabled, otherwise not.

No, Miquel, this may not be correct. There can be cases where an
operation does not require tuning but still can run at maximum
frequency (166 MHz, for instance). In such scenarios, simply setting
op->max_freq to the maximum frequency value and deciding whether to
enable tuning based on an op->max_freq comparison would not be
sufficient.

Regards,
Santhosh.

> 
> Thanks,
> Miquèl
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/