[PATCH v3 3/4] sysreset: provide type of reset in do_reset cmd

Igor Opaniuk igor.opaniuk at foundries.io
Thu Apr 1 13:01:21 CEST 2021


On Thu, Apr 1, 2021 at 2:44 AM Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> Am 1. April 2021 01:40:06 MESZ schrieb Igor Opaniuk <
> igor.opaniuk at gmail.com>:
> >Hi Heinrich
> >
> >On Thu, Apr 1, 2021 at 12:36 AM Heinrich Schuchardt
> ><xypron.glpk at gmx.de> wrote:
> >>
> >> On 3/31/21 10:16 PM, Igor Opaniuk wrote:
> >> > From: Igor Opaniuk <igor.opaniuk at foundries.io>
> >> >
> >> > Add additional param for reset cmd, which provides type of reset.
> >> >
> >> > Signed-off-by: Igor Opaniuk <igor.opaniuk at foundries.io>
> >>
> >> It seems you are missing a lot of do_reset() implementations in your
> >> series which may also have to be adjusted to conform to the changed
> >> reset command.
> >>
> >> arch/arc/lib/reset.c:16:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/arm/lib/reset.c:33:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/m68k/cpu/mcf5227x/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf523x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:145:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:180:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:269:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:359:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:378:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf52x2/cpu.c:410:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf530x/cpu.c:15:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf532x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf5445x/cpu.c:26:int do_reset(struct cmd_tbl *cmdtp,
> >int
> >> flag, int argc, char *const argv[])
> >> arch/m68k/cpu/mcf547x_8x/cpu.c:25:int do_reset(struct cmd_tbl *cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/microblaze/cpu/spl.c:53:int do_reset(struct cmd_tbl *cmdtp, int
> >> flag, int argc, char *const argv[])
> >> arch/mips/cpu/cpu.c:24:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >int
> >> argc, char *const argv[])
> >> arch/nds32/cpu/n1213/ae3xx/cpu.c:42:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/nds32/cpu/n1213/ag101/cpu.c:42:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/nios2/cpu/cpu.c:37:int do_reset(struct cmd_tbl *cmdtp, int flag,
> >> int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc83xx/cpu.c:128:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc85xx/cpu.c:301:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc86xx/cpu.c:112:int do_reset(struct cmd_tbl
> >*cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/powerpc/cpu/mpc8xx/cpu.c:199:int do_reset(struct cmd_tbl *cmdtp,
> >> int flag, int argc, char *const argv[])
> >> arch/riscv/lib/reset.c:10:int do_reset(struct cmd_tbl *cmdtp, int
> >flag,
> >> int argc, char *const argv[])
> >> arch/sh/cpu/sh4/cpu.c:32:int do_reset(struct cmd_tbl *cmdtp, int
> >flag,
> >> int argc, char *const argv[])
> >
> >This change don't really impact the behaviour of do_reset
> >implementations
> >you mentioned.:
> >-       reset, 1, 0,    do_reset,
> >+       reset, 2, 0,    do_reset,
> >
> >I have checked maybe half of that do_reset() implementations, and none
> >of them
> >has any logic that relies somehow on argc/argv[]
>
>
> The changed reyet command is meant to allow choosing between cold and warm
> reset.
>
> I would have expexted that this holds true for all architectures, e.g.
> RISC-V.
>

Well, I think this goes beyond this patch series, as :
1. The main scope of this patch is adjustments in PSCI firmware/sysreset
drivers, adding support
for the new version of PSCI API so it's possible to use alternative PSCI
RESET2 API call.
Extending do_reset() was just a consequence, as basically I wanted to avoid
introducing a new
arch-specific command which could be used for that.
If that doesn't work for you, I can create something like "psci_reset"
instead.

2. All do_reset() implementations you listed are all arch/SoC specific,
that's not even
theoretically feasible to adjust every function as I don't have neither
hardware nor time
for doing that. I strongly believe that it should be done by arch/soc/board
maintainers in
case they want to have support of alternative resets.

I can add an explicit notice to doc/usage/reset.rst that warm reset might
not be implemented for your
arch/soc/board, and in that case regular reset will be triggered.
Let me know what option works best for you.

Thanks


> Best regards
>
> Heinrich
>
>
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> > ---
> >> >
> >> >   cmd/boot.c                         |  2 +-
> >> >   drivers/sysreset/sysreset-uclass.c | 11 ++++++++++-
> >> >   2 files changed, 11 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/cmd/boot.c b/cmd/boot.c
> >> > index 36aba22b30..b84c0ed89e 100644
> >> > --- a/cmd/boot.c
> >> > +++ b/cmd/boot.c
> >> > @@ -56,7 +56,7 @@ U_BOOT_CMD(
> >> >   #endif
> >> >
> >> >   U_BOOT_CMD(
> >> > -     reset, 1, 0,    do_reset,
> >> > +     reset, 2, 0,    do_reset,
> >> >       "Perform RESET of the CPU",
> >> >       ""
> >> >   );
> >> > diff --git a/drivers/sysreset/sysreset-uclass.c
> >b/drivers/sysreset/sysreset-uclass.c
> >> > index 6c9dc7a384..0412c4a29b 100644
> >> > --- a/drivers/sysreset/sysreset-uclass.c
> >> > +++ b/drivers/sysreset/sysreset-uclass.c
> >> > @@ -122,10 +122,19 @@ void reset_cpu(ulong addr)
> >> >   #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET)
> >> >   int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char
> >*const argv[])
> >> >   {
> >> > +     enum sysreset_t reset_type = SYSRESET_COLD;
> >> > +
> >> > +     if (argc > 2)
> >> > +             return CMD_RET_USAGE;
> >> > +
> >> > +     if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> >> > +             reset_type = SYSRESET_WARM;
> >> > +     }
> >> > +
> >> >       printf("resetting ...\n");
> >> >       mdelay(100);
> >> >
> >> > -     sysreset_walk_halt(SYSRESET_COLD);
> >> > +     sysreset_walk_halt(reset_type);
> >> >
> >> >       return 0;
> >> >   }
> >> >
> >>
>
>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Embedded Software Engineer
T:  +380 938364067
E: igor.opaniuk at foundries.io
W: www.foundries.io


More information about the U-Boot mailing list