Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
> From: Bin Meng [mailto:bmeng...@gmail.com]
> Sent: Tuesday, September 11, 2018 12:55 PM
> To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> Cc: Lukas Auer
> Subject: [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
>
> We don't have a reset method on any RISC-V board yet. Instead of adding the
> same 'unsupported' message for each CPU variant it might make more sense to
> add a generic do_reset function for all CPU variants to lib/,
similar to the one for
> ARM (arch/arm/lib/reset.c).
>
> Suggested-by: Lukas Auer
> Signed-off-by: Bin Meng
>
> ---
>
> Changes in v2:
> - new patch to move do_reset() to a common place
>
> arch/riscv/cpu/ax25/cpu.c | 9 -
arch/riscv/cpu/qemu/cpu.c | 8
> arch/riscv/lib/Makefile | 1 +
> arch/riscv/lib/reset.c| 14 ++
> 4 files changed, 15 insertions(+), 17 deletions(-) create mode 100644
> arch/riscv/lib/reset.c
>
> diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c index
> ab05b57..fddcc15 100644
> --- a/arch/riscv/cpu/ax25/cpu.c
> +++ b/arch/riscv/cpu/ax25/cpu.c
> @@ -6,9 +6,6 @@
>
> /* CPU specific code */
> #include
> -#include
> -#include
> -#include
>
> /*
> * cleanup_before_linux() is called just before we call linux @@ -24,9 +21,3
> @@ int cleanup_before_linux(void)
>
> return 0;
> }
> -
> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{
> - disable_interrupts();
> - panic("ax25-ae350 wdt not support yet.\n");
> -}
> diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c index
> a064639..6c7a327 100644
> --- a/arch/riscv/cpu/qemu/cpu.c
> +++ b/arch/riscv/cpu/qemu/cpu.c
> @@ -4,7 +4,6 @@
> */
>
> #include
> -#include
>
> /*
> * cleanup_before_linux() is called just before we call linux @@
-20,10 +19,3
> @@ int cleanup_before_linux(void)
>
> return 0;
> }
> -
> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{
> - printf("reset unsupported yet\n");
> -
> - return 0;
> -}
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index
> cc562f9..b58db89 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> obj-$(CONFIG_CMD_GO) += boot.o
> obj-y+= cache.o
> obj-y+= interrupts.o
> +obj-y+= reset.o
> obj-y += setjmp.o
>
> # For building EFI apps
> diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c new
file mode 100644
> index 000..5d9b99c
> --- /dev/null
> +++ b/arch/riscv/lib/reset.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018, Bin Meng */
> +
> +#include
> +#include
> +
> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + printf("reset unsupported yet\n");
> +
> + return 0;
> +}
Reviewed-by: Rick Chen
> --
> 2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
Hi Bin,
On Tue, 2018-09-18 at 16:50 +0800, Bin Meng wrote:
> Hi Lukas,
>
> On Tue, Sep 18, 2018 at 6:01 AM Auer, Lukas
> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 2018-09-17 at 13:02 +0800, Bin Meng wrote:
> > > Hi Lukas,
> > >
> > > On Mon, Sep 17, 2018 at 5:09 AM Auer, Lukas
> > > wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> > > > > We don't have a reset method on any RISC-V board yet. Instead
> > > > > of
> > > > > adding the same 'unsupported' message for each CPU variant it
> > > > > might
> > > > > make more sense to add a generic do_reset function for all
> > > > > CPU
> > > > > variants to lib/, similar to the one for ARM
> > > > > (arch/arm/lib/reset.c).
> > > > >
> > > > > Suggested-by: Lukas Auer
> > > > > Signed-off-by: Bin Meng
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - new patch to move do_reset() to a common place
> > > > >
> > > > > arch/riscv/cpu/ax25/cpu.c | 9 -
> > > > > arch/riscv/cpu/qemu/cpu.c | 8
> > > > > arch/riscv/lib/Makefile | 1 +
> > > > > arch/riscv/lib/reset.c| 14 ++
> > > > > 4 files changed, 15 insertions(+), 17 deletions(-)
> > > > > create mode 100644 arch/riscv/lib/reset.c
> > > > >
> > > > > diff --git a/arch/riscv/cpu/ax25/cpu.c
> > > > > b/arch/riscv/cpu/ax25/cpu.c
> > > > > index ab05b57..fddcc15 100644
> > > > > --- a/arch/riscv/cpu/ax25/cpu.c
> > > > > +++ b/arch/riscv/cpu/ax25/cpu.c
> > > > > @@ -6,9 +6,6 @@
> > > > >
> > > > > /* CPU specific code */
> > > > > #include
> > > > > -#include
> > > > > -#include
> > > > > -#include
> > > > >
> > > > > /*
> > > > > * cleanup_before_linux() is called just before we call
> > > > > linux
> > > > > @@ -24,9 +21,3 @@ int cleanup_before_linux(void)
> > > > >
> > > > > return 0;
> > > > > }
> > > > > -
> > > > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *
> > > > > const
> > > > > argv[])
> > > > > -{
> > > > > - disable_interrupts();
> > > > > - panic("ax25-ae350 wdt not support yet.\n");
> > > > > -}
> > > > > diff --git a/arch/riscv/cpu/qemu/cpu.c
> > > > > b/arch/riscv/cpu/qemu/cpu.c
> > > > > index a064639..6c7a327 100644
> > > > > --- a/arch/riscv/cpu/qemu/cpu.c
> > > > > +++ b/arch/riscv/cpu/qemu/cpu.c
> > > > > @@ -4,7 +4,6 @@
> > > > > */
> > > > >
> > > > > #include
> > > > > -#include
> > > > >
> > > > > /*
> > > > > * cleanup_before_linux() is called just before we call
> > > > > linux
> > > > > @@ -20,10 +19,3 @@ int cleanup_before_linux(void)
> > > > >
> > > > > return 0;
> > > > > }
> > > > > -
> > > > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *
> > > > > const
> > > > > argv[])
> > > > > -{
> > > > > - printf("reset unsupported yet\n");
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > diff --git a/arch/riscv/lib/Makefile
> > > > > b/arch/riscv/lib/Makefile
> > > > > index cc562f9..b58db89 100644
> > > > > --- a/arch/riscv/lib/Makefile
> > > > > +++ b/arch/riscv/lib/Makefile
> > > > > @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> > > > > obj-$(CONFIG_CMD_GO) += boot.o
> > > > > obj-y+= cache.o
> > > > > obj-y+= interrupts.o
> > > > > +obj-y+= reset.o
> > > > > obj-y += setjmp.o
> > > > >
> > > > > # For building EFI apps
> > > > > diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c
> > > > > new file mode 100644
> > > > > index 000..5d9b99c
> > > > > --- /dev/null
> > > > > +++ b/arch/riscv/lib/reset.c
> > > > > @@ -0,0 +1,14 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright (C) 2018, Bin Meng
> > > > > + */
> > > > > +
> > > > > +#include
> > > > > +#include
> > > > > +
> > > > > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *
> > > > > const
> > > > > argv[])
> > > > > +{
> > > > > + printf("reset unsupported yet\n");
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > >
> > > > Thanks for adding this patch!
> > > > I see one possible problem with it. The way you implemented it,
> > > > arch /
> > > > board code can't overwrite the function to add a reset method.
> > > > How
> > > > about something like this?
> > > >
> > >
> > > Thanks for your review and comments. I believe current
> > > implementation
> > > in this patch is an interim approach, for now to save some
> > > duplicates.
> > > The problem you mentioned can be resolved by implementing a
> > > SYSRESET
> > > uclass driver for that CPU/board in the future.
> > >
> >
> > That's true, still, two minor things I would consider to change.
> >
> > - A simple wording changes to make it clear that the reset is not
> > only
> > not available, but u-boot actually tried to reset the system. So
> > something like this: "Resetting... reset not supported yet".
> >
>
> Sure, will reword this in v3.
>
> - I don't know how much of an issue it is that u-boot keeps running
> > after a reset. I would tend to use
Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
Hi Lukas,
On Tue, Sep 18, 2018 at 6:01 AM Auer, Lukas
wrote:
>
> Hi Bin,
>
> On Mon, 2018-09-17 at 13:02 +0800, Bin Meng wrote:
> > Hi Lukas,
> >
> > On Mon, Sep 17, 2018 at 5:09 AM Auer, Lukas
> > wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> > > > We don't have a reset method on any RISC-V board yet. Instead of
> > > > adding the same 'unsupported' message for each CPU variant it
> > > > might
> > > > make more sense to add a generic do_reset function for all CPU
> > > > variants to lib/, similar to the one for ARM
> > > > (arch/arm/lib/reset.c).
> > > >
> > > > Suggested-by: Lukas Auer
> > > > Signed-off-by: Bin Meng
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - new patch to move do_reset() to a common place
> > > >
> > > > arch/riscv/cpu/ax25/cpu.c | 9 -
> > > > arch/riscv/cpu/qemu/cpu.c | 8
> > > > arch/riscv/lib/Makefile | 1 +
> > > > arch/riscv/lib/reset.c| 14 ++
> > > > 4 files changed, 15 insertions(+), 17 deletions(-)
> > > > create mode 100644 arch/riscv/lib/reset.c
> > > >
> > > > diff --git a/arch/riscv/cpu/ax25/cpu.c
> > > > b/arch/riscv/cpu/ax25/cpu.c
> > > > index ab05b57..fddcc15 100644
> > > > --- a/arch/riscv/cpu/ax25/cpu.c
> > > > +++ b/arch/riscv/cpu/ax25/cpu.c
> > > > @@ -6,9 +6,6 @@
> > > >
> > > > /* CPU specific code */
> > > > #include
> > > > -#include
> > > > -#include
> > > > -#include
> > > >
> > > > /*
> > > > * cleanup_before_linux() is called just before we call linux
> > > > @@ -24,9 +21,3 @@ int cleanup_before_linux(void)
> > > >
> > > > return 0;
> > > > }
> > > > -
> > > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > > argv[])
> > > > -{
> > > > - disable_interrupts();
> > > > - panic("ax25-ae350 wdt not support yet.\n");
> > > > -}
> > > > diff --git a/arch/riscv/cpu/qemu/cpu.c
> > > > b/arch/riscv/cpu/qemu/cpu.c
> > > > index a064639..6c7a327 100644
> > > > --- a/arch/riscv/cpu/qemu/cpu.c
> > > > +++ b/arch/riscv/cpu/qemu/cpu.c
> > > > @@ -4,7 +4,6 @@
> > > > */
> > > >
> > > > #include
> > > > -#include
> > > >
> > > > /*
> > > > * cleanup_before_linux() is called just before we call linux
> > > > @@ -20,10 +19,3 @@ int cleanup_before_linux(void)
> > > >
> > > > return 0;
> > > > }
> > > > -
> > > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > > argv[])
> > > > -{
> > > > - printf("reset unsupported yet\n");
> > > > -
> > > > - return 0;
> > > > -}
> > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > > > index cc562f9..b58db89 100644
> > > > --- a/arch/riscv/lib/Makefile
> > > > +++ b/arch/riscv/lib/Makefile
> > > > @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> > > > obj-$(CONFIG_CMD_GO) += boot.o
> > > > obj-y+= cache.o
> > > > obj-y+= interrupts.o
> > > > +obj-y+= reset.o
> > > > obj-y += setjmp.o
> > > >
> > > > # For building EFI apps
> > > > diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c
> > > > new file mode 100644
> > > > index 000..5d9b99c
> > > > --- /dev/null
> > > > +++ b/arch/riscv/lib/reset.c
> > > > @@ -0,0 +1,14 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright (C) 2018, Bin Meng
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > > argv[])
> > > > +{
> > > > + printf("reset unsupported yet\n");
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > Thanks for adding this patch!
> > > I see one possible problem with it. The way you implemented it,
> > > arch /
> > > board code can't overwrite the function to add a reset method. How
> > > about something like this?
> > >
> >
> > Thanks for your review and comments. I believe current implementation
> > in this patch is an interim approach, for now to save some
> > duplicates.
> > The problem you mentioned can be resolved by implementing a SYSRESET
> > uclass driver for that CPU/board in the future.
> >
>
> That's true, still, two minor things I would consider to change.
>
> - A simple wording changes to make it clear that the reset is not only
> not available, but u-boot actually tried to reset the system. So
> something like this: "Resetting... reset not supported yet".
>
Sure, will reword this in v3.
- I don't know how much of an issue it is that u-boot keeps running
> after a reset. I would tend to use panic() together with PANIC_HANG, to
> make sure u-boot halts. You know u-boot better than me, what do you
> think?
>
Yes, I think using hang() can be a good choice for now.
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
Hi Bin,
On Mon, 2018-09-17 at 13:02 +0800, Bin Meng wrote:
> Hi Lukas,
>
> On Mon, Sep 17, 2018 at 5:09 AM Auer, Lukas
> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> > > We don't have a reset method on any RISC-V board yet. Instead of
> > > adding the same 'unsupported' message for each CPU variant it
> > > might
> > > make more sense to add a generic do_reset function for all CPU
> > > variants to lib/, similar to the one for ARM
> > > (arch/arm/lib/reset.c).
> > >
> > > Suggested-by: Lukas Auer
> > > Signed-off-by: Bin Meng
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - new patch to move do_reset() to a common place
> > >
> > > arch/riscv/cpu/ax25/cpu.c | 9 -
> > > arch/riscv/cpu/qemu/cpu.c | 8
> > > arch/riscv/lib/Makefile | 1 +
> > > arch/riscv/lib/reset.c| 14 ++
> > > 4 files changed, 15 insertions(+), 17 deletions(-)
> > > create mode 100644 arch/riscv/lib/reset.c
> > >
> > > diff --git a/arch/riscv/cpu/ax25/cpu.c
> > > b/arch/riscv/cpu/ax25/cpu.c
> > > index ab05b57..fddcc15 100644
> > > --- a/arch/riscv/cpu/ax25/cpu.c
> > > +++ b/arch/riscv/cpu/ax25/cpu.c
> > > @@ -6,9 +6,6 @@
> > >
> > > /* CPU specific code */
> > > #include
> > > -#include
> > > -#include
> > > -#include
> > >
> > > /*
> > > * cleanup_before_linux() is called just before we call linux
> > > @@ -24,9 +21,3 @@ int cleanup_before_linux(void)
> > >
> > > return 0;
> > > }
> > > -
> > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > argv[])
> > > -{
> > > - disable_interrupts();
> > > - panic("ax25-ae350 wdt not support yet.\n");
> > > -}
> > > diff --git a/arch/riscv/cpu/qemu/cpu.c
> > > b/arch/riscv/cpu/qemu/cpu.c
> > > index a064639..6c7a327 100644
> > > --- a/arch/riscv/cpu/qemu/cpu.c
> > > +++ b/arch/riscv/cpu/qemu/cpu.c
> > > @@ -4,7 +4,6 @@
> > > */
> > >
> > > #include
> > > -#include
> > >
> > > /*
> > > * cleanup_before_linux() is called just before we call linux
> > > @@ -20,10 +19,3 @@ int cleanup_before_linux(void)
> > >
> > > return 0;
> > > }
> > > -
> > > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > argv[])
> > > -{
> > > - printf("reset unsupported yet\n");
> > > -
> > > - return 0;
> > > -}
> > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > > index cc562f9..b58db89 100644
> > > --- a/arch/riscv/lib/Makefile
> > > +++ b/arch/riscv/lib/Makefile
> > > @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> > > obj-$(CONFIG_CMD_GO) += boot.o
> > > obj-y+= cache.o
> > > obj-y+= interrupts.o
> > > +obj-y+= reset.o
> > > obj-y += setjmp.o
> > >
> > > # For building EFI apps
> > > diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c
> > > new file mode 100644
> > > index 000..5d9b99c
> > > --- /dev/null
> > > +++ b/arch/riscv/lib/reset.c
> > > @@ -0,0 +1,14 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2018, Bin Meng
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +
> > > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > > argv[])
> > > +{
> > > + printf("reset unsupported yet\n");
> > > +
> > > + return 0;
> > > +}
> >
> > Thanks for adding this patch!
> > I see one possible problem with it. The way you implemented it,
> > arch /
> > board code can't overwrite the function to add a reset method. How
> > about something like this?
> >
>
> Thanks for your review and comments. I believe current implementation
> in this patch is an interim approach, for now to save some
> duplicates.
> The problem you mentioned can be resolved by implementing a SYSRESET
> uclass driver for that CPU/board in the future.
>
That's true, still, two minor things I would consider to change.
- A simple wording changes to make it clear that the reset is not only
not available, but u-boot actually tried to reset the system. So
something like this: "Resetting... reset not supported yet".
- I don't know how much of an issue it is that u-boot keeps running
after a reset. I would tend to use panic() together with PANIC_HANG, to
make sure u-boot halts. You know u-boot better than me, what do you
think?
Thanks,
Lukas
Reviewed-by: Lukas Auer
> > - do_reset() only prints "resetting..." and then calls cpu_reset()
> > - reset.c includes a weak cpu_reset() function that prints the
> > warning
> > you currently have in this patch (by the way, it should say "reset
> > not
> > supported yet"). It might also make sense to print the warning with
> > the
> > panic() function and define PANIC_HANG, so that u-boot is halted on
> > resets. cpu_reset() can then be defined by boards that have an
> > actual
> > reset method.
>
> Regards,
> Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
Hi Lukas,
On Mon, Sep 17, 2018 at 5:09 AM Auer, Lukas
wrote:
>
> Hi Bin,
>
> On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> > We don't have a reset method on any RISC-V board yet. Instead of
> > adding the same 'unsupported' message for each CPU variant it might
> > make more sense to add a generic do_reset function for all CPU
> > variants to lib/, similar to the one for ARM (arch/arm/lib/reset.c).
> >
> > Suggested-by: Lukas Auer
> > Signed-off-by: Bin Meng
> >
> > ---
> >
> > Changes in v2:
> > - new patch to move do_reset() to a common place
> >
> > arch/riscv/cpu/ax25/cpu.c | 9 -
> > arch/riscv/cpu/qemu/cpu.c | 8
> > arch/riscv/lib/Makefile | 1 +
> > arch/riscv/lib/reset.c| 14 ++
> > 4 files changed, 15 insertions(+), 17 deletions(-)
> > create mode 100644 arch/riscv/lib/reset.c
> >
> > diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c
> > index ab05b57..fddcc15 100644
> > --- a/arch/riscv/cpu/ax25/cpu.c
> > +++ b/arch/riscv/cpu/ax25/cpu.c
> > @@ -6,9 +6,6 @@
> >
> > /* CPU specific code */
> > #include
> > -#include
> > -#include
> > -#include
> >
> > /*
> > * cleanup_before_linux() is called just before we call linux
> > @@ -24,9 +21,3 @@ int cleanup_before_linux(void)
> >
> > return 0;
> > }
> > -
> > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > -{
> > - disable_interrupts();
> > - panic("ax25-ae350 wdt not support yet.\n");
> > -}
> > diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
> > index a064639..6c7a327 100644
> > --- a/arch/riscv/cpu/qemu/cpu.c
> > +++ b/arch/riscv/cpu/qemu/cpu.c
> > @@ -4,7 +4,6 @@
> > */
> >
> > #include
> > -#include
> >
> > /*
> > * cleanup_before_linux() is called just before we call linux
> > @@ -20,10 +19,3 @@ int cleanup_before_linux(void)
> >
> > return 0;
> > }
> > -
> > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > -{
> > - printf("reset unsupported yet\n");
> > -
> > - return 0;
> > -}
> > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > index cc562f9..b58db89 100644
> > --- a/arch/riscv/lib/Makefile
> > +++ b/arch/riscv/lib/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> > obj-$(CONFIG_CMD_GO) += boot.o
> > obj-y+= cache.o
> > obj-y+= interrupts.o
> > +obj-y+= reset.o
> > obj-y += setjmp.o
> >
> > # For building EFI apps
> > diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c
> > new file mode 100644
> > index 000..5d9b99c
> > --- /dev/null
> > +++ b/arch/riscv/lib/reset.c
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2018, Bin Meng
> > + */
> > +
> > +#include
> > +#include
> > +
> > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > +{
> > + printf("reset unsupported yet\n");
> > +
> > + return 0;
> > +}
>
> Thanks for adding this patch!
> I see one possible problem with it. The way you implemented it, arch /
> board code can't overwrite the function to add a reset method. How
> about something like this?
>
Thanks for your review and comments. I believe current implementation
in this patch is an interim approach, for now to save some duplicates.
The problem you mentioned can be resolved by implementing a SYSRESET
uclass driver for that CPU/board in the future.
> - do_reset() only prints "resetting..." and then calls cpu_reset()
> - reset.c includes a weak cpu_reset() function that prints the warning
> you currently have in this patch (by the way, it should say "reset not
> supported yet"). It might also make sense to print the warning with the
> panic() function and define PANIC_HANG, so that u-boot is halted on
> resets. cpu_reset() can then be defined by boards that have an actual
> reset method.
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RESEND PATCH v2 15/15] riscv: Move do_reset() to a common place
Hi Bin,
On Mon, 2018-09-10 at 21:54 -0700, Bin Meng wrote:
> We don't have a reset method on any RISC-V board yet. Instead of
> adding the same 'unsupported' message for each CPU variant it might
> make more sense to add a generic do_reset function for all CPU
> variants to lib/, similar to the one for ARM (arch/arm/lib/reset.c).
>
> Suggested-by: Lukas Auer
> Signed-off-by: Bin Meng
>
> ---
>
> Changes in v2:
> - new patch to move do_reset() to a common place
>
> arch/riscv/cpu/ax25/cpu.c | 9 -
> arch/riscv/cpu/qemu/cpu.c | 8
> arch/riscv/lib/Makefile | 1 +
> arch/riscv/lib/reset.c| 14 ++
> 4 files changed, 15 insertions(+), 17 deletions(-)
> create mode 100644 arch/riscv/lib/reset.c
>
> diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c
> index ab05b57..fddcc15 100644
> --- a/arch/riscv/cpu/ax25/cpu.c
> +++ b/arch/riscv/cpu/ax25/cpu.c
> @@ -6,9 +6,6 @@
>
> /* CPU specific code */
> #include
> -#include
> -#include
> -#include
>
> /*
> * cleanup_before_linux() is called just before we call linux
> @@ -24,9 +21,3 @@ int cleanup_before_linux(void)
>
> return 0;
> }
> -
> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> -{
> - disable_interrupts();
> - panic("ax25-ae350 wdt not support yet.\n");
> -}
> diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
> index a064639..6c7a327 100644
> --- a/arch/riscv/cpu/qemu/cpu.c
> +++ b/arch/riscv/cpu/qemu/cpu.c
> @@ -4,7 +4,6 @@
> */
>
> #include
> -#include
>
> /*
> * cleanup_before_linux() is called just before we call linux
> @@ -20,10 +19,3 @@ int cleanup_before_linux(void)
>
> return 0;
> }
> -
> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> -{
> - printf("reset unsupported yet\n");
> -
> - return 0;
> -}
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> index cc562f9..b58db89 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> obj-$(CONFIG_CMD_GO) += boot.o
> obj-y+= cache.o
> obj-y+= interrupts.o
> +obj-y+= reset.o
> obj-y += setjmp.o
>
> # For building EFI apps
> diff --git a/arch/riscv/lib/reset.c b/arch/riscv/lib/reset.c
> new file mode 100644
> index 000..5d9b99c
> --- /dev/null
> +++ b/arch/riscv/lib/reset.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018, Bin Meng
> + */
> +
> +#include
> +#include
> +
> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> +{
> + printf("reset unsupported yet\n");
> +
> + return 0;
> +}
Thanks for adding this patch!
I see one possible problem with it. The way you implemented it, arch /
board code can't overwrite the function to add a reset method. How
about something like this?
- do_reset() only prints "resetting..." and then calls cpu_reset()
- reset.c includes a weak cpu_reset() function that prints the warning
you currently have in this patch (by the way, it should say "reset not
supported yet"). It might also make sense to print the warning with the
panic() function and define PANIC_HANG, so that u-boot is halted on
resets. cpu_reset() can then be defined by boards that have an actual
reset method.
Thanks,
Lukas
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
