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

[Madhavan Srinivasan]

On Tuesday 13 June 2017 11:59 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.
>>
>> 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);
> 		}
>         }

OK will fix it.

>
>> +
>> +       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.

ok sure, will fix it.

Maddy

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