[U-Boot] [PATCH] sysreset: Add support for gpio-restart

[Simon Glass]

Hi Michal,

On 15 July 2018 at 23:31, Michal Simek <michal.simek at xilinx.com> wrote:
> On 16.7.2018 07:20, Simon Glass wrote:
>> Hi Michal,
>>
>> On 13 July 2018 at 03:15, Michal Simek <michal.simek at xilinx.com> wrote:
>>> The Linux kernel has binding for gpio-restart node.
>>> This patch is adding basic support without supporting any optional
>>> properties.
>>> This driver was tested on Microblaze system where gpio is connected to
>>> SoC reset logic.
>>> Output value is handled via gpios cells values.
>>>
>>> In gpio_reboot_request() set_value is writing 1 because
>>> dm_gpio_set_value() is capable to changing it when it is ACTIVE_LOW.
>>> ...
>>>         if (desc->flags & GPIOD_ACTIVE_LOW)
>>>                 value = !value;
>>> ...
>>>
>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>> ---
>>>
>>>  drivers/sysreset/Kconfig         |  6 ++++
>>>  drivers/sysreset/Makefile        |  1 +
>>>  drivers/sysreset/sysreset_gpio.c | 59 ++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 66 insertions(+)
>>>  create mode 100644 drivers/sysreset/sysreset_gpio.c
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> But please see below.
>>
>>>
>>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>>> index a6d48e8a662c..1e228b97443a 100644
>>> --- a/drivers/sysreset/Kconfig
>>> +++ b/drivers/sysreset/Kconfig
>>> @@ -15,6 +15,12 @@ config SYSRESET
>>>
>>>  if SYSRESET
>>>
>>> +config SYSRESET_GPIO
>>> +       bool "Enable support for GPIO restart driver"
>>> +       select GPIO
>>> +       help
>>> +         Restart support via GPIO pin connected reset logic.
>>
>> What does this mean? Please expand this to explain what it means.
>>
>> Also, what is the difference between restart and reset? If there is no
>> difference please use the word 'reset'. If there is a difference,
>> please explain it here.
>
> I have taken restart name because this is what it is written Linux kernel.
>
> Based on this explanation:
> https://kb.netgear.com/1001/Defining-Terms-Power-Cycle-Boot-Reboot-Restart-Reset-and-Hard-Reset
>
> "Unlike a reset which changes something, a restart means to turn
> something on, possibly without changing settings. When upgrading
> firmware or software you are often asked to restart. A restart would be
> probably be used if there were a major change to functionality, while a
> reset often just changes settings of existing functionality."
>

Perhaps this corresponds to warm and cold reset? I'm not sure. But I
don't know of a board which supports resetting without changing
anything. At the least the CPU is put back into a start where it can
start at its reset vector.

This really doesn't make any sense to me. I think we should stick with
'reset' to avoid confusion. By all means add some notes to the uclass
header file about what it should mean. We already have enum
sysreset_t, so you could define 'restart' there, perhaps?

Regards,
Simon