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

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

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.

> 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