Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1593656074-10092-1-git-send-email-Anson.Huang@nxp.com> <1593656074-10092-6-git-send-email-Anson.Huang@nxp.com>
In-Reply-To: <1593656074-10092-6-git-send-email-Anson.Huang@nxp.com>
From:   Dong Aisheng <dongas86@gmail.com>
Date:   Thu, 2 Jul 2020 11:17:05 +0800
Message-ID: <CAA+hA=T1+F-oTfWdV22ztUT3Ru-dFPDzhC7aJsdj0R3yP4wzbA@mail.gmail.com>
Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock
 driver as module
To:     Anson Huang <Anson.Huang@nxp.com>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Sascha Hauer <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        Peng Fan <peng.fan@nxp.com>, Arnd Bergmann <arnd@arndb.de>,
        Abel Vesa <abel.vesa@nxp.com>,
        Dong Aisheng <aisheng.dong@nxp.com>,
        Fugang Duan <fugang.duan@nxp.com>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Stephen Rothwell <sfr@canb.auug.org.au>,
        linux-clk <linux-clk@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        dl-linux-imx <Linux-imx@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 2, 2020 at 10:20 AM Anson Huang <Anson.Huang@nxp.com> wrote:
>
> Change configuration to "tristate", use device_initcall() instead
> of builtin_platform_driver(), add module author, description and
> license to support building i.MX8QXP clock drivers as module.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V3:
>         - use device_initcall() instead of builtin_platform_driver();
>         - add module author/description;
>         - link common scu/lpcg clock driver to i.MX8QXP clock driver, then
>           no need to have exports.
> ---
>  drivers/clk/imx/Kconfig            |  6 +++---
>  drivers/clk/imx/Makefile           |  9 ++++-----
>  drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++++++++-
>  drivers/clk/imx/clk-imx8qxp.c      | 11 ++++++++++-
>  4 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index 1867111..8340dfe 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -5,8 +5,8 @@ config MXC_CLK
>         depends on ARCH_MXC
>
>  config MXC_CLK_SCU
> -       bool
> -       depends on IMX_SCU
> +       tristate "IMX SCU clock"
> +       depends on ARCH_MXC && IMX_SCU
>
>  config CLK_IMX1
>           bool "IMX1 CCM Clock Driver"
> @@ -127,7 +127,7 @@ config CLK_IMX8MQ
>             Build the driver for i.MX8MQ CCM Clock Driver
>
>  config CLK_IMX8QXP
> -       bool "IMX8QXP SCU Clock"
> +       tristate "IMX8QXP SCU Clock"
>         depends on ARCH_MXC && IMX_SCU && ARM64
>         select MXC_CLK_SCU
>         help
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 17f5d12..79e53f2 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -21,15 +21,14 @@ mxc-clk-objs += clk-pll14xx.o
>  mxc-clk-objs += clk-sscg-pll.o
>  obj-$(CONFIG_MXC_CLK) += mxc-clk.o
>
> -obj-$(CONFIG_MXC_CLK_SCU) += \
> -       clk-scu.o \
> -       clk-lpcg-scu.o
> -
>  obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
>  obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
>  obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
>  obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
> -obj-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o
> +
> +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> +clk-imx-scu-$(CONFIG_CLK_IMX8QXP) += clk-scu.o clk-imx8qxp.o
> +clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
>
>  obj-$(CONFIG_CLK_IMX1)   += clk-imx1.o
>  obj-$(CONFIG_CLK_IMX21)  += clk-imx21.o
> diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> index 04c8ee3..5b6648e 100644
> --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> @@ -231,4 +231,12 @@ static struct platform_driver imx8qxp_lpcg_clk_driver = {
>         .probe = imx8qxp_lpcg_clk_probe,
>  };
>
> -builtin_platform_driver(imx8qxp_lpcg_clk_driver);
> +static int __init imx8qxp_lpcg_clk_init(void)

Does __init work for module?

> +{
> +       return platform_driver_register(&imx8qxp_lpcg_clk_driver);
> +}
> +device_initcall(imx8qxp_lpcg_clk_init);

Any reason to change to device_initcall which looks a bit strange?
Is it because the following line?
+obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
But it looks to me they're still two modules. Aren't they?

Regards
Aisheng

> +
> +MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX8QXP LPCG clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index 5e2903e..9bcf0d1 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -151,4 +151,13 @@ static struct platform_driver imx8qxp_clk_driver = {
>         },
>         .probe = imx8qxp_clk_probe,
>  };
> -builtin_platform_driver(imx8qxp_clk_driver);
> +
> +static int __init imx8qxp_clk_init(void)
> +{
> +       return platform_driver_register(&imx8qxp_clk_driver);
> +}
> +device_initcall(imx8qxp_clk_init);
> +
> +MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>