quoted

-----Original Message-----
From: Stephen Boyd [mailto:sboyd@kernel.org]
Sent: Thursday, December 13, 2018 3:58 PM
To: linux-clk@vger.kernel.org; Aisheng Dong <aisheng.dong@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
shawnguo@kernel.org; Fabio Estevam [off-list ref]; dl-linux-imx
[off-list ref]; kernel@pengutronix.de; Aisheng Dong
[off-list ref]; Arnd Bergmann [off-list ref]; Olof Johansson
[off-list ref]
Subject: Re: [PATCH V11 1/7] clk: imx: add configuration option for mmio clks

Quoting Aisheng Dong (2018-12-12 17:07:52)

quoted

The patch introduces CONFIG_MXC_CLK option for legacy MMIO clocks,
this is required to compile legacy MMIO clock conditionally when
adding SCU based clocks for MX8 platforms later.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <redacted>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> diff --git
a/drivers/clk/Makefile b/drivers/clk/Makefile index a47430b..8a9440a
100644

--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile

@@ -72,7 +72,7 @@ obj-$(CONFIG_ARCH_DAVINCI)            +=

davinci/

quoted

 obj-$(CONFIG_H8300)                    += h8300/
 obj-$(CONFIG_ARCH_HISI)                        += hisilicon/
 obj-y                                  += imgtec/
-obj-$(CONFIG_ARCH_MXC)                 += imx/
+obj-y                                  += imx/
 obj-y                                  += ingenic/
 obj-$(CONFIG_ARCH_K3)                  += keystone/
 obj-$(CONFIG_ARCH_KEYSTONE)            += keystone/

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig new

file mode 100644 index 0000000..37478ba

--- /dev/null
+++ b/drivers/clk/imx/Kconfig

@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+# common clock support for NXP i.MX SoC family.
+config MXC_CLK
+       bool
+       def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 &&
+SOC_IMX8MQ)

Ok I looked closer, SOC_IMX8MQ is only being introduced now and has only
been in linux-next for a few days. Having such a config symbol is highly unusual
for arm64 code. Has that been accepted by arm-soc? I don't know why it
would be because we don't add SoC specific config options anymore. I imagine
it will be rejected and we'll need to have this new MXC_CLK config be enabled
in the defconfig instead of selected from the arch layer.

Given all that, is there any damage if this is just a def_bool ARCH_MXC right
now? If it's enabled on the platforms that don't use it because they have SCU,
does it actually matter? Or we just get some code bloat?
If it's just some extra code, then even more reason to make a user visible config
option that can be selected in the configurations that care to save some space.

It make sense to me. No actual damage as we support multiplatforms using both
MXC_CLK and MXC_CLK_SCU on ARMv8. And for users who want to save one of them,
they do can change via memuconfig later.
I will make it a def_bool ARCH_MXC.
Thanks for the suggestion.

BTW, as I also introduced SOC_IMX8QXP in Patch 4 and 7.

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
+obj-$(CONFIG_SOC_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o 

According to your former suggestion, I think we should remove it.
So I'm going to make imx8qxp clk an user selectable configuration option which
will select the common MXC_CLK_SCU. So a bit difference from former discussion,
MXC_CLK_SCU will still be kept a hide option.

This way is mostly like how QCOM does and should work for IMX as well.

Then it will look like:
config MXC_CLK_SCU
        bool 

config CLK_IMX8QXP
        bool "IMX8QXP SCU Clock"
        depends on ARCH_MXC && IMX_SCU && ARM64
        select MXC_CLK_SCU
        help
          Build the driver for IMX8QXP SCU based clocks.

If any issue, please let me know.