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

[Nicolas Ferre <nicolas.ferre@microchip.com>]

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
> 

-- 
Nicolas Ferre