MPC83xx reset status register (RSR, offset 0x910)

MPC83xx reset status register (RSR, offset 0x910)

[Radu Rendec]

Hi Everyone,

Is there any kernel code that handles the "reset status register" (RSR)
on MPC83xx? I looked at arch/powerpc/platforms/83xx/misc.c, but it seems
to only map the reset register area and it's static. The watchdog driver
(drivers/watchdog/mpc8xxx_wdt.c) doesn't seem to look at it either (for
the bootstatus flags).

Basically I need to check the CPU reset reason and I thought I would ask
first, before starting to write any code of my own.

Thanks,
Radu Rendec

Re: MPC83xx reset status register (RSR, offset 0x910)

[Christophe Leroy]

Hi

On 08/03/2018 04:36 PM, Radu Rendec wrote:
> Hi Everyone,
> 
> Is there any kernel code that handles the "reset status register" (RSR)
> on MPC83xx? I looked at arch/powerpc/platforms/83xx/misc.c, but it seems
> to only map the reset register area and it's static. The watchdog driver
> (drivers/watchdog/mpc8xxx_wdt.c) doesn't seem to look at it either (for
> the bootstatus flags).

How do you boot your Linux kernel ?

My 832x board boots using U-boot, and U-boot reads the RSR then clears 
it. So when Linux kernel reads it, it is just 0.

> 
> Basically I need to check the CPU reset reason and I thought I would ask
> first, before starting to write any code of my own.

Anyway, find below a set of two patches I used for testing. Feel free to 
use them if you bootloader doesn't clear the register

Christophe

commit f5171b463b149d6d60816fd9673e0367c0d6a90c
Author: Christophe Leroy <christophe.leroy@c-s.fr>
Date:   Wed Aug 22 12:49:42 2018 +0000

     powerpc/83xx: Show reset status at startup

     The 83xx has a register called Reset Status Register that
     shows the reason of the last reset.

     Print this reason at startup.

     Suggested-by: Radu Rendec <radu.rendec@gmail.com>

diff --git a/arch/powerpc/platforms/83xx/misc.c 
b/arch/powerpc/platforms/83xx/misc.c
index 773559431459..a3dbc22088d9 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -24,6 +24,16 @@
  #include "mpc83xx.h"

  #define RST_OFFSET	0x00000900
+#define RST_STAT_REG	0x00000010
+#define RSR_BSF		BIT(15)
+#define RSR_SWSR	BIT(18)
+#define RSR_SWHR	BIT(19)
+#define RSR_JSRS	BIT(23)
+#define RSR_CSHR	BIT(27)
+#define RSR_SWRS	BIT(28)
+#define RSR_BMRS	BIT(29)
+#define RSR_SRS		BIT(30)
+#define RSR_HRS		BIT(31)
  #define RST_PROT_REG	0x00000018
  #define RST_CTRL_REG	0x0000001c

@@ -34,6 +44,32 @@ static int __init mpc83xx_restart_init(void)
  	/* map reset restart_reg_baseister space */
  	restart_reg_base = ioremap(get_immrbase() + RST_OFFSET, 0xff);

+	if (restart_reg_base) {
+		u32 status = in_be32(restart_reg_base + (RST_STAT_REG >> 2));
+
+		if (status & RSR_BSF)
+			pr_info("Reset Status: Boot Sequencer Fail\n");
+		if (status & RSR_SWSR)
+			pr_info("Reset Status: Software soft reset\n");
+		if (status & RSR_SWHR)
+			pr_info("Reset Status: Software hard reset\n");
+		if (status & RSR_JSRS)
+			pr_info("Reset Status: JTAG soft reset\n");
+		if (status & RSR_CSHR)
+			pr_info("Reset Status: Checkstop reset\n");
+		if (status & RSR_SWRS)
+			pr_info("Reset Status: Software watchdog reset\n");
+		if (status & RSR_BMRS)
+			pr_info("Reset Status: Bus monitor reset\n");
+		if (status & RSR_SRS)
+			pr_info("Reset Status: Soft reset\n");
+		if (status & RSR_HRS)
+			pr_info("Reset Status: Hard reset\n");
+
+		/* clear reset statuses */
+		out_be32(restart_reg_base + (RST_STAT_REG >> 2), status);
+	}
+
  	return 0;
  }


