Re: [PATCH 3/3] watchdog: bcm7038_wdt: support BCM4908 SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29.10.2021 16:15, Guenter Roeck wrote:
On 10/29/21 5:15 AM, Rafał Miłecki wrote:
On 28.10.2021 18:57, Guenter Roeck wrote:
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.

I used name used in Broadcom's SDK (and as I guess also in their
documentation too).

Take a look at this BCM60333 example:

typedef struct Timer {
     uint32    TimerInts;        /* 0x00 */
     uint32    TimerCtl0;        /* 0x04 */
     uint32    TimerCtl1;        /* 0x08 */
     uint32    TimerCtl2;        /* 0x0c */
     uint32    TimerCnt0;        /* 0x10 */
     uint32    TimerCnt1;        /* 0x14 */
     uint32    TimerCnt2;        /* 0x18 */
     uint32    WatchDogDefCount;    /* 0x1c */
     uint32    WatchDogCtl;        /* 0x20 */
     uint32    WDResetCount;        /* 0x24 */
} Timer;

I got impression that Linux driver registers names usually follow what
is used in hardware documentation.

Still, the key part of the register name is "Count", not "Def",
and there is no "val" in there.

Absolutely right. No idea where did I take it from and how did I miss that.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux