Linaro-acpi May 2015

linaro-acpi@lists.linaro.org

40 participants
44 discussions

by fu.wei＠linaro.org

From: Fu Wei <fu.wei(a)linaro.org> This driver is designed for ARM AArch64 HW originally. Now It's tested on Foundation model with Linux watchdog deamon. It support two stages watch period, and will trigger a SPI after first timeout. Fu Wei (1): Watchdog: add SBSA watchdog driver (1)use linux kernel watchdog framework (2)support getting info from ACPI GTDT table drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 805 +++++++++++++++++++++++++++++++++++++++++++ include/linux/sbsa_gwdt.h | 134 +++++++ 4 files changed, 950 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c create mode 100644 include/linux/sbsa_gwdt.h -- 1.9.1

8 years, 12 months

Re: [Linaro-acpi] [PATCH] watchdog: introduce the ARM64 SBSA watchdog driver

by Timur Tabi

On 04/30/2015 10:30 PM, Guenter Roeck wrote: >> +/* Watchdog Refresh Frame */ >> +struct arm_sbsa_watchdog_refresh { >> + uint32_t wrr; /* Watchdog Refresh Register */ >> + uint8_t res1[0xFCC - 0x004]; >> + struct arm_sbsa_watchdog_ident ident; >> +}; >> + >> +/* Watchdog Control Frame */ >> +struct arm_sbsa_watchdog_control { >> + uint32_t wcs; >> + uint32_t res1; >> + uint32_t wor; >> + uint32_t res2; >> + uint64_t wcv; >> + uint8_t res3[0xFCC - 0x018]; >> + struct arm_sbsa_watchdog_ident ident; >> +}; >> + > > Why not just use defines instead of all those structures ? I like structures. I think hardware register blocks should be defined with structures that provide type checking. >> +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_rate() * wdev->timeout, &data->control->wor); > > Do you also have to reset the counter ? No. Programming a new timeout automatically resets the timer. >> +static int __init arm_sbsa_wdt_probe(struct platform_device *pdev) >> +{ >> + struct arm_sbsa_watchdog_data *data; >> + struct resource *res; >> + uint32_t iidr; >> + int ret; >> + >> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!res || !res->start) { >> + dev_err(&pdev->dev, "could not get control address\n"); >> + return -ENOMEM; >> + } >> + > devm_ioremap_resource already prints an error message if res is NULL. > And res->start can not be 0 unless there is a severe infrastructure > problem. Will fix. >> + data->control = devm_ioremap_resource(&pdev->dev, res); >> + if (!data->control) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + if (!res || !res->start) { >> + dev_err(&pdev->dev, "could not get refresh address\n"); >> + return -ENOMEM; >> + } > Same here. > >> + data->refresh = devm_ioremap_resource(&pdev->dev, res); >> + if (!data->refresh) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> + if (!res || !res->start) { > > res->start checking is unnecessary. On a side note, irq == 0 might be a > valid > interrupt number. Will fix. > >> + dev_err(&pdev->dev, "could not get interrupt\n"); >> + return -ENOMEM; >> + } >> + >> + iidr = readl(&data->control->ident.w_iidr); >> + >> + /* We only support architecture version 0 */ >> + if (((iidr >> 16) & 0xf) != 0) { >> + dev_info(&pdev->dev, "only architecture version 0 is >> supported\n"); >> + return -ENODEV; >> + } >> + >> + ret = devm_request_irq(&pdev->dev, res->start, >> arm_sbsa_wdt_interrupt, >> + 0, DRV_NAME, NULL); > > Please align continuation lines with '('. Will fix. >> + if (ret < 0) { >> + dev_err(&pdev->dev, "could not request irq %u (ret=%i)\n", >> + (unsigned int)res->start, ret); >> + return ret; >> + } >> + >> + 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_rate(); >> + >> + 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_info(&pdev->dev, "implementer code is %03x, max timeout is %u >> seconds\n", >> + (iidr & 0xf00) >> 1 | (iidr & 0x7f), data->wdev.max_timeout); > > "Implementer code" sounds very much like a debug message to me. > It means nothing to me, and it won't mean anything to a user. Fair enough. I thought it would be a handy way to know that the driver found the hardware, but I'll move it to a pr_debug(). >> + >> + /* >> + * Bits [15:12] are an implementation-defined revision number >> + * for the component. >> + */ >> + arm_sbsa_wdt_info.firmware_version = (iidr >> 12) & 0xf; >> + > It might make sense to set that prior to registering the driver. Ok. >> +static struct platform_device *arm_sbsa_wdt_pdev; >> + >> +static int __init arm_sbsa_wdt_parse_gtdt(struct acpi_subtable_header >> *header, >> + const unsigned long end) >> +{ >> + struct acpi_gtdt_watchdog *wdg = (struct acpi_gtdt_watchdog >> *)header; >> + struct platform_device *arm_sbsa_wdt_pdev; > > That is an interesting one. Makes me wonder if you ever tried to unload > this driver. > Did you ? The driver can only be compiled in-kernel. >> + struct resource res[3] = {}; >> + int trigger, polarity; >> + int ret; >> + >> + if (!header || > > That error check is kind of weird. Sure, 0 is an invalid address, but so > are many other > addresses. Is there any reason to believe that acpi_get_table_with_size > would return > a NULL pointer (but not some other invalid address) ? If the table is uninitialized, then all the values are probably zero. I was trying to come up with something. These are the only tests I could come up with I know work. >> + (unsigned long)header + sizeof(*wdg) > end || >> + header->length < sizeof(*wdg)) { > > So I really wonder how this is supposed to work. > > struct acpi_subtable_header { > u8 type; > u8 length; > }; > > struct acpi_gtdt_watchdog { > struct acpi_gtdt_header header; > ... > > struct acpi_gtdt_header { > u8 type; > u16 length; > }; > > Either the length is 16 bit or 8 bit. But both ??? > Or is it just luck and the value happens to be stored as little endian > value and the length is less than 256 bytes ? > > I understand this seems to be copied from BAD_MADT_ENTRY or similar code, > but it is still odd. It's the best I could come up with. Sure it seems weird, but it works, and since it's copied from BAD_MADT_ENTRY, I figured it was acceptable. > >> + pr_err("malformed subtable entry\n"); >> + return -EINVAL; >> + } >> + >> + if (!wdg->control_frame_address || !wdg->refresh_frame_address) { >> + pr_err("invalid control or refresh address\n"); >> + return -ENXIO; >> + } >> + >> + arm_sbsa_wdt_pdev = platform_device_alloc(DRV_NAME, 0); >> + if (!arm_sbsa_wdt_pdev) >> + return -ENOMEM; >> + >> + res[0].name = "control"; >> + res[0].flags = IORESOURCE_MEM; >> + res[0].start = wdg->control_frame_address; >> + res[0].end = res[0].start + sizeof(struct >> arm_sbsa_watchdog_control); > > Really ? Or should there be a -1 somewhere ? You're right. >> + >> + res[1].name = "refresh"; >> + res[1].flags = IORESOURCE_MEM; >> + res[1].start = wdg->refresh_frame_address; >> + res[1].end = res[1].start + sizeof(struct >> arm_sbsa_watchdog_refresh); > > Same here. Ok. >> + trigger = (wdg->timer_flags & ACPI_GTDT_WATCHDOG_IRQ_MODE) ? >> + ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; >> + >> + polarity = (wdg->timer_flags & ACPI_GTDT_WATCHDOG_IRQ_POLARITY) ? >> + ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; >> + >> + /* This should be the WS0 interrupt */ >> + ret = acpi_register_gsi(&arm_sbsa_wdt_pdev->dev, >> wdg->timer_interrupt, >> + trigger, polarity); >> + if (ret <= 0) { > > 0 is not an error (the interrupt number could be 0). Ok. >> +static int __init arm_sbsa_wdt_init(void) >> +{ >> + struct acpi_table_header *table; >> + acpi_size tbl_size; >> + acpi_status status; >> + int count; >> + >> + pr_info("ARM Server Base System Architecture watchdog driver\n"); >> + > This seems to assume that the watchdog is always supported, which is > quite unlikely. I'll make it a pr_debug. >> + status = acpi_get_table_with_size(ACPI_SIG_GTDT, 0, &table, >> &tbl_size); >> + if (ACPI_FAILURE(status)) { >> + pr_err("cannot locate GTDT table\n"); >> + return -EINVAL; >> + } > > I am kind of completely unhappy with the noisiness here and below. > Is this how acpi detection is supposed to happen ? > And is it really an _error_ if the device isn't there, > or does it just mean that the device isn't there ? I think the GTDT is required. Most likely, the kernel will fail to boot long before we get to this point if the GTDT is missing or corrupt. However, I'm not an ACPI expert by any means. I'm hoping that someone who knows a lot more than I do will review it. This driver was reviewed and approved by some of our internal ACPI experts, so I presume that it is correct. >> + count = acpi_parse_entries(ACPI_SIG_GTDT, sizeof(struct >> acpi_table_gtdt), >> + arm_sbsa_wdt_parse_gtdt, table, ACPI_GTDT_TYPE_WATCHDOG, 0); >> + if (count <= 0) { >> + pr_err("no watchdog subtables found\n"); >> + return -EINVAL; >> + } >> + >> + return platform_driver_probe(&arm_sbsa_wdt_driver, >> arm_sbsa_wdt_probe); > > Another oddity. Is this how acpi device instantiation is supposed to > happen ? > > I thought there are functions to register acpi drivers, such as > acpi_bus_register_driver(). > Why doesn't this work here ? I can't use acpi_bus_register_driver() because there are no ACPI IDs to probe on. Watchdog information is stored as a subtable of the GTDT table, and there's nothing in the ACPI layer that automatically creates platforms from subtables. I have to create the platform from scratch, which is why the code looks so messy. -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

9 years

Re: [Linaro-acpi] [PATCH] watchdog: introduce the ARM64 SBSA watchdog driver

by Ashwin Chaugule

+ Linaro-acpi On 29 April 2015 at 15:33, Timur Tabi <timur(a)codeaurora.org> wrote: > 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 <timur(a)codeaurora.org> > --- > drivers/watchdog/Kconfig | 10 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/arm_sbsa_wdt.c | 368 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 379 insertions(+) > create mode 100644 drivers/watchdog/arm_sbsa_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index e5e7c55..a2a133f 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -514,6 +514,16 @@ config MEDIATEK_WATCHDOG > To compile this driver as a module, choose M here: the > module will be called mtk_wdt. > > +config ARM_SBSA_WDT > + bool "ARM Server Base System Architecture watchdog" > + depends on ACPI > + depends on ARM64 > + 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 > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 5c19294..063ab8c 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -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 > diff --git a/drivers/watchdog/arm_sbsa_wdt.c b/drivers/watchdog/arm_sbsa_wdt.c > new file mode 100644 > index 0000000..2a2be40 > --- /dev/null > +++ b/drivers/watchdog/arm_sbsa_wdt.c > @@ -0,0 +1,368 @@ > +/* > + * 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 DRV_NAME "arm-sbsa-wdt" > +#define pr_fmt(fmt) DRV_NAME ": " fmt > + > +#include <linux/module.h> > +#include <linux/watchdog.h> > +#include <linux/platform_device.h> > +#include <linux/device.h> > +#include <linux/io.h> > + > +#include <linux/irqdomain.h> > +#include <linux/interrupt.h> > +#include <linux/reboot.h> > + > +#include <clocksource/arm_arch_timer.h> > +#include <linux/acpi.h> > + > +/* Watchdog Interface Identification Registers */ > +struct arm_sbsa_watchdog_ident { > + uint32_t w_iidr; /* Watchdog Interface Identification Register */ > + uint8_t res2[0xFE8 - 0xFD0]; > + uint32_t w_pidr2; /* Peripheral ID2 Register */ > +}; > + > +/* Watchdog Refresh Frame */ > +struct arm_sbsa_watchdog_refresh { > + uint32_t wrr; /* Watchdog Refresh Register */ > + uint8_t res1[0xFCC - 0x004]; > + struct arm_sbsa_watchdog_ident ident; > +}; > + > +/* Watchdog Control Frame */ > +struct arm_sbsa_watchdog_control { > + uint32_t wcs; > + uint32_t res1; > + uint32_t wor; > + uint32_t res2; > + uint64_t wcv; > + uint8_t res3[0xFCC - 0x018]; > + struct arm_sbsa_watchdog_ident ident; > +}; > + > +struct arm_sbsa_watchdog_data { > + struct watchdog_device wdev; > + 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); > + > + 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_rate() * 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); > + > + 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_timer_read_counter(); > + > + return diff / arch_timer_get_rate(); > +} > + > +static struct watchdog_info arm_sbsa_wdt_info = { > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING, > + .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 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. > + */ > + emergency_restart(); > + > + return IRQ_HANDLED; > +} > + > +static int __init arm_sbsa_wdt_probe(struct platform_device *pdev) > +{ > + struct arm_sbsa_watchdog_data *data; > + struct resource *res; > + uint32_t iidr; > + int ret; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res || !res->start) { > + dev_err(&pdev->dev, "could not get control address\n"); > + return -ENOMEM; > + } > + > + data->control = devm_ioremap_resource(&pdev->dev, res); > + if (!data->control) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res || !res->start) { > + dev_err(&pdev->dev, "could not get refresh address\n"); > + return -ENOMEM; > + } > + data->refresh = devm_ioremap_resource(&pdev->dev, res); > + if (!data->refresh) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (!res || !res->start) { > + dev_err(&pdev->dev, "could not get interrupt\n"); > + return -ENOMEM; > + } > + > + iidr = readl(&data->control->ident.w_iidr); > + > + /* We only support architecture version 0 */ > + if (((iidr >> 16) & 0xf) != 0) { > + dev_info(&pdev->dev, "only architecture version 0 is supported\n"); > + return -ENODEV; > + } > + > + ret = devm_request_irq(&pdev->dev, res->start, arm_sbsa_wdt_interrupt, > + 0, DRV_NAME, NULL); > + if (ret < 0) { > + dev_err(&pdev->dev, "could not request irq %u (ret=%i)\n", > + (unsigned int)res->start, ret); > + return ret; > + } > + > + 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_rate(); > + > + 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_info(&pdev->dev, "implementer code is %03x, max timeout is %u seconds\n", > + (iidr & 0xf00) >> 1 | (iidr & 0x7f), data->wdev.max_timeout); > + > + /* > + * Bits [15:12] are an implementation-defined revision number > + * for the component. > + */ > + arm_sbsa_wdt_info.firmware_version = (iidr >> 12) & 0xf; > + > + 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; > +} > + > +static struct platform_device *arm_sbsa_wdt_pdev; > + > +static int __init arm_sbsa_wdt_parse_gtdt(struct acpi_subtable_header *header, > + const unsigned long end) > +{ > + struct acpi_gtdt_watchdog *wdg = (struct acpi_gtdt_watchdog *)header; > + struct platform_device *arm_sbsa_wdt_pdev; > + struct resource res[3] = {}; > + int trigger, polarity; > + int ret; > + > + if (!header || > + (unsigned long)header + sizeof(*wdg) > end || > + header->length < sizeof(*wdg)) { > + pr_err("malformed subtable entry\n"); > + return -EINVAL; > + } > + > + if (!wdg->control_frame_address || !wdg->refresh_frame_address) { > + pr_err("invalid control or refresh address\n"); > + return -ENXIO; > + } > + > + arm_sbsa_wdt_pdev = platform_device_alloc(DRV_NAME, 0); > + if (!arm_sbsa_wdt_pdev) > + return -ENOMEM; > + > + res[0].name = "control"; > + res[0].flags = IORESOURCE_MEM; > + res[0].start = wdg->control_frame_address; > + res[0].end = res[0].start + sizeof(struct arm_sbsa_watchdog_control); > + > + res[1].name = "refresh"; > + res[1].flags = IORESOURCE_MEM; > + res[1].start = wdg->refresh_frame_address; > + res[1].end = res[1].start + sizeof(struct arm_sbsa_watchdog_refresh); > + > + trigger = (wdg->timer_flags & ACPI_GTDT_WATCHDOG_IRQ_MODE) ? > + ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; > + > + polarity = (wdg->timer_flags & ACPI_GTDT_WATCHDOG_IRQ_POLARITY) ? > + ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; > + > + /* This should be the WS0 interrupt */ > + ret = acpi_register_gsi(&arm_sbsa_wdt_pdev->dev, wdg->timer_interrupt, > + trigger, polarity); > + if (ret <= 0) { > + pr_err("could not obtain interrupt\n"); > + goto error_platform; > + } > + > + res[2].name = "irq"; > + res[2].flags = IORESOURCE_IRQ; > + res[2].start = ret; > + res[2].end = res[2].start; > + > + ret = platform_device_add_resources(arm_sbsa_wdt_pdev, res, > + ARRAY_SIZE(res)); > + if (ret) { > + pr_err("could not add platform resources\n"); > + goto error_acpi; > + } > + > + ret = platform_device_add(arm_sbsa_wdt_pdev); > + if (ret) { > + pr_err("could not add platform device\n"); > + goto error_acpi; > + } > + > + return 0; > + > +error_acpi: > + acpi_unregister_gsi(res[2].start); > + > +error_platform: > + platform_device_put(arm_sbsa_wdt_pdev); > + > + return ret; > +} > + > +static struct platform_driver arm_sbsa_wdt_driver = { > + .remove = __exit_p(arm_sbsa_wdt_remove), > + .driver = { > + .name = DRV_NAME, > + .owner = THIS_MODULE, > + }, > +}; > + > +static int __init arm_sbsa_wdt_init(void) > +{ > + struct acpi_table_header *table; > + acpi_size tbl_size; > + acpi_status status; > + int count; > + > + pr_info("ARM Server Base System Architecture watchdog driver\n"); > + > + status = acpi_get_table_with_size(ACPI_SIG_GTDT, 0, &table, &tbl_size); > + if (ACPI_FAILURE(status)) { > + pr_err("cannot locate GTDT table\n"); > + return -EINVAL; > + } > + count = acpi_parse_entries(ACPI_SIG_GTDT, sizeof(struct acpi_table_gtdt), > + arm_sbsa_wdt_parse_gtdt, table, ACPI_GTDT_TYPE_WATCHDOG, 0); > + if (count <= 0) { > + pr_err("no watchdog subtables found\n"); > + return -EINVAL; > + } > + > + return platform_driver_probe(&arm_sbsa_wdt_driver, arm_sbsa_wdt_probe); > +} > + > +static void __exit arm_sbsa_wdt_exit(void) > +{ > + platform_device_unregister(arm_sbsa_wdt_pdev); > + platform_driver_unregister(&arm_sbsa_wdt_driver); > +} > + > +module_init(arm_sbsa_wdt_init); > +module_exit(arm_sbsa_wdt_exit); > + > +MODULE_DESCRIPTION("ARM Server Base System Architecture Watchdog Driver"); > +MODULE_LICENSE("GPL v2"); > -- > Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project. >