commit c1ef17dd9c736c5e1b94da68852ae761a193090f
Author: Christophe Leroy <christophe.leroy@c-s.fr>
Date:   Wed Aug 22 12:19:02 2018 +0000

     powerpc/83xx: Move reset registers defines before functions

     Move the registers defines before/out of the functions.
     Reuse in another function.

diff --git a/arch/powerpc/platforms/83xx/misc.c 
b/arch/powerpc/platforms/83xx/misc.c
index d75c9816a5c9..773559431459 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -23,12 +23,16 @@

  #include "mpc83xx.h"

+#define RST_OFFSET	0x00000900
+#define RST_PROT_REG	0x00000018
+#define RST_CTRL_REG	0x0000001c
+
  static __be32 __iomem *restart_reg_base;

  static int __init mpc83xx_restart_init(void)
  {
  	/* map reset restart_reg_baseister space */
-	restart_reg_base = ioremap(get_immrbase() + 0x900, 0xff);
+	restart_reg_base = ioremap(get_immrbase() + RST_OFFSET, 0xff);

  	return 0;
  }
@@ -37,10 +41,6 @@ arch_initcall(mpc83xx_restart_init);

  void __noreturn mpc83xx_restart(char *cmd)
  {
-#define RST_OFFSET	0x00000900
-#define RST_PROT_REG	0x00000018
-#define RST_CTRL_REG	0x0000001c
-
  	local_irq_disable();

  	if (restart_reg_base) {

Re: MPC83xx reset status register (RSR, offset 0x910)

[Radu Rendec]

Hi,

On Fri, 2018-08-24 at 16:20 +0000, Christophe Leroy wrote:
> > On 08/03/2018 04:36 PM, Radu Rendec wrote:
> >
> > Is there any kernel code that handles the "reset status register" (RSR)
> > on MPC83xx? I looked at arch/powerpc/platforms/83xx/misc.c, but it seems
> > to only map the reset register area and it's static. The watchdog driver
> > (drivers/watchdog/mpc8xxx_wdt.c) doesn't seem to look at it either (for
> > the bootstatus flags).
>
> How do you boot your Linux kernel ?
>
> My 832x board boots using U-boot, and U-boot reads the RSR then clears
> it. So when Linux kernel reads it, it is just 0.

I'm using U-boot as well, but it's just not configured to read or clear
the RSR. I'm curious: if U-boot reads/clears the RSR in your case, how 
do you make the initial value available to user space programs running 
under Linux?

> > Basically I need to check the CPU reset reason and I thought I would ask
> > first, before starting to write any code of my own.
>
> Anyway, find below a set of two patches I used for testing. Feel free to
> use them if you bootloader doesn't clear the register

Thank you very much for the patches. Is there any chance they can be 
submitted upstream?

Of course, just printing the decoded bits is only helpful for testing. I
was thinking of a way to make the value available to both the kernel 
(as an exported symbol) and user space (e.g. via sysfs). Is there a
standard or preferred way to do this?

I tried to look for something similar on other platforms or architectures,
but couldn't find anything.

Thanks,
Radu Rendec

Re: MPC83xx reset status register (RSR, offset 0x910)

[Christophe LEROY]

Hi,

Le 10/09/2018 à 01:13, Radu Rendec a écrit :
> Hi,
> 
> On Fri, 2018-08-24 at 16:20 +0000, Christophe Leroy wrote:
>>> On 08/03/2018 04:36 PM, Radu Rendec wrote:
>>>
>>> Is there any kernel code that handles the "reset status register" (RSR)
>>> on MPC83xx? I looked at arch/powerpc/platforms/83xx/misc.c, but it seems
>>> to only map the reset register area and it's static. The watchdog driver
>>> (drivers/watchdog/mpc8xxx_wdt.c) doesn't seem to look at it either (for
>>> the bootstatus flags).
>>
>> How do you boot your Linux kernel ?
>>
>> My 832x board boots using U-boot, and U-boot reads the RSR then clears
>> it. So when Linux kernel reads it, it is just 0.
> 
> I'm using U-boot as well, but it's just not configured to read or clear
> the RSR. I'm curious: if U-boot reads/clears the RSR in your case, how
> do you make the initial value available to user space programs running
> under Linux?

I'm surprised. When looking at U-boot code, I don't see any way to 
configure that. It seems just do by default in function cpu_init_f():

