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

[Santhosh Kumar K]

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@xxxxxxxxxx> wrote:
>
> > On Wed, Jun 3, 2026 at 11:01 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> > > Hello,
> > >
> > > On 02/06/2026 at 11:49:45 -05, Rob Herring <robh@xxxxxxxxxx> 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