| Message ID | 20200729160942.28867-6-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ARM: samsung: Cleanup of various S3C bits | expand |
On Wed, Jul 29, 2020 at 06:09:40PM +0200, Krzysztof Kozlowski wrote: > A separate Kconfig option HAVE_S3C2410_WATCHDOG for Samsung SoCs does > not have sense, because: > 1. All ARMv7 and ARMv8 Samsung SoCs have watchdog, > 2. All architecture Kconfigs were selecting it (if WATCHDOG framework is > chosen), > 3. HAVE_S3C2410_WATCHDOG is doing nothing except being a dependency of > actual Samsung SoC watchdog driver, which is enabled manually by > specific defconfigs. > > HAVE_S3C2410_WATCHDOG can be safely removed. > That is not really correct. HAVE_S3C2410_WATCHDOG is used to ensure that users can only enable S3C2410_WATCHDOG if the watchdog actually exists in a system. With this change, it can be enabled for all architectures and platforms. NACK. Guenter > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/Kconfig | 1 - > arch/arm/mach-exynos/Kconfig | 1 - > arch/arm/mach-s3c64xx/Kconfig | 2 -- > arch/arm/mach-s5pv210/Kconfig | 1 - > arch/arm64/Kconfig.platforms | 1 - > drivers/watchdog/Kconfig | 8 -------- > 6 files changed, 14 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 7564f293f107..fe95777af653 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -504,7 +504,6 @@ config ARCH_S3C24XX > select GPIOLIB > select GENERIC_IRQ_MULTI_HANDLER > select HAVE_S3C2410_I2C if I2C > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select HAVE_S3C_RTC if RTC_CLASS > select NEED_MACH_IO_H > select SAMSUNG_ATAGS > diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig > index f185cd3d4c62..d2d249706ebb 100644 > --- a/arch/arm/mach-exynos/Kconfig > +++ b/arch/arm/mach-exynos/Kconfig > @@ -24,7 +24,6 @@ menuconfig ARCH_EXYNOS > select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 > select HAVE_ARM_SCU if SMP > select HAVE_S3C2410_I2C if I2C > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select HAVE_S3C_RTC if RTC_CLASS > select PINCTRL > select PINCTRL_EXYNOS > diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig > index ac3e3563487f..e208c2b48853 100644 > --- a/arch/arm/mach-s3c64xx/Kconfig > +++ b/arch/arm/mach-s3c64xx/Kconfig > @@ -13,7 +13,6 @@ menuconfig ARCH_S3C64XX > select GPIO_SAMSUNG if ATAGS > select GPIOLIB > select HAVE_S3C2410_I2C if I2C > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select HAVE_TCM > select PLAT_SAMSUNG > select PM_GENERIC_DOMAINS if PM > @@ -165,7 +164,6 @@ config MACH_SMDK6410 > bool "SMDK6410" > depends on ATAGS > select CPU_S3C6410 > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select S3C64XX_SETUP_FB_24BPP > select S3C64XX_SETUP_I2C1 > select S3C64XX_SETUP_IDE > diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig > index 03984a791879..b3db1191e437 100644 > --- a/arch/arm/mach-s5pv210/Kconfig > +++ b/arch/arm/mach-s5pv210/Kconfig > @@ -14,7 +14,6 @@ config ARCH_S5PV210 > select COMMON_CLK_SAMSUNG > select GPIOLIB > select HAVE_S3C2410_I2C if I2C > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select HAVE_S3C_RTC if RTC_CLASS > select PINCTRL > select PINCTRL_EXYNOS > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index cd58f8495c45..d235b27cf372 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -80,7 +80,6 @@ config ARCH_EXYNOS > select EXYNOS_CHIPID > select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS > select EXYNOS_PMU > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > select HAVE_S3C_RTC if RTC_CLASS > select PINCTRL > select PINCTRL_EXYNOS > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 4f4687c46d38..ae86ea135d2b 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -478,16 +478,8 @@ config IXP4XX_WATCHDOG > > Say N if you are unsure. > > -config HAVE_S3C2410_WATCHDOG > - bool > - help > - This will include watchdog timer support for Samsung SoCs. If > - you want to include watchdog support for any machine, kindly > - select this in the respective mach-XXXX/Kconfig file. > - > config S3C2410_WATCHDOG > tristate "S3C2410 Watchdog" > - depends on HAVE_S3C2410_WATCHDOG || COMPILE_TEST > select WATCHDOG_CORE > select MFD_SYSCON if ARCH_EXYNOS > help > -- > 2.17.1 >
2020年7月29日(水) 19:02 Guenter Roeck <linux@roeck-us.net>: > > On Wed, Jul 29, 2020 at 06:09:40PM +0200, Krzysztof Kozlowski wrote: > > A separate Kconfig option HAVE_S3C2410_WATCHDOG for Samsung SoCs does > > not have sense, because: > > 1. All ARMv7 and ARMv8 Samsung SoCs have watchdog, > > 2. All architecture Kconfigs were selecting it (if WATCHDOG framework is > > chosen), > > 3. HAVE_S3C2410_WATCHDOG is doing nothing except being a dependency of > > actual Samsung SoC watchdog driver, which is enabled manually by > > specific defconfigs. > > > > HAVE_S3C2410_WATCHDOG can be safely removed. > > > > That is not really correct. HAVE_S3C2410_WATCHDOG is used to ensure > that users can only enable S3C2410_WATCHDOG if the watchdog actually > exists in a system. With this change, it can be enabled for all > architectures and platforms. > > NACK. > > Guenter > I'd side with Guenter on this. We better not flood users' screens with options that are not relevant to their hardware. An alternative here could be making CONFIG_S3C2410_WATCHDOG depend on a general symbol for Samsung SoC support if there is such, but then, are we 100% sure that all the Samsung SoCs would actually have exactly this watchdog? If a new one shows up, one would have to bring back this HAVE_S3C2410_WATCHDOG symbol. Best regards, Tomasz > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > arch/arm/Kconfig | 1 - > > arch/arm/mach-exynos/Kconfig | 1 - > > arch/arm/mach-s3c64xx/Kconfig | 2 -- > > arch/arm/mach-s5pv210/Kconfig | 1 - > > arch/arm64/Kconfig.platforms | 1 - > > drivers/watchdog/Kconfig | 8 -------- > > 6 files changed, 14 deletions(-) > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > index 7564f293f107..fe95777af653 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -504,7 +504,6 @@ config ARCH_S3C24XX > > select GPIOLIB > > select GENERIC_IRQ_MULTI_HANDLER > > select HAVE_S3C2410_I2C if I2C > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select HAVE_S3C_RTC if RTC_CLASS > > select NEED_MACH_IO_H > > select SAMSUNG_ATAGS > > diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig > > index f185cd3d4c62..d2d249706ebb 100644 > > --- a/arch/arm/mach-exynos/Kconfig > > +++ b/arch/arm/mach-exynos/Kconfig > > @@ -24,7 +24,6 @@ menuconfig ARCH_EXYNOS > > select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 > > select HAVE_ARM_SCU if SMP > > select HAVE_S3C2410_I2C if I2C > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select HAVE_S3C_RTC if RTC_CLASS > > select PINCTRL > > select PINCTRL_EXYNOS > > diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig > > index ac3e3563487f..e208c2b48853 100644 > > --- a/arch/arm/mach-s3c64xx/Kconfig > > +++ b/arch/arm/mach-s3c64xx/Kconfig > > @@ -13,7 +13,6 @@ menuconfig ARCH_S3C64XX > > select GPIO_SAMSUNG if ATAGS > > select GPIOLIB > > select HAVE_S3C2410_I2C if I2C > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select HAVE_TCM > > select PLAT_SAMSUNG > > select PM_GENERIC_DOMAINS if PM > > @@ -165,7 +164,6 @@ config MACH_SMDK6410 > > bool "SMDK6410" > > depends on ATAGS > > select CPU_S3C6410 > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select S3C64XX_SETUP_FB_24BPP > > select S3C64XX_SETUP_I2C1 > > select S3C64XX_SETUP_IDE > > diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig > > index 03984a791879..b3db1191e437 100644 > > --- a/arch/arm/mach-s5pv210/Kconfig > > +++ b/arch/arm/mach-s5pv210/Kconfig > > @@ -14,7 +14,6 @@ config ARCH_S5PV210 > > select COMMON_CLK_SAMSUNG > > select GPIOLIB > > select HAVE_S3C2410_I2C if I2C > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select HAVE_S3C_RTC if RTC_CLASS > > select PINCTRL > > select PINCTRL_EXYNOS > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > index cd58f8495c45..d235b27cf372 100644 > > --- a/arch/arm64/Kconfig.platforms > > +++ b/arch/arm64/Kconfig.platforms > > @@ -80,7 +80,6 @@ config ARCH_EXYNOS > > select EXYNOS_CHIPID > > select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS > > select EXYNOS_PMU > > - select HAVE_S3C2410_WATCHDOG if WATCHDOG > > select HAVE_S3C_RTC if RTC_CLASS > > select PINCTRL > > select PINCTRL_EXYNOS > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 4f4687c46d38..ae86ea135d2b 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -478,16 +478,8 @@ config IXP4XX_WATCHDOG > > > > Say N if you are unsure. > > > > -config HAVE_S3C2410_WATCHDOG > > - bool > > - help > > - This will include watchdog timer support for Samsung SoCs. If > > - you want to include watchdog support for any machine, kindly > > - select this in the respective mach-XXXX/Kconfig file. > > - > > config S3C2410_WATCHDOG > > tristate "S3C2410 Watchdog" > > - depends on HAVE_S3C2410_WATCHDOG || COMPILE_TEST > > select WATCHDOG_CORE > > select MFD_SYSCON if ARCH_EXYNOS > > help > > -- > > 2.17.1 > >
On Wed, Jul 29, 2020 at 07:36:38PM +0200, Tomasz Figa wrote: > 2020年7月29日(水) 19:02 Guenter Roeck <linux@roeck-us.net>: > > > > On Wed, Jul 29, 2020 at 06:09:40PM +0200, Krzysztof Kozlowski wrote: > > > A separate Kconfig option HAVE_S3C2410_WATCHDOG for Samsung SoCs does > > > not have sense, because: > > > 1. All ARMv7 and ARMv8 Samsung SoCs have watchdog, > > > 2. All architecture Kconfigs were selecting it (if WATCHDOG framework is > > > chosen), > > > 3. HAVE_S3C2410_WATCHDOG is doing nothing except being a dependency of > > > actual Samsung SoC watchdog driver, which is enabled manually by > > > specific defconfigs. > > > > > > HAVE_S3C2410_WATCHDOG can be safely removed. > > > > > > > That is not really correct. HAVE_S3C2410_WATCHDOG is used to ensure > > that users can only enable S3C2410_WATCHDOG if the watchdog actually > > exists in a system. With this change, it can be enabled for all > > architectures and platforms. > > > > NACK. > > > > Guenter > > > > I'd side with Guenter on this. We better not flood users' screens with > options that are not relevant to their hardware. > > An alternative here could be making CONFIG_S3C2410_WATCHDOG depend on > a general symbol for Samsung SoC support if there is such, but then, > are we 100% sure that all the Samsung SoCs would actually have exactly > this watchdog? If a new one shows up, one would have to bring back > this HAVE_S3C2410_WATCHDOG symbol. Ah, good points. Indeed for all of such SoC drivers we usually just depend on architecture to limit the choices on other architectures. In this case it would be: depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || COMPILE_TEST I admit it is pretty long, but we already use this pattern. In shorter version (less ARCH*) for all drivers, in full version also in: drivers/iio/adc/Kconfig drivers/gpu/drm/exynos/Kconfig Have in mind that in general we follow the first approach and only three drivers have still the HAVE_xxx option (also HAVE_S3C2410_I2C and HAVE_S3C_RTC). I can update therefore the "depends" while removing the HAVE_S3C2410_WATCHDOG option or just keep it. Best regards, Krzysztof
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 7564f293f107..fe95777af653 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -504,7 +504,6 @@ config ARCH_S3C24XX select GPIOLIB select GENERIC_IRQ_MULTI_HANDLER select HAVE_S3C2410_I2C if I2C - select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_S3C_RTC if RTC_CLASS select NEED_MACH_IO_H select SAMSUNG_ATAGS diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index f185cd3d4c62..d2d249706ebb 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -24,7 +24,6 @@ menuconfig ARCH_EXYNOS select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 select HAVE_ARM_SCU if SMP select HAVE_S3C2410_I2C if I2C - select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PINCTRL_EXYNOS diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index ac3e3563487f..e208c2b48853 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -13,7 +13,6 @@ menuconfig ARCH_S3C64XX select GPIO_SAMSUNG if ATAGS select GPIOLIB select HAVE_S3C2410_I2C if I2C - select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_TCM select PLAT_SAMSUNG select PM_GENERIC_DOMAINS if PM @@ -165,7 +164,6 @@ config MACH_SMDK6410 bool "SMDK6410" depends on ATAGS select CPU_S3C6410 - select HAVE_S3C2410_WATCHDOG if WATCHDOG select S3C64XX_SETUP_FB_24BPP select S3C64XX_SETUP_I2C1 select S3C64XX_SETUP_IDE diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig index 03984a791879..b3db1191e437 100644 --- a/arch/arm/mach-s5pv210/Kconfig +++ b/arch/arm/mach-s5pv210/Kconfig @@ -14,7 +14,6 @@ config ARCH_S5PV210 select COMMON_CLK_SAMSUNG select GPIOLIB select HAVE_S3C2410_I2C if I2C - select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PINCTRL_EXYNOS diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index cd58f8495c45..d235b27cf372 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -80,7 +80,6 @@ config ARCH_EXYNOS select EXYNOS_CHIPID select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select EXYNOS_PMU - select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PINCTRL_EXYNOS diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 4f4687c46d38..ae86ea135d2b 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -478,16 +478,8 @@ config IXP4XX_WATCHDOG Say N if you are unsure. -config HAVE_S3C2410_WATCHDOG - bool - help - This will include watchdog timer support for Samsung SoCs. If - you want to include watchdog support for any machine, kindly - select this in the respective mach-XXXX/Kconfig file. - config S3C2410_WATCHDOG tristate "S3C2410 Watchdog" - depends on HAVE_S3C2410_WATCHDOG || COMPILE_TEST select WATCHDOG_CORE select MFD_SYSCON if ARCH_EXYNOS help
A separate Kconfig option HAVE_S3C2410_WATCHDOG for Samsung SoCs does not have sense, because: 1. All ARMv7 and ARMv8 Samsung SoCs have watchdog, 2. All architecture Kconfigs were selecting it (if WATCHDOG framework is chosen), 3. HAVE_S3C2410_WATCHDOG is doing nothing except being a dependency of actual Samsung SoC watchdog driver, which is enabled manually by specific defconfigs. HAVE_S3C2410_WATCHDOG can be safely removed. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- arch/arm/Kconfig | 1 - arch/arm/mach-exynos/Kconfig | 1 - arch/arm/mach-s3c64xx/Kconfig | 2 -- arch/arm/mach-s5pv210/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/watchdog/Kconfig | 8 -------- 6 files changed, 14 deletions(-)