https://elixir.bootlin.com/u-boot/v2018.07/source/arch/powerpc/cpu/mpc83xx/cpu_init.c#L217

	/* RSR - Reset Status Register - clear all status (4.6.1.3) */
	gd->arch.reset_status = __raw_readl(&im->reset.rsr);
	__raw_writel(~(RSR_RES), &im->reset.rsr);


Do you know any user space program in Linux that needs this value ?

> 
>>> Basically I need to check the CPU reset reason and I thought I would ask
>>> first, before starting to write any code of my own.
>>
>> Anyway, find below a set of two patches I used for testing. Feel free to
>> use them if you bootloader doesn't clear the register
> 
> Thank you very much for the patches. Is there any chance they can be
> submitted upstream?

I see no problem submitting them upstream, but are they really worth it 
? Adding Michael in copy to get his opinion.

> 
> Of course, just printing the decoded bits is only helpful for testing. I
> was thinking of a way to make the value available to both the kernel
> (as an exported symbol) and user space (e.g. via sysfs). Is there a
> standard or preferred way to do this?
> 
> I tried to look for something similar on other platforms or architectures,
> but couldn't find anything.

I believe furst thing is to identify some app needing such an 
information, then we'll be able to investigate how to handle it.

Christophe

> 
> Thanks,
> Radu Rendec
> 

Re: MPC83xx reset status register (RSR, offset 0x910)

[Radu Rendec]

Hi,

On Mon, 2018-09-10 at 07:37 +0200, Christophe Leroy wrote:
> Le 10/09/2018 =C3=A0 01:13, Radu Rendec a =C3=A9crit :
> >
> > I'm using U-boot as well, but it's just not configured to read or clear
> > the RSR. I'm curious: if U-boot reads/clears the RSR in your case, how
> > do you make the initial value available to user space programs running
> > under Linux?
>
> I'm surprised. When looking at U-boot code, I don't see any way to
> configure that. It seems just do by default in function cpu_init_f():
>
> https://elixir.bootlin.com/u-boot/v2018.07/source/arch/powerpc/cpu/mpc83x=
x/cpu_init.c#L217
>
>         /* RSR - Reset Status Register - clear all status (4.6.1.3) */
>         gd->arch.reset_status =3D __raw_readl(&im->reset.rsr);
>         __raw_writel(~(RSR_RES), &im->reset.rsr);

I'm working as a contractor in a large embedded project, so I don't know
all the bits and pieces. I just checked the U-boot code. Whoever was
maintaining it, "configured" it by commenting out the __raw_writel()
that clears the register :)

Probably the reason was specifically to be able to read it from Linux,
but unfortunately the guy is not here any more to ask him.

It may make more sense to read it from U-boot, but (1) the value must
still be passed to Linux somehow and (2) in my case, I would prefer not
to touch U-boot.

> Do you know any user space program in Linux that needs this value ?

I don't know of any "standard" program that needs it. In the project I'm
working on, there are multiple peripherals on the board and initialization
is slightly different when the reset line is physically asserted vs. a
soft CPU reset. Besides, we need to show the reset reason to the user.

I guess in the embedded world this is a fairly common use case, so
perhaps others can benefit from that if I fix it in a way that can be
pushed upstream.

> > Thank you very much for the patches. Is there any chance they can be
> > submitted upstream?
>
> I see no problem submitting them upstream, but are they really worth it
> ? Adding Michael in copy to get his opinion.

I guess it's worth if they are changed to make the value available to
the kernel and user space rather than just decoding/printing it, for the
reasons I mentioned above.

The MPC83xx also has a watchdog and the kernel driver (mpc8xxx_wdt.c)
could also be improved to support the WDIOC_GETBOOTSTATUS ioctl and
properly report if the system rebooted due to a watchdog.

> > I tried to look for something similar on other platforms or architectur=
es,
> > but couldn't find anything.
>
> I believe furst thing is to identify some app needing such an
> information, then we'll be able to investigate how to handle it.

Well, I guess I explained my reasons and use case. If there is any
interest in that, I will gladly implement it in a way that makes sense
to upstream. Let's see what Michael thinks.

Thanks,
Radu Rendec

Re: MPC83xx reset status register (RSR, offset 0x910)

[Christophe LEROY]

