| Message ID | 49b95f19-4da6-4491-6ed7-5238ecfc35a8@free.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] clk: qcom: msm8916: Don't build support by default | expand |
On Wed 12 Jun 08:52 PDT 2019, Marc Gonzalez wrote: > Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when > we're building MSM_GCC_8916. > > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> Not sure why these are default at all. But both drivers are used on platforms other than 8916 as well, so if anything a fix would be to rename the APCS_MSM8916 to something more generic (such as QCOM_CLK_APCS_GLOBAL) - but then the content should be updated and the APCS mailbox driver as well... Regards, Bjorn > --- > drivers/clk/qcom/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index e1ff83cc361e..d5b065f64afc 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -21,7 +21,7 @@ if COMMON_CLK_QCOM > > config QCOM_A53PLL > tristate "MSM8916 A53 PLL" > - default ARCH_QCOM > + default MSM_GCC_8916 > help > Support for the A53 PLL on MSM8916 devices. It provides > the CPU with frequencies above 1GHz. > @@ -31,7 +31,7 @@ config QCOM_A53PLL > config QCOM_CLK_APCS_MSM8916 > tristate "MSM8916 APCS Clock Controller" > depends on QCOM_APCS_IPC || COMPILE_TEST > - default ARCH_QCOM > + default MSM_GCC_8916 > help > Support for the APCS Clock Controller on msm8916 devices. The > APCS is managing the mux and divider which feeds the CPUs. > -- > 2.17.1
Quoting Bjorn Andersson (2019-06-12 12:13:47) > On Wed 12 Jun 08:52 PDT 2019, Marc Gonzalez wrote: > > > Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when > > we're building MSM_GCC_8916. > > > > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> > > Not sure why these are default at all. > > But both drivers are used on platforms other than 8916 as well, so if > anything a fix would be to rename the APCS_MSM8916 to something more > generic (such as QCOM_CLK_APCS_GLOBAL) - but then the content should be > updated and the APCS mailbox driver as well... > I don't see any use in being this specific. I'd prefer we just leave this at the ARCH_FOO config level and not try anything more fancy.
On 12/06/2019 21:49, Stephen Boyd wrote: > Quoting Bjorn Andersson (2019-06-12 12:13:47) > >> On Wed 12 Jun 08:52 PDT 2019, Marc Gonzalez wrote: >> >>> Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when >>> we're building MSM_GCC_8916. >>> >>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> >> >> Not sure why these are default at all. >> >> But both drivers are used on platforms other than 8916 as well, so if >> anything a fix would be to rename the APCS_MSM8916 to something more >> generic (such as QCOM_CLK_APCS_GLOBAL) - but then the content should be >> updated and the APCS mailbox driver as well... > > I don't see any use in being this specific. I'd prefer we just leave > this at the ARCH_FOO config level and not try anything more fancy. As Bjorn pointed out, why do these default "on" at all? https://elixir.bootlin.com/linux/latest/source/drivers/clk/qcom/Kconfig There are dozens of config knobs in drivers/clk/qcom/Kconfig and only these two force the default. Let's remove the default altogether. Regards.
Quoting Marc Gonzalez (2019-06-12 13:03:22) > On 12/06/2019 21:49, Stephen Boyd wrote: > > > Quoting Bjorn Andersson (2019-06-12 12:13:47) > > > >> On Wed 12 Jun 08:52 PDT 2019, Marc Gonzalez wrote: > >> > >>> Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when > >>> we're building MSM_GCC_8916. > >>> > >>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> > >> > >> Not sure why these are default at all. > >> > >> But both drivers are used on platforms other than 8916 as well, so if > >> anything a fix would be to rename the APCS_MSM8916 to something more > >> generic (such as QCOM_CLK_APCS_GLOBAL) - but then the content should be > >> updated and the APCS mailbox driver as well... > > > > I don't see any use in being this specific. I'd prefer we just leave > > this at the ARCH_FOO config level and not try anything more fancy. > > As Bjorn pointed out, why do these default "on" at all? > > https://elixir.bootlin.com/linux/latest/source/drivers/clk/qcom/Kconfig > > There are dozens of config knobs in drivers/clk/qcom/Kconfig > and only these two force the default. > > Let's remove the default altogether. > Sure.
On 12/06/2019 21:13, Bjorn Andersson wrote: > On Wed 12 Jun 08:52 PDT 2019, Marc Gonzalez wrote: > >> Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when >> we're building MSM_GCC_8916. >> >> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> > > Not sure why these are default at all. > > But both drivers are used on platforms other than 8916 as well, so if > anything a fix would be to rename the APCS_MSM8916 to something more > generic (such as QCOM_CLK_APCS_GLOBAL) - but then the content should be > updated and the APCS mailbox driver as well... Used on platforms other than 8916? do you see that? $ git grep compatible drivers/clk/qcom/a53-pll.c { .compatible = "qcom,msm8916-a53pll" }, $ git grep qcom,msm8916-a53pll arch/arm64/boot/dts arch/arm64/boot/dts/qcom/msm8916.dtsi: compatible = "qcom,msm8916-a53pll"; drivers/clk/qcom/apcs-msm8916.c doesn't seem to support DT... $ git grep qcom-apcs-msm8916-clk drivers/clk/qcom/apcs-msm8916.c: .name = "qcom-apcs-msm8916-clk", drivers/mailbox/qcom-apcs-ipc-mailbox.c: "qcom-apcs-msm8916-clk", if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) { apcs->clk = platform_device_register_data(&pdev->dev, "qcom-apcs-msm8916-clk", -1, NULL, 0); $ git grep qcom,msm8916-apcs-kpss-global Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt: "qcom,msm8916-apcs-kpss-global", Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt: compatible = "qcom,msm8916-apcs-kpss-global"; arch/arm64/boot/dts/qcom/msm8916.dtsi: compatible = "qcom,msm8916-apcs-kpss-global", "syscon"; drivers/mailbox/qcom-apcs-ipc-mailbox.c: if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) { drivers/mailbox/qcom-apcs-ipc-mailbox.c: { .compatible = "qcom,msm8916-apcs-kpss-global", .data = (void *)8 }, Are you sure about other platforms? Regards.
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index e1ff83cc361e..d5b065f64afc 100644 --- a/drivers/clk/qcom/Kconfig +++ b/drivers/clk/qcom/Kconfig @@ -21,7 +21,7 @@ if COMMON_CLK_QCOM config QCOM_A53PLL tristate "MSM8916 A53 PLL" - default ARCH_QCOM + default MSM_GCC_8916 help Support for the A53 PLL on MSM8916 devices. It provides the CPU with frequencies above 1GHz. @@ -31,7 +31,7 @@ config QCOM_A53PLL config QCOM_CLK_APCS_MSM8916 tristate "MSM8916 APCS Clock Controller" depends on QCOM_APCS_IPC || COMPILE_TEST - default ARCH_QCOM + default MSM_GCC_8916 help Support for the APCS Clock Controller on msm8916 devices. The APCS is managing the mux and divider which feeds the CPUs.
Build QCOM_A53PLL and QCOM_CLK_APCS_MSM8916 by default only when we're building MSM_GCC_8916. Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> --- drivers/clk/qcom/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)