Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1662032 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] of: Introduce of_node_get_match_data() helper Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2017-06-09 09:40 +0200
    Re: [PATCH] of: Introduce of_node_get_match_data() helper Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-09 10:00 +0200

#1662032 — Re: [PATCH] of: Introduce of_node_get_match_data() helper

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@glider.be>

The change itself is good.

Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

[toc] | [next] | [standalone]

#1662057

Hi Vladimir,

On Fri, Jun 9, 2017 at 9:30 AM, Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web