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 <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
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(-)
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
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 <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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