[Skiboot] [PATCH v12 04/10] dt: Add phandle fixup helpers
Michael Neuling
mikey at neuling.org
Wed Jun 14 15:44:05 AEST 2017
On Tue, 2017-06-13 at 19:35 +0530, Madhavan Srinivasan wrote:
>
> On Tuesday 13 June 2017 11:47 AM, Michael Neuling wrote:
> > On Sun, 2017-05-21 at 20:40 +0530, Madhavan Srinivasan wrote:
> > > New subtree needs would have "phandle"
> > > already assigned to some of it's nodes.
> > > If this new substree is attached to main dt,
> > > without fixing the phandles, would lead to
> > > phandle duplicate errors.
> >
> > This commit message doesn't make a lot of sense to me. Can you reword?
>
> When there is a new device tree that needs to be added to
> the main dt of the opal (ex, IMC catalog dtb loaded from pnor partition),
> we need to check for the phandle values in the new incoming
> device tree before attaching it. Reason is that, incoming device tree could
> already have phandle values initialized for its nodes. Now, if we attach
> this new device tree to the main opal DT, we could potentially hit phandle
> duplicate error (since the phandles value usually start with 1).
>
> To avoid this, a new helper function dt_adjust_subtree_phandle()
> is added to scan the incoming device tree and update node
> "phandle" accordingly based on the opal "last_phandle" value.
>
> Add to this, helper function also supports updates of "properties"
> with in a node which may refer the "phandle" value in the incoming
> device tree. Helper function will also fix the "properties" field
> accordingly.
OK, thanks, that makes a lot more sense the original commit message.
Mikey
>
>
> Maddy
>
> >
> > Also, can you fix the wrapping so it's around 80 column rather than
> > (seemingly)
> > 50?
> >
> > > Also, new subtree could have "properties"
> > > refering to the phandle. So these "properties"
> > > should also get updated accordingly when fixing
> > > the "phandle".
> > >
> > > New helper dt_adjust_subtree_phandle() added to
> > > core/device.c to do both. It does phandle fixup
> > > with a single pass through the subtree.
> > >
> > > Patch also adds test in core/test/run-device.c
> > > to exercise dt_fixup().
> > >
> > > Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> > > ---
> > > core/device.c | 38 ++++++++++++++++++++++++++++++++++++++
> > > core/test/run-device.c | 35 +++++++++++++++++++++++++++++++++--
> > > include/device.h | 4 ++++
> > > 3 files changed, 75 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/core/device.c b/core/device.c
> > > index 7211570e1b3c..7f2c94c10386 100644
> > > --- a/core/device.c
> > > +++ b/core/device.c
> > > @@ -1069,3 +1069,41 @@ bool dt_node_is_enabled(struct dt_node *node)
> > >
> > > return p->len > 1 && p->prop[0] == 'o' && p->prop[1] == 'k';
> > > }
> > > +
> > > +/*
> > > + * Function to fixup the phandle in the subtree.
> > > + */
> > > +void dt_adjust_subtree_phandle(struct dt_node *dev,
> > > + const char** (get_properties_to_fix)(struct
> > > dt_node
> > > *n))
> > > +{
> > > + struct dt_node *node;
> > > + struct dt_property *prop;
> > > + u32 phandle, max_phandle = 0, import_phandle = new_phandle();
> > > + const char **name;
> > > +
> > > + dt_for_each_node(dev, node) {
> > > + const char **props_to_update;
> > > + node->phandle += import_phandle;
> > > +
> > > + /*
> > > + * calculate max_phandle(new_tree), needed to update
> > > + * last_phandle.
> > > + */
> > > + if (node->phandle >= max_phandle)
> > > + max_phandle = node->phandle;
> > > +
> > > + props_to_update = get_properties_to_fix(node);
> > > + if (props_to_update) {
> > > + for (name = props_to_update; *name != NULL;
> > > name++) {
> > > + prop = __dt_find_property(node, *name);
> > > + if (prop) {
> > > + phandle = dt_prop_get_u32(node,
> > > *name);
> > > + phandle += import_phandle;
> > > + memcpy((char *)&prop->prop,
> > > &phandle,
> > > prop->len);
> > > + }
> > > + }
> > > + }
> > > + }
> > > +
> > > + set_last_phandle(max_phandle);
> > > +}
> > > diff --git a/core/test/run-device.c b/core/test/run-device.c
> > > index 5939afc70e54..ffc8a4b9d1e1 100644
> > > --- a/core/test/run-device.c
> > > +++ b/core/test/run-device.c
> > > @@ -33,6 +33,8 @@ static inline bool fake_is_rodata(const void *p)
> > > #include "../device.c"
> > > #include <assert.h>
> > > #include "../../test/dt_common.c"
> > > +const char *prop_to_fix[] = {"something", NULL};
> > > +const char **imc_prop_to_fix(struct dt_node *node);
> > >
> > > static void check_path(const struct dt_node *node, const char *
> > > expected_path)
> > > {
> > > @@ -87,18 +89,29 @@ static bool is_sorted(const struct dt_node *root)
> > > return true;
> > > }
> > >
> > > +/*handler for phandle fixup test */
> > > +const char **imc_prop_to_fix(struct dt_node *node)
> > > +{
> > > + const struct dt_property *prop;
> > > +
> > > + prop = dt_find_property(node, "something");
> > > + if (prop)
> > > + return prop_to_fix;
> > > +
> > > + return NULL;
> > > +}
> > >
> > > int main(void)
> > > {
> > > struct dt_node *root, *c1, *c2, *gc1, *gc2, *gc3, *ggc1, *ggc2;
> > > struct dt_node *addrs, *addr1, *addr2;
> > > - struct dt_node *i;
> > > + struct dt_node *i, *subtree, *ev1, *ut1, *ut2;
> > > const struct dt_property *p;
> > > struct dt_property *p2;
> > > unsigned int n;
> > > char *s;
> > > size_t sz;
> > > - u32 phandle;
> > > + u32 phandle, ev1_ph, new_prop_ph;
> > >
> > > root = dt_new_root("");
> > > assert(!list_top(&root->properties, struct dt_property, list));
> > > @@ -412,6 +425,24 @@ int main(void)
> > >
> > > dt_free(root);
> > >
> > > + /* phandle fixup test */
> > > + subtree = dt_new_root("subtree");
> > > + ev1 = dt_new(subtree, "ev at 1");
> > > + ev1_ph = ev1->phandle;
> > > + dt_new(ev1,"a at 1");
> > > + dt_new(ev1,"a at 2");
> > > + dt_new(ev1,"a at 3");
> > > + ut1 = dt_new(subtree, "ut at 1");
> > > + dt_add_property(ut1, "something", (const void *)&ev1->phandle,
> > > 4);
> > > + ut2 = dt_new(subtree, "ut at 2");
> > > + dt_add_property(ut2, "something", (const void *)&ev1->phandle,
> > > 4);
> > > +
> > > + dt_adjust_subtree_phandle(subtree, imc_prop_to_fix);
> > > + assert(!(ev1->phandle == ev1_ph));
> > > + new_prop_ph = dt_prop_get_u32(ut1, "something");
> > > + assert(!(new_prop_ph == ev1_ph));
> > > + new_prop_ph = dt_prop_get_u32(ut2, "something");
> > > + assert(!(new_prop_ph == ev1_ph));
> > > return 0;
> > > }
> > >
> > > diff --git a/include/device.h b/include/device.h
> > > index c51b3eea298e..1e5875dca584 100644
> > > --- a/include/device.h
> > > +++ b/include/device.h
> > > @@ -268,4 +268,8 @@ struct dt_node *__dt_find_by_name_addr(struct dt_node
> > > *parent, const char *name,
> > > struct dt_node *dt_find_by_name_addr(struct dt_node *parent, const char
> > > *name,
> > > uint64_t addr);
> > >
> > > +/* phandle fixup helper */
> > > +void dt_adjust_subtree_phandle(struct dt_node *subtree,
> > > + const char**
> > > (get_properties_to_fix)(struct
> > > dt_node *n));
> > > +
> > > #endif /* __DEVICE_H */
>
>
More information about the Skiboot
mailing list