Re: [RFC PATCH 01/10] spi: spi-mem: Introduce support for tuning controller

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

On 14/08/25 18:04, Mark Brown wrote:
> On Thu, Aug 14, 2025 at 05:04:33PM +0530, Santhosh Kumar K wrote:
>> On 14/08/25 01:56, Mark Brown wrote:
> 
>>> Should we have something that blocks these tuning required modes without
>>> the appropriate tuning, and/or allows discovery of which modes require
>>> this tuning?  This all feels very landmineish - client drivers just have
>>> to know when tuning is required.
> 
>> The flash's maximum operating frequency determines whether PHY tuning is
>> required, as we need tuning in case of Cadence controller for frequencies
>> over 50 MHz.
> 
> That's entirely specific to the Candence controller from the sounds of
> it, that makes it hard to write a client driver if you need to know
> exactly what the controller you're dealing with is and what it's
> requirements are.

PHY tuning is not very specific to the Cadence controller; this has been 
added for other controllers as well. [1] - [3]

spi_mem simply verifies the execute_tuning hook within the controller's 
mem_ops and invokes it if it exists, and the tuning implementation is 
entirely controller-dependent - ranging from straightforward parameter 
configuration of PHY registers to advanced tuning algorithms such as the 
one implemented in this tuning series.

Currently, spi_mem_execute_tuning() is called by default from flash. In 
the future, this could be improved by asking the controller if tuning is 
actually needed (considering different factors such as frequency), 
similar to *_get_tuning_params implementation. Let me know your opinion 
in this.

The get_tuning_params and execute_tuning hooks in spi_mem can also be 
utilized by any non-MTD spi-mem users.

> 
>> And we do check for this condition - see Patch 07/10,
>> cqspi_phy_op_eligible_sdr(), which currently verifies the flash frequency
>> against 166 MHz. This logic can be improved by implementing both min and max
>> frequency checks, will update in the following version.
> 
> I can't actually tell how that verifies if the tuning has been done
> appropriately TBH, at least not without more effort than I'd care to

The *_execute_tuning function takes the read_op as an argument from 
flash, and considering flash continues to utilize the same read_op and 
frequency, it should make sure the tuning is appropriately completed. In 
the Cadence controller, the tuning process is validated by performing a 
read-back of a pre-defined tuning pattern using the read_op provided by 
flash.

> (and the tuning only gets added in patch 10?).

Patches 7 and 8 add PHY read/write support, and patch 9 adds tuning. 
These three patches could be squashed into one, but kept them separate 
to make it more granular for the reviewers.

[1] https://lore.kernel.org/linux-spi/20220509175616.1089346-1-clg@kaod.org/
[2] https://lore.kernel.org/all/20230322090451.3179431-2-haibo.chen@nxp.com/
[3] 
https://lore.kernel.org/all/20241128174316.3209354-1-csokas.bence@prolan.hu/