[PATCH 2/3] watchdog: mtk_wdt: add support for 16-bit control registers

[Boris Lysov]

В Sun, 31 Jan 2021 16:31:09 -0800
Guenter Roeck <linux at roeck-us.net> пишет:

> We can't do this. With this flag enabled, the watchdog won't
> support other SoCs, and there is nothing that prevents the flag
> from being set for those SoCs.
> 
> This has to be handled differently, without configuration
> flag. Maybe use regmap for register addresses, [snip],
> or use accessor functions in mtk_wdt_dev.

Thank you for reviewing my patch!

I will consider suggested fixes, and I will come up with better solution
in V2. I'm beginner developer, and am still learning.

> use the compatible string to determine which regmap settings to use

I think relying on hardcoded "compatible string - settings" pairs in
driver is not good. Whilst most Mediatek watchdogs I've seen use
similar drivers, no one (except Mediatek itself, of course) knows for
sure how many devices use 16-bit mode, and specifying each one in C
code may _theoretically_ bloat it. (well, on the other hand, I've seen
other watchdog drivers with many compatible devices listed in C code,
and they didn't seem bloated at all)

What do you think about implementing a simple boolean flag in
dt-binding for enabling the 16-bit operation mode? Something like
"mediatek,watchdog-16bits" [1] , which the driver would check for in
the `mtk_wdt_probe` and set corresponding regmaps. As result, there
won't be a need for kernel configuration flag, and other watchdogs
would be supported.

Most likely this idea doesn't sound good as I portray it, but I would
still like to hear your opinion about it. Thanks.

References:
[1] Mediatek UART APDMA driver uses similar flag
called `mediatek,dma-33bits`
Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt