diff mbox series

power: reset: make reboot-mode user selectable

Message ID 20190805075812.1056069-1-arnd@arndb.de
State New
Headers show
Series power: reset: make reboot-mode user selectable | expand

Commit Message

Arnd Bergmann Aug. 5, 2019, 7:57 a.m. UTC 
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(-)

-- 
2.20.0

Comments

Sebastian Reichel Sept. 2, 2019, 8:38 p.m. UTC | #1 
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

>
Arnd Bergmann Sept. 2, 2019, 9:16 p.m. UTC | #2 
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
Sebastian Reichel Sept. 3, 2019, 12:04 a.m. UTC | #3 
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
Nandor Han Sept. 4, 2019, 12:13 p.m. UTC | #4 
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 mbox series

Patch

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