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

[Santhosh Kumar K]

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@xxxxxx> wrote:
>
> > On 05/02/26 23:09, Miquel Raynal wrote:
> > > On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@xxxxxx> 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@xxxxxx>
> > > > ---
> > > >    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/