Re: [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured with BD71828

From: Vaittinen, Matti
Date: Fri Jan 08 2021 - 01:12:46 EST



On Thu, 2021-01-07 at 20:15 +0200, Matti Vaittinen wrote:
> Thanks a lot for taking a careful look at this Guenter!
>
> On Thu, 2021-01-07 at 07:12 -0800, Guenter Roeck wrote:
> > On Thu, Jan 07, 2021 at 08:37:03AM +0200, Matti Vaittinen wrote:
> > > If config for BD70528 watchdog is enabled when BD71828 or BD71815
> > > are used the RTC module will issue call to BD70528 watchdog with
> > > NULL data. Ignore this call and don't crash.
> > >
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx
> > > >
> >
> > I really think this should be handled in the calling code.
> > Also, I am curious how this is supposed to work.
> >
> > The code is called with
> >
> > ret = bd70528_wdt_set(r->parent, new_state &
> > BD70528_WDT_STATE_BIT,
> > old_state);
>
> My brainfart.
> The bd70528_wdt_set is not called as it is protected in RTC by
> has_rtc_timers flag.
>
> I inserted this check in wrong function. The bd70528_wdt_lock()
> is where we may hit the problem as it is not protected.

Actually, after a fresh look - it seems the bd70528_wdt_lock() is also
just fine. The RTC is not grabbing the lock on other PMICs but the
BD70528. I'm not really sure what I have been thinking of. @_@ I
should've been more careful. Thanks for spotting this in the review!

Best Regards
Matti