[U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode

[Simon Goldschmidt]

Simon,

I know I've slept for a while before retriggering this, but...

Am 04.05.2019 um 20:23 schrieb Simon Goldschmidt:
> Am 03.05.2019 um 23:10 schrieb Simon Glass:
>> Hi Simon,
>>
>> On Fri, 3 May 2019 at 14:43, Marek Vasut <marex at denx.de> wrote:
>>>
>>> On 5/3/19 10:40 PM, Simon Goldschmidt wrote:
>>>>
>>>>
>>>> On 03.05.19 22:37, Marek Vasut wrote:
>>>>> On 5/3/19 10:33 PM, Simon Goldschmidt wrote:
>>>>>>
>>>>>>
>>>>>> On 03.05.19 22:27, Marek Vasut wrote:
>>>>>>> On 5/3/19 10:25 PM, Simon Goldschmidt wrote:
>>>>>>>> This patch adds parameter support for the 'reset' command to specify
>>>>>>>> the reboot mode (cold vs. warm).
>>>>>>>>
>>>>>>>> Checking these parameters is implemented in the DM implementation.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>      cmd/boot.c                         |  4 ++--
>>>>>>>>      drivers/sysreset/sysreset-uclass.c | 17 ++++++++++++++++-
>>>>>>>>      2 files changed, 18 insertions(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/cmd/boot.c b/cmd/boot.c
>>>>>>>> index 9150fce80b..c3f33a9ca3 100644
>>>>>>>> --- a/cmd/boot.c
>>>>>>>> +++ b/cmd/boot.c
>>>>>>>> @@ -56,9 +56,9 @@ U_BOOT_CMD(
>>>>>>>>      #endif
>>>>>>>>        U_BOOT_CMD(
>>>>>>>> -    reset, 1, 0,    do_reset,
>>>>>>>> +    reset, 2, 0,    do_reset,
>>>>>>>>          "Perform RESET of the CPU",
>>>>>>>> -    ""
>>>>>>>> +    "[<cold|warm>] - type of reboot"
>>>>>>>>      );
>>>>>>>>        #ifdef CONFIG_CMD_POWEROFF
>>>>>>>> diff --git a/drivers/sysreset/sysreset-uclass.c
>>>>>>>> b/drivers/sysreset/sysreset-uclass.c
>>>>>>>> index ad831c703a..fbda3f44f2 100644
>>>>>>>> --- a/drivers/sysreset/sysreset-uclass.c
>>>>>>>> +++ b/drivers/sysreset/sysreset-uclass.c
>>>>>>>> @@ -111,9 +111,24 @@ void reset_cpu(ulong addr)
>>>>>>>>        int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>>>>>>>> argv[])
>>>>>>>>      {
>>>>>>>> +    enum sysreset_t reboot_mode = SYSRESET_COLD;
>>>>>>>> +
>>>>>>>> +    if (argc > 1 && argv[1]) {
>>>>>>>> +        switch (*argv[1]) {
>>>>>>>> +        case 'w':
>>>>>>>> +            reboot_mode = SYSRESET_WARM;
>>>>>>>> +            printf("warm ");
>>>>>>>> +            break;
>>>>>>>> +        case 'c':
>>>>>>>> +            reboot_mode = SYSRESET_COLD;
>>>>>>>> +            printf("cold ");
>>>>>>>> +            break;
>>
>> Please can we have a pytest for this command?
> 
> There's 'test_sandbox_exit.py' that seems to test that the "reset"
> command exits sandbox process. How would I test differing between "warm"
> and "cold" exit?

If this request for a test is what remains, can you please elaborate on 
what I should write to get this accepted?

Thanks,
Simon

> 
> Regards,
> Simon
> 
>>
>> Regards,
>> Simon
>>
>>>>>>>
>>>>>>> This looks like a platform or driver specific stuff ?
>>>>>>
>>>>>> Ouch, I just saw the extra printf might have to be removed in a v2...
>>>>>>
>>>>>> Anyway, except for that, I don't think it's platform or driver specific.
>>>>>> It's just a way to make the cmd 'reset' take arguments that map to enum
>>>>>> sysreset_t.
>>>>>
>>>>> Cold vs. Warm reset is socfpga specific. The reset driver should
>>>>> probably somehow register supported reset modes (warm/cold) with the
>>>>> reset core code.
>>>>
>>>> But it's a thing the UCLASS_SYSRESET already exposes. I haven't added
>>>> the enum values for that, I merely exposed them to the cmd "API". I
>>>> can't see anything socfpga specific about it.
>>>
>>> Ah, then socfpga just maps to that "API" very well, nice. I wasn't aware
>>> of it.
>>>
>>> --
>>> Best regards,
>>> Marek Vasut
>