В Sun, 31 Jan 2021 16:31:09 -0800 Guenter Roeck <linux@xxxxxxxxxxxx> пишет: > 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
