linux-kernel - Re: [PATCH v9 2/4] power: reset: add reboot mode driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <5767E22F.5030207@samsung.com>
Date:	Mon, 20 Jun 2016 14:31:43 +0200
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Andy Yan <andy.yan@...k-chips.com>, robh+dt@...nel.org,
	heiko@...ech.de, john.stultz@...aro.org, arnd@...db.de,
	bjorn.andersson@...aro.org
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	alexandre.belloni@...e-electrons.com, dbaryshkov@...il.com,
	sre@...nel.org, matthias.bgg@...il.com, linux-pm@...r.kernel.org,
	mbrugger@...e.com, lorenzo.pieralisi@....com,
	moritz.fischer@...us.com, richard@....at,
	Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: Re: [PATCH v9 2/4] power: reset: add reboot mode driver

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@...il.com>
>>> Reviewed-by: Moritz Fischer <moritz.fischer@...us.com>
>>> Tested-by: John Stultz <john.stultz@...aro.org>
>>> Acked-by: John Stultz <john.stultz@...aro.org>
>>> Signed-off-by: Andy Yan <andy.yan@...k-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