[PATCH] clk: renesas: r9a06g032: add restart handler

[PATCH] clk: renesas: r9a06g032: add restart handler

[Wolfram Sang]

The SYSCTRL module also does reset handling. Start supporting that by
allowing software resets which can then be utilized by a restart
handler. Finally 'reboot' will do something useful on RZ/N1D. Watchdog
support to be added later. Use BIT() macro consistently while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

I got a RZ/N1D now. Before I start the real work, I need working
'reboot' for fluent hacking :)

 drivers/clk/renesas/r9a06g032-clocks.c | 33 +++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index c1348e2d450c..8451567579c2 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -20,15 +20,24 @@
 #include <linux/platform_device.h>
 #include <linux/pm_clock.h>
 #include <linux/pm_domain.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/soc/renesas/r9a06g032-sysctrl.h>
 #include <linux/spinlock.h>
 #include <dt-bindings/clock/r9a06g032-sysctrl.h>
 
 #define R9A06G032_SYSCTRL_USB    0x00
-#define R9A06G032_SYSCTRL_USB_H2MODE  (1<<1)
+#define R9A06G032_SYSCTRL_USB_H2MODE BIT(1)
 #define R9A06G032_SYSCTRL_DMAMUX 0xA0
 
+#define R9A06G032_SYSCTRL_RSTEN 0x120
+#define R9A06G032_SYSCTRL_RSTEN_MRESET_EN BIT(0)
+#define R9A06G032_SYSCTRL_RSTCTRL 0x198
+/* These work for both reset registers */
+#define R9A06G032_SYSCTRL_SWRST BIT(6)
+#define R9A06G032_SYSCTRL_WDA7RST_1 BIT(2)
+#define R9A06G032_SYSCTRL_WDA7RST_0 BIT(1)
+
 /**
  * struct regbit - describe one bit in a register
  * @reg: offset of register relative to base address,
@@ -670,6 +679,7 @@ struct r9a06g032_priv {
 	struct clk_onecell_data data;
 	spinlock_t lock; /* protects concurrent access to gates */
 	void __iomem *reg;
+	struct notifier_block restart_nb;
 };
 
 static struct r9a06g032_priv *sysctrl_priv;
@@ -1270,6 +1280,13 @@ static void r9a06g032_clocks_del_clk_provider(void *data)
 	of_clk_del_provider(data);
 }
 
+static int r9a06g032_restart_notifier(struct notifier_block *nb,
+				      unsigned long action, void *data)
+{
+	writel(R9A06G032_SYSCTRL_SWRST, sysctrl_priv->reg + R9A06G032_SYSCTRL_RSTCTRL);
+	return NOTIFY_DONE;
+}
+
 static void __init r9a06g032_init_h2mode(struct r9a06g032_priv *clocks)
 {
 	struct device_node *usbf_np;
@@ -1324,6 +1341,20 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
 
 	r9a06g032_init_h2mode(clocks);
 
+	/* Clear potentially pending resets */
+	writel(R9A06G032_SYSCTRL_WDA7RST_0 | R9A06G032_SYSCTRL_WDA7RST_1,
+	       clocks->reg + R9A06G032_SYSCTRL_RSTCTRL);
+	/* Allow software reset */
+	writel(R9A06G032_SYSCTRL_SWRST | R9A06G032_SYSCTRL_RSTEN_MRESET_EN,
+	       clocks->reg + R9A06G032_SYSCTRL_RSTEN);
+
+	clocks->restart_nb.notifier_call = r9a06g032_restart_notifier;
+	clocks->restart_nb.priority = 192;
+
+	error = register_restart_handler(&clocks->restart_nb);
+	if (error)
+		dev_warn(dev, "couldn't register restart handler (%d)\n", error);
+
 	for (i = 0; i < ARRAY_SIZE(r9a06g032_clocks); ++i) {
 		const struct r9a06g032_clkdesc *d = &r9a06g032_clocks[i];
 		const char *parent_name = d->source ?
-- 
2.39.2

Re: [PATCH] clk: renesas: r9a06g032: add restart handler

[Geert Uytterhoeven]

Hi Wolfram,

On Thu, Nov 7, 2024 at 8:32 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> The SYSCTRL module also does reset handling. Start supporting that by
> allowing software resets which can then be utilized by a restart
> handler. Finally 'reboot' will do something useful on RZ/N1D. Watchdog
> support to be added later. Use BIT() macro consistently while here.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> I got a RZ/N1D now. Before I start the real work, I need working
> 'reboot' for fluent hacking :)

Yes, we all do ;-)

> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -20,15 +20,24 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_clock.h>
>  #include <linux/pm_domain.h>
> +#include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/soc/renesas/r9a06g032-sysctrl.h>
>  #include <linux/spinlock.h>
>  #include <dt-bindings/clock/r9a06g032-sysctrl.h>
>
>  #define R9A06G032_SYSCTRL_USB    0x00
> -#define R9A06G032_SYSCTRL_USB_H2MODE  (1<<1)
> +#define R9A06G032_SYSCTRL_USB_H2MODE BIT(1)
>  #define R9A06G032_SYSCTRL_DMAMUX 0xA0
>
> +#define R9A06G032_SYSCTRL_RSTEN 0x120
> +#define R9A06G032_SYSCTRL_RSTEN_MRESET_EN BIT(0)
> +#define R9A06G032_SYSCTRL_RSTCTRL 0x198
> +/* These work for both reset registers */
> +#define R9A06G032_SYSCTRL_SWRST BIT(6)
> +#define R9A06G032_SYSCTRL_WDA7RST_1 BIT(2)
> +#define R9A06G032_SYSCTRL_WDA7RST_0 BIT(1)
> +
>  /**
>   * struct regbit - describe one bit in a register
>   * @reg: offset of register relative to base address,
> @@ -670,6 +679,7 @@ struct r9a06g032_priv {
>         struct clk_onecell_data data;
>         spinlock_t lock; /* protects concurrent access to gates */
>         void __iomem *reg;
> +       struct notifier_block restart_nb;
>  };
>
>  static struct r9a06g032_priv *sysctrl_priv;
> @@ -1270,6 +1280,13 @@ static void r9a06g032_clocks_del_clk_provider(void *data)
>         of_clk_del_provider(data);
>  }
>
> +static int r9a06g032_restart_notifier(struct notifier_block *nb,
> +                                     unsigned long action, void *data)
> +{
> +       writel(R9A06G032_SYSCTRL_SWRST, sysctrl_priv->reg + R9A06G032_SYSCTRL_RSTCTRL);
> +       return NOTIFY_DONE;
> +}
> +
>  static void __init r9a06g032_init_h2mode(struct r9a06g032_priv *clocks)
>  {
>         struct device_node *usbf_np;
> @@ -1324,6 +1341,20 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
>
>         r9a06g032_init_h2mode(clocks);
>
> +       /* Clear potentially pending resets */
> +       writel(R9A06G032_SYSCTRL_WDA7RST_0 | R9A06G032_SYSCTRL_WDA7RST_1,
> +              clocks->reg + R9A06G032_SYSCTRL_RSTCTRL);
> +       /* Allow software reset */
> +       writel(R9A06G032_SYSCTRL_SWRST | R9A06G032_SYSCTRL_RSTEN_MRESET_EN,
> +              clocks->reg + R9A06G032_SYSCTRL_RSTEN);
> +
> +       clocks->restart_nb.notifier_call = r9a06g032_restart_notifier;
> +       clocks->restart_nb.priority = 192;
> +
> +       error = register_restart_handler(&clocks->restart_nb);

You can simplify using devm_register_sys_off_handler(), cfr.
drivers/power/reset/rmobile-reset.c.

> +       if (error)
> +               dev_warn(dev, "couldn't register restart handler (%d)\n", error);
> +
>         for (i = 0; i < ARRAY_SIZE(r9a06g032_clocks); ++i) {
>                 const struct r9a06g032_clkdesc *d = &r9a06g032_clocks[i];
>                 const char *parent_name = d->source ?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] clk: renesas: r9a06g032: add restart handler

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

Hi Geert,

> > I got a RZ/N1D now. Before I start the real work, I need working
> > 'reboot' for fluent hacking :)
> 
> Yes, we all do ;-)

Well, previous N1D developers didn't, it seems...

> > +       error = register_restart_handler(&clocks->restart_nb);
> 
> You can simplify using devm_register_sys_off_handler(), cfr.
> drivers/power/reset/rmobile-reset.c.

Ok, will do, thanks for the pointer. One question, though...

> 
> > +       if (error)
> > +               dev_warn(dev, "couldn't register restart handler (%d)\n", error);

rmobile bails out on error. I still think a warning is enough. What is
your preference?

Thanks and happy hacking,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] clk: renesas: r9a06g032: add restart handler

[Geert Uytterhoeven]

Hi Wolfram,

On Thu, Nov 28, 2024 at 9:45 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:

> > > +       error = register_restart_handler(&clocks->restart_nb);
> >
> > You can simplify using devm_register_sys_off_handler(), cfr.
> > drivers/power/reset/rmobile-reset.c.
>
> Ok, will do, thanks for the pointer. One question, though...
>
> > > +       if (error)
> > > +               dev_warn(dev, "couldn't register restart handler (%d)\n", error);
>
> rmobile bails out on error. I still think a warning is enough. What is
> your preference?

The mobile-reset driver is only a reset driver, hence failure is fatal.
For a combined clock/reset driver, a warning is fine, IMO.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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