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

[Rafał Miłecki]

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 at milecki.pl>
>>>
>>> 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 at milecki.pl>
>>> ---
>>
>> [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.