MIME-Version: 1.0
References: <20230310140218.326626-1-arnd@kernel.org>
In-Reply-To: <20230310140218.326626-1-arnd@kernel.org>
From:   Saravana Kannan <saravanak@google.com>
Date:   Fri, 10 Mar 2023 09:21:57 -0800
Message-ID: <CAGETcx-9PDaWc7eFQ5ajk0BFY7HRhvf30R6uR0Mo1qb+6Af0jQ@mail.gmail.com>
Subject: Re: [PATCH] clk: avoid build failure on CLK_OF_DECLARE() with invalid name
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Ricardo Ribalda <ribalda@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Maxime Ripard <maxime@cerno.tech>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Marijn Suijten <marijn.suijten@somainline.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Christian Marangi <ansuelsmth@gmail.com>,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Mar 10, 2023 at 6:02=E2=80=AFAM Arnd Bergmann <arnd@kernel.org> wro=
te:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Generating an init function function from CLK_OF_DECLARE() broke for the
> 98dx1135_clk declaration because that string starts with a digit and
> is not a valid C identifier:

Nathan already sent out a fix for this and it's been picked up.

https://lore.kernel.org/lkml/20230308-clk_of_declare-fix-v1-1-317b741e2532@=
kernel.org/

-Saravana

>
> In file included from drivers/clk/mvebu/kirkwood.c:15:
> drivers/clk/mvebu/kirkwood.c:358:16: error: invalid suffix "dx1135_clk_of=
_clk_init_declare" on integer constant
>   358 | CLK_OF_DECLARE(98dx1135_clk, "marvell,mv98dx1135-core-clock",
>       |                ^~~~~~~~~~~~
> include/linux/clk-provider.h:1367:28: note: in definition of macro 'CLK_O=
F_DECLARE'
>  1367 |         static void __init name##_of_clk_init_declare(struct devi=
ce_node *np) \
>       |                            ^~~~
> drivers/clk/mvebu/kirkwood.c:358:16: error: expected identifier or '(' be=
fore numeric constant
>   358 | CLK_OF_DECLARE(98dx1135_clk, "marvell,mv98dx1135-core-clock",
>       |                ^~~~~~~~~~~~
> include/linux/clk-provider.h:1367:28: note: in definition of macro 'CLK_O=
F_DECLARE'
>  1367 |         static void __init name##_of_clk_init_declare(struct devi=
ce_node *np) \
>       |                            ^~~~
>
> This could be fixed in the driver by renaming 98dx1135_clk to a valid
> C identifier, but it's easy enough to make the macro more robust by
> reversing the two parts of the name, which makes it work for other files
> that may have the same issue. Since CLK_OF_DECLARE_DRIVER() has a very
> similar definition, do the same change in both.
>
> Fixes: c7296c51ce5d ("clk: core: New macro CLK_OF_DECLARE_DRIVER")
> Fixes: c28cd1f3433c ("clk: Mark a fwnode as initialized when using CLK_OF=
_DECLARE() macro")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/clk-provider.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index c9f5276006a0..3586a029db05 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -1364,24 +1364,24 @@ struct clk_hw_onecell_data {
>  };
>
>  #define CLK_OF_DECLARE(name, compat, fn) \
> -       static void __init name##_of_clk_init_declare(struct device_node =
*np) \
> +       static void __init of_clk_init_declare##name(struct device_node *=
np) \
>         {                                                               \
>                 fn(np);                                                 \
>                 fwnode_dev_initialized(of_fwnode_handle(np), true);     \
>         }                                                               \
> -       OF_DECLARE_1(clk, name, compat, name##_of_clk_init_declare)
> +       OF_DECLARE_1(clk, name, compat, of_clk_init_declare##name)
>
>  /*
>   * Use this macro when you have a driver that requires two initializatio=
n
>   * routines, one at of_clk_init(), and one at platform device probe
>   */
>  #define CLK_OF_DECLARE_DRIVER(name, compat, fn) \
> -       static void __init name##_of_clk_init_driver(struct device_node *=
np) \
> +       static void __init of_clk_init_driver##name(struct device_node *n=
p) \
>         {                                                               \
>                 of_node_clear_flag(np, OF_POPULATED);                   \
>                 fn(np);                                                 \
>         }                                                               \
> -       OF_DECLARE_1(clk, name, compat, name##_of_clk_init_driver)
> +       OF_DECLARE_1(clk, name, compat, of_clk_init_driver##name)
>
>  #define CLK_HW_INIT(_name, _parent, _ops, _flags)              \
>         (&(struct clk_init_data) {                              \
> --
> 2.39.2
>