[PATCH] of: Introduce of_node_get_match_data() helper

[PATCH] of: Introduce of_node_get_match_data() helper

[Geert Uytterhoeven]

If CONFIG_OF=n, code using

    info = of_match_node(matchs, np)->data;

fails to compile:

    warning: dereferencing ‘void *’ pointer
    error: request for member ‘data’ in something not a structure or union

Follow the example set by of_device_get_match_data(), and introduce a
new helper of_node_get_match_data().  This will allow to increase
compile-testing coverage later.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/of/base.c  | 13 +++++++++++++
 include/linux/of.h |  7 +++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2decb6..473d002ba7203115 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1022,6 +1022,19 @@ const struct of_device_id *of_match_node(const struct of_device_id *matches,
 }
 EXPORT_SYMBOL(of_match_node);
 
+const void *of_node_get_match_data(const struct of_device_id *matches,
+				   const struct device_node *node)
+{
+	const struct of_device_id *match;
+
+	match = of_match_node(matches, node);
+	if (!match)
+		return NULL;
+
+	return match->data;
+}
+EXPORT_SYMBOL(of_node_get_match_data);
+
 /**
  *	of_find_matching_node_and_match - Find a node based on an of_device_id
  *					  match table.
diff --git a/include/linux/of.h b/include/linux/of.h
index bfd1a23221735161..214e41eac4e5634f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -337,6 +337,8 @@ extern int of_n_addr_cells(struct device_node *np);
 extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
+extern const void *of_node_get_match_data(const struct of_device_id *matches,
+					  const struct device_node *node);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
 extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args);
 extern struct device_node *of_parse_phandle(const struct device_node *np,
@@ -844,8 +846,9 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
 {
 }
 
-#define of_match_ptr(_ptr)	NULL
-#define of_match_node(_matches, _node)	NULL
+#define of_match_ptr(_ptr)			NULL
+#define of_match_node(_matches, _node)		NULL
+#define of_node_get_match_data(_matches, _node)	NULL
 #endif /* CONFIG_OF */
 
 /* Default string compare functions, Allow arch asm/prom.h to override */
-- 
2.7.4

Re: [PATCH] of: Introduce of_node_get_match_data() helper

[Vladimir Zapolskiy]

Hello Geert,

On 04/25/2017 08:40 PM, Geert Uytterhoeven wrote:
> If CONFIG_OF=n, code using
> 
>     info = of_match_node(matchs, np)->data;
> 
> fails to compile:
> 
>     warning: dereferencing ‘void *’ pointer
>     error: request for member ‘data’ in something not a structure or union

IMHO firstly all the cases like one above must be fixed in place, in
parallel do an overall conversion of subcases:

-	of_id = of_match_node(some_dt_ids, dev->of_node);
+	of_id = of_match_device(some_dt_ids, dev);

> Follow the example set by of_device_get_match_data(), and introduce a
> new helper of_node_get_match_data().  This will allow to increase
> compile-testing coverage later.

Unfortunately the proposed of_node_get_match_data() is not so similar
to the of_device_get_match_data(), the latter one has only one input
argument in opposite to of_match_device(), which is truly convenient.

> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

The change itself is good.

Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: Introduce of_node_get_match_data() helper

[Geert Uytterhoeven]

Hi Vladimir,

On Fri, Jun 9, 2017 at 9:30 AM, Vladimir Zapolskiy
<vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> wrote:
> On 04/25/2017 08:40 PM, Geert Uytterhoeven wrote:
>> If CONFIG_OF=n, code using
>>
>>     info = of_match_node(matchs, np)->data;
>>
>> fails to compile:
>>
>>     warning: dereferencing ‘void *’ pointer
>>     error: request for member ‘data’ in something not a structure or union
>
> IMHO firstly all the cases like one above must be fixed in place, in
> parallel do an overall conversion of subcases:
>
> -       of_id = of_match_node(some_dt_ids, dev->of_node);
> +       of_id = of_match_device(some_dt_ids, dev);
>
>> Follow the example set by of_device_get_match_data(), and introduce a
>> new helper of_node_get_match_data().  This will allow to increase
>> compile-testing coverage later.
>
> Unfortunately the proposed of_node_get_match_data() is not so similar
> to the of_device_get_match_data(), the latter one has only one input
> argument in opposite to of_match_device(), which is truly convenient.

Thanks to your comment, I realized I can just use of_device_get_match_data().
Hence I'd like to withdraw my patch.

Background: I encountered this in a clock driver, which started life as
a CLK_OF_DECLARE() driver, but ended up as a platform driver anyway.
CLK_OF_DECLARE() drivers don't have a struct device to use, so the
driver still used of_match_node() instead of of_match_device().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html