9 years

[PATCH v3] ArmPlatformPkg: fix gtdt.asl for ArmVExpressPkg

by fu.wei＠linaro.org

From: Fu Wei <fu.wei(a)linaro.org> Add Memory-mapped GT and SBSA Generic Watchdog timer info base on Foundation Model Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Fu Wei <fu.wei(a)linaro.org> --- .../AcpiTables/rtsm_ve-aemv8a/gtdt.asl | 67 +++++++++++++++++----- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/ArmPlatformPkg/ArmVExpressPkg/AcpiTables/rtsm_ve-aemv8a/gtdt.asl b/ArmPlatformPkg/ArmVExpressPkg/AcpiTables/rtsm_ve-aemv8a/gtdt.asl index d304243..11d5f89 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/AcpiTables/rtsm_ve-aemv8a/gtdt.asl +++ b/ArmPlatformPkg/ArmVExpressPkg/AcpiTables/rtsm_ve-aemv8a/gtdt.asl @@ -36,16 +36,16 @@ */ [0004] Signature : "GTDT" -[0004] Table Length : 00000050 +[0004] Table Length : 000000D8 [0001] Revision : 02 [0001] Checksum : F1 [0006] Oem ID : "LINARO" [0008] Oem Table ID : "RTSMVEV8" -[0004] Oem Revision : 00000001 +[0004] Oem Revision : 00000002 [0004] Asl Compiler ID : "INTL" [0004] Asl Compiler Revision : 20110623 -[0008] Counter Block Address : 0000000000000000 +[0008] Counter Block Address : 000000002a430000 [0004] Reserved : 00000000 /* In RTSM model's dts file, the last cell of interrupts @@ -83,26 +83,35 @@ Always-on : 0 /* The 64-bit physical address at which the Counter Read block is located */ -[0008] CntReadBase Physical address : 0000000000000000 +[0008] CntReadBase Physical address : 000000002a800000 -[0004] Platform Timer Count : 00000001 -[0004] Platform Timer Offset : 0000005C +[0004] Platform Timer Count : 00000002 +[0004] Platform Timer Offset : 00000060 /* Memory-mapped GT (Generic Timer) structures */ [0001] Subtable Type : 00 [0002] Length : 0064 [0001] Reserved : 000000 [0008] Block Address : 000000002a810000 -[0004] Timer Count : 00000001 -[0004] Timer Offset : 00000010 +[0004] Timer Count : 00000002 +[0004] Timer Offset : 00000014 -/* One frame is available */ +/* Two frames are available + * In FVP base model's dts file, the last cell of interrupts + * is 0x04, it means its trigger type is 4 = active high level-sensitive. + * + * So in ACPI the Trigger Mode is 0 - Level triggered, and + * Polarity is 0 - Active high as ACPI spec describled. + * + * Using direct mapping for hwirqs, it means that we using IRQ ID: + * (SPI offset + 32), not SPI offset used in FDT. + */ [0001] Frame Number : 00 [0003] Reserved : 000000 [0008] Base Address : 000000002a820000 -[0008] EL0 Base Address : FFFFFFFFFFFFFFFF -[0004] Timer Interrupt : 00000029 /* 25+16 */ -[0004] Timer Flags (decoded below) : 00000000 /* Active high level-sensitive */ +[0008] EL0 Base Address : 000000002a820000 +[0004] Timer Interrupt : 00000039 /* 25(SPI)+32 */ +[0004] Timer Flags (decoded below) : 00000000 Trigger Mode : 0 Polarity : 0 /* No virtual timer */ @@ -110,7 +119,37 @@ [0004] Virtual Timer Flags (decoded below) : 00000000 Trigger Mode : 0 Polarity : 0 -[0004] Common Flags (decoded below) : 00000000 - Secure : 0 +/* CNTPSIRQ Secure physical timer event.*/ +[0004] Common Flags (decoded below) : 00000001 + Secure : 1 + Always On : 0 + +[0001] Frame Number : 01 +[0003] Reserved : 000000 +[0008] Base Address : 000000002a830000 +[0008] EL0 Base Address : 000000002a830000 +[0004] Timer Interrupt : 0000003A /* 26(SPI)+32 */ +[0004] Timer Flags (decoded below) : 00000000 + Trigger Mode : 0 + Polarity : 0 +/* No virtual timer */ +[0004] Virtual Timer Interrupt : 00000000 +[0004] Virtual Timer Flags (decoded below) : 00000000 + Trigger Mode : 0 + Polarity : 0 +/* CNTPSIRQ1 Secure physical timer event.*/ +[0004] Common Flags (decoded below) : 00000001 + Secure : 1 Always On : 0 +/* One EL2 SBSA Generic Watchdog timer is available */ +[0001] Subtable Type : 01 [SBSA Generic Watchdog] +[0001] Length : 1C +[0002] Reserved : 0000 +[0008] Refresh Frame Address : 000000002a450000 +[0008] Control Frame Address : 000000002a440000 +[0004] Timer Interrupt : 0000003B /* 27(SPI)+32 */ +[0004] Timer Flags (decoded below) : 00000000 + Trigger Mode : 0 + Polarity : 0 + Secure : 0 -- 1.8.3.1

9 years

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

Linaro-acpi May 2015