[Skiboot] [PATCH v9 05/11] dt: Add phandle fixup helpers
Madhavan Srinivasan
maddy at linux.vnet.ibm.com
Tue May 2 21:29:46 AEST 2017
On Tuesday 02 May 2017 09:54 AM, Stewart Smith wrote:
> Madhavan Srinivasan <maddy at linux.vnet.ibm.com> writes:
>
>> On Monday 01 May 2017 12:28 PM, Stewart Smith wrote:
>>> Anju T Sudhakar <anju at linux.vnet.ibm.com> writes:
>>>> Patch to add a phandle fixup function for a new
>>> You don't need to say "patch to add". You can drop the whole first
>>> sentence.
>>>
>>>> incoming subtree. When attaching a "subtree" to
>>>> main dt, subtree could have "property" referring
>>>> different nodes or multiple nodes via phandle as
>>>> "labels". For this reason, new subtree will have
>>>> "phandle" already assigned to some of it's nodes.
>>>> If this new subtree is attached to main dt without
>>>> phandle fixup, could lead to duplicate phandle errors
>>>>
>>>> dt_fixup_populate_hash() does two main things. It first
>>>> uses a hash primitive to create a hashtable using phandle
>>>> as key. It then loops through the list of properties specified
>>>> as an input to this function (which needs to be fixed up),
>>>> and calls the fixup "handler".
>>>>
>>>> Signed-off-by: Anju T Sudhakar <anju at linux.vnet.ibm.com>
>>>> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>>>> ---
>>>> core/device.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/device.h | 19 +++++++++++++++++++
>>>> 2 files changed, 66 insertions(+)
>>>>
>>>> diff --git a/core/device.c b/core/device.c
>>>> index 170e0ee..e933c27 100644
>>>> --- a/core/device.c
>>>> +++ b/core/device.c
>>>> @@ -17,6 +17,7 @@
>>>> #include <device.h>
>>>> #include <stdlib.h>
>>>> #include <skiboot.h>
>>>> +#include <hash.h>
>>>> #include <libfdt/libfdt.h>
>>>> #include <libfdt/libfdt_internal.h>
>>>> #include <ccan/str/str.h>
>>>> @@ -849,6 +850,52 @@ void dt_expand(const void *fdt)
>>>> abort();
>>>> }
>>>>
>>>> +/* dt_fixup_populate_hash: populate the hash table, and invoke the fixup-handler
>>>> + * to fixup the phandle value of the nodes.
>>>> + */
>>>> +int dt_fixup_populate_hash(struct dt_node *dev, hash_table_p hash, phandle_fixup_p list_prop )
>>>> +{
>>>> + struct dt_node *node;
>>>> + hash_entry_p entry;
>>>> + fixup_prop_list *list_pp;
>>>> + u32 phandle;
>>>> + phandle_fixup_n *val;
>>>> +
>>>> + /* Initialize the hashtable */
>>>> + hash_init(hash);
>>>> +
>>>> + /* For each node of 'dev', insert the node-phandle pair into hashtable */
>>>> + dt_for_each_node(dev, node) {
>>>> + if (node->phandle > 0) {
>>>> + val = (phandle_fixup_n *)malloc(sizeof(phandle_fixup_n));
>>>> + if(!val) {
>>>> + prerror("Failed to allocate memory");
>>>> + return -1;
>>>> + }
>>>> + val->node = node;
>>>> + val->fixed = false;
>>>> + entry = hash_insert_entry(hash, node->phandle, val);
>>>> + if(!entry) {
>>>> + prerror("Failed to make hash table entry");
>>>> + return -1;
>>>> + }
>>>> + }
>>>> + }
>>>> + list_for_each(&list_prop->list, list_pp, list) {
>>>> + phandle = dt_prop_get_u32(list_pp->node, list_pp->prop->name);
>>>> + /* look for the entry, with phandle as the key to hashtable */
>>>> + entry = hash_lookup_entry(hash, phandle);
>>>> + if (entry) {
>>>> + /* If find a valid entry in the hashtable, do the fixup */
>>>> + list_prop->fixup(entry, list_pp->prop);
>>>> + } else {
>>>> + prerror("%s: Missing hash entry phandle = %x\n", __FUNCTION__, phandle);
>>>> + return -1;
>>>> + }
>>>> + }
>>>> + return 0;
>>>> +}
>>>> +
>>> This looks overly complex for what needs to be done.
>>>
>>> If we're importing a subtree that uses phandles say 1-100, and our
>>> existing last_phandle=42, then all we need to do is go through and add
>>> 42 to all of the phandles in the subtree, and at the end, set
>>> last_phandle=42+100.
>> Basically this function does two things.
>> 1)Fix the phandle
>> 2)properties that refers to the phandle
>>
>> For example (from imc dtb), node here carries the events supported by
>> "mcs" unit.
>> And this node is common and referred by all "mcs" units supported.
>>
>> + nest-mcs-events {
>> + #address-cells = <0x1>;
>> + #size-cells = <0x1>;
>> + phandle = <0x10000242>;
>> +
>>
>> Here is the "mcs" unit node which refers to the above event node via
>> a property called "events". "events" property contains the phandle.
>>
>> + mcs3 {
>> + compatible = "ibm,imc-counters-nest";
>> + events-prefix = "PM_MCS3_"; /* denotes event name to be prefixed to get complete event name supported by this device */
>> +
>> + phandle = <0x10000241>;
>> + events = <0x10000242>; /* phandle of the events node supported by this device */
>>
>>
>> So when we fix the incoming dtb for the phandles, we also need to
>> look for any property that refers that phandle.
>>
>> Now my initial implementation was using llist from the
>> current ccan lib in the skiboot, But benh suggested to
>> have a simple hash primitive. Since this is a generic
>> function, he suggested to have a list of properties from
>> the caller along with a "handler" function which knows to fix.
>>
>> Hope this clarifies.
> I don't see why any of that would require anything more complex than a
> single pass through the subtree we're adding though.
>
> We just need a function like this:
>
> int dt_adjust_subtree_phandle(dt_node *subtree,
> const char** (get_properties_to_fix*)(dt_node *n))
> {
> phandle_import_start = ++last_phandle;
>
> foreach node and subnode {
> phandle += phandle_import_start;
> const char **properties_to_update;
> props_to_update = get_properties_to_fix(node);
> if (props_to_update) {
> for (const char* p = props_to_update; *p != NULL; p++) {
> set_value(prop, get_value(prop) + phandle_import_start);
> }
> }
> }
> }
How about something like this.
+
+/*
+ * Function to fixup the phandle in the subtree.
+ */
+void dt_adjust_subtree_phandle(struct dt_node *dev, const char
**prop_name )
+{
+ struct dt_node *node;
+ struct dt_property *prop;
+ u32 phandle, max_phandle, import_phandle = ++last_phandle;
+ const char **name;
+
+ dt_for_each_node(dev, node) {
+ node->phandle += import_phandle;
+
+ /*
+ * calculate max_pahndle(new_tree), needed to update
+ * last_phandle.
+ */
+ if (node->phandle >= max_phandle)
+ max_phandle = node->phandle;
+
+ /* Add the import_phandle to each property in the prop
array */
+ for (name = prop_name; *name != NULL &&
(__dt_find_property(node, *name)); name++) {
+ prop = __dt_find_property(node, *name);
+ phandle = dt_prop_get_u32(node, *name);
+ phandle += import_phandle;
+ memcpy((char *)&prop->prop, &phandle, prop->len);
+ }
+ }
+
+ update_last_phandle(max_phandle);
+}
+
Have also handled update of last_phande.
Maddy
> With this for IMC:
>
> const char** imc_properties_to_fix(dt_node *node)
> {
> if (node_is_compatible("ibm,imc-counters-nest"))
> return { "events" , NULL };
> else
> return NULL;
> }
>
> ?
>
More information about the Skiboot
mailing list