* [PATCH 1/2] of: property: Get rid of code duplication in port getting
@ 2020-12-10 20:29 Sam Protsenko
2020-12-10 20:59 ` Dmitry Osipenko
0 siblings, 1 reply; 3+ messages in thread
From: Sam Protsenko @ 2020-12-10 20:29 UTC (permalink / raw)
To: devicetree, linux-kernel
Cc: Rob Herring, Frank Rowand, John Stultz, Andy Shevchenko,
Sam Ravnborg, Dmitry Osipenko, Laurent Pinchart
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()")
Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
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;
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] of: property: Get rid of code duplication in port getting
2020-12-10 20:29 [PATCH 1/2] of: property: Get rid of code duplication in port getting Sam Protsenko
@ 2020-12-10 20:59 ` Dmitry Osipenko
2020-12-11 13:26 ` Sam Protsenko
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Osipenko @ 2020-12-10 20:59 UTC (permalink / raw)
To: Sam Protsenko, devicetree, linux-kernel
Cc: Rob Herring, Frank Rowand, John Stultz, Andy Shevchenko,
Sam Ravnborg, Laurent Pinchart
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@linaro.org>
> ---
> 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:
https://lore.kernel.org/patchwork/patch/1266028/#1461493
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] of: property: Get rid of code duplication in port getting
2020-12-10 20:59 ` Dmitry Osipenko
@ 2020-12-11 13:26 ` Sam Protsenko
0 siblings, 0 replies; 3+ messages in thread
From: Sam Protsenko @ 2020-12-11 13:26 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: devicetree, linux-kernel, Rob Herring, Frank Rowand, John Stultz,
Andy Shevchenko, Sam Ravnborg, Laurent Pinchart
Hi,
On Thu, 10 Dec 2020 at 22:59, Dmitry Osipenko <digetx@gmail.com> 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@linaro.org>
> > ---
> > 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-11 13:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-10 20:29 [PATCH 1/2] of: property: Get rid of code duplication in port getting Sam Protsenko
2020-12-10 20:59 ` Dmitry Osipenko
2020-12-11 13:26 ` Sam Protsenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).