watchdog: introduce the ARM64 SBSA watchdog driver
PATCH v3 1/1 watchdog: introduce the ARM64 SBSA watchdog driver
The ARM Server Base System Architecture is a specification for ARM-based server systems. Among other things, it defines the behavior and register interface for a watchdog timer.
---
[v3]
removed cpu_to_le32
used __maybe_unused instead of #ifdef
drivers/watchdog/Kconfig | 9 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_sbsa_wdt.c | 296 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+) create mode 100644 drivers/watchdog/arm_sbsa_wdt.c
drivers/watchdog/Kconfig +9 -0
@@ -514,6 +514,15 @@ config MEDIATEK_WATCHDOG To compile this driver as a module, choose M here: the module will be called mtk_wdt. +config ARM_SBSA_WDT + tristate "ARM Server Base System Architecture watchdog" + depends on ARM64 || COMPILE_TEST + depends on ARM_ARCH_TIMER + select WATCHDOG_CORE + help + Say Y here to include watchdog timer support for ARM Server Base + System Architecture (SBSA) systems. + # AVR32 Architecture config AT32AP700X_WDT
drivers/watchdog/Makefile +1 -0
@@ -64,6 +64,7 @@ obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o +obj-$(CONFIG_ARM_SBSA_WDT) += arm_sbsa_wdt.o # AVR32 Architecture obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
drivers/watchdog/arm_sbsa_wdt.c new file +296 -0
296 lines not on this page · raw message
Signature · 3 lines
Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Guenter Roeck
maintainerQuoted 22 lines · Timur Tabi
The ARM Server Base System Architecture is a specification for ARM-based server systems. Among other things, it defines the behavior and register interface for a watchdog timer.
Signed-off-by Timur Tabi <[email protected]>---
[v3]
removedcpu_to_le32
used__maybe_unusedinstead of #ifdefdrivers/watchdog/Kconfig | 9 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_sbsa_wdt.c | 296 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+)create mode 100644
drivers/watchdog/arm_sbsa_wdt.cdrivers/watchdog/Kconfig +0
-0
drivers/watchdog/Kconfig config MEDIATEK_WATCHDOG · line 519
Quoted 6 lines
drivers/watchdog/Kconfig +3
-0@@ -514,6 +514,15 @@ config MEDIATEK_WATCHDOG To compile this driver as a module, choose M here: the module will be called mtk_wdt. +config ARM_SBSA_WDT + tristate "ARM Server Base System Architecture watchdog" + depends on ARM64 || COMPILE_TEST
COMPILE_TEST doesn't work with the current code - see below.
Quoted 51 lines
drivers/watchdog/Kconfig +6
-0+ depends on ARM_ARCH_TIMER + select WATCHDOG_CORE + help + Say Y here to include watchdog timer support for ARM Server Base + System Architecture (SBSA) systems. + # AVR32 Architecture config AT32AP700X_WDTdrivers/watchdog/Makefile +1
-0@@ -64,6 +64,7 @@ obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o +obj-$(CONFIG_ARM_SBSA_WDT) += arm_sbsa_wdt.o # AVR32 Architecture obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.odrivers/watchdog/arm_sbsa_wdt.c new file +24
-0@@ -0,0 +1,296 @@ +/* +* Watchdog driver for SBSA-compliant watchdog timers +* +* Copyright (c) 2015, The Linux Foundation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 and +* only version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* ARM Server Base System Architecture watchdog driver. +* +* Register descriptions are taken from the ARM Server Base System +* Architecture document (ARM-DEN-0029) +*/ + +#define pr_fmt (fmt) "sbsa-gwdt: " fmt + +#include <linux/module.h> +#include <linux/watchdog.h>
drivers/watchdog/arm_sbsa_wdt.c line 34
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/device.h> +#include <linux/io.h> + +#include <linux/irqdomain.h> +#include <linux/interrupt.h> +#include <linux/reboot.h> + +#include <asm/arch_timer.h>
Only exists for arm and arm64, so COMPILE_TEST isn't really very useful (also see readq problem below).
I still don't understand why you can not use the standard arch_sys_counter provided for arm but have to call arm-internal functions directly. That seems quite broken to me.
drivers/watchdog/arm_sbsa_wdt.c line 41
Quoted 7 lines
drivers/watchdog/arm_sbsa_wdt.c new file +7
-0+#include <linux/acpi.h> + +/* Watchdog Interface Identification Registers */ +struct arm_sbsa_watchdog_ident { +__le32 w_iidr;/* Watchdog Interface Identification Register */ +uint8_t res2[0xFE8 -0xFD0 ]; +__le32 w_pidr2;/* Peripheral ID2 Register */
Does this register have any use ?
Quoted 13 lines
drivers/watchdog/arm_sbsa_wdt.c new file +13
-0+}; + +/* Watchdog Refresh Frame */ +struct arm_sbsa_watchdog_refresh { +__le32 wrr;/* Watchdog Refresh Register */ +uint8_t res1[0xFCC -0x004 ]; +struct arm_sbsa_watchdog_ident ident; +}; + +/* Watchdog Control Frame */ +struct arm_sbsa_watchdog_control { +__le32 wcs; +__le32 res1;
drivers/watchdog/arm_sbsa_wdt.c line 64
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+__le32 wor; +__le32 res2; +__le64 wcv; +uint8_t res3[0xFCC -0x018 ]; +struct arm_sbsa_watchdog_ident ident; +}; + +struct arm_sbsa_watchdog_data { +struct watchdog_device wdev; +unsigned int pm_status;
You can use bool here.
drivers/watchdog/arm_sbsa_wdt.c line 76
Quoted 12 lines
drivers/watchdog/arm_sbsa_wdt.c new file +12
-0+struct arm_sbsa_watchdog_refresh __iomem *refresh; +struct arm_sbsa_watchdog_control __iomem *control; +}; + +static int arm_sbsa_wdt_start (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + +/* Writing to the control register will also reset the counter */ +writel (1 , &data->control->wcs); +
Is it ok to always overwrite the entire wcs register ?
Quoted 39 lines
drivers/watchdog/arm_sbsa_wdt.c new file +39
-0+return 0 ; +} + +static int arm_sbsa_wdt_stop (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + +writel (0 , &data->control->wcs); + +return 0 ; +} + +static int arm_sbsa_wdt_set_timeout (struct watchdog_device *wdev, +unsigned int timeout) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + + wdev->timeout = timeout; +writel (arch_timer_get_cntfrq () * wdev->timeout, &data->control->wor); + +return 0 ; +} + +static int arm_sbsa_wdt_ping (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + +writel (1 , &data->refresh->wrr); + +return 0 ; +} + +static unsigned int arm_sbsa_wdt_status (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
drivers/watchdog/arm_sbsa_wdt.c line 125
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+ +return (readl (&data->control->wcs) &1 ) <<WDOG_ACTIVE ; +} + +static unsigned int arm_sbsa_wdt_timeleft (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); +uint64_t diff =readq (&data->control->wcv) -arch_counter_get_cntvct (); +
Due to this readq the code doesn't compile on arm, nor on any other architecture which does not implement readq.
drivers/watchdog/arm_sbsa_wdt.c line 126
Quoted 1 line
drivers/watchdog/arm_sbsa_wdt.c new file +1
-0+return diff /arch_timer_get_cntfrq ();
.. and as mentioned before this won't compile on 32 bit targets.
Quoted 39 lines
drivers/watchdog/arm_sbsa_wdt.c new file +39
-0+} + +static irqreturn_t arm_sbsa_wdt_interrupt (int irq,void *p) +{ +/* +* The WS0 interrupt occurs after the first timeout, so we attempt +* a manual reboot. If this doesn't work, the WS1 timeout will +* cause a hardware reset. +*/ +pr_crit ("Initiating system reboot\n" ); +emergency_restart (); + +return IRQ_HANDLED ; +} + +/* +* Disable watchdog if it is active during suspend +*/ +static int __maybe_unused arm_sbsa_wdt_suspend (struct device *dev) +{ +struct arm_sbsa_watchdog_data *data =dev_get_drvdata (dev); + + data->pm_status =readl (&data->control->wcs) &1 ; + +if (data->pm_status) +writel (0 , &data->control->wcs); + +return 0 ; +} + +/* +* Enable watchdog and configure it if necessary +*/ +static int __maybe_unused arm_sbsa_wdt_resume (struct device *dev) +{ +struct arm_sbsa_watchdog_data *data =dev_get_drvdata (dev); + +if (data->pm_status) +writel (1 , &data->control->wcs);
drivers/watchdog/arm_sbsa_wdt.c
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+ +return 0 ; +} + +static const struct dev_pm_ops arm_sbsa_wdt_pm_ops = { +SET_SYSTEM_SLEEP_PM_OPS (arm_sbsa_wdt_suspend, arm_sbsa_wdt_resume) +}; + +static struct watchdog_info arm_sbsa_wdt_info = { + .options =WDIOF_SETTIMEOUT | _WDIOF_KEEPALIVEPING,
WDIOF_MAGICCLOSE ?
Quoted 29 lines
drivers/watchdog/arm_sbsa_wdt.c new file +29
-0+ .identity = "ARM SBSA watchdog", +}; + +static struct watchdog_ops arm_sbsa_wdt_ops = { + .owner =THIS_MODULE , + .start = arm_sbsa_wdt_start, + .stop = arm_sbsa_wdt_stop, + .ping = arm_sbsa_wdt_ping, + .set_timeout = arm_sbsa_wdt_set_timeout, + .status = arm_sbsa_wdt_status, + .get_timeleft = arm_sbsa_wdt_timeleft, +}; + +static int __init arm_sbsa_wdt_probe (struct platform_device *pdev) +{ +struct arm_sbsa_watchdog_data *data; +struct resource *res; +uint32_t iidr; +int irq, ret; + + data =devm_kzalloc (&pdev->dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"control" ); + data->control =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (data->control)) +return PTR_ERR (data->control); +
drivers/watchdog/arm_sbsa_wdt.c line 214
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+ res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"refresh" ); + data->refresh =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (data->refresh)) +return PTR_ERR (data->refresh); + +/* We only support architecture version 0 */ + iidr =readl (&data->control->ident.w_iidr); +if (((iidr >>16 ) &0xf ) !=0 ) { +dev_info (&pdev->dev, +"only architecture version 0 is supported\n" );
dev_err ?
It might make sense to display the detected version as well.
Quoted 17 lines
drivers/watchdog/arm_sbsa_wdt.c new file +17
-0+return -ENODEV ; + } + + irq =platform_get_irq_byname (pdev,"ws0" ); +if (irq <0 ) { +dev_err (&pdev->dev,"could not get interrupt\n" ); +return irq; + } + + ret =devm_request_irq (&pdev->dev, irq, arm_sbsa_wdt_interrupt, +0 , pdev->name,NULL ); +if (ret <0 ) { +dev_err (&pdev->dev,"could not request irq %i (ret=%i)\n" , + irq, ret); +return ret; + } +
drivers/watchdog/arm_sbsa_wdt.c line 241
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c new file +10
-0+ data->wdev.info = &arm_sbsa_wdt_info; + data->wdev.ops = &arm_sbsa_wdt_ops; + data->wdev.min_timeout =1 ; + data->wdev.status =WATCHDOG_NOWAYOUT_INIT_STATUS ; + +/* Calculate the maximum timeout in seconds that we can support */ + data->wdev.max_timeout =U32_MAX /arch_timer_get_cntfrq (); + +watchdog_set_drvdata (&data->wdev, data); +
You don't ever call watchdog_get_drvdata(), so this is unnecessary.
Quoted 56 lines
drivers/watchdog/arm_sbsa_wdt.c new file +55
-0+/* +* Bits [15:12] are an implementation-defined revision number +* for the component. +*/ + arm_sbsa_wdt_info.firmware_version = (iidr >>12 ) &0xf ; + + ret =watchdog_register_device (&data->wdev); +if (ret <0 ) { +dev_err (&pdev->dev, +"could not register watchdog device (ret=%i)\n" , ret); +return ret; + } + +dev_dbg (&pdev->dev,"implementer code is %03x\n" , + (iidr &0xf00 ) >>1 | (iidr &0x7f )); +dev_info (&pdev->dev,"maximum timeout is %u seconds\n" , + data->wdev.max_timeout); + +platform_set_drvdata (pdev, data); + +return 0 ; +} + +static int __exit arm_sbsa_wdt_remove(struct platform_device *pdev) +{ +struct arm_sbsa_watchdog_data *data =platform_get_drvdata (pdev); + +watchdog_unregister_device (&data->wdev); + +return 0 ; +} + +#if IS_ENABLED (CONFIG_OF ) +static const struct of_device_id arm_sbsa_wdt_of_match[] = { + { .compatible ="arm,sbsa-gwdt" }, + {}, +}; +MODULE_DEVICE_TABLE (of, arm_sbsa_wdt_of_match); +#endif + +static struct platform_driver arm_sbsa_wdt_driver = { + .driver = { + .name ="sbsa-gwdt" , + .owner =THIS_MODULE , + .pm = &arm_sbsa_wdt_pm_ops, + .of_match_table =of_match_ptr (arm_sbsa_wdt_of_match), + }, + .probe = arm_sbsa_wdt_probe, + .remove =__exit_p (arm_sbsa_wdt_remove), +}; + +module_platform_driver (arm_sbsa_wdt_driver); + +MODULE_DESCRIPTION ("ARM Server Base System Architecture Watchdog Driver" ); +MODULE_LICENSE ("GPL v2" );
Quoted 5 lines · Guenter Roeck
drivers/watchdog/Kconfig config MEDIATEK_WATCHDOG · line 519
Quoted 3 lines
drivers/watchdog/Kconfig +3
-0+config ARM_SBSA_WDT + tristate "ARM Server Base System Architecture watchdog" + depends on ARM64 || COMPILE_TEST
COMPILE_TESTdoesn't work with the current code - see below.
Ok, I'll remove it. I never liked it anyway.
Quoted 3 lines
I still don't understand why you can not use the standard
arch_sys_counterprovided for arm but have to call arm-internal functions directly. That seems quite broken to me.
I tried to use clk_get_sys(), but that failed because there are no clocks defined on my platform (the 'clocks' list in clkdev.c is empty).
I don't think the clock API is fully functional on an ARM ACPI system.
I was originally using functions like arch_timer_read_counter(), but that function is not exported, so I couldn't compile my driver as module if I used it.
Quoted 9 lines
drivers/watchdog/arm_sbsa_wdt.c line 41
Quoted 7 lines
drivers/watchdog/arm_sbsa_wdt.c new file +7
-0+#include <linux/acpi.h> + +/* Watchdog Interface Identification Registers */ +struct arm_sbsa_watchdog_ident { +__le32 w_iidr;/* Watchdog Interface Identification Register */ +uint8_t res2[0xFE8 -0xFD0 ]; +__le32 w_pidr2;/* Peripheral ID2 Register */ Does this register have any use ?
I don't use it in this driver, but it is part of the hardware.
Quoted 5 lines
drivers/watchdog/arm_sbsa_wdt.c line 64
Quoted 3 lines
drivers/watchdog/arm_sbsa_wdt.c new file +3
-0+struct arm_sbsa_watchdog_data { +struct watchdog_device wdev; +unsigned int pm_status;You can use bool here.
Ok.
Quoted 10 lines
drivers/watchdog/arm_sbsa_wdt.c line 76
Quoted 8 lines
drivers/watchdog/arm_sbsa_wdt.c new file +8
-0+static int arm_sbsa_wdt_start (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + +/* Writing to the control register will also reset the counter */ +writel (1 , &data->control->wcs); +Is it ok to always overwrite the entire wcs register ?
Yes. Writes to bits 1-31 have no affect.
Quoted 10 lines
Quoted 7 lines
drivers/watchdog/arm_sbsa_wdt.c new file +5
-0+static unsigned int arm_sbsa_wdt_timeleft (struct watchdog_device *wdev) +{ +struct arm_sbsa_watchdog_data *data = + container_of(wdev, struct arm_sbsa_watchdog_data, wdev); + uint64_t diff = readq(&data->control->wcv) -
arch_counter_get_cntvct();
+Due to this readq the code doesn't compile on arm, nor on any other architecture which does not implement readq.
Quoted 3 lines
Quoted 1 line
+ return diff /
arch_timer_get_cntfrq();.. and as mentioned before this won't compile on 32 bit targets.
That's why I have in my Kconfig:
depends on ARM64
It will never be compiled on a 32-bit ARM platform.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+static struct watchdog_info arm_sbsa_wdt_info = { + .options = WDIOF_SETTIMEOUT | _WDIOF_KEEPALIVEPING,
WDIOF_MAGICCLOSE?
Is it preferred to enable this feature? It seems like a policy setting that shouldn't be defined by the driver.
Quoted 8 lines
drivers/watchdog/arm_sbsa_wdt.c line 214
Quoted 5 lines
drivers/watchdog/arm_sbsa_wdt.c new file +5
-0+/* We only support architecture version 0 */ + iidr =readl (&data->control->ident.w_iidr); +if (((iidr >>16 ) &0xf ) !=0 ) { +dev_info (&pdev->dev, +"only architecture version 0 is supported\n" );
dev_err?
It might make sense to display the detected version as well.
Ok.
Quoted 3 lines
drivers/watchdog/arm_sbsa_wdt.c line 241
Quoted 2 lines
drivers/watchdog/arm_sbsa_wdt.c new file +2
-0+watchdog_set_drvdata (&data->wdev, data); +You don't ever call
watchdog_get_drvdata(), so this is unnecessary.
Ok
Signature · 3 lines
Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.