Le 11/09/2018 à 00:17, Radu Rendec a écrit :
> Hi,
> 
> On Mon, 2018-09-10 at 07:37 +0200, Christophe Leroy wrote:
>> Le 10/09/2018 à 01:13, Radu Rendec a écrit :
>>>
>>> I'm using U-boot as well, but it's just not configured to read or clear
>>> the RSR. I'm curious: if U-boot reads/clears the RSR in your case, how
>>> do you make the initial value available to user space programs running
>>> under Linux?
>>
>> I'm surprised. When looking at U-boot code, I don't see any way to
>> configure that. It seems just do by default in function cpu_init_f():
>>
>> https://elixir.bootlin.com/u-boot/v2018.07/source/arch/powerpc/cpu/mpc83xx/cpu_init.c#L217
>>
>>          /* RSR - Reset Status Register - clear all status (4.6.1.3) */
>>          gd->arch.reset_status = __raw_readl(&im->reset.rsr);
>>          __raw_writel(~(RSR_RES), &im->reset.rsr);
> 
> I'm working as a contractor in a large embedded project, so I don't know
> all the bits and pieces. I just checked the U-boot code. Whoever was
> maintaining it, "configured" it by commenting out the __raw_writel()
> that clears the register :)
> 
> Probably the reason was specifically to be able to read it from Linux,
> but unfortunately the guy is not here any more to ask him.
> 
> It may make more sense to read it from U-boot, but (1) the value must
> still be passed to Linux somehow and (2) in my case, I would prefer not
> to touch U-boot.
> 
>> Do you know any user space program in Linux that needs this value ?
> 
> I don't know of any "standard" program that needs it. In the project I'm
> working on, there are multiple peripherals on the board and initialization
> is slightly different when the reset line is physically asserted vs. a
> soft CPU reset. Besides, we need to show the reset reason to the user.
> 
> I guess in the embedded world this is a fairly common use case, so
> perhaps others can benefit from that if I fix it in a way that can be
> pushed upstream.
> 
>>> Thank you very much for the patches. Is there any chance they can be
>>> submitted upstream?
>>
>> I see no problem submitting them upstream, but are they really worth it
>> ? Adding Michael in copy to get his opinion.
> 
> I guess it's worth if they are changed to make the value available to
> the kernel and user space rather than just decoding/printing it, for the
> reasons I mentioned above.
> 
> The MPC83xx also has a watchdog and the kernel driver (mpc8xxx_wdt.c)
> could also be improved to support the WDIOC_GETBOOTSTATUS ioctl and
> properly report if the system rebooted due to a watchdog.

Very good idea.

I just submitted a patch for that. Please look at it.
I'm sure any driver which needs reset status information can do the same.

If we want to do something more central, maybe we should look at what 
was done on ARM:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04fef228fb00

Christophe


> 
>>> I tried to look for something similar on other platforms or architectures,
>>> but couldn't find anything.
>>
>> I believe furst thing is to identify some app needing such an
>> information, then we'll be able to investigate how to handle it.
> 
> Well, I guess I explained my reasons and use case. If there is any
> interest in that, I will gladly implement it in a way that makes sense
> to upstream. Let's see what Michael thinks.
> 
> Thanks,
> Radu Rendec
> 

Re: MPC83xx reset status register (RSR, offset 0x910)

[Radu Rendec]

Hi Christophe,

On Thu, 2018-09-13 at 10:21 +0200, Christophe LEROY wrote:
>
> Le 11/09/2018 à 00:17, Radu Rendec a écrit :
> >
> > The MPC83xx also has a watchdog and the kernel driver (mpc8xxx_wdt.c)
> > could also be improved to support the WDIOC_GETBOOTSTATUS ioctl and
> > properly report if the system rebooted due to a watchdog.
>
> Very good idea.
>
> I just submitted a patch for that. Please look at it.
> I'm sure any driver which needs reset status information can do the same.

Thanks for submitting the patch and sorry for the late reply! I followed
the conversation between you and Guenter and it seems your patches are
almost accepted. That's a good thing.

> If we want to do something more central, maybe we should look at what
> was done on ARM:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04fef228fb00

Thanks for pointing out that commit. It's very similar to what I wanted
to do (for MPC83xx) in the first place: read the RSR value on start-up
into a variable and export it as a symbol to make it available to other
drivers.

I would take on the work to implement something similar for PowerPC, but
I need some guidance as to what goes where. For instance, what would be
the PowerPC equivalent of arch/arm/mach-pxa/reset.c, which defines the
reset_status variable?

Another question is if the device tree should be used. We already have
separate directories for each platform in arch/powerpc/platforms and I
guess for each platform the RSR is always there and at a fixed, well
known address.

Thanks,
Radu