Re: [PATCH] of: replace of_match_node() macro by a function when !CONFIG_OF

[Rob Herring]

On Mon, Jul 8, 2024 at 2:55 AM Théo Lebrun <theo.lebrun@xxxxxxxxxxx> wrote:
>
> In the !CONFIG_OF case, replace the of_match_node() macro implementation
> by a static function. This ensures drivers calling of_match_node() can
> be COMPILE_TESTed.
>
> include/linux/of.h declares of_match_node() like this:
>
>         #ifdef CONFIG_OF
>         extern const struct of_device_id *of_match_node(
>                 const struct of_device_id *matches, const struct device_node *node);
>         #else
>         #define of_match_node(_matches, _node)  NULL
>         #endif
>
> When used inside an expression, those two implementations behave truly
> differently. The macro implementation has (at least) two pitfalls:
>
>  - Arguments are removed by the preprocessor meaning they do not appear
>    to the compiler. This can give "defined but not used" warnings.

It also means the arguments don't have to be defined at all which is
the reasoning the commit adding the macro gave:

    I have chosen to use a macro instead of a function to
    be able to avoid defining the first parameter.
    In fact, this "struct of_device_id *" first parameter
    is usualy not defined as well on non-dt builds.

We could change our mind here, but I suspect applying this would
result in some build failures.

>  - The returned value type is (void *)
>    versus (const struct of_device_id *).
>    It works okay if the value is stored in a variable, thanks to C's
>    implicit void pointer casting rules. It causes build errors if used
>    like `of_match_data(...)->data`.

Really, the only places of_match_node() should be used are ones
without a struct device. Otherwise, of_device_get_match_data() or
device_get_match_data() should be used instead.

Rob