2021-01-04 17:02:50

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> Let the LiteX SoC Controller a register a restart handler, which resets
> the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Tested with linux-on-litex-vexriscv.
>
> This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> CPUs"
> (https://lore.kernel.org/lkml/[email protected]/)

Bummer, and that's why the RESET_REG_* definitions are no longer
next to the SCRATCH_REG_* definitions :-(

Well, I assume that will be fixed by evolution ;-)

> v2:
> - Rebase on top of v5.11-rc1,
> - Change reset handler priority to recommended default value of 128
> (was 192).
>
> (v1 was not sent to a mailing list)
> ---
> drivers/soc/litex/litex_soc_ctrl.c | 33 +++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
> index 1217cafdfd4d1d2b..d729ad50d4ffca5e 100644
> --- a/drivers/soc/litex/litex_soc_ctrl.c
> +++ b/drivers/soc/litex/litex_soc_ctrl.c
> @@ -15,6 +15,11 @@
> #include <linux/module.h>
> #include <linux/errno.h>
> #include <linux/io.h>
> +#include <linux/reboot.h>
> +
> +/* reset register located at the base address */
> +#define RESET_REG_OFF 0x00
> +#define RESET_REG_VALUE 0x00000001
>
> /*
> * LiteX SoC Generator, depending on the configuration, can split a single

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2021-01-14 02:05:33

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > Let the LiteX SoC Controller a register a restart handler, which resets
> > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Tested with linux-on-litex-vexriscv.
> >
> > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > CPUs"
> > (https://lore.kernel.org/lkml/[email protected]/)
>
> Bummer, and that's why the RESET_REG_* definitions are no longer
> next to the SCRATCH_REG_* definitions :-(

If it helps I have accepted Gabriel's patches and put them onto for-next.

https://github.com/openrisc/linux/commits/for-next

I am happy to take and test a patch based on that. Or I can do the adjustments
to base the patch on that myself. Let me know.

-Stafford

> Well, I assume that will be fixed by evolution ;-)
>
> > v2:
> > - Rebase on top of v5.11-rc1,
> > - Change reset handler priority to recommended default value of 128
> > (was 192).
> >
> > (v1 was not sent to a mailing list)
> > ---
> > drivers/soc/litex/litex_soc_ctrl.c | 33 +++++++++++++++++++++++++++---
> > 1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
> > index 1217cafdfd4d1d2b..d729ad50d4ffca5e 100644
> > --- a/drivers/soc/litex/litex_soc_ctrl.c
> > +++ b/drivers/soc/litex/litex_soc_ctrl.c
> > @@ -15,6 +15,11 @@
> > #include <linux/module.h>
> > #include <linux/errno.h>
> > #include <linux/io.h>
> > +#include <linux/reboot.h>
> > +
> > +/* reset register located at the base address */
> > +#define RESET_REG_OFF 0x00
> > +#define RESET_REG_VALUE 0x00000001
> >
> > /*
> > * LiteX SoC Generator, depending on the configuration, can split a single
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2021-01-14 13:52:04

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

Hi Stafford,

On Thu, Jan 14, 2021 at 3:03 AM Stafford Horne <[email protected]> wrote:
> On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> > On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > > Let the LiteX SoC Controller a register a restart handler, which resets
> > > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> > >
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > ---
> > > Tested with linux-on-litex-vexriscv.
> > >
> > > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > > CPUs"
> > > (https://lore.kernel.org/lkml/[email protected]/)
> >
> > Bummer, and that's why the RESET_REG_* definitions are no longer
> > next to the SCRATCH_REG_* definitions :-(
>
> If it helps I have accepted Gabriel's patches and put them onto for-next.
>
> https://github.com/openrisc/linux/commits/for-next
>
> I am happy to take and test a patch based on that. Or I can do the adjustments
> to base the patch on that myself. Let me know.

Thanks for letting me know! V3 sent.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-01-18 12:01:13

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

On Thu, Jan 14, 2021 at 02:48:49PM +0100, Geert Uytterhoeven wrote:
> Hi Stafford,
>
> On Thu, Jan 14, 2021 at 3:03 AM Stafford Horne <[email protected]> wrote:
> > On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> > > On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > Let the LiteX SoC Controller a register a restart handler, which resets
> > > > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > > ---
> > > > Tested with linux-on-litex-vexriscv.
> > > >
> > > > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > > > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > > > CPUs"
> > > > (https://lore.kernel.org/lkml/[email protected]/)
> > >
> > > Bummer, and that's why the RESET_REG_* definitions are no longer
> > > next to the SCRATCH_REG_* definitions :-(
> >
> > If it helps I have accepted Gabriel's patches and put them onto for-next.
> >
> > https://github.com/openrisc/linux/commits/for-next
> >
> > I am happy to take and test a patch based on that. Or I can do the adjustments
> > to base the patch on that myself. Let me know.
>
> Thanks for letting me know! V3 sent.

Hi Geert,

I don't seem to see v3 anywhere. Where did you send it and what is the subject?

-Stafford

2021-01-18 14:07:44

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

Hi Stafford,

On Mon, Jan 18, 2021 at 12:43 PM Stafford Horne <[email protected]> wrote:
> On Thu, Jan 14, 2021 at 02:48:49PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Jan 14, 2021 at 3:03 AM Stafford Horne <[email protected]> wrote:
> > > On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> > > > On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > > Let the LiteX SoC Controller a register a restart handler, which resets
> > > > > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> > > > >
> > > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > > > ---
> > > > > Tested with linux-on-litex-vexriscv.
> > > > >
> > > > > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > > > > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > > > > CPUs"
> > > > > (https://lore.kernel.org/lkml/[email protected]/)
> > > >
> > > > Bummer, and that's why the RESET_REG_* definitions are no longer
> > > > next to the SCRATCH_REG_* definitions :-(
> > >
> > > If it helps I have accepted Gabriel's patches and put them onto for-next.
> > >
> > > https://github.com/openrisc/linux/commits/for-next
> > >
> > > I am happy to take and test a patch based on that. Or I can do the adjustments
> > > to base the patch on that myself. Let me know.
> >
> > Thanks for letting me know! V3 sent.
>
> Hi Geert,
>
> I don't seem to see v3 anywhere. Where did you send it and what is the subject?

https://lore.kernel.org/linux-riscv/[email protected]/

So "b4 am [email protected]" should give you
a copy.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-01-18 21:21:45

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

On Mon, Jan 18, 2021 at 01:27:32PM +0100, Geert Uytterhoeven wrote:
> Hi Stafford,
>
> On Mon, Jan 18, 2021 at 12:43 PM Stafford Horne <[email protected]> wrote:
> > On Thu, Jan 14, 2021 at 02:48:49PM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Jan 14, 2021 at 3:03 AM Stafford Horne <[email protected]> wrote:
> > > > On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> > > > > On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > > > Let the LiteX SoC Controller a register a restart handler, which resets

I think there is a typo here:

Let the LiteX SoC Controller a register a restart ...

should remove the first 'a' and say

Let the LiteX SoC Controller register a restart ...

> > > > > > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> > > > > >
> > > > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > > > > ---
> > > > > > Tested with linux-on-litex-vexriscv.
> > > > > >
> > > > > > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > > > > > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > > > > > CPUs"
> > > > > > (https://lore.kernel.org/lkml/[email protected]/)
> > > > >
> > > > > Bummer, and that's why the RESET_REG_* definitions are no longer
> > > > > next to the SCRATCH_REG_* definitions :-(
> > > >
> > > > If it helps I have accepted Gabriel's patches and put them onto for-next.
> > > >
> > > > https://github.com/openrisc/linux/commits/for-next
> > > >
> > > > I am happy to take and test a patch based on that. Or I can do the adjustments
> > > > to base the patch on that myself. Let me know.
> > >
> > > Thanks for letting me know! V3 sent.
> >
> > Hi Geert,
> >
> > I don't seem to see v3 anywhere. Where did you send it and what is the subject?
>
> https://lore.kernel.org/linux-riscv/[email protected]/
>
> So "b4 am [email protected]" should give you
> a copy.

Thanks I got it, I am not sure why it does not show up in my inbox anywhere,
sometimes gmail drops mails. Hence, I am replying here.

As per the typo above I can fix during applying or you could send during a v4.

One more small nit is that you move soc_ctrl_dev out to a static instance it
might help to mention. But it's easy to see why.

-Stafford

2021-01-19 08:07:35

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/soc/litex: Add restart handler

Hi Stafford,

On Mon, Jan 18, 2021 at 10:16 PM Stafford Horne <[email protected]> wrote:
> On Mon, Jan 18, 2021 at 01:27:32PM +0100, Geert Uytterhoeven wrote:
> > On Mon, Jan 18, 2021 at 12:43 PM Stafford Horne <[email protected]> wrote:
> > > On Thu, Jan 14, 2021 at 02:48:49PM +0100, Geert Uytterhoeven wrote:
> > > > On Thu, Jan 14, 2021 at 3:03 AM Stafford Horne <[email protected]> wrote:
> > > > > On Mon, Jan 04, 2021 at 05:49:03PM +0100, Geert Uytterhoeven wrote:
> > > > > > On Mon, Jan 4, 2021 at 5:45 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > > > > Let the LiteX SoC Controller a register a restart handler, which resets
>
> I think there is a typo here:
>
> Let the LiteX SoC Controller a register a restart ...
>
> should remove the first 'a' and say
>
> Let the LiteX SoC Controller register a restart ...
>
> > > > > > > the LiteX SoC by writing 1 to CSR_CTRL_RESET_ADDR.
> > > > > > >
> > > > > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > > > > > ---
> > > > > > > Tested with linux-on-litex-vexriscv.
> > > > > > >
> > > > > > > This patch is based on upstream, i.e. not on top of Gabriel Somlo's
> > > > > > > "[PATCH v5 0/4] drivers/soc/litex: support 32-bit subregisters, 64-bit
> > > > > > > CPUs"
> > > > > > > (https://lore.kernel.org/lkml/[email protected]/)
> > > > > >
> > > > > > Bummer, and that's why the RESET_REG_* definitions are no longer
> > > > > > next to the SCRATCH_REG_* definitions :-(
> > > > >
> > > > > If it helps I have accepted Gabriel's patches and put them onto for-next.
> > > > >
> > > > > https://github.com/openrisc/linux/commits/for-next
> > > > >
> > > > > I am happy to take and test a patch based on that. Or I can do the adjustments
> > > > > to base the patch on that myself. Let me know.
> > > >
> > > > Thanks for letting me know! V3 sent.
> > >
> > > Hi Geert,
> > >
> > > I don't seem to see v3 anywhere. Where did you send it and what is the subject?
> >
> > https://lore.kernel.org/linux-riscv/[email protected]/
> >
> > So "b4 am [email protected]" should give you
> > a copy.
>
> Thanks I got it, I am not sure why it does not show up in my inbox anywhere,
> sometimes gmail drops mails. Hence, I am replying here.

You may want to add a rule to never mark as spam emails with "PATCH"
in the subject. And check your spam folder regularly, and teach gmail by
marking non-spam as non-spam.

> As per the typo above I can fix during applying or you could send during a v4.
>
> One more small nit is that you move soc_ctrl_dev out to a static instance it
> might help to mention. But it's easy to see why.

Found a way to get rid of it. Will send v4 shortly.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds