[Skiboot] [PATCH v12 04/10] dt: Add phandle fixup helpers

[Michael Neuling]

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.
> 
> 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);
> > +				}
> > +			}
> > +		}
> +       }

Few too many levels of indentation.  Could do something like:

	dt_for_each_node(dev, node) {

		<snip>

		props_to_update = get_properties_to_fix(node);
		if (!props_to_update)
			continue;
		for (name = props_to_update; *name != NULL; name++) {
			  prop = __dt_find_property(node, *name);
			  if (!prop)
				continue;
			  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)

I'm not sure you want to call this imc_.  imc is just a distraction here.

> +{
> +	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 */