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

[Vaittinen, Matti]

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.
> 
> from bd70528_set_rtc_based_timers(). That same function subsequently
> calls bd70528_set_elapsed_tmr() with the same parameter, and that
> parameter is dereferenced in bd70528_set_elapsed_tmr() without
> checking.
> 
> Conceptually, it should not be necessary to determine at compile-time
> which of the chips is in the system. It should be posible to compile
> a single kernel which supports all chips.

I agree. The information whether WDT should be accessed should be
judged by dt-compatible. MFD has this knowledge and passes it to RTC.
So yes, RTC should omit the call if BD70528 is not used. Please ignore
these patches, I'll do changes to RTC driver :)


Best Regards
Matti