[U-Boot] [PATCH 2/2] drivers: watchdog: Add brcm iproc sp805 watchdog driver

[Stefan Roese]

Hi Vladimir,
Hi Pramod,

On 23.11.19 14:52, Stefan Roese wrote:
> On 23.11.19 00:23, Vladimir Olovyannikov wrote:
>> From: Pramod Kumar <pramod.kumar at broadcom.com>
>>
>> Add sp805 watchdog driver for Broadcom iproc socs (DM).
>>
>> Signed-off-by: Pramod Kumar <pramod.kumar at broadcom.com>
>> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>
>> ---
>>    drivers/watchdog/Kconfig        |  10 ++
>>    drivers/watchdog/Makefile       |   1 +
>>    drivers/watchdog/sp805_wdt_dm.c | 181 ++++++++++++++++++++++++++++++++
>>    3 files changed, 192 insertions(+)
>>    create mode 100644 drivers/watchdog/sp805_wdt_dm.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 8c16d69d33..74a5319467 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -40,6 +40,16 @@ config OMAP_WATCHDOG
>>    	help
>>    	  Say Y here to enable the OMAP3+ watchdog driver.
>>    
>> +config SP805_WATCHDOG
>> +	bool "Enable ARM SP805 watchdog driver (DM)"
>> +	depends on WDT
>> +	imply WATCHDOG
>> +	help
>> +	  Say Y here to enable the sp805 watchdog (DM)
>> +
>> +	  This provides basic infrastructure to support sp805 watchdog
>> +	  hardware; driver model.
>> +
>>    config ULP_WATCHDOG
>>    	bool "i.MX7ULP watchdog"
>>    	help
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 955caef815..aacabf1f2e 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_S5P)               += s5p_wdt.o
>>    obj-$(CONFIG_XILINX_TB_WATCHDOG) += xilinx_tb_wdt.o
>>    obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>>    obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
>> +obj-$(CONFIG_SP805_WATCHDOG) += sp805_iproc_dm.o
>>    obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o
>>    obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o
>>    obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o
>> diff --git a/drivers/watchdog/sp805_wdt_dm.c b/drivers/watchdog/sp805_wdt_dm.c
>> new file mode 100644
>> index 0000000000..56d0e77080
>> --- /dev/null
>> +++ b/drivers/watchdog/sp805_wdt_dm.c
>> @@ -0,0 +1,181 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2018 Broadcom.
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <dm/device.h>
>> +#include <dm/read.h>
>> +#include <log.h>
>> +#include <wdt.h>
>> +#include <asm/io.h>
>> +#include <linux/bitops.h>
>> +
>> +/* SP805 register offset */
>> +#define SP805_WDOG_LOAD_OFF		0x000
>> +#define SP805_WDOG_CTR_OFF		0x008
>> +#define SP805_WDOG_CLR_OFF		0x00c
>> +#define SP805_WDOG_LOCK_OFF		0xc00
>> +
>> +/* Magic word to unlock the wd registers */
>> +#define WDOG_UNLOCK_KEY			0x1ACCE551
>> +
>> +/* Register field definitions */
>> +#define SP805_CTR_RESEN			BIT(1)
>> +#define SP805_CTR_INTEN			BIT(0)
>> +
>> +struct sp805_wdt_platdata {
>> +	void __iomem *regs;
>> +	u32 timeout_msec;
>> +	u32 clk_mhz;
>> +};
>> +
>> +/* Inline register access functions */
>> +
>> +static inline void sp805_write_wdog_load(void __iomem *base, u32 value)
>> +{
>> +	writel(value, base + SP805_WDOG_LOAD_OFF);
>> +}
>> +
>> +static inline void sp805_write_wdog_ctrl(void __iomem *base, u32 value)
>> +{
>> +	writel(value, base + SP805_WDOG_CTR_OFF);
>> +}
>> +
>> +static inline void sp805_write_wdog_lock(void __iomem *base, u32 value)
>> +{
>> +	writel(value, base + SP805_WDOG_LOCK_OFF);
>> +}
>> +
>> +static inline void sp805_write_wdog_kick(void __iomem *base, u32 value)
>> +{
>> +	writel(value, base + SP805_WDOG_CLR_OFF);
>> +}
>> +
>> +static u32 msec_to_ticks(struct udevice *dev)
>> +{
>> +	u32 timeout_msec;
>> +	u32 msec;
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +
>> +	timeout_msec = env_get_ulong("wdt_timeout_msec", 16, 0);
>> +	if (timeout_msec) {
>> +		dev_dbg(dev, "Overriding timeout :%u\n", timeout_msec);
>> +		msec = timeout_msec;
>> +	} else {
>> +		msec = pd->timeout_msec;
>> +	}
>> +
>> +	timeout_msec = (msec / 2) * (pd->clk_mhz / 1000);
>> +
>> +	dev_dbg(dev, "ticks :%u\n", timeout_msec);
>> +
>> +	return timeout_msec;
>> +}
>> +
>> +static int sp805_wdt_expire_now(struct udevice *dev, ulong flags)
>> +{
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +
>> +	sp805_write_wdog_lock(pd->regs, WDOG_UNLOCK_KEY);
>> +	sp805_write_wdog_load(pd->regs, 0);
>> +	sp805_write_wdog_lock(pd->regs, 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sp805_wdt_reset(struct udevice *dev)
>> +{
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +	u32 ticks;
>> +
>> +	ticks = msec_to_ticks(dev);
>> +
>> +	sp805_write_wdog_lock(pd->regs, WDOG_UNLOCK_KEY);
>> +	sp805_write_wdog_load(pd->regs, ticks);
>> +	sp805_write_wdog_lock(pd->regs, 0);
>> +
>> +	dev_dbg(dev, "%s ", __func__);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sp805_wdt_stop(struct udevice *dev)
>> +{
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +
>> +	sp805_write_wdog_lock(pd->regs, WDOG_UNLOCK_KEY);
>> +	sp805_write_wdog_ctrl(pd->regs, 0);
>> +
>> +	dev_dbg(dev, "Watchdog disabled!\n");
>> +
>> +	return 0;
>> +}
>> +
>> +static int sp805_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>> +{
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +	u32 ticks;
>> +
>> +	ticks = msec_to_ticks(dev);
>> +
>> +	dev_dbg(dev, "%s:\n", __func__);
>> +
>> +	sp805_write_wdog_load(pd->regs, ticks);
>> +	sp805_write_wdog_ctrl(pd->regs, SP805_CTR_RESEN | SP805_CTR_INTEN);
>> +	/* Lock registers access */
>> +	sp805_write_wdog_lock(pd->regs, 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sp805_wdt_probe(struct udevice *dev)
>> +{
>> +	dev_dbg(dev, "%s: Probing wdt%u\n", __func__, dev->seq);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sp805_wdt_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +	struct sp805_wdt_platdata *pd = dev_get_platdata(dev);
>> +
>> +	dev_dbg(dev, "%s->\n", __func__);
>> +	pd->regs = dev_read_addr_ptr(dev);
>> +	if (!pd->regs)
>> +		return -ENODEV;
>> +
>> +	if (dev_read_u32(dev, "timeout-msec", &pd->timeout_msec))
>> +		return -ENODATA;
> 
> Why are you not using the already supported "timeout-sec" DT property
> instead? This one is also supported in Linux, so please move to this
> one. This DT values is already read in initr_watchdog() and provided to
> your DM enabled driver.

Could you please comment on this?

Thanks,
Stefan