clk: mediatek: adjust dependency of reset.c to avoid unexpectedly being built

Commit Message

Sean Wang Dec. 26, 2017, 3:52 a.m. UTC

From: Sean Wang <[e-mail]>

commit 74cb0d6dde8 ("clk: mediatek: fixup test-building of MediaTek clock
drivers") can let the build system looking into the directory where the
clock drivers resides and then allow test-building the drivers.

But the change also gives rise to certain incorrect behavior which is
reset.c being built even not depending on either COMPILE_TEST or
ARCH_MEDIATEK alternative dependency. To get rid of reset.c being built
unexpectedly on the other platforms, it would be a good change that the
file should be built depending on its own specific configuration rather
than just on generic RESET_CONTROLLER one.

Signed-off-by: Sean Wang <[e-mail]>
Cc: Jean Delvare <[e-mail]>
---
 drivers/clk/mediatek/Kconfig   | 7 +++++++
 drivers/clk/mediatek/Makefile  | 2 +-
 drivers/clk/mediatek/clk-mtk.h | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

Comments

Stephen Boyd Dec. 27, 2017, 1:19 a.m. UTC | #1

On 12/26, [e-mail] wrote:
> From: Sean Wang <[e-mail]>
> 
> commit 74cb0d6dde8 ("clk: mediatek: fixup test-building of MediaTek clock
> drivers") can let the build system looking into the directory where the
> clock drivers resides and then allow test-building the drivers.
> 
> But the change also gives rise to certain incorrect behavior which is
> reset.c being built even not depending on either COMPILE_TEST or
> ARCH_MEDIATEK alternative dependency. To get rid of reset.c being built
> unexpectedly on the other platforms, it would be a good change that the
> file should be built depending on its own specific configuration rather
> than just on generic RESET_CONTROLLER one.
> 
> Signed-off-by: Sean Wang <[e-mail]>
> Cc: Jean Delvare <[e-mail]>

I've typically seen vendor Kconfigs select the RESET_CONTROLLER
framework if the vendor Kconfig is enabled. Any reason that same
method isn't followed here?

Sean Wang Dec. 27, 2017, 9:43 a.m. UTC | #2

On Tue, 2017-12-26 at 17:19 -0800, Stephen Boyd wrote:
> On 12/26, [e-mail] wrote:
> > From: Sean Wang <[e-mail]>
> > 
> > commit 74cb0d6dde8 ("clk: mediatek: fixup test-building of MediaTek clock
> > drivers") can let the build system looking into the directory where the
> > clock drivers resides and then allow test-building the drivers.
> > 
> > But the change also gives rise to certain incorrect behavior which is
> > reset.c being built even not depending on either COMPILE_TEST or
> > ARCH_MEDIATEK alternative dependency. To get rid of reset.c being built
> > unexpectedly on the other platforms, it would be a good change that the
> > file should be built depending on its own specific configuration rather
> > than just on generic RESET_CONTROLLER one.
> > 
> > Signed-off-by: Sean Wang <[e-mail]>
> > Cc: Jean Delvare <[e-mail]>
> 
> I've typically seen vendor Kconfigs select the RESET_CONTROLLER
> framework if the vendor Kconfig is enabled. Any reason that same
> method isn't followed here?
> 

I just thought explicit dependency added in Kconfig seems a little good
no matter how the vendor Kconfig forces to select.

But, I believe reset controller is always present on every mediatek SoC,
at least it can be found on infracfg and pericfg subsystem, which is
really fundamental hardware block. So, it's still quite reasonable to
add "RESET_CONTROLLER" to vendor Kconfig.

Once we did it in vendor Kconfig, the Kconfig maybe could become
something like that.

config RESET_MEDIATEK
       bool "MediaTek Reset Driver"
       depends on ARCH_MEDIATEK || (RESET_CONTROLLER && COMPILE_TEST)
       help
         This enables the reset controller driver used on MediaTek SoCs.

where COMPILE_TEST still has to depend on RESET_CONTROLLER to avoid any
compiling error.

I'll make the next version based on above and relevant vendor Kconfig
changes

	Sean



--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stephen Boyd Dec. 27, 2017, 6:19 p.m. UTC | #3

On 12/27, Sean Wang wrote:
> On Tue, 2017-12-26 at 17:19 -0800, Stephen Boyd wrote:
> > On 12/26, [e-mail] wrote:
> > > From: Sean Wang <[e-mail]>
> > > 
> > > commit 74cb0d6dde8 ("clk: mediatek: fixup test-building of MediaTek clock
> > > drivers") can let the build system looking into the directory where the
> > > clock drivers resides and then allow test-building the drivers.
> > > 
> > > But the change also gives rise to certain incorrect behavior which is
> > > reset.c being built even not depending on either COMPILE_TEST or
> > > ARCH_MEDIATEK alternative dependency. To get rid of reset.c being built
> > > unexpectedly on the other platforms, it would be a good change that the
> > > file should be built depending on its own specific configuration rather
> > > than just on generic RESET_CONTROLLER one.
> > > 
> > > Signed-off-by: Sean Wang <[e-mail]>
> > > Cc: Jean Delvare <[e-mail]>
> > 
> > I've typically seen vendor Kconfigs select the RESET_CONTROLLER
> > framework if the vendor Kconfig is enabled. Any reason that same
> > method isn't followed here?
> > 
> 
> I just thought explicit dependency added in Kconfig seems a little good
> no matter how the vendor Kconfig forces to select.
> 
> But, I believe reset controller is always present on every mediatek SoC,
> at least it can be found on infracfg and pericfg subsystem, which is
> really fundamental hardware block. So, it's still quite reasonable to
> add "RESET_CONTROLLER" to vendor Kconfig.
> 
> Once we did it in vendor Kconfig, the Kconfig maybe could become
> something like that.
> 
> config RESET_MEDIATEK
>        bool "MediaTek Reset Driver"
>        depends on ARCH_MEDIATEK || (RESET_CONTROLLER && COMPILE_TEST)
>        help
>          This enables the reset controller driver used on MediaTek SoCs.
> 
> where COMPILE_TEST still has to depend on RESET_CONTROLLER to avoid any
> compiling error.
> 

Why can't we have 'select RESET_CONTROLLER' under
COMMON_CLK_MEDIATEK? Is there something that RESET_CONTROLLER
depends on that may not be enabled?

Patch

diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index 7338f81..52d880a 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -9,6 +9,13 @@  config COMMON_CLK_MEDIATEK
 	---help---
 	  MediaTek SoCs' clock support.
 
+config RESET_MEDIATEK
+	bool "MediaTek Reset Driver"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on RESET_CONTROLLER
+	help
+	  This enables the reset controller driver used on MediaTek SoCs.
+
 config COMMON_CLK_MT2701
 	bool "Clock driver for MediaTek MT2701"
 	depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST
diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index c421ffc..2a48006 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o
-obj-$(CONFIG_RESET_CONTROLLER) += reset.o
+obj-$(CONFIG_RESET_MEDIATEK) += reset.o
 obj-$(CONFIG_COMMON_CLK_MT6797) += clk-mt6797.o
 obj-$(CONFIG_COMMON_CLK_MT6797_IMGSYS) += clk-mt6797-img.o
 obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index bf8006d..0322dec 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -229,7 +229,7 @@  void mtk_clk_register_plls(struct device_node *node,
 struct clk *mtk_clk_register_ref2usb_tx(const char *name,
 			const char *parent_name, void __iomem *reg);
 
-#ifdef CONFIG_RESET_CONTROLLER
+#ifdef CONFIG_RESET_MEDIATEK
 void mtk_register_reset_controller(struct device_node *np,
 			unsigned int num_regs, int regofs);
 #else