Messages in this thread | | | Subject | Re: [PATCH v9 2/4] power: reset: add reboot mode driver | From | Krzysztof Kozlowski <> | Date | Mon, 20 Jun 2016 14:31:43 +0200 |
| |
On 06/20/2016 10:28 AM, Andy Yan wrote: > Hi Krzysztof: > > On 2016年06月20日 16:09, Krzysztof Kozlowski wrote: >> On 06/20/2016 08:38 AM, Andy Yan wrote: >>> This driver parses the reboot commands like "reboot bootloader" >>> and "reboot recovery" to get a boot mode described in the >>> device tree , then call the write interfae to store the boot >>> mode in some place like special register or sram, which can >>> be read by the bootloader after system reboot, then the bootloader >>> can take different action according to the mode stored. >>> >>> This is commonly used on Android based devices, in order to >>> reboot the device into fastboot or recovery mode. >>> >>> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >>> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >>> Tested-by: John Stultz <john.stultz@linaro.org> >>> Acked-by: John Stultz <john.stultz@linaro.org> >>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >>> >>> --- >>> >>> Changes in v9: >>> - select MFD_SYSCON when syscon-reboot-mode driver enabled >>> - refactoring error handling in reboot_mode_register >>> >>> Changes in v8: >>> - do some cleanup when driver ubind >>> >>> Changes in v7: >>> - address some suggestions from Krzysztof, make syscon-reboot-mode >>> driver data self-contained. >>> >>> Changes in v6: None >>> Changes in v5: >>> - use two blank space under help in Kconfig >>> - use unsigned int instead of int for member magic in struct mode_info >>> >>> Changes in v4: >>> - make this driver depends on OF to avoid kbuild test error >>> >>> Changes in v3: >>> - scan multi properities >>> - add mask value for some platform which only use some bits of the >>> register >>> to store boot mode magic value >>> >>> Changes in v2: >>> - move to dir drivers/power/reset/ >>> - make syscon-reboot-mode a generic driver >>> >>> Changes in v1: >>> - fix the embarrassed compile warning >>> - correct the maskrom magic number >>> - check for the normal reboot >>> >>> drivers/power/reset/Kconfig | 14 ++++ >>> drivers/power/reset/Makefile | 2 + >>> drivers/power/reset/reboot-mode.c | 128 >>> +++++++++++++++++++++++++++++++ >>> drivers/power/reset/reboot-mode.h | 14 ++++ >>> drivers/power/reset/syscon-reboot-mode.c | 100 >>> ++++++++++++++++++++++++ >>> 5 files changed, 258 insertions(+) >>> create mode 100644 drivers/power/reset/reboot-mode.c >>> create mode 100644 drivers/power/reset/reboot-mode.h >>> create mode 100644 drivers/power/reset/syscon-reboot-mode.c >>> >>> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig >>> index 9bb2622..4ceaf74 100644 >>> --- a/drivers/power/reset/Kconfig >>> +++ b/drivers/power/reset/Kconfig >>> @@ -183,5 +183,19 @@ config POWER_RESET_ZX >>> help >>> Reboot support for ZTE SoCs. >>> +config REBOOT_MODE >>> + tristate >>> + >>> +config SYSCON_REBOOT_MODE >>> + bool "Generic SYSCON regmap reboot mode driver" >> Why not tristate? > > I see many reset drivers in this directories use bool, so I follow > them.
+Cc Paul,
I don't mind that although I don't see any particular objections for making it module-capable. In the same time I just reminded myself about Paul Gortmaker's long effort (like [1] [2]) about removing module capability from non-modular drivers.
Following his rationale, I think either this should be a tristate or the module stuff should be removed.
Best regards, Krzysztof
[1] https://lkml.org/lkml/2016/2/21/180 [2] https://lkml.org/lkml/2016/6/13/682
| |