Re: [PATCH 1/5] of: introduce of_graph_get_remote_node

[Rob Herring]

On Mon, Feb 6, 2017 at 4:32 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
> Hi Rob,
>
> thanks for this clean-up series! I was not aware how far the duplication
> has spread over time.
>
> On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote:
>> The OF graph API leaves too much of the graph walking to clients when
>> in many cases the driver doesn't care about accessing the port or
>> endpoint nodes. The drivers typically just want the device connected via
>> a particular graph connection. of_graph_get_remote_node provides this
>> functionality.
>>
>> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>> ---
>>  drivers/of/base.c        | 28 ++++++++++++++++++++++++++++
>>  include/linux/of_graph.h |  8 ++++++++
>>  2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index d4bea3c797d6..ea18ab16b92c 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
>>       return of_get_next_parent(np);
>>  }
>>  EXPORT_SYMBOL(of_graph_get_remote_port);
>> +
>> +struct device_node *of_graph_get_remote_node(const struct device_node *node,
>> +                                          int port, int endpoint)
>
> I think this should have a documentation comment, similar to the
> of_graph_get_endpoint_by_regs one, as it is not really clear from the
> function name that the returned device node is the parent (or
> grandparent) device node containing the remote port to the specified
> node & port & endpoint.
> Also it might be interesting to the user that -1 is a wildcard value for
> port / endpoint.

I really want to not allow using a wildcard here. Drivers should know
what port they want (or iterate over all of them). It didn't look like
any drivers were depending on the wildcard, but were just using -1 for
"no reg property" when really that should 0. Of course, I may have
missed something.

I guess I could enforce port/endpoint > 0 here as there's no existing users.

Rob