| Message ID | 20190805075812.1056069-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | power: reset: make reboot-mode user selectable | expand |
Hi Arnd and Nandor, On Mon, Aug 05, 2019 at 09:57:15AM +0200, Arnd Bergmann wrote: > Nandor Han points out that there might be drivers that can use > the reboot-mode interfaces but might also be usable on configurations > without device tree. > > Move the 'depends on OF' dependency into CONFIG_REBOOT_MODE since > that is the only thing that truely has a compile-time dependency > on CONFIG_OF, and make it user visible to make it possible to > disable it. > > The drivers that used to 'select REBOOT_MODE' of course now have to > use 'depends on REBOOT_MODE instead'. With this, we can soften the > dependency and allow compile-testing the three front-end drivers > on non-OF platforms. > > Note: anyone who was using a reboot mode driver in their kernel > configuration now has to enable CONFIG_REBOOT_MODE as well. > > Suggested-by: Nandor Han <nandor.han@vaisala.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > Nandor, this is so far untested, could you make sure this > works in all configurations and forward it along with the > bugfix? > > Sebastian, I'm not convinced this is a good idea, just sending > it as Nandor requested. Please decide for yourself. > --- (Sorry for late reply) This patch does not look good to me. Better patch would be to allow compiling CONFIG_REBOOT_MODE without CONFIG_OF. Obviously the configuration would not be useful for anything except compile testing, but that is also true for this patch. Making the small OF dependent section in reboot_mode_register() would be a smaller patch with bigger test coverage, without breakng existing configurations. -- Sebastian > arch/arm/configs/davinci_all_defconfig | 1 + > arch/arm64/configs/defconfig | 1 + > drivers/power/reset/Kconfig | 22 +++++++++++++--------- > include/linux/reboot-mode.h | 20 ++++++++++++++++++++ > 4 files changed, 35 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig > index b34970ce6b31..ebf506c01899 100644 > --- a/arch/arm/configs/davinci_all_defconfig > +++ b/arch/arm/configs/davinci_all_defconfig > @@ -139,6 +139,7 @@ CONFIG_GPIO_PCA953X_IRQ=y > CONFIG_RESET_CONTROLLER=y > CONFIG_POWER_RESET=y > CONFIG_POWER_RESET_GPIO=y > +CONFIG_REBOOT_MODE=m > CONFIG_SYSCON_REBOOT_MODE=m > CONFIG_BATTERY_LEGO_EV3=m > CONFIG_WATCHDOG=y > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > index 0e58ef02880c..bb7d7bec1413 100644 > --- a/arch/arm64/configs/defconfig > +++ b/arch/arm64/configs/defconfig > @@ -419,6 +419,7 @@ CONFIG_ROCKCHIP_IODOMAIN=y > CONFIG_POWER_RESET_MSM=y > CONFIG_POWER_RESET_XGENE=y > CONFIG_POWER_RESET_SYSCON=y > +CONFIG_REBOOT_MODE=y > CONFIG_SYSCON_REBOOT_MODE=y > CONFIG_BATTERY_SBS=m > CONFIG_BATTERY_BQ27XXX=y > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index a564237278ff..997323d443f5 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -9,6 +9,13 @@ menuconfig POWER_RESET > > if POWER_RESET > > +config REBOOT_MODE > + tristate "Pass reboot-mode to firmware" > + depends on OF > + help > + Some drivers allow setting the reboot mode through a platform > + interface that the boot firmware can read. > + > config POWER_RESET_AS3722 > bool "ams AS3722 power-off driver" > depends on MFD_AS3722 > @@ -107,9 +114,9 @@ config POWER_RESET_MSM > > config POWER_RESET_QCOM_PON > tristate "Qualcomm power-on driver" > - depends on ARCH_QCOM > + depends on ARCH_QCOM || COMPILE_TEST > depends on MFD_SPMI_PMIC > - select REBOOT_MODE > + depends on REBOOT_MODE || !REBOOT_MODE > help > Power On support for Qualcomm boards. > If you have a Qualcomm platform and need support for > @@ -223,14 +230,11 @@ config POWER_RESET_ZX > help > Reboot support for ZTE SoCs. > > -config REBOOT_MODE > - tristate > - > config SYSCON_REBOOT_MODE > tristate "Generic SYSCON regmap reboot mode driver" > - depends on OF > + depends on OF || COMPILE_TEST > depends on MFD_SYSCON > - select REBOOT_MODE > + depends on REBOOT_MODE || !REBOOT_MODE > help > Say y here will enable reboot mode driver. This will > get reboot mode arguments and store it in SYSCON mapped > @@ -248,8 +252,8 @@ config POWER_RESET_SC27XX > > config NVMEM_REBOOT_MODE > tristate "Generic NVMEM reboot mode driver" > - depends on OF > - select REBOOT_MODE > + depends on OF || COMPILE_TEST > + depends on REBOOT_MODE || !REBOOT_MODE > help > Say y here will enable reboot mode driver. This will > get reboot mode arguments and store it in a NVMEM cell, > diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h > index 4a2abb38d1d6..bd002060e3d0 100644 > --- a/include/linux/reboot-mode.h > +++ b/include/linux/reboot-mode.h > @@ -9,11 +9,31 @@ struct reboot_mode_driver { > struct notifier_block reboot_notifier; > }; > > +#if IS_ENABLED(CONFIG_REBOOT_MODE) > int reboot_mode_register(struct reboot_mode_driver *reboot); > int reboot_mode_unregister(struct reboot_mode_driver *reboot); > int devm_reboot_mode_register(struct device *dev, > struct reboot_mode_driver *reboot); > void devm_reboot_mode_unregister(struct device *dev, > struct reboot_mode_driver *reboot); > +#else > +static inline int reboot_mode_register(struct reboot_mode_driver *reboot) > +{ > + return 0; > +} > +static inline int reboot_mode_unregister(struct reboot_mode_driver *reboot) > +{ > + return 0; > +} > +static inline int devm_reboot_mode_register(struct device *dev, > + struct reboot_mode_driver *reboot) > +{ > + return 0; > +} > +static inline void devm_reboot_mode_unregister(struct device *dev, > + struct reboot_mode_driver *reboot) > +{ > +} > +#endif > > #endif > -- > 2.20.0 >
On Mon, Sep 2, 2019 at 10:39 PM Sebastian Reichel <sre@kernel.org> wrote: > > This patch does not look good to me. Better patch would be to > allow compiling CONFIG_REBOOT_MODE without CONFIG_OF. Obviously > the configuration would not be useful for anything except compile > testing, but that is also true for this patch. Ok, I'd suggest we leave it with the bugfix you already applied then. [caa2b55784, power: reset: nvmem-reboot-mode: add CONFIG_OF dependency] Arnd
Hi, On Mon, Sep 02, 2019 at 11:16:27PM +0200, Arnd Bergmann wrote: > On Mon, Sep 2, 2019 at 10:39 PM Sebastian Reichel <sre@kernel.org> wrote: > > This patch does not look good to me. Better patch would be to > > allow compiling CONFIG_REBOOT_MODE without CONFIG_OF. Obviously > > the configuration would not be useful for anything except compile > > testing, but that is also true for this patch. > > Ok, I'd suggest we leave it with the bugfix you already applied then. > [caa2b55784, power: reset: nvmem-reboot-mode: add CONFIG_OF dependency] That's also fine with me. -- Sebastian
On 9/3/19 3:04 AM, Sebastian Reichel wrote: > Hi, > > On Mon, Sep 02, 2019 at 11:16:27PM +0200, Arnd Bergmann wrote: >> On Mon, Sep 2, 2019 at 10:39 PM Sebastian Reichel <sre@kernel.org> wrote: >>> This patch does not look good to me. Better patch would be to >>> allow compiling CONFIG_REBOOT_MODE without CONFIG_OF. Obviously >>> the configuration would not be useful for anything except compile >>> testing, but that is also true for this patch. >> >> Ok, I'd suggest we leave it with the bugfix you already applied then. >> [caa2b55784, power: reset: nvmem-reboot-mode: add CONFIG_OF dependency] > > That's also fine with me. > > -- Sebastian > Sounds good to me. --- Nandor
diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig index b34970ce6b31..ebf506c01899 100644 --- a/arch/arm/configs/davinci_all_defconfig +++ b/arch/arm/configs/davinci_all_defconfig @@ -139,6 +139,7 @@ CONFIG_GPIO_PCA953X_IRQ=y CONFIG_RESET_CONTROLLER=y CONFIG_POWER_RESET=y CONFIG_POWER_RESET_GPIO=y +CONFIG_REBOOT_MODE=m CONFIG_SYSCON_REBOOT_MODE=m CONFIG_BATTERY_LEGO_EV3=m CONFIG_WATCHDOG=y diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 0e58ef02880c..bb7d7bec1413 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -419,6 +419,7 @@ CONFIG_ROCKCHIP_IODOMAIN=y CONFIG_POWER_RESET_MSM=y CONFIG_POWER_RESET_XGENE=y CONFIG_POWER_RESET_SYSCON=y +CONFIG_REBOOT_MODE=y CONFIG_SYSCON_REBOOT_MODE=y CONFIG_BATTERY_SBS=m CONFIG_BATTERY_BQ27XXX=y diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig index a564237278ff..997323d443f5 100644 --- a/drivers/power/reset/Kconfig +++ b/drivers/power/reset/Kconfig @@ -9,6 +9,13 @@ menuconfig POWER_RESET if POWER_RESET +config REBOOT_MODE + tristate "Pass reboot-mode to firmware" + depends on OF + help + Some drivers allow setting the reboot mode through a platform + interface that the boot firmware can read. + config POWER_RESET_AS3722 bool "ams AS3722 power-off driver" depends on MFD_AS3722 @@ -107,9 +114,9 @@ config POWER_RESET_MSM config POWER_RESET_QCOM_PON tristate "Qualcomm power-on driver" - depends on ARCH_QCOM + depends on ARCH_QCOM || COMPILE_TEST depends on MFD_SPMI_PMIC - select REBOOT_MODE + depends on REBOOT_MODE || !REBOOT_MODE help Power On support for Qualcomm boards. If you have a Qualcomm platform and need support for @@ -223,14 +230,11 @@ config POWER_RESET_ZX help Reboot support for ZTE SoCs. -config REBOOT_MODE - tristate - config SYSCON_REBOOT_MODE tristate "Generic SYSCON regmap reboot mode driver" - depends on OF + depends on OF || COMPILE_TEST depends on MFD_SYSCON - select REBOOT_MODE + depends on REBOOT_MODE || !REBOOT_MODE help Say y here will enable reboot mode driver. This will get reboot mode arguments and store it in SYSCON mapped @@ -248,8 +252,8 @@ config POWER_RESET_SC27XX config NVMEM_REBOOT_MODE tristate "Generic NVMEM reboot mode driver" - depends on OF - select REBOOT_MODE + depends on OF || COMPILE_TEST + depends on REBOOT_MODE || !REBOOT_MODE help Say y here will enable reboot mode driver. This will get reboot mode arguments and store it in a NVMEM cell, diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h index 4a2abb38d1d6..bd002060e3d0 100644 --- a/include/linux/reboot-mode.h +++ b/include/linux/reboot-mode.h @@ -9,11 +9,31 @@ struct reboot_mode_driver { struct notifier_block reboot_notifier; }; +#if IS_ENABLED(CONFIG_REBOOT_MODE) int reboot_mode_register(struct reboot_mode_driver *reboot); int reboot_mode_unregister(struct reboot_mode_driver *reboot); int devm_reboot_mode_register(struct device *dev, struct reboot_mode_driver *reboot); void devm_reboot_mode_unregister(struct device *dev, struct reboot_mode_driver *reboot); +#else +static inline int reboot_mode_register(struct reboot_mode_driver *reboot) +{ + return 0; +} +static inline int reboot_mode_unregister(struct reboot_mode_driver *reboot) +{ + return 0; +} +static inline int devm_reboot_mode_register(struct device *dev, + struct reboot_mode_driver *reboot) +{ + return 0; +} +static inline void devm_reboot_mode_unregister(struct device *dev, + struct reboot_mode_driver *reboot) +{ +} +#endif #endif
Nandor Han points out that there might be drivers that can use the reboot-mode interfaces but might also be usable on configurations without device tree. Move the 'depends on OF' dependency into CONFIG_REBOOT_MODE since that is the only thing that truely has a compile-time dependency on CONFIG_OF, and make it user visible to make it possible to disable it. The drivers that used to 'select REBOOT_MODE' of course now have to use 'depends on REBOOT_MODE instead'. With this, we can soften the dependency and allow compile-testing the three front-end drivers on non-OF platforms. Note: anyone who was using a reboot mode driver in their kernel configuration now has to enable CONFIG_REBOOT_MODE as well. Suggested-by: Nandor Han <nandor.han@vaisala.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- Nandor, this is so far untested, could you make sure this works in all configurations and forward it along with the bugfix? Sebastian, I'm not convinced this is a good idea, just sending it as Nandor requested. Please decide for yourself. --- arch/arm/configs/davinci_all_defconfig | 1 + arch/arm64/configs/defconfig | 1 + drivers/power/reset/Kconfig | 22 +++++++++++++--------- include/linux/reboot-mode.h | 20 ++++++++++++++++++++ 4 files changed, 35 insertions(+), 9 deletions(-)