Re: [PATCH v3 02/13] spi: dt-bindings: cdns,qspi-nor: add PHY tuning pattern partition property

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

Hello Rob and Miquel,

On 04/06/26 12:58, Miquel Raynal wrote:
> On 03/06/2026 at 12:38:46 -05, Rob Herring <robh@kernel.org> wrote:
> 
>> On Wed, Jun 3, 2026 at 11:01 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>>>
>>> Hello,
>>>
>>> On 02/06/2026 at 11:49:45 -05, Rob Herring <robh@kernel.org> wrote:
>>>
>>>> On Wed, May 27, 2026 at 11:25:16PM +0530, Santhosh Kumar K wrote:
>>>>> PHY tuning requires a known data pattern to be readable from flash.
>>>>> When no partition is explicitly identified, the controller must search
>>>>> all available partitions to locate the pattern by label, which adds
>>>>> overhead and relies on label naming conventions outside the
>>>>> controller's control.
>>>>
>>>> I agree 'label' is not the best choice. Software should not care what
>>>> 'label' contains. It should really be 'compatible' instead.
>>>
>>> But compatible does not seem relevant in this case, right? We are just
>>> flagging the location of "some useful data for the controller".
>>
>> compatible is what tells us what a region contains and how to use it.
>> That seems exactly what we need to define here.
> 
> We usually talk about "programming model" when it comes to compatible,
> here we just need to point at an offset which is in no way different
> (from a hardware standpoint) than the other offsets. I honestly feel
> like a phandle property would be simpler, also because compatibles in
> MTD are already quite complex to manage and I would prefer not to add
> more complexity into the parsing logic.
> 
>>>>> Add cdns,phy-pattern-partition, a phandle property that allows the DT
>>>>> author to directly reference the flash partition holding the PHY tuning
>>>>> pattern. The controller uses this partition during calibration, avoiding
>>>>> the partition search entirely.
>>>>
>>>> Do you have any data that this approach being "direct" is faster? In
>>>> fact, it might be worse. Instead of searching just the limited number of
>>>> partition subnodes, you now search the entire tree for a matching
>>>> phandle value. We do have phandle caching, so that might save you
>>>> here.
>>>
>>> True, but besides performance considerations, I personally do not find
>>> elegant using a partition name/label, but maybe that's just personal
>>> taste :-)
>>
>> I agree. That's true for all the partition nodes with only node name
>> or label to go on. We should fix that at the source. However, you
>> already have to support using label,
> 
> Hum, no? There is downstream support for labels in TI kernels, but we
> explicitly asked Santhosh to drop it for mainline inclusion. His commit
> message may be a bit misleading on this regard because he is mentioning
> labels like if we were already using them, despite the fact that we are
> not.

Yeah, my bad, the commit message is a bit misleading.

Just to clarify:
  - The phandle was chosen for correctness, not performance
  - There is no existing support to reference via label

I'll update the commit message in v4 to make this clearer.

Thanks,
Santhosh.

> 
>> so anything else is supporting a
>> 2nd way whether it is compatible or a phandle property. Is it really
>> worth it here?
>>
>> Rob
> 
> Thanks,
> Miquèl