Hi Geert,
On Monday, February 13, 2017, Geert Uytterhoeven wrote:
> On Fri, Feb 10, 2017 at 8:46 PM, Chris Brandt <Chris.Brandt@xxxxxxxxxxx>
> 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.
On my board (the RSK), I don't really care. I was thinking more about
other users boards. In other words, what should I tell them what they
should do?
I will suggest your recommendations above (not include the weak modifier).
> > 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
OK. Thanks!
Chris
