Hi,
On Thu, 10 Dec 2020 at 22:59, Dmitry Osipenko <digetx@xxxxxxxxx> wrote:
>
> 10.12.2020 23:29, Sam Protsenko пишет:
> > Both of_graph_is_present() and of_graph_get_next_endpoint() functions
> > share common piece of code for obtaining the graph port. Extract it into
> > separate static function to get rid of code duplication and avoid
> > possible coding errors in future.
> >
> > Fixes: 4ec0a44ba8d7 ("of_graph: add of_graph_is_present()")
>
> The "fixes" tag should be used only for bug-fixes and there is no bug
> fixed in this patch.
>
> > Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
> > ---
> > drivers/of/property.c | 34 ++++++++++++++++++++--------------
> > 1 file changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > index 408a7b5f06a9..da111fcf37ac 100644
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -30,13 +30,13 @@
> > #include "of_private.h"
> >
> > /**
> > - * of_graph_is_present() - check graph's presence
> > + * of_graph_get_port - find the "port" node in a given node
> > * @node: pointer to device_node containing graph port
> > *
> > - * Return: True if @node has a port or ports (with a port) sub-node,
> > - * false otherwise.
> > + * Return: A 'port' node pointer with refcount incremented if found or NULL
> > + * otherwise. The caller has to use of_node_put() on it when done.
> > */
> > -bool of_graph_is_present(const struct device_node *node)
> > +static struct device_node *of_graph_get_port(const struct device_node *node)
> > {
> > struct device_node *ports, *port;
> >
> > @@ -46,8 +46,22 @@ bool of_graph_is_present(const struct device_node *node)
> >
> > port = of_get_child_by_name(node, "port");
> > of_node_put(ports);
> > - of_node_put(port);
> >
> > + return port;
> > +}
> > +
> > +/**
> > + * of_graph_is_present() - check graph's presence
> > + * @node: pointer to device_node containing graph port
> > + *
> > + * Return: True if @node has a port or ports (with a port) sub-node,
> > + * false otherwise.
> > + */
> > +bool of_graph_is_present(const struct device_node *node)
> > +{
> > + struct device_node *port = of_graph_get_port(node);
> > +
> > + of_node_put(port);
> > return !!port;
> > }
> > EXPORT_SYMBOL(of_graph_is_present);
> > @@ -631,15 +645,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> > * parent port node.
> > */
> > if (!prev) {
> > - struct device_node *node;
> > -
> > - node = of_get_child_by_name(parent, "ports");
> > - if (node)
> > - parent = node;
> > -
> > - port = of_get_child_by_name(parent, "port");
> > - of_node_put(node);
> > -
> > + port = of_graph_get_port(parent);
> > if (!port) {
> > pr_err("graph: no port node found in %pOF\n", parent);
> > return NULL;
> >
>
> This repeats the problem which was made once before:
>
You are right. Inlining is probably the best solution here. Let's drop
this patch and keep everything as is. Thanks for catching this!
> https://lore.kernel.org/patchwork/patch/1266028/#1461493
