Hi Geert,
On Thursday, September 06, 2018, Geert Uytterhoeven wrote:
> Thanks for your patch!
Thank you for your review.
> "timeout * rate" may overflow
> DIV_ROUND_UP()?
OK.
> > + pr_debug("%s: timeout set to %d (WTCNT=%d)\n", __func__,
>
> %u for unsigned.
OK.
> > + if (of_device_is_compatible(np, "renesas,r7s9210-wdt"))
> > + priv->ext_cks = true;
>
> That's not the proper way to support multiple devices.
> Please add an entry for "renesas,r7s9210-wdt" to rza_wdt_of_match[].
> "renesas,r7s9210-wdt" must be documented in the DT bindings.
>
> I suggest storing cks in rza_wdt_of_match[].data, and retrieving it with
> of_device_get_match_data() in your probe function, so you can use that to
> differentiate,
OK, I will change to that.
> and get rid of the ext_cks flag.
I still need to keep track if it's a RZ/A1 or RZ/A2 (ext_cks) for
functions outside of probe (rza_wdt_calc_timeout).
> BTW, is the RZ/A2 WDT compatible with the RZ/A1 WDT, i.e. does it work
> with the unmodified driver? If not, "renesas,rza-wdt" must not be used as
> a
> fallback.
I would say it is not, because the dividers are different (meaning the
calculated timeouts would not be correct).
Thanks,
Chris
