diff mbox series

[v3,1/2] power: reset: at91-reset: change the power on reason prototype

Message ID 20230616135252.2787679-2-miquel.raynal@bootlin.com
State Accepted
Commit cba266a4f62bcf82528c45cf569412c542c007c9
Headers show
Series Expose reset reason through sysfs | expand

Commit Message

Miquel Raynal June 16, 2023, 1:52 p.m. UTC
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 the helper which returns the power on
reason. While we change the parameters, we can as well rename the
function from at91_reset_status() to at91_reset_reason() to be more
accurate with what the helper actually does, and finally because we don't
really need the pdev argument in this helper besides for printing the
reset reason, we can move the dev_info() call into the probe.

All these modifications prepare the introduction of a sysfs entry to
access this information. This way the diff will be much smaller. Thus,
there is no intended functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/power/reset/at91-reset.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Nicolas Ferre June 19, 2023, 8:10 a.m. UTC | #1
On 16/06/2023 at 15:52, Miquel Raynal wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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 the helper which returns the power on
> reason. While we change the parameters, we can as well rename the
> function from at91_reset_status() to at91_reset_reason() to be more
> accurate with what the helper actually does, and finally because we don't
> really need the pdev argument in this helper besides for printing the
> reset reason, we can move the dev_info() call into the probe.
> 
> All these modifications prepare the introduction of a sysfs entry to
> access this information. This way the diff will be much smaller. Thus,
> there is no intended functional change.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks Miquel:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
   Nicolas

> ---
>   drivers/power/reset/at91-reset.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> index 741e44a017c3..d6884841a6dc 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 const char * __init at91_reset_reason(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:
> @@ -185,7 +184,7 @@ static void __init at91_reset_status(struct platform_device *pdev,
>                  break;
>          }
> 
> -       dev_info(&pdev->dev, "Starting after %s\n", reason);
> +       return reason;
>   }
> 
>   static const struct of_device_id at91_ramc_of_match[] = {
> @@ -392,7 +391,7 @@ static int __init at91_reset_probe(struct platform_device *pdev)
>          if (ret)
>                  goto disable_clk;
> 
> -       at91_reset_status(pdev, reset->rstc_base);
> +       dev_info(&pdev->dev, "Starting after %s\n", at91_reset_reason(reset));
> 
>          return 0;
> 
> --
> 2.34.1
>
Sebastian Reichel June 19, 2023, 9:46 p.m. UTC | #2
Hi,

On Mon, Jun 19, 2023 at 10:10:05AM +0200, Nicolas Ferre wrote:
> On 16/06/2023 at 15:52, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > 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 the helper which returns the power on
> > reason. While we change the parameters, we can as well rename the
> > function from at91_reset_status() to at91_reset_reason() to be more
> > accurate with what the helper actually does, and finally because we don't
> > really need the pdev argument in this helper besides for printing the
> > reset reason, we can move the dev_info() call into the probe.
> > 
> > All these modifications prepare the introduction of a sysfs entry to
> > access this information. This way the diff will be much smaller. Thus,
> > there is no intended functional change.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> Thanks Miquel:
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks, queued.

-- Sebastian

> 
> Best regards,
>   Nicolas
> 
> > ---
> >   drivers/power/reset/at91-reset.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> > index 741e44a017c3..d6884841a6dc 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 const char * __init at91_reset_reason(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:
> > @@ -185,7 +184,7 @@ static void __init at91_reset_status(struct platform_device *pdev,
> >                  break;
> >          }
> > 
> > -       dev_info(&pdev->dev, "Starting after %s\n", reason);
> > +       return reason;
> >   }
> > 
> >   static const struct of_device_id at91_ramc_of_match[] = {
> > @@ -392,7 +391,7 @@ static int __init at91_reset_probe(struct platform_device *pdev)
> >          if (ret)
> >                  goto disable_clk;
> > 
> > -       at91_reset_status(pdev, reset->rstc_base);
> > +       dev_info(&pdev->dev, "Starting after %s\n", at91_reset_reason(reset));
> > 
> >          return 0;
> > 
> > --
> > 2.34.1
> > 
> 
> -- 
> Nicolas Ferre
>
diff mbox series

Patch

diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 741e44a017c3..d6884841a6dc 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 const char * __init at91_reset_reason(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:
@@ -185,7 +184,7 @@  static void __init at91_reset_status(struct platform_device *pdev,
 		break;
 	}
 
-	dev_info(&pdev->dev, "Starting after %s\n", reason);
+	return reason;
 }
 
 static const struct of_device_id at91_ramc_of_match[] = {
@@ -392,7 +391,7 @@  static int __init at91_reset_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_clk;
 
-	at91_reset_status(pdev, reset->rstc_base);
+	dev_info(&pdev->dev, "Starting after %s\n", at91_reset_reason(reset));
 
 	return 0;