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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

>>>> 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.

Yes, none of the core parts are ready for this, we may need extra logic
to handle this gracefully. But with such an option, once tuning has
happened, the core could use the correct frequency for each operation?

Thanks,
Miquèl