Re: [PATCH v2 1/6] wdt: sunxi: Move restart code to the watchdog driver

[Guenter Roeck]

On 06/23/2014 07:42 AM, Arnd Bergmann wrote:
> On Monday 23 June 2014 07:30:56 Guenter Roeck wrote:
> > On 06/23/2014 03:31 AM, Maxime Ripard wrote:
> > > On Thu, May 22, 2014 at 02:12:07PM -0700, Guenter Roeck wrote:
> > > > On Thu, May 22, 2014 at 10:34:44PM +0200, Maxime Ripard wrote:
> > > > > On Mon, May 19, 2014 at 05:04:22PM +0200, Maxime Ripard wrote:
> > > >
> > > > The patches _are_ in my watchdog-next branch and get some coverage from
> > > > both my auto-builders and from Fenguang's build robots, so while they are
> > > > not in linux-next, they are not completely in the dark either.
> > >
> > > So, this patch finally didn't make it into 3.16. Great. Now, we can't
> > > even reboot the boards.
> > >
> > > Given how it's just impossible to get something merged reliably
> > > through the watchdog tree, I guess I should just start merging the
> > > patches through mine?
> >
> > You can not really blame Wim here.
> >
> > In this case, I suspect the major reason for not accepting the patch
> > is that I tried to provide a clean method / API for "reset through watchdog
> > subsystem", which went nowhere, in my understanding because someone objected
> > that it would be the wrong thing to do [1] and it didn't get approval /
> > acceptance from the arm maintainers. If it is wrong to reset the board
> > from the watchdog subsystem in a clean way, it is for sure even more wrong
> > to do it as you proposed in your patch.
> >
> > My conclusion therefore is that all board reset code should move back out
> > of the watchdog subsystem, and that we should not accept such code in the
> > future. This is not my personal preference, but I do believe that we should
> > do it in a clean way or not at all.
>
> Moved to where?
>
> I certainly don't want it in the platform directories, and for arm64 we
> intentionally don't have a place to put this stuff.

I have no idea, but setting the arm reset function pointer from a watchdog
driver doesn't seem like a good idea either. The arm code _does_ provide
and expect platform code to set the reset function, so having it in the arm
code would at least make more sense than expecting some unrelated driver
code to set it - especially since it is inherently racy [1].

> drivers/power/reset would be an option, but then we have to solve the
> problem of loading two drivers for one device first.
It sounds crazy and may be huge overkill, but one solution for this is
would be to use mfd to bridge the gap.

Guenter

---
[1] https://lkml.org/lkml/2014/5/15/838

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/