[Skiboot] [PATCH V5 1/3] core/device: Add function to return child node using name at substring "@"

Athira Rajeev atrajeev at linux.vnet.ibm.com
Mon Sep 11 15:15:20 AEST 2023



> On 10-Aug-2023, at 3:21 AM, Reza Arbab <arbab at linux.ibm.com> wrote:
> 
> Hi Athira,
> 
> I still have a couple of the same questions I asked in v4.
> 
> On Mon, Jul 17, 2023 at 08:54:29AM +0530, Athira Rajeev wrote:
>> Add a function dt_find_by_name_before_addr() that returns the child node if
>> it matches till first occurrence at "@" of a given name, otherwise NULL.
> 
> Given this summary, I don't userstand the following:
> 
>> + assert(dt_find_by_name(root, "node at 1") == addr1);
>> + assert(dt_find_by_name(root, "node0_1 at 2") == addr2);
> 
> Is this behavior required? I don't think it makes sense to call this function with a second argument containing '@', so I wouldn't expect it to match anything in these cases. The function seems to specifically enable it:

Hi Reza,

Yes makes sense. dt_find_by_name can be removed in this test since its intention is to find device by name.
I will remove these two checks.

> 
>> +struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const char *name)
>> +{
> [snip]
>> + node = strdup(name);
>> + if (!node)
>> + return NULL;
>> + node = strtok(node, "@");
> 
> Seems like you could get rid of this and just use name as-is.

Ok Reza
> 
> I was curious about something else; say we have 'node at 1' and 'node at 2'.  Is there an expectation of which it should match?
> 
>    addr1 = dt_new_addr(root, "node", 0x1);
>    addr2 = dt_new_addr(root, "node", 0x2);
>    assert(dt_find_by_name_substr(root, "node") == ???????);
>                                                   ^^^^^^^

In this case, dt_find_by_name_before_addr is not the right function to use.
We have other functions like dt_find_by_name_addr that can be made use of.

I will address other changes in next version

Thanks
Athira
> 
> -- 
> Reza Arbab



More information about the Skiboot mailing list