| Message ID | 20220310195229.109477-3-nick.hawkins@hpe.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Thu, Mar 10, 2022 at 01:52:22PM -0600, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > Adding support for the HPE GXP Watchdog. It is new to the linux > community and this along with several other patches is the first > support for it. The GXP asic contains a full compliment of > timers one of which is the watchdog timer. The watchdog timer > is 16 bit and has 10ms resolution. > > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> > --- > drivers/watchdog/Kconfig | 8 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/gxp-wdt.c | 191 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 200 insertions(+) > create mode 100644 drivers/watchdog/gxp-wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index c8fa79da23b3..cb210d2978d2 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -1820,6 +1820,14 @@ config RALINK_WDT > help > Hardware driver for the Ralink SoC Watchdog Timer. > > +config GXP_WATCHDOG > + tristate "HPE GXP watchdog support" > + depends on ARCH_HPE_GXP > + select WATCHDOG_CORE > + help > + Say Y here to include support for the watchdog timer > + in HPE GXP SoCs. > + > config MT7621_WDT > tristate "Mediatek SoC watchdog" > select WATCHDOG_CORE > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index f7da867e8782..e2acf3a0d0fc 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -92,6 +92,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o > obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o > obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o > obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o > +obj-$(CONFIG_GXP_WATCHDOG) += gxp-wdt.o > obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > diff --git a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c > new file mode 100644 > index 000000000000..d2b489cb4774 > --- /dev/null > +++ b/drivers/watchdog/gxp-wdt.c > @@ -0,0 +1,191 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. > + * > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of_address.h> > +#include <linux/of_platform.h> > +#include <linux/types.h> > +#include <linux/watchdog.h> > + > +#define MASK_WDGCS_ENABLE 0x01 > +#define MASK_WDGCS_RELOAD 0x04 > +#define MASK_WDGCS_NMIEN 0x08 > +#define MASK_WDGCS_WARN 0x80 > + > +#define WDT_MAX_TIMEOUT_MS 655000 > +#define WDT_DEFAULT_TIMEOUT 30 > +#define SECS_TO_WDOG_TICKS(x) ((x) * 100) > +#define WDOG_TICKS_TO_SECS(x) ((x) / 100) > + > +struct gxp_wdt { > + void __iomem *counter; > + void __iomem *control; > + struct watchdog_device wdd; Odd variable alignment. Might as well just use spaces before the variable names. > +}; > + > +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) > +{ > + uint8_t val; > + > + val = readb(drvdata->control); > + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD); > + writeb(val, drvdata->control); > +} > + > +static int gxp_wdt_start(struct watchdog_device *wdd) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + > + writew((SECS_TO_WDOG_TICKS(wdd->timeout)), drvdata->counter); Unnecessary iand confusing () around SECS_TO_WDOG_TICKS(). > + gxp_wdt_enable_reload(drvdata); > + return 0; > +} > + > +static int gxp_wdt_stop(struct watchdog_device *wdd) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + uint8_t val; > + > + val = readb_relaxed(drvdata->control); > + val &= ~MASK_WDGCS_ENABLE; > + writeb(val, drvdata->control); > + return 0; > +} > + > +static int gxp_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + uint32_t actual; Please use u32 as suggested by checkpatch. Same everywhere. > + > + wdd->timeout = timeout; > + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000); > + writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter); Unnecessary () > + > + return 0; > +} > + > +static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + uint32_t val = readw(drvdata->counter); > + > + return WDOG_TICKS_TO_SECS(val); > +} > + > +static int gxp_wdt_ping(struct watchdog_device *wdd) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + > + gxp_wdt_enable_reload(drvdata); > + return 0; > +} > + > +static int gxp_restart(struct watchdog_device *wdd, unsigned long action, > + void *data) > +{ > + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); > + > + writew(10, drvdata->counter); > + gxp_wdt_enable_reload(drvdata); > + mdelay(100); > + return 0; > +} > + > +static const struct watchdog_ops gxp_wdt_ops = { > + .owner = THIS_MODULE, > + .start = gxp_wdt_start, > + .stop = gxp_wdt_stop, > + .ping = gxp_wdt_ping, > + .set_timeout = gxp_wdt_set_timeout, > + .get_timeleft = gxp_wdt_get_timeleft, > + .restart = gxp_restart, Please be consistent with alignments. > +}; > + > +static const struct watchdog_info gxp_wdt_info = { > + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, > + .identity = "HPE GXP Watchdog timer", > +}; > + > +static int gxp_wdt_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct device *dev = &pdev->dev; > + struct gxp_wdt *drvdata; > + int err; > + uint8_t val; Please use u8. > + > + drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + platform_set_drvdata(pdev, drvdata); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + drvdata->counter = devm_ioremap_resource(dev, res); > + if (IS_ERR(drvdata->counter)) > + return PTR_ERR(drvdata->counter); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + drvdata->control = devm_ioremap_resource(dev, res); > + if (IS_ERR(drvdata->control)) > + return PTR_ERR(drvdata->control); Please use a single resource and offsets, or explain in a comment why that does not work for this driver. > + > + drvdata->wdd.info = &gxp_wdt_info; > + drvdata->wdd.ops = &gxp_wdt_ops; > + drvdata->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS; > + drvdata->wdd.parent = &pdev->dev; > + > + watchdog_set_drvdata(&drvdata->wdd, drvdata); > + watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev); That disables setting the watchdog timeout through devicetree, and is thus mostly pointless. It is better to set the default timeout via variable assignment above if you don't want to use devicetree to set the timeout, or pass a timeout value of 0 to get the timeout from devicetree (if configured). > + watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT); > + > + val = readb(drvdata->control); > + if (val & MASK_WDGCS_ENABLE) > + set_bit(WDOG_HW_RUNNING, &drvdata->wdd.status); > + > + watchdog_set_restart_priority(&drvdata->wdd, 128); > + > + watchdog_stop_on_reboot(&drvdata->wdd); > + err = devm_watchdog_register_device(dev, &drvdata->wdd); > + if (err) { > + dev_err(dev, "Failed to register watchdog device"); > + return err; > + } > + > + dev_info(dev, "HPE GXP watchdog timer"); > + return 0; > +} > + > +static int gxp_wdt_remove(struct platform_device *pdev) > +{ > + return 0; > +} Pointless remove function. > + > +static const struct of_device_id gxp_wdt_of_match[] = { > + { .compatible = "hpe,gxp-wdt", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, gxp_wdt_of_match); > + > +static struct platform_driver gxp_wdt_driver = { > + .probe = gxp_wdt_probe, > + .remove = gxp_wdt_remove, > + .driver = { > + .name = "gxp-wdt", > + .of_match_table = gxp_wdt_of_match, > + }, > +}; > +module_platform_driver(gxp_wdt_driver); > + > +MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>"); > +MODULE_AUTHOR("Jean-Marie Verdun <verdun@hpe.com>"); > +MODULE_DESCRIPTION("Driver for GXP watchdog timer");
-----Original Message----- From: Guenter Roeck [mailto:linux@roeck-us.net] Sent: Monday, April 4, 2022 9:29 AM To: Hawkins, Nick <nick.hawkins@hpe.com> Cc: Verdun, Jean-Marie <verdun@hpe.com>; Wim Van Sebroeck <wim@linux-watchdog.org>; linux-kernel@vger.kernel.org; linux-watchdog@vger.kernel.org Subject: Re: [PATCH v3 03/10] drivers: wdt: Introduce HPE GXP SoC Watchdog On Thu, Mar 10, 2022 at 01:52:22PM -0600, nick.hawkins@hpe.com wrote: >> From: Nick Hawkins <nick.hawkins@hpe.com> >> >> Adding support for the HPE GXP Watchdog. It is new to the linux >> community and this along with several other patches is the first >> support for it. The GXP asic contains a full compliment of timers one >> of which is the watchdog timer. The watchdog timer is 16 bit and has >> 10ms resolution. >> >> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> >> --- >> drivers/watchdog/Kconfig | 8 ++ >> drivers/watchdog/Makefile | 1 + >> drivers/watchdog/gxp-wdt.c | 191 >> +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 200 insertions(+) >> create mode 100644 drivers/watchdog/gxp-wdt.c >> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index >> c8fa79da23b3..cb210d2978d2 100644 >> --- a/drivers/watchdog/Kconfig >> +++ b/drivers/watchdog/Kconfig >> @@ -1820,6 +1820,14 @@ config RALINK_WDT >> help >> Hardware driver for the Ralink SoC Watchdog Timer. >> >> +config GXP_WATCHDOG >> + tristate "HPE GXP watchdog support" >> + depends on ARCH_HPE_GXP >> + select WATCHDOG_CORE >> + help >> + Say Y here to include support for the watchdog timer >> + in HPE GXP SoCs. >> + >> config MT7621_WDT >> tristate "Mediatek SoC watchdog" >> select WATCHDOG_CORE >> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile >> index f7da867e8782..e2acf3a0d0fc 100644 >> --- a/drivers/watchdog/Makefile >> +++ b/drivers/watchdog/Makefile >> @@ -92,6 +92,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o >> obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o >> obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o >> obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o >> +obj-$(CONFIG_GXP_WATCHDOG) += gxp-wdt.o >> obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o >> obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o >> obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o diff --git >> a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c new file >> mode 100644 index 000000000000..d2b489cb4774 >> --- /dev/null >> +++ b/drivers/watchdog/gxp-wdt.c >> @@ -0,0 +1,191 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. >> + * >> + * >> + * This program is free software; you can redistribute it and/or >> +modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include <linux/delay.h> >> +#include <linux/io.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/of_address.h> >> +#include <linux/of_platform.h> >> +#include <linux/types.h> >> +#include <linux/watchdog.h> >> + >> +#define MASK_WDGCS_ENABLE 0x01 >> +#define MASK_WDGCS_RELOAD 0x04 >> +#define MASK_WDGCS_NMIEN 0x08 >> +#define MASK_WDGCS_WARN 0x80 >> + >> +#define WDT_MAX_TIMEOUT_MS 655000 >> +#define WDT_DEFAULT_TIMEOUT 30 >> +#define SECS_TO_WDOG_TICKS(x) ((x) * 100) #define >> +WDOG_TICKS_TO_SECS(x) ((x) / 100) >> + >> +struct gxp_wdt { >> + void __iomem *counter; >> + void __iomem *control; >> + struct watchdog_device wdd; > Odd variable alignment. Might as well just use spaces before the variable names. Fixed >> +}; >> + >> +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) { >> + uint8_t val; >> + >> + val = readb(drvdata->control); >> + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD); >> + writeb(val, drvdata->control); >> +} >> + >> +static int gxp_wdt_start(struct watchdog_device *wdd) { >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + >> + writew((SECS_TO_WDOG_TICKS(wdd->timeout)), drvdata->counter); > Unnecessary iand confusing () around SECS_TO_WDOG_TICKS(). Fixed >> + gxp_wdt_enable_reload(drvdata); >> + return 0; >> +} >> + >> +static int gxp_wdt_stop(struct watchdog_device *wdd) { >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + uint8_t val; >> + >> + val = readb_relaxed(drvdata->control); >> + val &= ~MASK_WDGCS_ENABLE; >> + writeb(val, drvdata->control); >> + return 0; >> +} >> + >> +static int gxp_wdt_set_timeout(struct watchdog_device *wdd, >> + unsigned int timeout) >> +{ >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + uint32_t actual; > Please use u32 as suggested by checkpatch. Same everywhere. Fixed, checkpatch did not flag this, is there an option I should be using with checkpatch.pl? >> + >> + wdd->timeout = timeout; >> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000); >> + writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter); > Unnecessary () Fixed >> + >> + return 0; >> +} >> + >> +static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd) >> +{ >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + uint32_t val = readw(drvdata->counter); >> + >> + return WDOG_TICKS_TO_SECS(val); >> +} >> + >> +static int gxp_wdt_ping(struct watchdog_device *wdd) { >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + >> + gxp_wdt_enable_reload(drvdata); >> + return 0; >> +} >> + >> +static int gxp_restart(struct watchdog_device *wdd, unsigned long action, >> + void *data) >> +{ >> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >> + >> + writew(10, drvdata->counter); >> + gxp_wdt_enable_reload(drvdata); >> + mdelay(100); >> + return 0; >> +} >> + >> +static const struct watchdog_ops gxp_wdt_ops = { >> + .owner = THIS_MODULE, >> + .start = gxp_wdt_start, >> + .stop = gxp_wdt_stop, >> + .ping = gxp_wdt_ping, >> + .set_timeout = gxp_wdt_set_timeout, >> + .get_timeleft = gxp_wdt_get_timeleft, >> + .restart = gxp_restart, > Please be consistent with alignments. Fixed. >> +}; >> + >> +static const struct watchdog_info gxp_wdt_info = { >> + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, >> + .identity = "HPE GXP Watchdog timer", }; >> + >> +static int gxp_wdt_probe(struct platform_device *pdev) { >> + struct resource *res; >> + struct device *dev = &pdev->dev; >> + struct gxp_wdt *drvdata; >> + int err; >> + uint8_t val; > Please use u8. Fixed. >> + >> + drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + platform_set_drvdata(pdev, drvdata); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + drvdata->counter = devm_ioremap_resource(dev, res); >> + if (IS_ERR(drvdata->counter)) >> + return PTR_ERR(drvdata->counter); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + drvdata->control = devm_ioremap_resource(dev, res); >> + if (IS_ERR(drvdata->control)) >> + return PTR_ERR(drvdata->control); > Please use a single resource and offsets, or explain in a comment why that does not work for this driver. I actually was flagged for this on our separate device tree review and have trying to determine a better solution for it. The primary issue is that there are many controls in the register region. The current suggestion I am pursuing is having the timer be the parent and watchdog be the child all in the same driver. I need to get feedback from the clocksource owners on this. For reference here is the discussion: https://lore.kernel.org/all/CAK8P3a1Cc+2oY9djdp11PuOW+TBQ0zf+p8QaDY3aerk1QqaG-g@mail.gmail.com/ Any input on this is welcome. >> + >> + drvdata->wdd.info = &gxp_wdt_info; >> + drvdata->wdd.ops = &gxp_wdt_ops; >> + drvdata->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS; >> + drvdata->wdd.parent = &pdev->dev; >> + >> + watchdog_set_drvdata(&drvdata->wdd, drvdata); >> + watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev); > That disables setting the watchdog timeout through devicetree, and is thus mostly pointless. It is better to set the default timeout via variable assignment above if you don't want to use devicetree to set the timeout, or pass a timeout value of 0 to get the timeout from devicetree (if configured). Thank you. >> + watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT); >> + >> + val = readb(drvdata->control); >> + if (val & MASK_WDGCS_ENABLE) >> + set_bit(WDOG_HW_RUNNING, &drvdata->wdd.status); >> + >> + watchdog_set_restart_priority(&drvdata->wdd, 128); >> + >> + watchdog_stop_on_reboot(&drvdata->wdd); >> + err = devm_watchdog_register_device(dev, &drvdata->wdd); >> + if (err) { >> + dev_err(dev, "Failed to register watchdog device"); >> + return err; >> + } >> + >> + dev_info(dev, "HPE GXP watchdog timer"); >> + return 0; >> +} >> + >> +static int gxp_wdt_remove(struct platform_device *pdev) { >> + return 0; >> +} > Pointless remove function. Fixed. >> + >> +static const struct of_device_id gxp_wdt_of_match[] = { >> + { .compatible = "hpe,gxp-wdt", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, gxp_wdt_of_match); >> + >> +static struct platform_driver gxp_wdt_driver = { >> + .probe = gxp_wdt_probe, >> + .remove = gxp_wdt_remove, >> + .driver = { >> + .name = "gxp-wdt", >> + .of_match_table = gxp_wdt_of_match, >> + }, >> +}; >> +module_platform_driver(gxp_wdt_driver); >> + >> +MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>"); >> +MODULE_AUTHOR("Jean-Marie Verdun <verdun@hpe.com>"); >> +MODULE_DESCRIPTION("Driver for GXP watchdog timer");
On 4/4/22 09:25, Hawkins, Nick wrote: > > > -----Original Message----- > From: Guenter Roeck [mailto:linux@roeck-us.net] > Sent: Monday, April 4, 2022 9:29 AM > To: Hawkins, Nick <nick.hawkins@hpe.com> > Cc: Verdun, Jean-Marie <verdun@hpe.com>; Wim Van Sebroeck <wim@linux-watchdog.org>; linux-kernel@vger.kernel.org; linux-watchdog@vger.kernel.org > Subject: Re: [PATCH v3 03/10] drivers: wdt: Introduce HPE GXP SoC Watchdog > > On Thu, Mar 10, 2022 at 01:52:22PM -0600, nick.hawkins@hpe.com wrote: >>> From: Nick Hawkins <nick.hawkins@hpe.com> >>> >>> Adding support for the HPE GXP Watchdog. It is new to the linux >>> community and this along with several other patches is the first >>> support for it. The GXP asic contains a full compliment of timers one >>> of which is the watchdog timer. The watchdog timer is 16 bit and has >>> 10ms resolution. >>> >>> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> >>> --- >>> drivers/watchdog/Kconfig | 8 ++ >>> drivers/watchdog/Makefile | 1 + >>> drivers/watchdog/gxp-wdt.c | 191 >>> +++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 200 insertions(+) >>> create mode 100644 drivers/watchdog/gxp-wdt.c >>> >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index >>> c8fa79da23b3..cb210d2978d2 100644 >>> --- a/drivers/watchdog/Kconfig >>> +++ b/drivers/watchdog/Kconfig >>> @@ -1820,6 +1820,14 @@ config RALINK_WDT >>> help >>> Hardware driver for the Ralink SoC Watchdog Timer. >>> >>> +config GXP_WATCHDOG >>> + tristate "HPE GXP watchdog support" >>> + depends on ARCH_HPE_GXP >>> + select WATCHDOG_CORE >>> + help >>> + Say Y here to include support for the watchdog timer >>> + in HPE GXP SoCs. >>> + >>> config MT7621_WDT >>> tristate "Mediatek SoC watchdog" >>> select WATCHDOG_CORE >>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile >>> index f7da867e8782..e2acf3a0d0fc 100644 >>> --- a/drivers/watchdog/Makefile >>> +++ b/drivers/watchdog/Makefile >>> @@ -92,6 +92,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o >>> obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o >>> obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o >>> obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o >>> +obj-$(CONFIG_GXP_WATCHDOG) += gxp-wdt.o >>> obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o >>> obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o >>> obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o diff --git >>> a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c new file >>> mode 100644 index 000000000000..d2b489cb4774 >>> --- /dev/null >>> +++ b/drivers/watchdog/gxp-wdt.c >>> @@ -0,0 +1,191 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. >>> + * >>> + * >>> + * This program is free software; you can redistribute it and/or >>> +modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + */ >>> + >>> +#include <linux/delay.h> >>> +#include <linux/io.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_platform.h> >>> +#include <linux/types.h> >>> +#include <linux/watchdog.h> >>> + >>> +#define MASK_WDGCS_ENABLE 0x01 >>> +#define MASK_WDGCS_RELOAD 0x04 >>> +#define MASK_WDGCS_NMIEN 0x08 >>> +#define MASK_WDGCS_WARN 0x80 >>> + >>> +#define WDT_MAX_TIMEOUT_MS 655000 >>> +#define WDT_DEFAULT_TIMEOUT 30 >>> +#define SECS_TO_WDOG_TICKS(x) ((x) * 100) #define >>> +WDOG_TICKS_TO_SECS(x) ((x) / 100) >>> + >>> +struct gxp_wdt { >>> + void __iomem *counter; >>> + void __iomem *control; >>> + struct watchdog_device wdd; > >> Odd variable alignment. Might as well just use spaces before the variable names. > > Fixed > >>> +}; >>> + >>> +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) { >>> + uint8_t val; >>> + >>> + val = readb(drvdata->control); >>> + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD); >>> + writeb(val, drvdata->control); >>> +} >>> + >>> +static int gxp_wdt_start(struct watchdog_device *wdd) { >>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >>> + >>> + writew((SECS_TO_WDOG_TICKS(wdd->timeout)), drvdata->counter); > >> Unnecessary iand confusing () around SECS_TO_WDOG_TICKS(). > > Fixed > >>> + gxp_wdt_enable_reload(drvdata); >>> + return 0; >>> +} >>> + >>> +static int gxp_wdt_stop(struct watchdog_device *wdd) { >>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >>> + uint8_t val; >>> + >>> + val = readb_relaxed(drvdata->control); >>> + val &= ~MASK_WDGCS_ENABLE; >>> + writeb(val, drvdata->control); >>> + return 0; >>> +} >>> + >>> +static int gxp_wdt_set_timeout(struct watchdog_device *wdd, >>> + unsigned int timeout) >>> +{ >>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); >>> + uint32_t actual; > >> Please use u32 as suggested by checkpatch. Same everywhere. > > Fixed, checkpatch did not flag this, is there an option I should be using with checkpatch.pl? --strict Guenter
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index c8fa79da23b3..cb210d2978d2 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1820,6 +1820,14 @@ config RALINK_WDT help Hardware driver for the Ralink SoC Watchdog Timer. +config GXP_WATCHDOG + tristate "HPE GXP watchdog support" + depends on ARCH_HPE_GXP + select WATCHDOG_CORE + help + Say Y here to include support for the watchdog timer + in HPE GXP SoCs. + config MT7621_WDT tristate "Mediatek SoC watchdog" select WATCHDOG_CORE diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index f7da867e8782..e2acf3a0d0fc 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -92,6 +92,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o +obj-$(CONFIG_GXP_WATCHDOG) += gxp-wdt.o obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o diff --git a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c new file mode 100644 index 000000000000..d2b489cb4774 --- /dev/null +++ b/drivers/watchdog/gxp-wdt.c @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. + * + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/types.h> +#include <linux/watchdog.h> + +#define MASK_WDGCS_ENABLE 0x01 +#define MASK_WDGCS_RELOAD 0x04 +#define MASK_WDGCS_NMIEN 0x08 +#define MASK_WDGCS_WARN 0x80 + +#define WDT_MAX_TIMEOUT_MS 655000 +#define WDT_DEFAULT_TIMEOUT 30 +#define SECS_TO_WDOG_TICKS(x) ((x) * 100) +#define WDOG_TICKS_TO_SECS(x) ((x) / 100) + +struct gxp_wdt { + void __iomem *counter; + void __iomem *control; + struct watchdog_device wdd; +}; + +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) +{ + uint8_t val; + + val = readb(drvdata->control); + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD); + writeb(val, drvdata->control); +} + +static int gxp_wdt_start(struct watchdog_device *wdd) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + + writew((SECS_TO_WDOG_TICKS(wdd->timeout)), drvdata->counter); + gxp_wdt_enable_reload(drvdata); + return 0; +} + +static int gxp_wdt_stop(struct watchdog_device *wdd) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + uint8_t val; + + val = readb_relaxed(drvdata->control); + val &= ~MASK_WDGCS_ENABLE; + writeb(val, drvdata->control); + return 0; +} + +static int gxp_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + uint32_t actual; + + wdd->timeout = timeout; + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000); + writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter); + + return 0; +} + +static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + uint32_t val = readw(drvdata->counter); + + return WDOG_TICKS_TO_SECS(val); +} + +static int gxp_wdt_ping(struct watchdog_device *wdd) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + + gxp_wdt_enable_reload(drvdata); + return 0; +} + +static int gxp_restart(struct watchdog_device *wdd, unsigned long action, + void *data) +{ + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd); + + writew(10, drvdata->counter); + gxp_wdt_enable_reload(drvdata); + mdelay(100); + return 0; +} + +static const struct watchdog_ops gxp_wdt_ops = { + .owner = THIS_MODULE, + .start = gxp_wdt_start, + .stop = gxp_wdt_stop, + .ping = gxp_wdt_ping, + .set_timeout = gxp_wdt_set_timeout, + .get_timeleft = gxp_wdt_get_timeleft, + .restart = gxp_restart, +}; + +static const struct watchdog_info gxp_wdt_info = { + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, + .identity = "HPE GXP Watchdog timer", +}; + +static int gxp_wdt_probe(struct platform_device *pdev) +{ + struct resource *res; + struct device *dev = &pdev->dev; + struct gxp_wdt *drvdata; + int err; + uint8_t val; + + drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + platform_set_drvdata(pdev, drvdata); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + drvdata->counter = devm_ioremap_resource(dev, res); + if (IS_ERR(drvdata->counter)) + return PTR_ERR(drvdata->counter); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + drvdata->control = devm_ioremap_resource(dev, res); + if (IS_ERR(drvdata->control)) + return PTR_ERR(drvdata->control); + + drvdata->wdd.info = &gxp_wdt_info; + drvdata->wdd.ops = &gxp_wdt_ops; + drvdata->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS; + drvdata->wdd.parent = &pdev->dev; + + watchdog_set_drvdata(&drvdata->wdd, drvdata); + watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev); + watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT); + + val = readb(drvdata->control); + if (val & MASK_WDGCS_ENABLE) + set_bit(WDOG_HW_RUNNING, &drvdata->wdd.status); + + watchdog_set_restart_priority(&drvdata->wdd, 128); + + watchdog_stop_on_reboot(&drvdata->wdd); + err = devm_watchdog_register_device(dev, &drvdata->wdd); + if (err) { + dev_err(dev, "Failed to register watchdog device"); + return err; + } + + dev_info(dev, "HPE GXP watchdog timer"); + return 0; +} + +static int gxp_wdt_remove(struct platform_device *pdev) +{ + return 0; +} + +static const struct of_device_id gxp_wdt_of_match[] = { + { .compatible = "hpe,gxp-wdt", }, + {}, +}; +MODULE_DEVICE_TABLE(of, gxp_wdt_of_match); + +static struct platform_driver gxp_wdt_driver = { + .probe = gxp_wdt_probe, + .remove = gxp_wdt_remove, + .driver = { + .name = "gxp-wdt", + .of_match_table = gxp_wdt_of_match, + }, +}; +module_platform_driver(gxp_wdt_driver); + +MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>"); +MODULE_AUTHOR("Jean-Marie Verdun <verdun@hpe.com>"); +MODULE_DESCRIPTION("Driver for GXP watchdog timer");