On 18-12-13 16:12:41, Shawn Guo wrote:

On Thu, Dec 13, 2018 at 04:02:34PM +0800, Olof Johansson wrote:
...

quoted

quoted

quoted

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/
 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.

On arm64 we generally add an ARCH_<family> and leave it at that. So,
ARCH_MXC would be suitable, but we shouldn't add more finegrained than
that.

Olof,

I'm just about to send pull request for i.MX8MQ support, which
introduced SOC_IMX8MQ option.  It sounds like we should rework the
patches to get SOC_IMX8MQ killed?

That was introduced because of the way the clock were introduced by prior IMXs.
What I suggest is to change the way the clocks are built a little bit.
Something like this:

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 5c0b11e..fdea7d4 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile

@@ -3,13 +3,11 @@
 obj-y += \
        clk.o \
        clk-busy.o \
-       clk-composite-8m.o \
        clk-cpu.o \
        clk-composite-7ulp.o \
        clk-divider-gate.o \
        clk-fixup-div.o \
        clk-fixup-mux.o \
-       clk-frac-pll.o \
        clk-gate-exclusive.o \
        clk-gate2.o \
        clk-pfd.o \

@@ -18,7 +16,6 @@ obj-y += \
        clk-pllv2.o \
        clk-pllv3.o \
        clk-pllv4.o \
-       clk-sccg-pll.o

 obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
 obj-$(CONFIG_SOC_IMX21)  += clk-imx21.o

@@ -34,5 +31,9 @@ obj-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o
 obj-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o
 obj-$(CONFIG_SOC_IMX7D)  += clk-imx7d.o
 obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o
-obj-$(CONFIG_SOC_IMX8MQ) += clk-imx8mq.o
 obj-$(CONFIG_SOC_VF610)  += clk-vf610.o
+
+obj-$(CONFIG_ARCH_MXC) += clk-imx8mq.o \
+       clk-sccg-pll.o \
+       clk-composite-8m.o \
+       clk-frac-pll.o

This way the frac, composite-8m and the sccg will only be built for arm64,
and we also get rid of the SOC_IMX8MQ. But then, arm64 IMX clocks will not follow
the same pattern as the arm32 ones. If that's ok with everyone, I already have
a patch that solves this issue.

Shawn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel