On 2/1/21 5:33 PM, Boris Lysov wrote:

В Sun, 31 Jan 2021 16:31:09 -0800
Guenter Roeck [off-list ref] пишет:

quoted

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.

quoted

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

So you are saying that the existing mediatek watchdog driver is not good ?
Or that it is good for setting the number of supported reset pins,
but not for setting the register width ?

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.

I don't like it at all, I don't see your problem, and mediatek,dma-33bits
seems to be a completely different scope (all it does is to trigger a couple
of additional writes, not control register width). On the other side,
you would have to sell it to dt maintainers, not to me.

Guenter

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