Re: [PATCH] ARM: shmobile: r7s72100: add restart handler

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Chris,

On Fri, Feb 10, 2017 at 8:46 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, February 10, 2017, Geert Uytterhoeven wrote:
>> Alternatively, you can write a restart driver (cfr.
>> drivers/power/reset/rmobile-reset.c) that binds against a
>> "renesas,r7s72100-wdt" device node, but doesn't implement watchdog
>> functionality.
>> You're gonna need DT bindings anyway.
>
> I like that idea. That should take me no time at all.
> Thank you.
>
> Do you think I can still keep my 'weak function' idea in there??
>
>
> extern void __attribute__ ((weak)) prepare_for_restart(void)
> {
>         /* override to do board specific stuff */
> }
>
> static int renesas_wdt_reset_handler(struct notifier_block *this,
>                                  unsigned long mode, void *cmd)
> {
>         pr_debug("%s %lu\n", __func__, mode);
>
>         prepare_for_restart();
>
>         /* set WDT for reset */
>         . . .
>
>         return NOTIFY_DONE;
> }

What do you want to use the board-specific function for?
Have a board-specific reset handler, or do some preparatory cleanup?

In case of the former, please write a separate driver that registers  a
reset handler with a higher priority.
In case of the latter, please use register_reboot_notifier() in the driver
that needs it.

> Or...do you think I can just use the rmobile-reset.c driver and
> just add WDT to it?
>
> Honestly, the only thing different will be rmobile_reset_handler().
> I could make a rmobile_wdt_reset_handler() and I could just pass in
> a different notifier_block depending on the DT.
>
> What do you think?

Given the small amount of code to add to the existing driver, and the sheer
amount of boilerplate for writing a new driver, I'm inclined to say yes.
But in the end, it's up to the subsystem maintainer to decide.

> static const struct of_device_id rmobile_reset_of_match[] = {
>         { .compatible = "renesas,sysc-rmobile", },
>         { .compatible = "renesas,wdt-rmobile", },

renesas,r7s72100-wdt

>         { /* sentinel */ }
> };

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