Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Am 02.08.2019 um 16:41 schrieb Simon Glass: Hi SImon, On Thu, 1 Aug 2019 at 23:53, Simon Goldschmidt wrote: On Fri, Aug 2, 2019 at 12:45 AM Simon Glass wrote: Hi Simon, On Mon, 29 Jul 2019 at 04:47, Simon Goldschmidt wrote: Simon, On Wed, Jul 10, 2019 at 8:50 PM Simon Goldschmidt wrote: 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 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 --- 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", -"" +"[] - 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? Sorry, but I can't implement a test without you telling me what kind of test you mean. I'll be sending v2 (that removes the extra printf's from above) without such a test and hope it can still be accepted. Well we have test_unknown_cmd.py for example, and test_pinmux.py, which you could use as examples, just to check that the command works and provides the right output. The tests are run in sandbox, right? And my understanding was that the reset command makes sandbox exit. That's why I asked this question: I just don'tests know if/how I can write a test when the command under test exits sandbox. The sandbox sysreset driver has a way to avoid exiting. But even if you do have it set to exit, that should be OK for the test. I'll drop this one, as it's not required any more. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Hi SImon, On Thu, 1 Aug 2019 at 23:53, Simon Goldschmidt wrote: > > On Fri, Aug 2, 2019 at 12:45 AM Simon Glass wrote: > > > > Hi Simon, > > > > On Mon, 29 Jul 2019 at 04:47, Simon Goldschmidt > > wrote: > > > > > > Simon, > > > > > > On Wed, Jul 10, 2019 at 8:50 PM Simon Goldschmidt > > > wrote: > > > > > > > > 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 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 > > > > > > > > --- > > > > > > > > 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", > > > > -"" > > > > +"[] - 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? > > > > > > Sorry, but I can't implement a test without you telling me what kind > > > of test you mean. > > > > > > I'll be sending v2 (that removes the extra printf's from above) without > > > such a test and hope it can still be accepted. > > > > Well we have test_unknown_cmd.py for example, and test_pinmux.py, > > which you could use as examples, just to check that the command works > > and provides the right output. > > The tests are run in sandbox, right? And my understanding was that the reset > command makes sandbox exit. That's why I asked this question: I just don'tests > know if/how I can write a test when the command under test exits sandbox. The sandbox sysreset driver has a way to avoid exiting. But even if you do have it set to exit, that should be OK for the test. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
On Fri, Aug 2, 2019 at 12:45 AM Simon Glass wrote: > > Hi Simon, > > On Mon, 29 Jul 2019 at 04:47, Simon Goldschmidt > wrote: > > > > Simon, > > > > On Wed, Jul 10, 2019 at 8:50 PM Simon Goldschmidt > > wrote: > > > > > > 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 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 > > > > > > --- > > > > > > 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", > > > -"" > > > +"[] - 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? > > > > Sorry, but I can't implement a test without you telling me what kind > > of test you mean. > > > > I'll be sending v2 (that removes the extra printf's from above) without > > such a test and hope it can still be accepted. > > Well we have test_unknown_cmd.py for example, and test_pinmux.py, > which you could use as examples, just to check that the command works > and provides the right output. The tests are run in sandbox, right? And my understanding was that the reset command makes sandbox exit. That's why I asked this question: I just don'tests know if/how I can write a test when the command under test exits sandbox. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Hi Simon, On Mon, 29 Jul 2019 at 04:47, Simon Goldschmidt wrote: > > Simon, > > On Wed, Jul 10, 2019 at 8:50 PM Simon Goldschmidt > wrote: > > > > 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 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 > > --- > > > > 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", > > -"" > > +"[] - 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? > > Sorry, but I can't implement a test without you telling me what kind > of test you mean. > > I'll be sending v2 (that removes the extra printf's from above) without > such a test and hope it can still be accepted. Well we have test_unknown_cmd.py for example, and test_pinmux.py, which you could use as examples, just to check that the command works and provides the right output. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Simon, On Wed, Jul 10, 2019 at 8:50 PM Simon Goldschmidt wrote: > > 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 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 > --- > > 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", > -"" > +"[] - 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? Sorry, but I can't implement a test without you telling me what kind of test you mean. I'll be sending v2 (that removes the extra printf's from above) without such a test and hope it can still be accepted. Regards, Simon > > 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 > > > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 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 --- 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", -"" +"[] - 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Am 03.05.2019 um 23:10 schrieb Simon Glass: Hi Simon, On Fri, 3 May 2019 at 14:43, Marek Vasut 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 --- 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", -"" +"[] - 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? 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
Hi Simon, On Fri, 3 May 2019 at 14:43, Marek Vasut 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 > > --- > > > >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", > > -"" > > +"[] - 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? 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 > --- > > 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", > - "" > + "[] - 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; 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 --- 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", - "" + "[] - 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; 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. There is a problem in that other platforms without UCLASS_SYSRESET don't handle these arguments, but I thought UCLASS_SYSRESET would be the future, and the rest would be "legacy"? Yep. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 >>> --- >>> >>> 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", >>> - "" >>> + "[] - 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; >> >> 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. > There is a problem in that other platforms without UCLASS_SYSRESET don't > handle these arguments, but I thought UCLASS_SYSRESET would be the > future, and the rest would be "legacy"? Yep. -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 --- 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", - "" + "[] - 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; 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. There is a problem in that other platforms without UCLASS_SYSRESET don't handle these arguments, but I thought UCLASS_SYSRESET would be the future, and the rest would be "legacy"? Regards, SImon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 > --- > > 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", > - "" > + "[] - 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; This looks like a platform or driver specific stuff ? -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] cmd: reset: add parameters to specify reboot_mode
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 --- 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", - "" + "[] - 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; + } + } + printf("resetting ...\n"); - sysreset_walk_halt(SYSRESET_COLD); + sysreset_walk_halt(reboot_mode); return 0; } -- 2.20.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot