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

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]

On Wed Feb 18, 2026 at 7:07 PM CET, Santhosh Kumar K wrote:
> Hello Michael and Miquel,
>
> On 12/02/26 18:25, Miquel Raynal wrote:
>> Hello,
>> 
>>>>>> +	for_each_child_of_node(partition_np, part_np) {
>>>>>> +		if (of_property_read_string(part_np, "label", &label) ||
>>>>>> +		    !strstr(label, "phypattern"))
>>>>>> +			continue;
>>>>>
>>>>> There was already a review comment on the last version. Moving this
>>>>> into the driver doesn't make it any better. In fact this might
>>>>> create a (bad) precedent for future drivers.
>>>>
>>>> I remember complaining about it but not if there was a solution
>>>> foreseen. In SPI NAND the solution has been found: the pattern is in the
>>>> driver and we load it into cache before PHY tuning. But for SPI NOR I
>>>> understood this wasn't possible. What would be an alternative?
>>>
>>> I'm not complaining about using a partition for the pattern but
>>> about the hardcoded name of it.
>>>
>>> It was proposed to use at least a device tree phandle to point to a
>>> partition (or so).
>> 
>> Ah, yes indeed, thanks for clarifying this up (again) for me. I also
>> agree the hardcoded name is not ideal.
>
> I remember this was discussed in the previous version. As mentioned
> in v1, using a phandle may not be ideal since a single controller can
> be associated with multiple flashes. Regarding the suggestion to
> maintain an array of phandles - consider a configuration with three
> flashes (NAND, NOR, and another NAND). In such a case, we would not
> need a phandle for the NAND devices, right?

Miquel said:
 | I find pretty strange to have this property in the flash node,
 | even though I understand the reason. Perhaps an array of phandles
 | may work in the controller node instead?  So maybe the phandle
 | should be inside the flash node?

But why is this strange? Can't we treat it as some kind of hint for
the controller, esp. because as we now learned, that it's not needed
for NAND flashes?

I.e. for phy tuning to work you'll have either:
 - nand flash, there it just works out of the box
 - nor flash, you'll need a phy-calibration-pattern-hint property in
   the flash node pointing to a flash partition.

If you think about it, a flash partition with a fixed name is also
part of the flash node. You just add one more indirection to get rid
of the hardcoded name and have a proper DT ABI.

> Also, I'm trying to understand the practical difference between using
> the partition name versus a phandle. Since the phandle would still be
> named something like "phy_partition", it seems functionally similar.
> Please let me know if I'm missing something here.

A phandle will be part of the DT ABI. And it will be configurable by
the user.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]