On Mon, Feb 5, 2024 at 11:47 AM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > On Mon, Feb 5, 2024 at 9:36 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Fri, Feb 02, 2024 at 02:13:24AM -0800, Saravana Kannan wrote: > > > We have a more accurate function to find the right consumer of a > > > remote-endpoint property instead of searching for a parent with > > > compatible string property. So, use that instead. While at it, make the > > > code to find the consumer a bit more flexible and based on the property > > > being parsed. > > > > > > Fixes: f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint") > > > Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx> > > > --- > > > drivers/of/property.c | 52 +++++++++++++------------------------------ > > > 1 file changed, 15 insertions(+), 37 deletions(-) > > > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > > index 641a40cf5cf3..ba374a1f2072 100644 > > > --- a/drivers/of/property.c > > > +++ b/drivers/of/property.c > > > @@ -1063,36 +1063,6 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode, > > > return of_device_get_match_data(dev); > > > } > > > > > > -static struct device_node *of_get_compat_node(struct device_node *np) > > > -{ > > > - of_node_get(np); > > > - > > > - while (np) { > > > - if (!of_device_is_available(np)) { > > > - of_node_put(np); > > > - np = NULL; > > > - } > > > - > > > - if (of_property_present(np, "compatible")) > > > - break; > > > - > > > - np = of_get_next_parent(np); > > > - } > > > - > > > - return np; > > > -} > > > - > > > -static struct device_node *of_get_compat_node_parent(struct device_node *np) > > > -{ > > > - struct device_node *parent, *node; > > > - > > > - parent = of_get_parent(np); > > > - node = of_get_compat_node(parent); > > > - of_node_put(parent); > > > - > > > - return node; > > > -} > > > - > > > static void of_link_to_phandle(struct device_node *con_np, > > > struct device_node *sup_np) > > > { > > > @@ -1222,10 +1192,10 @@ static struct device_node *parse_##fname(struct device_node *np, \ > > > * parse_prop.prop_name: Name of property holding a phandle value > > > * parse_prop.index: For properties holding a list of phandles, this is the > > > * index into the list > > > + * @get_con_dev: If the consumer node containing the property is never converted > > > + * to a struct device, implement this ops so fw_devlink can use it > > > + * to find the true consumer. > > > * @optional: Describes whether a supplier is mandatory or not > > > - * @node_not_dev: The consumer node containing the property is never converted > > > - * to a struct device. Instead, parse ancestor nodes for the > > > - * compatible property to find a node corresponding to a device. > > > * > > > * Returns: > > > * parse_prop() return values are > > > @@ -1236,8 +1206,8 @@ static struct device_node *parse_##fname(struct device_node *np, \ > > > struct supplier_bindings { > > > struct device_node *(*parse_prop)(struct device_node *np, > > > const char *prop_name, int index); > > > + struct device_node *(*get_con_dev)(struct device_node *np); > > > bool optional; > > > - bool node_not_dev; > > > }; > > > > > > DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells") > > > @@ -1328,6 +1298,11 @@ static struct device_node *parse_interrupts(struct device_node *np, > > > return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > > > } > > > > > > +static struct device_node *get_remote_endpoint_dev(struct device_node *np) > > > +{ > > > + return to_of_node(fwnode_graph_get_port_parent(of_fwnode_handle(np))); > > > > DT APIs shouldn't depend on fwnode APIs. > > I get what you are saying, but is it an API though? It's a static > internal function that's eventually used by .add_link to add fwnode > links. Would it be better if I rolled this into two separate inline > code where this function is getting called from? I'm trying to avoid > duplicating the function that's already present in > fwnode_graph_get_port_parent(), but it also feels too specific an op > to introduce into drivers/of/property.c > > How would you like me to handle this? I don't see a good option. Nevermind, there's already of_graph_get_port_parent() that does exactly the same as fwnode_graph_get_port_parent(). I'll just use that. -Saravana