| Message ID | 1543934041-12572-2-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: imx: add imx8qxp clock support | expand |
Quoting Aisheng DONG (2018-12-04 06:39:22) > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > index c12a05c..303082c 100644 > --- a/arch/arm/mach-imx/Kconfig > +++ b/arch/arm/mach-imx/Kconfig > @@ -570,6 +580,7 @@ config SOC_IMX7ULP > config SOC_VF610 > bool "Vybrid Family VF610 support" > select ARM_GIC if ARCH_MULTI_V7 > + select MXC_CLK > select PINCTRL_VF610 > Instead of select can we break this dependency on the arch Kconfig and have: config MXC_CLK def_bool ARCH_MXC && !ARM64 > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index 5c0b11e..f850424 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -1,6 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > > -obj-y += \ > +obj-$(CONFIG_MXC_CLK) += \ Because this changes to obj-$(CONFIG) we should change the drivers/clk/Makefile to have obj-y for this Makefile instead of depending on ARCH_MXC.
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Sent: Tuesday, December 11, 2018 4:55 AM [...] > Subject: Re: [PATCH V9 1/7] clk: imx: add configuration option for mmio clks > > Quoting Aisheng DONG (2018-12-04 06:39:22) > > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > > index c12a05c..303082c 100644 > > --- a/arch/arm/mach-imx/Kconfig > > +++ b/arch/arm/mach-imx/Kconfig > > @@ -570,6 +580,7 @@ config SOC_IMX7ULP config SOC_VF610 > > bool "Vybrid Family VF610 support" > > select ARM_GIC if ARCH_MULTI_V7 > > + select MXC_CLK > > select PINCTRL_VF610 > > > > Instead of select can we break this dependency on the arch Kconfig and > have: > > config MXC_CLK > def_bool ARCH_MXC && !ARM64 > Thanks for the suggestion. One little problem is that we also have LS1021 supported under ARCH_MXC which does not use MXC_CLK. > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index > > 5c0b11e..f850424 100644 > > --- a/drivers/clk/imx/Makefile > > +++ b/drivers/clk/imx/Makefile > > @@ -1,6 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0 > > > > -obj-y += \ > > +obj-$(CONFIG_MXC_CLK) += \ > > Because this changes to obj-$(CONFIG) we should change the > drivers/clk/Makefile to have obj-y for this Makefile instead of depending on > ARCH_MXC. We also use ARCH_MXC for ARMv8 SoC clocks. Regards Dong Aisheng
Quoting Aisheng Dong (2018-12-10 18:36:29) > > -----Original Message----- > > From: Stephen Boyd [mailto:sboyd@kernel.org] > > Sent: Tuesday, December 11, 2018 4:55 AM > [...] > > Subject: Re: [PATCH V9 1/7] clk: imx: add configuration option for mmio clks > > > > Quoting Aisheng DONG (2018-12-04 06:39:22) > > > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > > > index c12a05c..303082c 100644 > > > --- a/arch/arm/mach-imx/Kconfig > > > +++ b/arch/arm/mach-imx/Kconfig > > > @@ -570,6 +580,7 @@ config SOC_IMX7ULP config SOC_VF610 > > > bool "Vybrid Family VF610 support" > > > select ARM_GIC if ARCH_MULTI_V7 > > > + select MXC_CLK > > > select PINCTRL_VF610 > > > > > > > Instead of select can we break this dependency on the arch Kconfig and > > have: > > > > config MXC_CLK > > def_bool ARCH_MXC && !ARM64 > > > > Thanks for the suggestion. > One little problem is that we also have LS1021 supported under ARCH_MXC which > does not use MXC_CLK. Ok. So then your change is also limiting the compilation of drivers/clk/imx/ to only the platforms that select it now, instead of all ARCH_MXC like it was done before. We can also have def_bool <big list of SoC platforms> if that helps for the optimization that this patch is making. arm-soc has generally pushed against having so many different arch level Kconfig options because they make for unwieldy Kconfig fragments that may become conflicting. Instead, the options for the different SoCs are removed and we just have one for the platform and rely on defconfigs to pick the right drivers. Why can't we do that here? > > > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index > > > 5c0b11e..f850424 100644 > > > --- a/drivers/clk/imx/Makefile > > > +++ b/drivers/clk/imx/Makefile > > > @@ -1,6 +1,6 @@ > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > -obj-y += \ > > > +obj-$(CONFIG_MXC_CLK) += \ > > > > Because this changes to obj-$(CONFIG) we should change the > > drivers/clk/Makefile to have obj-y for this Makefile instead of depending on > > ARCH_MXC. > > We also use ARCH_MXC for ARMv8 SoC clocks. Yes, and? I'm suggesting drivers/clk/Makefile have an obj-y += drivers/clk/imx/ and then decide to compile or not compile the MXC_CLK "core" bits based on CONFIG_MXC_CLK config option.
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Quoting Aisheng Dong (2018-12-10 18:36:29) > > > -----Original Message----- > > > From: Stephen Boyd [mailto:sboyd@kernel.org] > > > Sent: Tuesday, December 11, 2018 4:55 AM > > [...] > > > Subject: Re: [PATCH V9 1/7] clk: imx: add configuration option for > > > mmio clks > > > > > > Quoting Aisheng DONG (2018-12-04 06:39:22) > > > > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > > > > index c12a05c..303082c 100644 > > > > --- a/arch/arm/mach-imx/Kconfig > > > > +++ b/arch/arm/mach-imx/Kconfig > > > > @@ -570,6 +580,7 @@ config SOC_IMX7ULP config SOC_VF610 > > > > bool "Vybrid Family VF610 support" > > > > select ARM_GIC if ARCH_MULTI_V7 > > > > + select MXC_CLK > > > > select PINCTRL_VF610 > > > > > > > > > > Instead of select can we break this dependency on the arch Kconfig > > > and > > > have: > > > > > > config MXC_CLK > > > def_bool ARCH_MXC && !ARM64 > > > > > > > Thanks for the suggestion. > > One little problem is that we also have LS1021 supported under > > ARCH_MXC which does not use MXC_CLK. > > Ok. So then your change is also limiting the compilation of drivers/clk/imx/ to > only the platforms that select it now, instead of all ARCH_MXC like it was done > before. > > We can also have def_bool <big list of SoC platforms> if that helps for the > optimization that this patch is making. > > arm-soc has generally pushed against having so many different arch level > Kconfig options because they make for unwieldy Kconfig fragments that may > become conflicting. Instead, the options for the different SoCs are removed > and we just have one for the platform and rely on defconfigs to pick the right > drivers. > Thanks for telling the detailed background. Very clear now. > Why can't we do that here? > > > > > > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > > > > index > > > > 5c0b11e..f850424 100644 > > > > --- a/drivers/clk/imx/Makefile > > > > +++ b/drivers/clk/imx/Makefile > > > > @@ -1,6 +1,6 @@ > > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > > > -obj-y += \ > > > > +obj-$(CONFIG_MXC_CLK) += \ > > > > > > Because this changes to obj-$(CONFIG) we should change the > > > drivers/clk/Makefile to have obj-y for this Makefile instead of > > > depending on ARCH_MXC. > > > > We also use ARCH_MXC for ARMv8 SoC clocks. > > Yes, and? I'm suggesting drivers/clk/Makefile have an obj-y += drivers/clk/imx/ > and then decide to compile or not compile the MXC_CLK "core" bits based on > CONFIG_MXC_CLK config option. Sorry for the missunderstanding before. I guess you point is have an ojb-y += drivrs/clk/imx/ Then in drivers/clk/imx/Kconfig config MXC_CLK def_bool ARCH_MXC && !ARM64 select MXC_clk bits config MXC_CLK_SCU def_bool ARCH_MXC && ARM64 select SCU clock bits If wrong please let me know. Will update the patch series and re-send. Thanks for the suggestion. Regards Dong Aisheng
Hi Stephen, > > > > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > > > > > index > > > > > 5c0b11e..f850424 100644 > > > > > --- a/drivers/clk/imx/Makefile > > > > > +++ b/drivers/clk/imx/Makefile > > > > > @@ -1,6 +1,6 @@ > > > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > > > > > -obj-y += \ > > > > > +obj-$(CONFIG_MXC_CLK) += \ > > > > > > > > Because this changes to obj-$(CONFIG) we should change the > > > > drivers/clk/Makefile to have obj-y for this Makefile instead of > > > > depending on ARCH_MXC. > > > > > > We also use ARCH_MXC for ARMv8 SoC clocks. > > > > Yes, and? I'm suggesting drivers/clk/Makefile have an obj-y += > > drivers/clk/imx/ and then decide to compile or not compile the MXC_CLK > > "core" bits based on CONFIG_MXC_CLK config option. > > Sorry for the missunderstanding before. > I guess you point is have an ojb-y += drivrs/clk/imx/ Then in > drivers/clk/imx/Kconfig config MXC_CLK > def_bool ARCH_MXC && !ARM64 > select MXC_clk bits > config MXC_CLK_SCU > def_bool ARCH_MXC && ARM64 > select SCU clock bits > > If wrong please let me know. > > Will update the patch series and re-send. > Thanks for the suggestion. > I tried and met another problem that MX8MQ (ARM64) is also using legacy MXC_CLK. (And 3 more MX8M series based chips.) That means I have to write something like: config MXC_CLK bool def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && SOC_IMX8MQ) config MXC_CLK_SCU bool def_bool (ARCH_MXC && ARM64 && !SOC_IMX8MQ) But it can't work as ARM64 supports multiplatforms. e.g. we have also SOC_IMX8QXP, SOC_IMX8QM, SOC_IMX8DM ... Do you have a suggestion about this? Regards Dong Aisheng > Regards > Dong Aisheng
Quoting Aisheng Dong (2018-12-12 02:27:24) > Hi Stephen, > > > > > > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > > > > > > index > > > > > > 5c0b11e..f850424 100644 > > > > > > --- a/drivers/clk/imx/Makefile > > > > > > +++ b/drivers/clk/imx/Makefile > > > > > > @@ -1,6 +1,6 @@ > > > > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > > > > > > > -obj-y += \ > > > > > > +obj-$(CONFIG_MXC_CLK) += \ > > > > > > > > > > Because this changes to obj-$(CONFIG) we should change the > > > > > drivers/clk/Makefile to have obj-y for this Makefile instead of > > > > > depending on ARCH_MXC. > > > > > > > > We also use ARCH_MXC for ARMv8 SoC clocks. > > > > > > Yes, and? I'm suggesting drivers/clk/Makefile have an obj-y += > > > drivers/clk/imx/ and then decide to compile or not compile the MXC_CLK > > > "core" bits based on CONFIG_MXC_CLK config option. > > > > Sorry for the missunderstanding before. > > I guess you point is have an ojb-y += drivrs/clk/imx/ Then in > > drivers/clk/imx/Kconfig config MXC_CLK > > def_bool ARCH_MXC && !ARM64 > > select MXC_clk bits > > config MXC_CLK_SCU > > def_bool ARCH_MXC && ARM64 > > select SCU clock bits > > > > If wrong please let me know. > > > > Will update the patch series and re-send. > > Thanks for the suggestion. Yes that looks like what I'm asking for. > > > > I tried and met another problem that MX8MQ (ARM64) is also > using legacy MXC_CLK. (And 3 more MX8M series based chips.) > > That means I have to write something like: > config MXC_CLK > bool > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && SOC_IMX8MQ) > > config MXC_CLK_SCU > bool > def_bool (ARCH_MXC && ARM64 && !SOC_IMX8MQ) > > But it can't work as ARM64 supports multiplatforms. > e.g. we have also SOC_IMX8QXP, SOC_IMX8QM, SOC_IMX8DM ... > > Do you have a suggestion about this? > Can you put the enabling of MXC_CLK_SCU in the defconfig and exposed it as a user selectable option? And keep hiding the MXC_CLK option behind the def_bool? I hope that MXC_CLK could be user selectable eventually, but the def_bool is there to make it easier to bridge the transition and update everyone's defconfigs while it is moved into the defconfig.
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Sent: Thursday, December 13, 2018 6:24 AM > 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 <fabio.estevam@nxp.com>; dl-linux-imx > <linux-imx@nxp.com>; kernel@pengutronix.de > Subject: RE: [PATCH V9 1/7] clk: imx: add configuration option for mmio clks > > Quoting Aisheng Dong (2018-12-12 02:27:24) > > Hi Stephen, > > > > > > > > > diff --git a/drivers/clk/imx/Makefile > > > > > > > b/drivers/clk/imx/Makefile index > > > > > > > 5c0b11e..f850424 100644 > > > > > > > --- a/drivers/clk/imx/Makefile > > > > > > > +++ b/drivers/clk/imx/Makefile > > > > > > > @@ -1,6 +1,6 @@ > > > > > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > > > > > > > > > -obj-y += \ > > > > > > > +obj-$(CONFIG_MXC_CLK) += \ > > > > > > > > > > > > Because this changes to obj-$(CONFIG) we should change the > > > > > > drivers/clk/Makefile to have obj-y for this Makefile instead > > > > > > of depending on ARCH_MXC. > > > > > > > > > > We also use ARCH_MXC for ARMv8 SoC clocks. > > > > > > > > Yes, and? I'm suggesting drivers/clk/Makefile have an obj-y += > > > > drivers/clk/imx/ and then decide to compile or not compile the > > > > MXC_CLK "core" bits based on CONFIG_MXC_CLK config option. > > > > > > Sorry for the missunderstanding before. > > > I guess you point is have an ojb-y += drivrs/clk/imx/ Then in > > > drivers/clk/imx/Kconfig config MXC_CLK > > > def_bool ARCH_MXC && !ARM64 > > > select MXC_clk bits > > > config MXC_CLK_SCU > > > def_bool ARCH_MXC && ARM64 > > > select SCU clock bits > > > > > > If wrong please let me know. > > > > > > Will update the patch series and re-send. > > > Thanks for the suggestion. > > Yes that looks like what I'm asking for. > > > > > > > > I tried and met another problem that MX8MQ (ARM64) is also using > > legacy MXC_CLK. (And 3 more MX8M series based chips.) > > > > That means I have to write something like: > > config MXC_CLK > > bool > > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && > > SOC_IMX8MQ) > > > > config MXC_CLK_SCU > > bool > > def_bool (ARCH_MXC && ARM64 && !SOC_IMX8MQ) > > > > But it can't work as ARM64 supports multiplatforms. > > e.g. we have also SOC_IMX8QXP, SOC_IMX8QM, SOC_IMX8DM ... > > > > Do you have a suggestion about this? > > > > Can you put the enabling of MXC_CLK_SCU in the defconfig and exposed it as a > user selectable option? And keep hiding the MXC_CLK option behind the > def_bool? I hope that MXC_CLK could be user selectable eventually, but the > def_bool is there to make it easier to bridge the transition and update > everyone's defconfigs while it is moved into the defconfig. Sounds like a good suggestion. I will do it now. In order to avoid breaking MX8MQ, I will keep MXC_CLK as config MXC_CLK bool def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && SOC_IMX8MQ) and make MXC_CLK_SCU a user selectable option. And finally we will make MXC_CLK selectable as well after the transition. If any issue please let me know. Regards Dong Aisheng
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index c12a05c..303082c 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -57,23 +57,27 @@ config SOC_IMX21 select CPU_ARM926T select IMX_HAVE_IOMUX_V1 select MXC_AVIC + select MXC_CLK config SOC_IMX27 bool select CPU_ARM926T select IMX_HAVE_IOMUX_V1 select MXC_AVIC + select MXC_CLK select PINCTRL_IMX27 config SOC_IMX31 bool select CPU_V6 select MXC_AVIC + select MXC_CLK config SOC_IMX35 bool select ARCH_MXC_IOMUX_V3 select MXC_AVIC + select MXC_CLK select PINCTRL_IMX35 if ARCH_MULTI_V5 @@ -417,6 +421,7 @@ config SOC_IMX1 bool "i.MX1 support" select CPU_ARM920T select MXC_AVIC + select MXC_CLK select PINCTRL_IMX1 help This enables support for Freescale i.MX1 processor @@ -430,6 +435,7 @@ config SOC_IMX25 select ARCH_MXC_IOMUX_V3 select CPU_ARM926T select MXC_AVIC + select MXC_CLK select PINCTRL_IMX25 help This enables support for Freescale i.MX25 processor @@ -442,6 +448,7 @@ comment "Cortex-A platforms" config SOC_IMX5 bool select HAVE_IMX_SRC + select MXC_CLK select MXC_TZIC config SOC_IMX50 @@ -478,6 +485,7 @@ config SOC_IMX6 select HAVE_IMX_MMDC select HAVE_IMX_SRC select MFD_SYSCON + select MXC_CLK select PL310_ERRATA_769419 if CACHE_L2X0 config SOC_IMX6Q @@ -545,10 +553,12 @@ config SOC_IMX7D_CA7 select HAVE_IMX_MMDC select HAVE_IMX_SRC select IMX_GPCV2 + select MXC_CLK config SOC_IMX7D_CM4 bool select ARMV7M_SYSTICK + select MXC_CLK config SOC_IMX7D bool "i.MX7 Dual support" @@ -570,6 +580,7 @@ config SOC_IMX7ULP config SOC_VF610 bool "Vybrid Family VF610 support" select ARM_GIC if ARCH_MULTI_V7 + select MXC_CLK select PINCTRL_VF610 help diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 81cdb4e..1dbfcc2 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -287,6 +287,7 @@ source "drivers/clk/actions/Kconfig" source "drivers/clk/bcm/Kconfig" source "drivers/clk/hisilicon/Kconfig" source "drivers/clk/imgtec/Kconfig" +source "drivers/clk/imx/Kconfig" source "drivers/clk/ingenic/Kconfig" source "drivers/clk/keystone/Kconfig" source "drivers/clk/mediatek/Kconfig" diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig new file mode 100644 index 0000000..43a3ecc --- /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 + depends on ARCH_MXC diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index 5c0b11e..f850424 100644 --- a/drivers/clk/imx/Makefile +++ b/drivers/clk/imx/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y += \ +obj-$(CONFIG_MXC_CLK) += \ clk.o \ clk-busy.o \ clk-composite-8m.o \
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 <fabio.estevam@nxp.com> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Michael Turquette <mturquette@baylibre.com> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- arch/arm/mach-imx/Kconfig | 11 +++++++++++ drivers/clk/Kconfig | 1 + drivers/clk/imx/Kconfig | 5 +++++ drivers/clk/imx/Makefile | 2 +- 4 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/imx/Kconfig