mbox series

[0/2] Expose reset reason through sysfs

Message ID 20230609143912.849995-1-miquel.raynal@bootlin.com
Headers show
Series Expose reset reason through sysfs | expand

Message

Miquel Raynal June 9, 2023, 2:39 p.m. UTC 
Hello,

Back in 2019, my colleague Kamel did try to upstream a small change in the at91 reset driver, in order to expose the reset reason through sysfs instead of expecting userland to grep through dmesg to get it. There was basically no strong reason opposed to it, besides minor changes which needed fixing. 4 years ago I am seeing again the need for such exposure, so here is Kamel's patch with the minor comments addressed, as well as a small cleanup just before.

Link: https://lore.kernel.org/lkml/00f4e9a2-f6bd-9242-cafd-9c0c4f4dc619@microchip.com/T/

Cheers,
Miquèl

Changes in v2:
* Collected Nicolas' Acked-by
* Dropped the Xtal frequency information (as this may change between
  platforms of course).

Kamel Bouhara (1):
  power: reset: at91-reset: add sysfs interface to the power on reason

Miquel Raynal (1):
  power: reset: at91-reset: use driver structure as status parameter

 .../testing/sysfs-platform-power-on-reason    | 10 +++++
 drivers/power/reset/at91-reset.c              | 45 +++++++++++++------
 include/linux/power/power_on_reason.h         | 19 ++++++++
 3 files changed, 60 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-power-on-reason
 create mode 100644 include/linux/power/power_on_reason.h

Comments

Miquel Raynal June 15, 2023, 2:04 p.m. UTC | #1 
Hi Sebastian,

sebastian.reichel@collabora.com wrote on Sat, 10 Jun 2023 01:14:22
+0200:

> Hi,
> 
> On Fri, Jun 09, 2023 at 04:39:11PM +0200, Miquel Raynal wrote:
> > It is quite uncommon to use a driver helper with parameters like *pdev
> > and __iomem *base. It is much cleaner and close to today's standards to
> > provide the per-device driver structure and then access its
> > internals. Let's do this with at91_resete_status() before making more
> > modifications to this helper.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/power/reset/at91-reset.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> > index 741e44a017c3..a8a6f3997768 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -149,11 +149,10 @@ static int at91_reset(struct notifier_block *this, unsigned long mode,
> >  	return NOTIFY_DONE;
> >  }
> >  
> > -static void __init at91_reset_status(struct platform_device *pdev,
> > -				     void __iomem *base)
> > +static void __init at91_reset_status(struct at91_reset *reset)
> >  {
> > +	u32 reg = readl(reset->rstc_base + AT91_RSTC_SR);
> >  	const char *reason;
> > -	u32 reg = readl(base + AT91_RSTC_SR);
> >  
> >  	switch ((reg & AT91_RSTC_RSTTYP) >> 8) {
> >  	case RESET_TYPE_GENERAL:  
> 
> You also need to update the code calling this functions, otherwise
> the series is not bisectable.

Of course, I was not paying enough attention here, sorry about that.

Thanks,
Miquèl