[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aW2_Zm3RmOkNWkiO@zatzit>
Date: Mon, 19 Jan 2026 16:21:42 +1100
From: David Gibson <david@...son.dropbear.id.au>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Ayush Singh <ayush@...gleboard.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
devicetree-compiler@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree-spec@...r.kernel.org,
Hui Pu <hui.pu@...ealthcare.com>,
Ian Ray <ian.ray@...ealthcare.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [RFC PATCH 08/77] dtc: Introduce update_phandles_ref()
On Fri, Jan 16, 2026 at 12:26:30PM +0100, Herve Codina wrote:
> On Thu, 15 Jan 2026 11:46:52 +1100
> David Gibson <david@...son.dropbear.id.au> wrote:
>
> > On Mon, Jan 12, 2026 at 03:18:58PM +0100, Herve Codina wrote:
> > > With the introduction of FDT_REF_LOCAL dtb tag, a local phandle used
> > > by a property is identify when a dtb is parsed.
> > >
> > > In order to have consistent internal data, the reference related to this
> > > phandle usage needs to be updated based on the phandle value.
> > >
> > > This is done by update_phandles_ref().
> > >
> > > Signed-off-by: Herve Codina <herve.codina@...tlin.com>
> > > ---
> > > dtc.c | 2 ++
> > > dtc.h | 2 ++
> > > livetree.c | 38 ++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 42 insertions(+)
> > >
> > > diff --git a/dtc.c b/dtc.c
> > > index b3445b7..88f03ff 100644
> > > --- a/dtc.c
> > > +++ b/dtc.c
> > > @@ -333,6 +333,8 @@ int main(int argc, char *argv[])
> > > generate_fixups = 1;
> > > }
> > >
> > > + update_phandles_ref(dti);
> > > +
> > > process_checks(force, dti);
> > >
> > > if (auto_label_aliases)
> > > diff --git a/dtc.h b/dtc.h
> > > index 965321c..351fe41 100644
> > > --- a/dtc.h
> > > +++ b/dtc.h
> > > @@ -345,6 +345,8 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
> > > void generate_fixups_tree(struct dt_info *dti, const char *name);
> > > void generate_local_fixups_tree(struct dt_info *dti, const char *name);
> > >
> > > +void update_phandles_ref(struct dt_info *dti);
> > > +
> > > /* Checks */
> > >
> > > void parse_checks_option(bool warn, bool error, const char *arg);
> > > diff --git a/livetree.c b/livetree.c
> > > index 9b67934..9e30a63 100644
> > > --- a/livetree.c
> > > +++ b/livetree.c
> > > @@ -1158,3 +1158,41 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
> > > "Warning: Preexisting data in %s malformed, some content could not be added.\n",
> > > name);
> > > }
> > > +
> > > +static void update_phandles_ref_internal(struct dt_info *dti, struct node *node)
> > > +{
> > > + struct node *c;
> > > + struct property *prop;
> > > + struct marker *m;
> > > + struct node *refnode;
> > > + cell_t phandle;
> > > +
> > > + for_each_property(node, prop) {
> > > + m = prop->val.markers;
> > > + for_each_marker_of_type(m, REF_PHANDLE) {
> > > + if (m->ref)
> > > + continue;
> >
> > IIUC this means that REF_PHANDLE markers can be missing their ref.
>
> Yes, no ref are present in dtb for phandle pointing to local node, only
> the phandle value is present. We need at some point to set the ref from
> this phandle value.
>
> > Allowing the markers to be in an incomplete state is a footgun. If
> > possible it would be better to fully generate the markers when we
> > create them. If not, we should use a different marker type when it's
> > introduced, and convert it later.
> >
> > I think what's going on here is one type is saying "you have a
> > reference, fill in the phandle", the other is saying "you have a
> > phandle, fill in the reference". Those seem like they should be
> > different types to me, even if they can be converted once all the
> > fixups are applied.
>
> Right, I can introduce
> - REF_PHANDLE_FILL_PHANDLE: "you have a phandle, fill in the reference"
>
> - REF_PHANDLE_FILL_REF: "you have a reference, fill the phandle"
> Well, fill the phandle or not... Indeed, this reference can be external and
> so this can be an unresolved symbol.
>
> But anyway those new marker types will exist only between the parsing step and
> the step where we convert them to REF_PHANDLE markers.
So REF_PHANDLE always has a complete reference, and a
complete-as-possible phandle value (-1 if it's an external reference)?
I think that makes sense - I don't love the proposed names for the
earlier markers, but better ones aren't immediately occurring to me.
>
> Will see what I can propose in the next series iteration.
>
> Best regards,
> Hervé
>
>
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists