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

[Michal Simek]

On 17.7.2018 05:45, Simon Glass wrote:
> 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?

I have sent v2 with reset. TBH adding restart to enum will just cause
another confusion when we don't know what's the exact difference.

Thanks,
Michal