[PATCH] versal: watchdog: Add support for Xilinx window watchdog

[Michal Simek <michal.simek@xilinx.com>]

Hi Stefan,

On 11. 03. 20 12:25, Stefan Roese wrote:
> On 11.03.20 11:48, Michal Simek wrote:
>> From: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
>>
>> Add support for Xilinx window watchdog, which can be found on
>> Versal platforms.
>>
>> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> ? MAINTAINERS??????????????????? |?? 1 +
>> ? drivers/watchdog/Kconfig?????? |?? 8 ++
>> ? drivers/watchdog/Makefile????? |?? 1 +
>> ? drivers/watchdog/xilinx_wwdt.c | 185 +++++++++++++++++++++++++++++++++
>> ? 4 files changed, 195 insertions(+)
>> ? create mode 100644 drivers/watchdog/xilinx_wwdt.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 37ff21a037b4..7cb1bc0957a3 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -433,6 +433,7 @@ M:??? Michal Simek <michal.simek@xilinx.com>
>> ? S:??? Maintained
>> ? T:??? git
>> https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze.git
>> ? F:??? arch/arm/mach-versal/
>> +F:??? drivers/watchdog/xilinx_wwdt.c
>> ? N:??? (?<!uni)versal
>> ? ? ARM VERSATILE EXPRESS DRIVERS
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index d24c1e48353f..b3911cf346ed 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -185,6 +185,14 @@ config XILINX_TB_WATCHDOG
>> ???????? Select this to enable Xilinx Axi watchdog timer, which can be
>> found on some
>> ???????? Xilinx Microblaze Platforms.
>> ? +config WWDT_XILINX
>> +??? bool "Xilinx window watchdog timer support"
>> +??? depends on WDT && ARCH_VERSAL
>> +??? imply WATCHDOG
>> +??? help
>> +????? Select this to enable Xilinx window watchdog timer, which can
>> be found on
>> +????? Xilinx Versal Platforms.
>> +
>> ? config WDT_TANGIER
>> ????? bool "Intel Tangier watchdog timer support"
>> ????? depends on WDT && INTEL_MID
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 87f92a43b14e..dc25560dbadf 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -30,3 +30,4 @@ obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
>> ? obj-$(CONFIG_WDT_SP805) += sp805_wdt.o
>> ? obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o
>> ? obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
>> +obj-$(CONFIG_WWDT_XILINX) += xilinx_wwdt.o
>> diff --git a/drivers/watchdog/xilinx_wwdt.c
>> b/drivers/watchdog/xilinx_wwdt.c
>> new file mode 100644
>> index 000000000000..34018e2b2b34
>> --- /dev/null
>> +++ b/drivers/watchdog/xilinx_wwdt.c
>> @@ -0,0 +1,185 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx window watchdog timer driver.
>> + *
>> + * Author(s):??? Michal Simek <michal.simek@xilinx.com>
>> + *??????? Ashok Reddy Soma <ashokred@xilinx.com>
>> + *
>> + * Copyright (c) 2020, Xilinx Inc.
>> + */
>> +
>> +#include <clk.h>
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <wdt.h>
>> +#include <linux/io.h>
>> +
>> +/* Refresh Register Masks */
>> +#define XWT_WWREF_GWRR_MASK??? BIT(0) /* Refresh and start new period */
>> +
>> +/* Generic Control/Status Register Masks */
>> +#define XWT_WWCSR_GWEN_MASK??? BIT(0) /* Enable Bit */
>> +
>> +struct wwdt_regs {
>> +??? u32 reserved0[1024];
>> +??? u32 refresh;??????? /* Refresh Register [0x1000] */
>> +??? u32 reserved1[1023];
>> +??? u32 csr;??????? /* Control/Status Register [0x2000] */
>> +??? u32 reserved2;
>> +??? u32 offset;??????? /* Offset Register [0x2008] */
>> +??? u32 reserved3;
>> +??? u32 cmp0;??????? /* Compare Value Register0 [0x2010] */
>> +??? u32 cmp1;??????? /* Compare Value Register1 [0x2014] */
>> +??? u32 reserved4[1006];
>> +??? u32 warmrst;??????? /* Warm Reset Register [0x2FD0] */
>> +};
> 
> My understanding is, that we moved to using defines instead of structs
> for register definitions. So if you need to send a v2, then please
> consider using #defines here.

When was that decision done? Any link to documentation/commit message?

Origin driver had macros but I have asked Ashok to change it to
structure based.

Thanks,
Michal