Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure

[Santhosh Kumar K <s-k6@ti.com>]

Hello Miquel,

On 13/02/26 13:48, Miquel Raynal wrote:
> On 07/02/2026 at 00:55:49 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> 
>> On 05/02/26 23:09, Miquel Raynal wrote:
>>> On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>>>
>>>> Implement the spi_controller_mem_ops execute_tuning callback to enable
>>>> PHY tuning support for the Cadence controller. PHY tuning optimizes data
>>>> capture timing at high frequencies by calibrating the read data capture
>>>> delay through the controller's PHY interface.
>>>>
>>>> Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
>>>> cqspi_phy_pre/post_config) are placeholders to be implemented
>>>> in subsequent commits.
>>>>
>>>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>>>> ---
>>>>    drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
>>>>    1 file changed, 241 insertions(+)
>>>>
>>>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>>>> index 0df286d24256..b8b0e85f4f68 100644
>>>> --- a/drivers/spi/spi-cadence-quadspi.c
>>>> +++ b/drivers/spi/spi-cadence-quadspi.c
>>>> @@ -32,6 +32,7 @@
>>>>      #define CQSPI_NAME			"cadence-qspi"
>>>>    #define CQSPI_MAX_CHIPSELECT		4
>>>> +#define CQSPI_AM654_NON_PHY_CLK_RATE	25000000
>>>>      static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>>>>    @@ -65,6 +66,7 @@ struct cqspi_st;
>>>>    struct cqspi_flash_pdata {
>>>>    	struct cqspi_st	*cqspi;
>>>>    	u32		clk_rate;
>>>> +	u32		non_phy_clk_rate;
>>> This is the second (and last) main issue I have with the series as it
>>> is
>>> right now. We cannot set this type of frequency in the driver IMO, it is
>>> too board specific.
>>> We currently have a DT property for the SPI maximum supported
>>> frequency. I believe this is no longer enough. Why not making this
>>> frequency property an array? First frequency would be the default,
>>> non tuned maximum frequency. The second would be the maximum frequency
>>> reachable when tuning the PHY.
>>
>> If the concern is only about where this is set, we could introduce a DT
>> property such as "non-phy-max-freq" to carry this information. This
>> would allow us to avoid any changes to the existing "spi-max-frequency"
>> handling. Let me know your thoughts on this.
> 
> Naming is difficult, non-phy-max-freq is too TI specific. I was
> proposing the evolution of spi-max-frequency because it is backward
> compatible. The naming can be discussed after you send a proposal, but
> do not include "non-phy" in it. It shall reflect the fact that with fine
> tuning we can reach higher frequencies on certain operations.

I tried your suggestion of keeping an array of frequencies in
spi-max-frequency:

spi-max-frequency = <25000000 166000000>;
                   (non_phy_freq phy_freq)

and updating max_speed_hz with phy_freq once tuning succeeds.

Bad news! this doesn't seem to work as we expected. The
read_op->max_freq for both NOR and NAND is initially set to
non_phy_freq, and it does not appear to be updated again by
adjust_op_freq() after tuning completes as the if case fails.

Regards,
Santhosh.

> 
> Mark, any take on this?
> 
>> I'll also test the approach you suggested and share my inputs based on
>> the results. By the way, where are you insisting to adjust/switch to
>> the maximum frequency - within the controller driver or in the
>> spi-core?
> 
> It is preferable to make the decisions in the core and avoid being smart
> in controller drivers, if possible.
> 
> Thanks,
> Miquèl
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/