On 10/28/21 9:29 AM, Florian Fainelli wrote:
On 10/28/21 2:30 AM, Rafał Miłecki wrote:
From: Rafał Miłecki <rafal@xxxxxxxxxx>
Hardware supported by this driver goes back to the old bcm63xx days. It
was then reused in BCM7038 and later also in BCM4908.
Depending on SoC model registers layout differs a bit. This commit
introduces support for per-chipset registers offsets & adds BCM4908
layout.
Later on BCM63xx SoCs support should be added too (probably as platform
devices due to missing DT). Eventually this driver should replace
bcm63xx_wdt.c.
Seems unrelated / irrelevant in this commit log, except maybe after '---'.
Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
---
[snip]
+
+static const u16 bcm7038_wdt_regs_bcm4908[] = {
+ [BCM63XX_WDT_REG_DEFVAL] = 0x28,
REG_DEFVAL is an odd name for this register. I can see that the
bcm63xx driver uses it, but in reality it seems to be the timeout
value, not some default value, only the bcm63xx driver doesn't
seem to use it properly. I think REG_TIMEOUT or similar would
be a much better name.
+ [BCM63XX_WDT_REG_CTL] = 0x2c,
+ [BCM63XX_WDT_REG_SOFTRESET] = 0x34,
I don't understand what you are doing here and why you are not
offsetting the "reg" property appropriately when you create your
bcm4908-wdt Device Tree node such that the base starts at 0, and the
existing driver becomes usable as-is. This does not make any sense to me
when it is obviously the simplest way to make the driver "accept" the
resource being passed.
Agreed. This adds a lot of complexity that could be avoided.
I am going to send my patch series converting the bcm63xx_wdt.c driver
over to bcm7038_wdt.c, feel free to use or discard it.
For my part I am very much looking forward to it.
Guenter
