All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: fu.wei@linaro.org, Suravee.Suthikulpanit@amd.com,
	linaro-acpi@lists.linaro.org, linux-watchdog@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org
Cc: tekkamanninja@gmail.com, graeme.gregory@linaro.org,
	al.stone@linaro.org, hanjun.guo@linaro.org, timur@codeaurora.org,
	ashwin.chaugule@linaro.org, arnd@arndb.de,
	vgandhi@codeaurora.org, wim@iguana.be, jcm@redhat.com,
	leo.duran@amd.com, corbet@lwn.net, mark.rutland@arm.com,
	catalin.marinas@arm.com, will.deacon@arm.com
Subject: Re: [PATCH v3 5/6] Watchdog: introduce ARM SBSA watchdog driver
Date: Mon, 25 May 2015 12:39:25 -0700	[thread overview]
Message-ID: <55637A6D.3080403@roeck-us.net> (raw)
In-Reply-To: <1432548193-19569-6-git-send-email-fu.wei@linaro.org>

On 05/25/2015 03:03 AM, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> This driver bases on linux kernel watchdog framework, and
> use "pretimeout" in the framework. It supports getting timeout and
> pretimeout from parameter and FDT at the driver init stage.
> In first timeout, the interrupt routine run panic to save
> system context.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---

Comments inline.

Thanks,
Guenter

>   drivers/watchdog/Kconfig     |  11 +
>   drivers/watchdog/Makefile    |   1 +
>   drivers/watchdog/sbsa_gwdt.c | 474 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 486 insertions(+)
>   create mode 100644 drivers/watchdog/sbsa_gwdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e5e7c55..554f18a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,17 @@ config ARM_SP805_WATCHDOG
>   	  ARM Primecell SP805 Watchdog timer. This will reboot your system when
>   	  the timeout is reached.
>
> +config ARM_SBSA_WATCHDOG
> +	tristate "ARM SBSA Generic Watchdog"
> +	depends on ARM64
> +	depends on ARM_ARCH_TIMER
> +	select WATCHDOG_CORE
> +	help
> +	  ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
> +	  The first timeout will trigger a panic; the second timeout will
> +	  trigger a system reset.
> +	  More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> +
>   config AT91RM9200_WATCHDOG
>   	tristate "AT91RM9200 watchdog"
>   	depends on SOC_AT91RM9200 && MFD_SYSCON
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c19294..471f1b7c 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
>
>   # ARM Architecture
>   obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
> +obj-$(CONFIG_ARM_SBSA_WATCHDOG) += sbsa_gwdt.o
>   obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
>   obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
>   obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> new file mode 100644
> index 0000000..0d1aff1
> --- /dev/null
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -0,0 +1,474 @@
> +/*
> + * SBSA(Server Base System Architecture) Generic Watchdog driver
> + *
> + * Copyright (c) 2015, Linaro Ltd.
> + * Author: Fu Wei <fu.wei@linaro.org>
> + *         Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License 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.
> + *
> + * Note: This SBSA Generic watchdog driver is compatible with
> + *       the pretimeout concept of Linux kernel.
> + *       The timeout and pretimeout are set by the different REGs.
> + *       The first watch period is set by writing WCV directly,
> + *       that can support more than 10s timeout at the maximum
> + *       system counter frequency.
> + *       The second watch period is set by WOR(32bit) which will be loaded
> + *       automatically by hardware, when WS0 is triggered.
> + *       This gives a maximum watch period of around 10s at the maximum
> + *       system counter frequency.
> + *       The System Counter shall run at maximum of 400MHz.
> + *       More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> + *
> + * Kernel/API:                         P---------| pretimeout
> + *               |-------------------------------T timeout
> + * SBSA GWDT:                          P--WOR---WS1 pretimeout
> + *               |-------WCV----------WS0~~~~~~~~T timeout
> + */
> +
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/uaccess.h>
> +#include <linux/watchdog.h>
> +#include <asm/arch_timer.h>
> +
> +/* SBSA Generic Watchdog register definitions */
> +/* refresh frame */
> +#define SBSA_GWDT_WRR				0x000
> +
> +/* control frame */
> +#define SBSA_GWDT_WCS				0x000
> +#define SBSA_GWDT_WOR				0x008
> +#define SBSA_GWDT_WCV_LO			0x010
> +#define SBSA_GWDT_WCV_HI			0x014
> +
> +/* refresh/control frame */
> +#define SBSA_GWDT_W_IIDR			0xfcc
> +#define SBSA_GWDT_IDR				0xfd0
> +
> +/* Watchdog Control and Status Register */
> +#define SBSA_GWDT_WCS_EN			BIT(0)
> +#define SBSA_GWDT_WCS_WS0			BIT(1)
> +#define SBSA_GWDT_WCS_WS1			BIT(2)
> +
> +/**
> + * struct sbsa_gwdt - Internal representation of the SBSA GWDT
> + * @wdd:		kernel watchdog_device structure
> + * @clk:		store the System Counter clock frequency, in Hz.
> + * @refresh_base:	Virtual address of the watchdog refresh frame
> + * @control_base:	Virtual address of the watchdog control frame
> + */
> +struct sbsa_gwdt {
> +	struct watchdog_device	wdd;
> +	u32			clk;
> +	void __iomem		*refresh_base;
> +	void __iomem		*control_base;
> +};
> +
> +#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
> +
> +#define DEFAULT_TIMEOUT		10 /* seconds, the 1st + 2nd watch periods*/

That is a bit low for a default. Is that on purpose ?
Most drivers use 30 or 60 seconds.

> +#define DEFAULT_PRETIMEOUT	5  /* seconds, the 2nd watch period*/
> +
> +static unsigned int timeout_param;
> +module_param(timeout_param, uint, 0);
> +MODULE_PARM_DESC(timeout_param,
> +		 "Watchdog timeout in seconds. (>=0, default="
> +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
Why _param in the module parameter names ? Seems to be redundant.

> +static unsigned int max_timeout_param = UINT_MAX;
> +module_param(max_timeout_param, uint, 0);
> +MODULE_PARM_DESC(max_timeout_param,
> +		 "Watchdog max timeout in seconds. (>=0, default="
> +		 __MODULE_STRING(UINT_MAX) ")");
> +

Why do we want or need this parameter and max_pretimeout ? Those
are determined by the hardware.

> +static unsigned int pretimeout_param;
> +module_param(pretimeout_param, uint, 0);
> +MODULE_PARM_DESC(pretimeout_param,
> +		 "Watchdog pretimeout in seconds. (>=0, default="
> +		 __MODULE_STRING(DEFAULT_PRETIMEOUT) ")");
> +
> +static unsigned int max_pretimeout_param = U32_MAX;
> +module_param(max_pretimeout_param, uint, 0);
> +MODULE_PARM_DESC(max_pretimeout_param,
> +		 "Watchdog max pretimeout in seconds. (>=0, default="
> +		 __MODULE_STRING(U32_MAX) ")");
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, S_IRUGO);
> +MODULE_PARM_DESC(nowayout,
> +		 "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +/*
> + * help functions for accessing 32bit registers of SBSA Generic Watchdog
> + */
> +static void sbsa_gwdt_cf_write(unsigned int reg, u32 val,
> +			       struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	writel_relaxed(val, gwdt->control_base + reg);
> +}
> +
> +static void sbsa_gwdt_rf_write(unsigned int reg, u32 val,
> +			       struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	writel_relaxed(val, gwdt->refresh_base + reg);
> +}
> +
> +static u32 sbsa_gwdt_cf_read(unsigned int reg, struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	return readl_relaxed(gwdt->control_base + reg);
> +}
> +
> +/*
> + * help functions for accessing 64bit WCV register
> + */
> +static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd)
> +{
> +	u32 wcv_lo, wcv_hi;
> +
> +	do {
> +		wcv_hi = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_HI, wdd);
> +		wcv_lo = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_LO, wdd);
> +	} while (wcv_hi != sbsa_gwdt_cf_read(SBSA_GWDT_WCV_HI, wdd));
> +
> +	return (((u64)wcv_hi << 32) | wcv_lo);
> +}
> +
> +static void sbsa_gwdt_set_wcv(struct watchdog_device *wdd, u64 value)
> +{
> +	u32 wcv_lo, wcv_hi;
> +
> +	wcv_lo = value & U32_MAX;
> +	wcv_hi = (value >> 32) & U32_MAX;
> +
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCV_HI, wcv_hi, wdd);
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCV_LO, wcv_lo, wdd);
> +}
> +
> +static void reload_timeout_to_wcv(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 wcv;
> +
> +	wcv = arch_counter_get_cntvct() +
> +		(u64)(wdd->timeout - wdd->pretimeout) * gwdt->clk;
> +
> +	sbsa_gwdt_set_wcv(wdd, wcv);
> +}
> +
> +/*
> + * Use the following function to update the timeout limits
> + * after updating pretimeout
> + */
> +static void sbsa_gwdt_update_limits(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 first_period_max = U64_MAX;
> +
> +	do_div(first_period_max, gwdt->clk);
> +
> +	wdd->min_timeout = wdd->pretimeout + 1;
> +	wdd->max_timeout = min((uint)(wdd->pretimeout + first_period_max),
> +			       wdd->max_timeout);
> +}
> +

After understanding the watchdog a bit better now, I think you should drop
those updates and set
	min_timeout = 1
	max_timeout = min(UINT_MAX, U64_MAX / clk)
	min_pretimeout = 0
	max_pretimeout = U32_MAX / clk

and then ensure that pretimeout < timeout at runtime (if possible in
the infrastructure code).

Even at maximum clock rate, max_timeout is so large that anything else
is really overkill.

> +static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
> +				 unsigned int timeout)
> +{
> +	wdd->timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_set_pretimeout(struct watchdog_device *wdd,
> +				    unsigned int pretimeout)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u32 wor;
> +
> +	wdd->pretimeout = pretimeout;
> +	sbsa_gwdt_update_limits(wdd);
> +
> +	if (!pretimeout)
> +		/* gives sbsa_gwdt_start a chance to setup timeout */
> +		wor = gwdt->clk;
> +	else
> +		wor = pretimeout * gwdt->clk;
> +

So in practice you always have a pretimeout of at least one second,
correct ? That kind of violates the ABI, which says that the pretimeout
should be disabled if set to 0. Is there anything we can do about that ?

What exactly happens if WOR = 0 ? Doesn't that just mean that the second
timeout will happen immediately, and isn't that what we would want if
pretimeout = 0 ?

> +	/* refresh the WOR, that will cause an explicit watchdog refresh */

Except that we use wcv which needs a manual refresh, so this is a bit
misleading, isn't it ?

> +	sbsa_gwdt_cf_write(SBSA_GWDT_WOR, wor, wdd);
> +
> +	return 0;
> +}
> +
> +static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 timeleft = sbsa_gwdt_get_wcv(wdd) - arch_counter_get_cntvct();
> +
> +	do_div(timeleft, gwdt->clk);
> +
> +	return timeleft;
> +}
> +
> +static int sbsa_gwdt_start(struct watchdog_device *wdd)
> +{
> +	/* Force refresh */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);
> +	/* writing WCS will cause an explicit watchdog refresh */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCS, SBSA_GWDT_WCS_EN, wdd);
> +
> +	reload_timeout_to_wcv(wdd);
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_stop(struct watchdog_device *wdd)
> +{
> +	/* Force refresh */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);
> +	/* writing WCS will cause an explicit watchdog refresh */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCS, 0, wdd);
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
> +{
> +	/*
> +	 * Writing WRR for an explicit watchdog refresh
> +	 * You can write anyting(like 0xc0ffee)
> +	 */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);

Should that happen after writing wcv ?

> +
> +	reload_timeout_to_wcv(wdd);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
> +{
> +	struct sbsa_gwdt *gwdt = (struct sbsa_gwdt *)dev_id;
> +	struct watchdog_device *wdd = &gwdt->wdd;
> +	u32 status;
> +
> +	status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> +
> +	if (status & SBSA_GWDT_WCS_WS0)
> +		panic("SBSA Watchdog pre-timeout");
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct watchdog_info sbsa_gwdt_info = {
> +	.identity	= "SBSA Generic Watchdog",
> +	.options	= WDIOF_SETTIMEOUT |
> +			  WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE |
> +			  WDIOF_PRETIMEOUT |
> +			  WDIOF_CARDRESET,
> +};
> +
> +static struct watchdog_ops sbsa_gwdt_ops = {
> +	.owner		= THIS_MODULE,
> +	.start		= sbsa_gwdt_start,
> +	.stop		= sbsa_gwdt_stop,
> +	.ping		= sbsa_gwdt_keepalive,
> +	.set_timeout	= sbsa_gwdt_set_timeout,
> +	.set_pretimeout	= sbsa_gwdt_set_pretimeout,
> +	.get_timeleft	= sbsa_gwdt_get_timeleft,
> +};
> +
> +static int sbsa_gwdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sbsa_gwdt *gwdt;
> +	struct watchdog_device *wdd;
> +	struct resource *res;
> +	void *rf_base, *cf_base;
> +	int irq;
> +	u32 clk, status;
> +	int ret = 0;
> +	u64 first_period_max = U64_MAX;
> +
> +	/*
> +	 * Get the frequency of system counter from
> +	 * the cp15 interface of ARM Generic timer
> +	 */
> +	clk = arch_timer_get_cntfrq();

That can not return 0, presumably, from looking into its implementation.
And the system should panic if it is ever 0.

> +	if (!clk) {

Given that, I don't think this check is necessary.

> +		dev_err(dev, "System Counter frequency not available\n");
> +		return -EINVAL;
> +	}
> +
> +	gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL);
> +	if (!gwdt)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "refresh");
> +	rf_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(rf_base))
> +		return PTR_ERR(rf_base);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
> +	cf_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(cf_base))
> +		return PTR_ERR(cf_base);
> +
> +	irq = platform_get_irq_byname(pdev, "ws0");
> +	if (irq < 0) {
> +		dev_err(dev, "unable to get ws0 interrupt.\n");
> +		return irq;
> +	}
> +
> +	gwdt->refresh_base = rf_base;
> +	gwdt->control_base = cf_base;
> +	gwdt->clk = clk;
> +
> +	platform_set_drvdata(pdev, gwdt);
> +
> +	wdd = &gwdt->wdd;
> +	wdd->parent = dev;
> +	wdd->info = &sbsa_gwdt_info;
> +	wdd->ops = &sbsa_gwdt_ops;
> +	watchdog_set_drvdata(wdd, gwdt);
> +	watchdog_set_nowayout(wdd, nowayout);
> +
> +	status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> +	if (status & SBSA_GWDT_WCS_WS1) {
> +		dev_warn(dev, "System reset by WDT(WCS: %x, WCV: %llx)\n",
> +			 status, sbsa_gwdt_get_wcv(wdd));

Does this message (specifically the WCS / WCV values) have any
useful meaning for the user ?

> +		wdd->bootstatus |= WDIOF_CARDRESET;
> +	}
> +	/* Check if watchdog is already enabled */
> +	if (status & SBSA_GWDT_WCS_EN) {
> +		dev_warn(dev, "already enabled! WCS:0x%x\n", status);
> +		sbsa_gwdt_keepalive(wdd);

You have not configured wdd->timeout and wdd->pretimeout here,
yet the function calls reload_timeout_to_wcv which needs it.

> +	}
> +
> +	wdd->min_pretimeout = 0;
> +	wdd->max_pretimeout = min(U32_MAX / clk, max_pretimeout_param);
> +	wdd->min_timeout = 1;
> +	do_div(first_period_max, clk);
> +	wdd->max_timeout = min((uint)(wdd->pretimeout + first_period_max),
> +			       max_timeout_param);
> +
> +	wdd->pretimeout = DEFAULT_PRETIMEOUT;
> +	wdd->timeout = DEFAULT_TIMEOUT;
> +	watchdog_init_timeouts(wdd, pretimeout_param, timeout_param, dev);
> +	/* update pretimeout to WOR */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WOR, wdd->pretimeout * clk, wdd);

This is interesting because you set WOR to 0 here if pretimeout is 0.
Yet, when pretimeout is updated later on, you always set it to at least
one second. Any reason for doing this differently ?

If the above works, and presumably you have tested it, I don't see why
it would be necessary to set WOR to a value larger than 0 when updating
pretimeout.

> +
> +	ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, IRQF_TIMER,
> +			       pdev->name, gwdt);
> +	if (ret) {
> +		dev_err(dev, "unable to request IRQ %d\n", irq);
> +		return ret;
> +	}
> +
> +	ret = watchdog_register_device(wdd);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "Initialized with %ds timeout, %ds pretimeout @ %uHz\n",
> +		 wdd->timeout, wdd->pretimeout, gwdt->clk);

400000Hz reads a bit odd. How about a space between the number and Hz ?

> +
> +	return 0;
> +}
> +
> +static void sbsa_gwdt_shutdown(struct platform_device *pdev)
> +{
> +	struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev);
> +
> +	sbsa_gwdt_stop(&gwdt->wdd);
> +}
> +
> +static int sbsa_gwdt_remove(struct platform_device *pdev)
> +{
> +	struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev);
> +	int ret = 0;
> +
> +	if (!nowayout)
> +		ret = sbsa_gwdt_stop(&gwdt->wdd);
> +
I don't think you should do anything here. The driver can only be removed
if closed, and it that case the watchdog will already have been stopped,
or if nowayout was set it won't be stopped. Either case it is already in
the state we want it to be in.

> +	watchdog_unregister_device(&gwdt->wdd);
> +
> +	return ret;
> +}
> +
> +/* Disable watchdog if it is active during suspend */
> +static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
> +{
> +	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&gwdt->wdd))
> +		sbsa_gwdt_stop(&gwdt->wdd);
> +
> +	return 0;
> +}
> +
> +/* Enable watchdog and configure it if necessary */
> +static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
> +{
> +	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&gwdt->wdd))
> +		sbsa_gwdt_start(&gwdt->wdd);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
> +};
> +
> +static const struct of_device_id sbsa_gwdt_of_match[] = {
> +	{ .compatible = "arm,sbsa-gwdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match);
> +
> +static const struct platform_device_id sbsa_gwdt_pdev_match[] = {
> +	{ .name = "sbsa-gwdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
> +
> +static struct platform_driver sbsa_gwdt_driver = {
> +	.driver = {
> +		.name = "sbsa-gwdt",
> +		.pm = &sbsa_gwdt_pm_ops,
> +		.of_match_table = sbsa_gwdt_of_match,
> +	},
> +	.probe = sbsa_gwdt_probe,
> +	.remove = sbsa_gwdt_remove,
> +	.shutdown = sbsa_gwdt_shutdown,
> +	.id_table = sbsa_gwdt_pdev_match,
> +};
> +
> +module_platform_driver(sbsa_gwdt_driver);
> +
> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
> +MODULE_VERSION("v1.0");
> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> +MODULE_LICENSE("GPL v2");
>


WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
To: fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org,
	linaro-acpi-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: tekkamanninja-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	graeme.gregory-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	al.stone-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	ashwin.chaugule-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	vgandhi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org,
	jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	leo.duran-5C7GfCeVMHo@public.gmane.org,
	corbet-T1hC0tSOHrs@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	catalin.marinas-5wv7dgnIgG8@public.gmane.org,
	will.deacon-5wv7dgnIgG8@public.gmane.org
Subject: Re: [PATCH v3 5/6] Watchdog: introduce ARM SBSA watchdog driver
Date: Mon, 25 May 2015 12:39:25 -0700	[thread overview]
Message-ID: <55637A6D.3080403@roeck-us.net> (raw)
In-Reply-To: <1432548193-19569-6-git-send-email-fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 05/25/2015 03:03 AM, fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Fu Wei <fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> This driver bases on linux kernel watchdog framework, and
> use "pretimeout" in the framework. It supports getting timeout and
> pretimeout from parameter and FDT at the driver init stage.
> In first timeout, the interrupt routine run panic to save
> system context.
>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> Signed-off-by: Fu Wei <fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Comments inline.

Thanks,
Guenter

>   drivers/watchdog/Kconfig     |  11 +
>   drivers/watchdog/Makefile    |   1 +
>   drivers/watchdog/sbsa_gwdt.c | 474 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 486 insertions(+)
>   create mode 100644 drivers/watchdog/sbsa_gwdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e5e7c55..554f18a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,17 @@ config ARM_SP805_WATCHDOG
>   	  ARM Primecell SP805 Watchdog timer. This will reboot your system when
>   	  the timeout is reached.
>
> +config ARM_SBSA_WATCHDOG
> +	tristate "ARM SBSA Generic Watchdog"
> +	depends on ARM64
> +	depends on ARM_ARCH_TIMER
> +	select WATCHDOG_CORE
> +	help
> +	  ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
> +	  The first timeout will trigger a panic; the second timeout will
> +	  trigger a system reset.
> +	  More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> +
>   config AT91RM9200_WATCHDOG
>   	tristate "AT91RM9200 watchdog"
>   	depends on SOC_AT91RM9200 && MFD_SYSCON
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c19294..471f1b7c 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
>
>   # ARM Architecture
>   obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
> +obj-$(CONFIG_ARM_SBSA_WATCHDOG) += sbsa_gwdt.o
>   obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
>   obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
>   obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> new file mode 100644
> index 0000000..0d1aff1
> --- /dev/null
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -0,0 +1,474 @@
> +/*
> + * SBSA(Server Base System Architecture) Generic Watchdog driver
> + *
> + * Copyright (c) 2015, Linaro Ltd.
> + * Author: Fu Wei <fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *         Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License 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.
> + *
> + * Note: This SBSA Generic watchdog driver is compatible with
> + *       the pretimeout concept of Linux kernel.
> + *       The timeout and pretimeout are set by the different REGs.
> + *       The first watch period is set by writing WCV directly,
> + *       that can support more than 10s timeout at the maximum
> + *       system counter frequency.
> + *       The second watch period is set by WOR(32bit) which will be loaded
> + *       automatically by hardware, when WS0 is triggered.
> + *       This gives a maximum watch period of around 10s at the maximum
> + *       system counter frequency.
> + *       The System Counter shall run at maximum of 400MHz.
> + *       More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> + *
> + * Kernel/API:                         P---------| pretimeout
> + *               |-------------------------------T timeout
> + * SBSA GWDT:                          P--WOR---WS1 pretimeout
> + *               |-------WCV----------WS0~~~~~~~~T timeout
> + */
> +
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/uaccess.h>
> +#include <linux/watchdog.h>
> +#include <asm/arch_timer.h>
> +
> +/* SBSA Generic Watchdog register definitions */
> +/* refresh frame */
> +#define SBSA_GWDT_WRR				0x000
> +
> +/* control frame */
> +#define SBSA_GWDT_WCS				0x000
> +#define SBSA_GWDT_WOR				0x008
> +#define SBSA_GWDT_WCV_LO			0x010
> +#define SBSA_GWDT_WCV_HI			0x014
> +
> +/* refresh/control frame */
> +#define SBSA_GWDT_W_IIDR			0xfcc
> +#define SBSA_GWDT_IDR				0xfd0
> +
> +/* Watchdog Control and Status Register */
> +#define SBSA_GWDT_WCS_EN			BIT(0)
> +#define SBSA_GWDT_WCS_WS0			BIT(1)
> +#define SBSA_GWDT_WCS_WS1			BIT(2)
> +
> +/**
> + * struct sbsa_gwdt - Internal representation of the SBSA GWDT
> + * @wdd:		kernel watchdog_device structure
> + * @clk:		store the System Counter clock frequency, in Hz.
> + * @refresh_base:	Virtual address of the watchdog refresh frame
> + * @control_base:	Virtual address of the watchdog control frame
> + */
> +struct sbsa_gwdt {
> +	struct watchdog_device	wdd;
> +	u32			clk;
> +	void __iomem		*refresh_base;
> +	void __iomem		*control_base;
> +};
> +
> +#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
> +
> +#define DEFAULT_TIMEOUT		10 /* seconds, the 1st + 2nd watch periods*/

That is a bit low for a default. Is that on purpose ?
Most drivers use 30 or 60 seconds.

> +#define DEFAULT_PRETIMEOUT	5  /* seconds, the 2nd watch period*/
> +
> +static unsigned int timeout_param;
> +module_param(timeout_param, uint, 0);
> +MODULE_PARM_DESC(timeout_param,
> +		 "Watchdog timeout in seconds. (>=0, default="
> +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
Why _param in the module parameter names ? Seems to be redundant.

> +static unsigned int max_timeout_param = UINT_MAX;
> +module_param(max_timeout_param, uint, 0);
> +MODULE_PARM_DESC(max_timeout_param,
> +		 "Watchdog max timeout in seconds. (>=0, default="
> +		 __MODULE_STRING(UINT_MAX) ")");
> +

Why do we want or need this parameter and max_pretimeout ? Those
are determined by the hardware.

> +static unsigned int pretimeout_param;
> +module_param(pretimeout_param, uint, 0);
> +MODULE_PARM_DESC(pretimeout_param,
> +		 "Watchdog pretimeout in seconds. (>=0, default="
> +		 __MODULE_STRING(DEFAULT_PRETIMEOUT) ")");
> +
> +static unsigned int max_pretimeout_param = U32_MAX;
> +module_param(max_pretimeout_param, uint, 0);
> +MODULE_PARM_DESC(max_pretimeout_param,
> +		 "Watchdog max pretimeout in seconds. (>=0, default="
> +		 __MODULE_STRING(U32_MAX) ")");
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, S_IRUGO);
> +MODULE_PARM_DESC(nowayout,
> +		 "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +/*
> + * help functions for accessing 32bit registers of SBSA Generic Watchdog
> + */
> +static void sbsa_gwdt_cf_write(unsigned int reg, u32 val,
> +			       struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	writel_relaxed(val, gwdt->control_base + reg);
> +}
> +
> +static void sbsa_gwdt_rf_write(unsigned int reg, u32 val,
> +			       struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	writel_relaxed(val, gwdt->refresh_base + reg);
> +}
> +
> +static u32 sbsa_gwdt_cf_read(unsigned int reg, struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> +	return readl_relaxed(gwdt->control_base + reg);
> +}
> +
> +/*
> + * help functions for accessing 64bit WCV register
> + */
> +static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd)
> +{
> +	u32 wcv_lo, wcv_hi;
> +
> +	do {
> +		wcv_hi = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_HI, wdd);
> +		wcv_lo = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_LO, wdd);
> +	} while (wcv_hi != sbsa_gwdt_cf_read(SBSA_GWDT_WCV_HI, wdd));
> +
> +	return (((u64)wcv_hi << 32) | wcv_lo);
> +}
> +
> +static void sbsa_gwdt_set_wcv(struct watchdog_device *wdd, u64 value)
> +{
> +	u32 wcv_lo, wcv_hi;
> +
> +	wcv_lo = value & U32_MAX;
> +	wcv_hi = (value >> 32) & U32_MAX;
> +
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCV_HI, wcv_hi, wdd);
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCV_LO, wcv_lo, wdd);
> +}
> +
> +static void reload_timeout_to_wcv(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 wcv;
> +
> +	wcv = arch_counter_get_cntvct() +
> +		(u64)(wdd->timeout - wdd->pretimeout) * gwdt->clk;
> +
> +	sbsa_gwdt_set_wcv(wdd, wcv);
> +}
> +
> +/*
> + * Use the following function to update the timeout limits
> + * after updating pretimeout
> + */
> +static void sbsa_gwdt_update_limits(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 first_period_max = U64_MAX;
> +
> +	do_div(first_period_max, gwdt->clk);
> +
> +	wdd->min_timeout = wdd->pretimeout + 1;
> +	wdd->max_timeout = min((uint)(wdd->pretimeout + first_period_max),
> +			       wdd->max_timeout);
> +}
> +

After understanding the watchdog a bit better now, I think you should drop
those updates and set
	min_timeout = 1
	max_timeout = min(UINT_MAX, U64_MAX / clk)
	min_pretimeout = 0
	max_pretimeout = U32_MAX / clk

and then ensure that pretimeout < timeout at runtime (if possible in
the infrastructure code).

Even at maximum clock rate, max_timeout is so large that anything else
is really overkill.

> +static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
> +				 unsigned int timeout)
> +{
> +	wdd->timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_set_pretimeout(struct watchdog_device *wdd,
> +				    unsigned int pretimeout)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u32 wor;
> +
> +	wdd->pretimeout = pretimeout;
> +	sbsa_gwdt_update_limits(wdd);
> +
> +	if (!pretimeout)
> +		/* gives sbsa_gwdt_start a chance to setup timeout */
> +		wor = gwdt->clk;
> +	else
> +		wor = pretimeout * gwdt->clk;
> +

So in practice you always have a pretimeout of at least one second,
correct ? That kind of violates the ABI, which says that the pretimeout
should be disabled if set to 0. Is there anything we can do about that ?

What exactly happens if WOR = 0 ? Doesn't that just mean that the second
timeout will happen immediately, and isn't that what we would want if
pretimeout = 0 ?

> +	/* refresh the WOR, that will cause an explicit watchdog refresh */

Except that we use wcv which needs a manual refresh, so this is a bit
misleading, isn't it ?

> +	sbsa_gwdt_cf_write(SBSA_GWDT_WOR, wor, wdd);
> +
> +	return 0;
> +}
> +
> +static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +	u64 timeleft = sbsa_gwdt_get_wcv(wdd) - arch_counter_get_cntvct();
> +
> +	do_div(timeleft, gwdt->clk);
> +
> +	return timeleft;
> +}
> +
> +static int sbsa_gwdt_start(struct watchdog_device *wdd)
> +{
> +	/* Force refresh */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);
> +	/* writing WCS will cause an explicit watchdog refresh */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCS, SBSA_GWDT_WCS_EN, wdd);
> +
> +	reload_timeout_to_wcv(wdd);
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_stop(struct watchdog_device *wdd)
> +{
> +	/* Force refresh */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);
> +	/* writing WCS will cause an explicit watchdog refresh */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WCS, 0, wdd);
> +
> +	return 0;
> +}
> +
> +static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
> +{
> +	/*
> +	 * Writing WRR for an explicit watchdog refresh
> +	 * You can write anyting(like 0xc0ffee)
> +	 */
> +	sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd);

Should that happen after writing wcv ?

> +
> +	reload_timeout_to_wcv(wdd);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
> +{
> +	struct sbsa_gwdt *gwdt = (struct sbsa_gwdt *)dev_id;
> +	struct watchdog_device *wdd = &gwdt->wdd;
> +	u32 status;
> +
> +	status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> +
> +	if (status & SBSA_GWDT_WCS_WS0)
> +		panic("SBSA Watchdog pre-timeout");
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct watchdog_info sbsa_gwdt_info = {
> +	.identity	= "SBSA Generic Watchdog",
> +	.options	= WDIOF_SETTIMEOUT |
> +			  WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE |
> +			  WDIOF_PRETIMEOUT |
> +			  WDIOF_CARDRESET,
> +};
> +
> +static struct watchdog_ops sbsa_gwdt_ops = {
> +	.owner		= THIS_MODULE,
> +	.start		= sbsa_gwdt_start,
> +	.stop		= sbsa_gwdt_stop,
> +	.ping		= sbsa_gwdt_keepalive,
> +	.set_timeout	= sbsa_gwdt_set_timeout,
> +	.set_pretimeout	= sbsa_gwdt_set_pretimeout,
> +	.get_timeleft	= sbsa_gwdt_get_timeleft,
> +};
> +
> +static int sbsa_gwdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sbsa_gwdt *gwdt;
> +	struct watchdog_device *wdd;
> +	struct resource *res;
> +	void *rf_base, *cf_base;
> +	int irq;
> +	u32 clk, status;
> +	int ret = 0;
> +	u64 first_period_max = U64_MAX;
> +
> +	/*
> +	 * Get the frequency of system counter from
> +	 * the cp15 interface of ARM Generic timer
> +	 */
> +	clk = arch_timer_get_cntfrq();

That can not return 0, presumably, from looking into its implementation.
And the system should panic if it is ever 0.

> +	if (!clk) {

Given that, I don't think this check is necessary.

> +		dev_err(dev, "System Counter frequency not available\n");
> +		return -EINVAL;
> +	}
> +
> +	gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL);
> +	if (!gwdt)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "refresh");
> +	rf_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(rf_base))
> +		return PTR_ERR(rf_base);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
> +	cf_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(cf_base))
> +		return PTR_ERR(cf_base);
> +
> +	irq = platform_get_irq_byname(pdev, "ws0");
> +	if (irq < 0) {
> +		dev_err(dev, "unable to get ws0 interrupt.\n");
> +		return irq;
> +	}
> +
> +	gwdt->refresh_base = rf_base;
> +	gwdt->control_base = cf_base;
> +	gwdt->clk = clk;
> +
> +	platform_set_drvdata(pdev, gwdt);
> +
> +	wdd = &gwdt->wdd;
> +	wdd->parent = dev;
> +	wdd->info = &sbsa_gwdt_info;
> +	wdd->ops = &sbsa_gwdt_ops;
> +	watchdog_set_drvdata(wdd, gwdt);
> +	watchdog_set_nowayout(wdd, nowayout);
> +
> +	status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> +	if (status & SBSA_GWDT_WCS_WS1) {
> +		dev_warn(dev, "System reset by WDT(WCS: %x, WCV: %llx)\n",
> +			 status, sbsa_gwdt_get_wcv(wdd));

Does this message (specifically the WCS / WCV values) have any
useful meaning for the user ?

> +		wdd->bootstatus |= WDIOF_CARDRESET;
> +	}
> +	/* Check if watchdog is already enabled */
> +	if (status & SBSA_GWDT_WCS_EN) {
> +		dev_warn(dev, "already enabled! WCS:0x%x\n", status);
> +		sbsa_gwdt_keepalive(wdd);

You have not configured wdd->timeout and wdd->pretimeout here,
yet the function calls reload_timeout_to_wcv which needs it.

> +	}
> +
> +	wdd->min_pretimeout = 0;
> +	wdd->max_pretimeout = min(U32_MAX / clk, max_pretimeout_param);
> +	wdd->min_timeout = 1;
> +	do_div(first_period_max, clk);
> +	wdd->max_timeout = min((uint)(wdd->pretimeout + first_period_max),
> +			       max_timeout_param);
> +
> +	wdd->pretimeout = DEFAULT_PRETIMEOUT;
> +	wdd->timeout = DEFAULT_TIMEOUT;
> +	watchdog_init_timeouts(wdd, pretimeout_param, timeout_param, dev);
> +	/* update pretimeout to WOR */
> +	sbsa_gwdt_cf_write(SBSA_GWDT_WOR, wdd->pretimeout * clk, wdd);

This is interesting because you set WOR to 0 here if pretimeout is 0.
Yet, when pretimeout is updated later on, you always set it to at least
one second. Any reason for doing this differently ?

If the above works, and presumably you have tested it, I don't see why
it would be necessary to set WOR to a value larger than 0 when updating
pretimeout.

> +
> +	ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, IRQF_TIMER,
> +			       pdev->name, gwdt);
> +	if (ret) {
> +		dev_err(dev, "unable to request IRQ %d\n", irq);
> +		return ret;
> +	}
> +
> +	ret = watchdog_register_device(wdd);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "Initialized with %ds timeout, %ds pretimeout @ %uHz\n",
> +		 wdd->timeout, wdd->pretimeout, gwdt->clk);

400000Hz reads a bit odd. How about a space between the number and Hz ?

> +
> +	return 0;
> +}
> +
> +static void sbsa_gwdt_shutdown(struct platform_device *pdev)
> +{
> +	struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev);
> +
> +	sbsa_gwdt_stop(&gwdt->wdd);
> +}
> +
> +static int sbsa_gwdt_remove(struct platform_device *pdev)
> +{
> +	struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev);
> +	int ret = 0;
> +
> +	if (!nowayout)
> +		ret = sbsa_gwdt_stop(&gwdt->wdd);
> +
I don't think you should do anything here. The driver can only be removed
if closed, and it that case the watchdog will already have been stopped,
or if nowayout was set it won't be stopped. Either case it is already in
the state we want it to be in.

> +	watchdog_unregister_device(&gwdt->wdd);
> +
> +	return ret;
> +}
> +
> +/* Disable watchdog if it is active during suspend */
> +static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
> +{
> +	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&gwdt->wdd))
> +		sbsa_gwdt_stop(&gwdt->wdd);
> +
> +	return 0;
> +}
> +
> +/* Enable watchdog and configure it if necessary */
> +static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
> +{
> +	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&gwdt->wdd))
> +		sbsa_gwdt_start(&gwdt->wdd);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
> +};
> +
> +static const struct of_device_id sbsa_gwdt_of_match[] = {
> +	{ .compatible = "arm,sbsa-gwdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match);
> +
> +static const struct platform_device_id sbsa_gwdt_pdev_match[] = {
> +	{ .name = "sbsa-gwdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
> +
> +static struct platform_driver sbsa_gwdt_driver = {
> +	.driver = {
> +		.name = "sbsa-gwdt",
> +		.pm = &sbsa_gwdt_pm_ops,
> +		.of_match_table = sbsa_gwdt_of_match,
> +	},
> +	.probe = sbsa_gwdt_probe,
> +	.remove = sbsa_gwdt_remove,
> +	.shutdown = sbsa_gwdt_shutdown,
> +	.id_table = sbsa_gwdt_pdev_match,
> +};
> +
> +module_platform_driver(sbsa_gwdt_driver);
> +
> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
> +MODULE_VERSION("v1.0");
> +MODULE_AUTHOR("Fu Wei <fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
>

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-05-25 19:39 UTC|newest]

Thread overview: 550+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <=fu.wei@linaro.org>
2015-05-15 11:08 ` [PATCH 0/6] Watchdog: introdouce ARM SBSA watchdog driver fu.wei
2015-05-15 11:08   ` [PATCH 1/6] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-05-15 14:06     ` Arnd Bergmann
2015-05-15 14:06       ` Arnd Bergmann
2015-05-15 14:14       ` Fu Wei
2015-05-15 14:14         ` Fu Wei
2015-05-16 10:29       ` Fu Wei
2015-05-16 10:29         ` Fu Wei
2015-05-15 11:08   ` [PATCH 2/6] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-05-15 11:08   ` [PATCH 3/6] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-05-15 11:08     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-15 14:07   ` [Linaro-acpi] [PATCH 0/6] Watchdog: introdouce ARM SBSA watchdog driver Arnd Bergmann
2015-05-15 14:07     ` Arnd Bergmann
2015-05-16 10:33     ` Fu Wei
2015-05-16 10:33       ` Fu Wei
2015-05-15 11:24 ` [PATCH 4/6] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-05-15 11:24   ` [PATCH 5/6] Watchdog: introdouce ARM SBSA watchdog driver fu.wei
2015-05-15 13:57     ` Arnd Bergmann
2015-05-16 12:01       ` Fu Wei
2015-05-16 12:01         ` Fu Wei
2015-05-16 12:26         ` Timur Tabi
2015-05-15 22:57     ` Guenter Roeck
2015-05-15 22:57       ` Guenter Roeck
2015-05-18 17:38       ` Fu Wei
2015-05-18 17:38         ` Fu Wei
2015-05-15 11:24   ` [PATCH 6/6] ACPI: import watchdog info of GTDT into platform device fu.wei
2015-05-15 13:33   ` [PATCH 4/6] Watchdog: introdouce "pretimeout" into framework Guenter Roeck
2015-05-15 13:49     ` Fu Wei
2015-05-15 13:55       ` Timur Tabi
2015-05-15 13:55         ` Timur Tabi
2015-05-15 17:59         ` Guenter Roeck
2015-05-15 18:01       ` Guenter Roeck
2015-05-15 18:01         ` Guenter Roeck
2015-05-18 17:22         ` Fu Wei
2015-05-18 17:22           ` Fu Wei
2015-05-15 14:04   ` Arnd Bergmann
2015-05-18 17:19     ` Fu Wei
2015-05-18 17:19       ` Fu Wei
2015-05-18 17:23       ` Guenter Roeck
2015-05-18 17:39         ` Fu Wei
2015-05-18 20:03         ` Arnd Bergmann
2015-05-18 20:14           ` Guenter Roeck
2015-05-18 20:14             ` Guenter Roeck
2015-05-19  1:12             ` Fu Wei
2015-05-19  1:12               ` Fu Wei
2015-05-21  8:32 ` [PATCH v2 0/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-05-21  8:32   ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-21  8:32   ` [PATCH v2 1/7] clocksource: export "arch_timer_get_rate" for the other drivers fu.wei
2015-05-21  8:32     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-22 14:02     ` Hanjun Guo
2015-05-22 14:09     ` Timur Tabi
2015-05-22 15:16       ` Guenter Roeck
2015-05-22 16:22         ` Timur Tabi
2015-05-21  8:32   ` [PATCH v2 2/7] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-05-21  8:32   ` [PATCH v2 3/7] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-05-21  8:45     ` Arnd Bergmann
2015-05-21  8:49       ` Fu Wei
2015-05-21  8:49         ` Fu Wei
2015-05-21  8:32   ` [PATCH v2 4/7] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-05-21 20:33     ` Suravee Suthikulpanit
2015-05-21 20:33       ` Suravee Suthikulpanit
2015-05-21  8:32   ` [PATCH v2 5/7] Watchdog: introduce "pretimeout" into framework fu.wei
2015-05-21  9:04     ` Guenter Roeck
2015-05-21 10:05       ` Fu Wei
2015-05-21 10:05         ` Fu Wei
2015-05-21 10:17         ` Guenter Roeck
2015-05-21 10:50           ` Fu Wei
2015-05-21 10:50             ` Fu Wei
2015-05-21 13:28             ` Guenter Roeck
2015-05-21 10:11     ` Guenter Roeck
2015-05-21 15:32     ` Guenter Roeck
2015-05-21 15:32       ` Guenter Roeck
2015-05-22  5:17       ` Fu Wei
2015-05-25  3:09       ` Fu Wei
2015-05-25  3:09         ` Fu Wei
2015-05-22  6:30     ` Timo Kokkonen
2015-05-22  8:23       ` Fu Wei
2015-05-22  8:59         ` Timo Kokkonen
2015-05-22 10:46           ` Fu Wei
2015-05-22 10:46             ` Fu Wei
2015-05-22 12:14             ` Timo Kokkonen
2015-05-22 13:37               ` Guenter Roeck
2015-05-22 13:23             ` Guenter Roeck
2015-05-22 14:38               ` Fu Wei
2015-05-22 14:38                 ` Fu Wei
2015-05-22 15:05                 ` Guenter Roeck
2015-05-24 16:17                   ` Fu Wei
2015-05-21  8:32   ` [PATCH v2 6/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-05-21  8:32     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-21 10:34     ` Guenter Roeck
2015-05-21 10:34       ` Guenter Roeck
2015-05-21 11:08       ` Fu Wei
2015-05-21 15:18         ` Guenter Roeck
2015-05-21 15:18           ` Guenter Roeck
2015-05-21 15:46           ` Fu Wei
2015-05-21 15:59             ` Guenter Roeck
2015-05-21 16:12               ` Fu Wei
2015-05-21 16:12                 ` Fu Wei
2015-05-21 16:33                 ` Timur Tabi
2015-05-21 16:33                   ` Timur Tabi
2015-05-22  5:05                   ` Fu Wei
2015-05-22  5:05                     ` Fu Wei
2015-05-21 13:09       ` Timur Tabi
2015-05-21 15:28         ` Guenter Roeck
2015-05-21 15:28           ` Guenter Roeck
2015-05-25  3:43           ` Fu Wei
2015-05-25  3:43             ` Fu Wei
2015-05-25  3:46             ` Timur Tabi
2015-05-25  3:46               ` Timur Tabi
2015-05-25  4:11               ` Fu Wei
2015-05-25  4:11                 ` Fu Wei
2015-05-21 15:42     ` Timur Tabi
2015-05-21 15:42       ` Timur Tabi
2015-05-23 16:28       ` Fu Wei
2015-05-23 16:50         ` Fu Wei
2015-05-23 19:40         ` Timur Tabi
2015-05-23 19:40           ` Timur Tabi
2015-05-23 20:01           ` Guenter Roeck
2015-05-23 20:01             ` Guenter Roeck
2015-05-23 20:27             ` Timur Tabi
2015-05-23 20:27               ` Timur Tabi
2015-05-23 20:44               ` Guenter Roeck
2015-05-24 10:50                 ` Fu Wei
2015-05-24 10:15             ` Fu Wei
2015-05-24 14:15               ` Guenter Roeck
2015-05-24 14:15                 ` Guenter Roeck
2015-05-24 15:50                 ` Fu Wei
2015-05-24 15:50                   ` Fu Wei
2015-05-24 16:23                   ` Guenter Roeck
2015-05-24 16:47                     ` Fu Wei
2015-05-24 16:58                       ` Guenter Roeck
2015-05-24 16:58                         ` Guenter Roeck
2015-05-24 17:04                         ` Fu Wei
2015-05-24 17:04                           ` Fu Wei
2015-05-24 15:02               ` Timur Tabi
2015-05-24 16:04                 ` Fu Wei
2015-05-24 16:04                   ` Fu Wei
2015-05-24 16:13                   ` Timur Tabi
2015-05-24 16:13                     ` Timur Tabi
2015-05-24 16:29                     ` Guenter Roeck
2015-05-24 16:29                       ` Guenter Roeck
2015-05-24 16:33                       ` Fu Wei
2015-05-24 16:44                       ` Timur Tabi
2015-05-24 16:50                         ` Guenter Roeck
2015-05-24 16:52                         ` Fu Wei
2015-05-24 17:19                           ` Timur Tabi
2015-05-24 17:19                             ` Timur Tabi
2015-05-24 17:23                             ` Fu Wei
2015-05-24 17:32                             ` Guenter Roeck
2015-05-24 17:32                               ` Guenter Roeck
2015-05-24 17:47                               ` Timur Tabi
2015-05-25  2:03                                 ` Fu Wei
2015-05-25  2:00                               ` Fu Wei
2015-05-24 16:29                     ` Fu Wei
2015-05-22 14:50     ` Hanjun Guo
2015-05-22 14:50       ` Hanjun Guo
2015-05-22 14:55       ` Arnd Bergmann
2015-05-22 15:01         ` Guenter Roeck
2015-05-22 15:01           ` Guenter Roeck
2015-05-22 15:18           ` Hanjun Guo
2015-05-22 15:18             ` Hanjun Guo
2015-05-22 15:24             ` [Linaro-acpi] " Arnd Bergmann
2015-05-22 15:24               ` Arnd Bergmann
2015-05-22 16:19               ` Timur Tabi
2015-05-22 20:13                 ` Arnd Bergmann
2015-05-22 20:17                   ` Timur Tabi
2015-05-23  7:25               ` Fu Wei
2015-05-23  7:25                 ` Fu Wei
2015-05-22 16:19           ` Timur Tabi
2015-05-23 14:47           ` Fu Wei
2015-05-22 16:18         ` Timur Tabi
2015-05-23 15:08         ` Timur Tabi
2015-05-23 15:08           ` Timur Tabi
2015-05-23 17:26           ` Fu Wei
2015-05-23 18:35             ` Guenter Roeck
2015-05-23 18:37               ` Timur Tabi
2015-05-23 19:03                 ` Fu Wei
2015-05-23 19:51                 ` Guenter Roeck
2015-05-24  9:58                   ` Fu Wei
2015-05-24 14:06                     ` Guenter Roeck
2015-05-24 14:06                       ` Guenter Roeck
2015-05-24 15:06                       ` Timur Tabi
2015-05-24 15:06                         ` Timur Tabi
2015-05-24 15:37                       ` Fu Wei
2015-05-24 15:37                         ` Fu Wei
2015-05-23 18:40             ` Timur Tabi
2015-05-23 19:14               ` Fu Wei
2015-05-23 19:14                 ` Fu Wei
2015-05-23 19:21                 ` Timur Tabi
2015-05-21  8:32   ` [PATCH v2 7/7] ACPI: import watchdog info of GTDT into platform device fu.wei
2015-05-22 15:38     ` Hanjun Guo
2015-05-22 15:38       ` Hanjun Guo
2015-05-23 19:46     ` Timur Tabi
2015-05-23 19:46       ` Timur Tabi
2015-05-21  8:46   ` [Linaro-acpi] [PATCH v2 0/7] Watchdog: introduce ARM SBSA watchdog driver Arnd Bergmann
2015-05-21  8:46     ` Arnd Bergmann
2015-05-21  9:01     ` Fu Wei
2015-05-21  9:01       ` Fu Wei
2015-05-21 20:36   ` Suravee Suthikulpanit
2015-05-21 20:36     ` Suravee Suthikulpanit
2015-05-22  5:08     ` Fu Wei
2015-05-25 10:03 ` [PATCH v3 0/6] " fu.wei
2015-05-25 10:03   ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-25 10:03   ` [PATCH v3 1/6] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-05-25 10:03     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-05-25 10:03   ` [PATCH v3 2/6] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-05-25 10:03   ` [PATCH v3 3/6] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-05-25 10:03   ` [PATCH v3 4/6] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-05-25 19:28     ` Guenter Roeck
2015-05-25 19:28       ` Guenter Roeck
2015-05-25 10:03   ` [PATCH v3 5/6] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-05-25 19:39     ` Guenter Roeck [this message]
2015-05-25 19:39       ` Guenter Roeck
2015-05-29  9:11       ` Fu Wei
2015-05-29  9:11         ` Fu Wei
2015-05-29 14:54         ` Guenter Roeck
2015-05-29 15:05           ` Fu Wei
2015-05-26 16:50     ` Timur Tabi
2015-05-29 10:17       ` Fu Wei
2015-05-29 10:17         ` Fu Wei
2015-05-29 13:28         ` Timur Tabi
2015-05-29 13:28           ` Timur Tabi
2015-05-29 14:32           ` Fu Wei
2015-05-29 14:32             ` Fu Wei
2015-05-29 15:46             ` Timur Tabi
2015-05-29 15:46               ` Timur Tabi
2015-05-29 17:53               ` Fu Wei
2015-05-29 18:27                 ` Timur Tabi
2015-05-29 18:27                   ` Timur Tabi
2015-05-29 22:10               ` Guenter Roeck
2015-05-29 22:10                 ` Guenter Roeck
2015-06-01  7:50                 ` Fu Wei
2015-05-25 10:03   ` [PATCH v3 6/6] ACPI: import watchdog info of GTDT into platform device fu.wei
2015-05-26  8:28     ` Hanjun Guo
2015-05-26 16:35       ` Timur Tabi
2015-05-26 18:24         ` Guenter Roeck
2015-05-26 18:24           ` Guenter Roeck
2015-05-27  3:01         ` Hanjun Guo
2015-05-27  3:08           ` Timur Tabi
2015-05-27  3:08             ` Timur Tabi
2015-05-26 12:28     ` Will Deacon
2015-05-26 12:28       ` Will Deacon
2015-05-26 15:02       ` Ashwin Chaugule
2015-05-26 15:02         ` Ashwin Chaugule
2015-05-26 15:18         ` Will Deacon
2015-05-26 15:18           ` Will Deacon
2015-05-26 15:35           ` Ashwin Chaugule
2015-05-26 15:35             ` Ashwin Chaugule
2015-05-26 15:35             ` Ashwin Chaugule
2015-05-26 15:36           ` Guenter Roeck
2015-05-26 15:36             ` Guenter Roeck
2015-05-26 16:27             ` Fu Wei
2015-05-26 16:27               ` Fu Wei
2015-05-27 10:44               ` Will Deacon
2015-05-27 10:44                 ` Will Deacon
2015-05-29 11:13                 ` Fu Wei
2015-05-29 11:13                   ` Fu Wei
2015-06-02  4:05 ` [PATCH v4 0/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-02  4:05   ` [PATCH v4 1/7] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-06-02  4:05   ` [PATCH v4 2/7] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-06-02  4:05   ` [PATCH v4 3/7] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-06-02  4:05     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-02  4:05   ` [PATCH v4 4/7] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-06-02  4:05     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-02 16:12     ` Guenter Roeck
2015-06-02 16:12       ` Guenter Roeck
2015-06-08 16:44       ` Fu Wei
2015-06-02  4:05   ` [PATCH v4 5/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-02  4:05     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-02 15:32     ` Timur Tabi
2015-06-02 15:32       ` Timur Tabi
2015-06-02 15:37       ` Guenter Roeck
2015-06-02 15:37         ` Guenter Roeck
2015-06-02 16:55       ` Fu Wei
2015-06-02 16:55         ` Fu Wei
2015-06-02 17:07         ` Guenter Roeck
2015-06-02 17:07           ` Guenter Roeck
2015-06-08 16:05           ` Fu Wei
2015-06-08 16:05             ` Fu Wei
2015-06-08 18:26             ` Guenter Roeck
2015-06-08 18:26               ` Guenter Roeck
2015-06-09  3:59               ` Fu Wei
2015-06-09  4:37                 ` Guenter Roeck
2015-06-09  4:37                   ` Guenter Roeck
2015-06-09  6:37                   ` Fu Wei
2015-06-09  6:37                     ` Fu Wei
2015-06-09  8:04                     ` Guenter Roeck
2015-06-09  8:04                       ` Guenter Roeck
2015-06-09 10:46                       ` Fu Wei
2015-06-09 16:22                         ` Guenter Roeck
2015-06-09 16:29                           ` Timur Tabi
2015-06-09 16:29                             ` Timur Tabi
2015-06-09 16:45                             ` Guenter Roeck
2015-06-09 16:53                               ` Timur Tabi
2015-06-09 16:53                                 ` Timur Tabi
2015-06-10  3:41                               ` Fu Wei
2015-06-10  3:41                                 ` Fu Wei
2015-06-10  4:20                                 ` Fu Wei
2015-06-10 14:22                                   ` Timur Tabi
2015-06-10 14:22                                     ` Timur Tabi
2015-06-10 14:36                                     ` Fu Wei
2015-06-10 14:36                                       ` Fu Wei
2015-06-10 14:41                                       ` Fu Wei
2015-06-10 14:41                                         ` Fu Wei
2015-06-10 14:41                                         ` Fu Wei
2015-06-10 15:38                                 ` Fu Wei
2015-06-10 15:38                                   ` Fu Wei
2015-06-10 17:54                                   ` Fu Wei
2015-06-10 17:54                                     ` Fu Wei
2015-06-11  0:22                                 ` Timur Tabi
2015-06-11  0:22                                   ` Timur Tabi
2015-06-11  3:00                                   ` Fu Wei
2015-06-11  3:45                                     ` Timur Tabi
2015-06-11  3:45                                       ` Timur Tabi
2015-06-11  5:13                                       ` Guenter Roeck
2015-06-11  5:13                                         ` Guenter Roeck
2015-06-11  5:33                                         ` Fu Wei
2015-06-11  5:32                                       ` Fu Wei
2015-06-11  5:32                                         ` Fu Wei
2015-06-02 17:21         ` Timur Tabi
2015-06-02 17:21           ` Timur Tabi
2015-06-03 18:16     ` Timur Tabi
2015-06-03 18:16       ` Timur Tabi
2015-06-03 18:25       ` Guenter Roeck
2015-06-03 18:25         ` Guenter Roeck
2015-06-03 18:53         ` Timur Tabi
2015-06-03 18:53           ` Timur Tabi
2015-06-03 19:29           ` Arnd Bergmann
2015-09-10 22:45           ` Jon Masters
2015-09-14  4:21             ` Pratyush Anand
2015-09-14 15:27               ` Fu Wei
2015-09-14 15:27                 ` Fu Wei
2015-09-14  8:51             ` Catalin Marinas
2015-09-15  3:16             ` Fu Wei
2015-09-15  3:16               ` Fu Wei
2015-06-08 16:10         ` Fu Wei
2015-06-02  4:05   ` [PATCH v4 6/7] ACPI: add GTDT table parse driver into ACPI driver fu.wei
2015-06-02  4:05   ` [PATCH v4 7/7] clocksource: simplify ACPI code in arm_arch_timer.c fu.wei
2015-06-10 13:41 ` [PATCH v5 0/8] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-10 13:41   ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 1/8] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-06-10 13:41   ` [PATCH v5 2/8] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-06-10 13:41     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 3/8] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-06-10 13:41     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 4/8] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-06-10 16:21     ` Guenter Roeck
2015-06-10 16:21       ` Guenter Roeck
2015-06-11 11:22       ` Fu Wei
2015-06-11 16:38         ` Guenter Roeck
2015-06-11 16:38           ` Guenter Roeck
2015-06-10 13:41   ` [PATCH v5 5/8] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-10 13:41     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 6/8] ACPI: add GTDT table parse driver into ACPI driver fu.wei
2015-06-10 13:41     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 7/8] Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver fu.wei
2015-06-10 13:41     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 13:41   ` [PATCH v5 8/8] clocksource: simplify ACPI code in arm_arch_timer.c fu.wei
2015-06-10 17:47 ` [PATCH non-pretimeout 0/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-10 17:47   ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 17:47   ` [PATCH non-pretimeout 1/7] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-06-10 17:47     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-10 17:47   ` [PATCH non-pretimeout 2/7] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-06-10 17:47   ` [PATCH non-pretimeout 3/7] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-06-10 17:47     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-12 20:54     ` Timur Tabi
2015-06-12 20:54       ` Timur Tabi
2015-06-14 10:05       ` Fu Wei
2015-06-14 13:17         ` Timur Tabi
2015-06-14 13:17           ` Timur Tabi
2015-06-14 13:57         ` Guenter Roeck
2015-06-15 11:00           ` Fu Wei
2015-06-10 17:47   ` [PATCH non-pretimeout 4/7] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-11  5:33     ` Guenter Roeck
2015-06-11  5:33       ` Guenter Roeck
2015-06-11  5:44       ` Fu Wei
2015-06-11  5:49         ` Guenter Roeck
2015-06-11  5:59           ` Fu Wei
2015-06-11  5:59             ` Fu Wei
2015-06-11 16:28     ` [non-pretimeout,4/7] " Guenter Roeck
2015-06-23 13:26       ` Fu Wei
2015-06-23 15:21         ` Guenter Roeck
2015-06-23 15:21           ` Guenter Roeck
2015-06-23 16:17           ` Fu Wei
2015-06-23 16:43             ` Guenter Roeck
2015-06-23 17:01               ` Fu Wei
2015-06-23 17:01                 ` Fu Wei
2015-06-12  3:57     ` [PATCH non-pretimeout 4/7] " Timur Tabi
2015-06-12  3:57       ` Timur Tabi
2015-06-14 10:15       ` Fu Wei
2015-06-14 10:15         ` Fu Wei
2015-06-10 17:47   ` [PATCH non-pretimeout 5/7] ACPI: add GTDT table parse driver into ACPI driver fu.wei
2015-06-11 11:14     ` Hanjun Guo
2015-06-10 17:47   ` [PATCH non-pretimeout 6/7] Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver fu.wei
2015-06-10 17:47     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-12 13:16     ` Timur Tabi
2015-06-10 17:47   ` [PATCH non-pretimeout 7/7] clocksource: simplify ACPI code in arm_arch_timer.c fu.wei
2015-06-10 17:47     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-23 14:16 ` [PATCH v6 0/8] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-06-23 14:16   ` [PATCH v6 1/8] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-06-23 14:16     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-07-14 14:49     ` Rob Herring
2015-07-14 14:49       ` Rob Herring
2015-07-14 15:48       ` Fu Wei
2015-07-14 15:48         ` Fu Wei
2015-07-15 12:52         ` Fu Wei
2015-07-15 12:52           ` Fu Wei
2015-06-23 14:16   ` [PATCH v6 2/8] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-06-23 14:16     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-23 14:16   ` [PATCH v6 3/8] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-06-23 14:16   ` [PATCH v6 4/8] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-06-23 14:16     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-29 16:53   ` [PATCH v6 0/8] Watchdog: introduce ARM SBSA watchdog driver Fu Wei
2015-06-29 16:53     ` Fu Wei
2015-06-29 19:16     ` Guenter Roeck
2015-06-30 23:47       ` Fu Wei
2015-07-13  9:09       ` Fu Wei
2015-07-13  9:09         ` Fu Wei
2015-07-13 15:34         ` Guenter Roeck
2015-07-14  0:42           ` Fu Wei
2015-07-14  0:42             ` Fu Wei
2015-06-23 15:59 ` [PATCH v6 5/8] " fu.wei
2015-06-23 15:59   ` [PATCH v6 6/8] ACPI: add GTDT table parse driver into ACPI driver fu.wei
2015-06-23 15:59     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-07-23  8:32     ` Fu Wei
2015-07-23  8:32       ` Fu Wei
2015-07-23  8:32       ` Fu Wei
2015-06-23 15:59   ` [PATCH v6 7/8] Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver fu.wei
2015-06-23 15:59     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-06-23 15:59   ` [PATCH v6 8/8] clocksource: simplify ACPI code in arm_arch_timer.c fu.wei
2015-06-23 15:59     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-07-13  8:53 ` [PATCH v2 0/3] arm64: Add multiboot support (via fdt) for Xen boot fu.wei
2015-07-13  8:53   ` [PATCH v2 1/3] arm64: Add Xen boot support file fu.wei
2015-07-15 16:18     ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-15 16:18       ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-16 16:46       ` Ian Campbell
2015-07-16 16:46         ` Ian Campbell
2015-07-23 10:16       ` Fu Wei
2015-07-23 10:16         ` Fu Wei
2015-07-13  8:53   ` [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support fu.wei
2015-07-14  3:53     ` Andrei Borzenkov
2015-07-14  3:53       ` Andrei Borzenkov
2015-07-14  9:41       ` Ian Campbell
2015-07-14 15:23         ` [Xen-devel] " Konrad Rzeszutek Wilk
2015-07-14 13:09       ` Fu Wei
2015-07-14 13:09         ` Fu Wei
2015-07-15 16:24         ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-15 16:24           ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-13  8:54   ` [PATCH v2 3/3] arm64: Add the introduction of Xen boot command fu.wei
2015-07-14  9:29   ` [Xen-devel] [PATCH v2 0/3] arm64: Add multiboot support (via fdt) for Xen boot Ian Campbell
2015-07-14  9:29     ` Ian Campbell
2015-07-14 11:56     ` Fu Wei
2015-07-15 15:56   ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-15 15:56     ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-23  5:16 ` [PATCH v3 0/4] arm64: Add Xen boot support (via fdt) fu.wei
2015-07-23  5:16   ` [PATCH v3 1/4] arm64: Add and export some accessor functions for xen boot fu.wei
2015-10-29 12:03     ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-29 12:03       ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-30  7:11       ` Fu Wei
2015-10-30  7:11         ` Fu Wei
2015-07-23  5:16   ` [PATCH v3 2/4] arm64: Add xen_boot module file fu.wei
2015-10-29 14:27     ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-29 14:27       ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-30  8:08       ` Fu Wei
2015-10-30  8:08         ` Fu Wei
2015-11-03 14:57         ` Fu Wei
2015-11-03 14:57           ` Fu Wei
2015-11-03 15:22           ` Ian Campbell
2015-11-03 15:22             ` Ian Campbell
2015-11-05  9:46             ` Fu Wei
2015-07-23  5:16   ` [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64 fu.wei
2015-10-29 15:25     ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-29 15:25       ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-29 19:53       ` Andrei Borzenkov
2015-10-29 19:53         ` Andrei Borzenkov
2015-10-30  8:44       ` Fu Wei
2015-10-30  8:44         ` Fu Wei
2015-10-30  9:50         ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-30  9:50           ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-10-30 10:07           ` Andrei Borzenkov
2015-10-30 10:07             ` Andrei Borzenkov
     [not found]             ` <CAEaD8JOHR9QXa=ySc36a-ECF-7MUBgvaSVFw_BZxrXrvZA5u_A@mail.gmail.com>
     [not found]               ` <CAA91j0XKvpgVVWuv79AEv6bguxrQrV4q+eG5Xtw+UBvMSmDF-w@mail.gmail.com>
2015-11-12 10:43                 ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-07-23  5:16   ` [PATCH v3 4/4] arm64: Add the introduction of xen boot commands in docs/grub.texi fu.wei
2015-08-04  8:34   ` [PATCH v3 0/4] arm64: Add Xen boot support (via fdt) Fu Wei
2015-09-08  3:38     ` Fu Wei
2015-09-30 16:00       ` Stefano Stabellini
2015-10-01 16:19         ` Stefano Stabellini
2015-10-29  2:43           ` Fu Wei
2015-10-29  6:06         ` Fu Wei
2015-10-29 11:00           ` Stefano Stabellini
2015-08-14 12:35 ` [PATCH] acpi, apei, arm64: APEI initial support for aarch64 fu.wei
2015-08-14 12:39   ` Fu Wei
2015-08-14 18:27     ` Zhang, Jonathan Zhixiong
2015-08-15  6:45       ` Fu Wei
2015-08-17 10:01   ` Will Deacon
2015-08-17 23:19     ` Zhang, Jonathan Zhixiong
2015-08-18  8:31       ` Will Deacon
2015-08-18  9:26         ` Fu Wei
2015-08-18 16:44 ` [PATCH v2 0/2] acpi, apei: add BERT support fu.wei
2015-08-18 16:44   ` [PATCH v2 1/2] acpi, apei: add Boot Error Record Table (BERT) support fu.wei
2015-12-16 10:29     ` Borislav Petkov
2016-01-06 18:21       ` Fu Wei
2016-01-06 18:31         ` Borislav Petkov
2015-08-18 16:44   ` [PATCH v2 2/2] acpi, apei, bert: Clear error status at the end of error handling fu.wei
2015-12-16 10:30     ` Borislav Petkov
2015-12-15 16:39   ` [Linaro-acpi] [PATCH v2 0/2] acpi, apei: add BERT support Timur Tabi
2016-01-06 18:24     ` Fu Wei
2015-08-24 17:01 ` [PATCH v7 0/8] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-08-24 17:01   ` [PATCH v7 1/8] Documentation: add sbsa-gwdt.txt documentation fu.wei
2015-08-24 17:01   ` [PATCH v7 2/8] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2015-09-15  8:43     ` Dave Young
2015-09-15  8:43       ` Dave Young
2015-09-15  9:44       ` Pratyush Anand
2015-09-15 10:23         ` Fu Wei
2015-09-15 10:15       ` Fu Wei
2015-08-24 17:01   ` [PATCH v7 3/8] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2015-08-24 17:01   ` [PATCH v7 4/8] Watchdog: introdouce "pretimeout" into framework fu.wei
2015-08-24 17:01   ` [PATCH v7 5/8] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2015-09-10 22:29     ` Jon Masters
2015-09-11  2:05       ` Guenter Roeck
2015-09-11  2:50       ` Guenter Roeck
2015-09-11  2:50         ` Guenter Roeck
2015-09-14 17:11         ` Fu Wei
2015-09-15  8:38     ` Dave Young
2015-09-15 10:07       ` Fu Wei
2015-09-16  1:57         ` Dave Young
2015-10-13  8:34           ` Fu Wei
2015-10-13  8:34             ` Fu Wei
2015-08-24 17:01   ` [PATCH v7 6/8] ACPI: add GTDT table parse driver into ACPI driver fu.wei
2015-08-24 17:01   ` [PATCH v7 7/8] Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver fu.wei
2015-08-24 17:01   ` [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c fu.wei
2015-08-24 17:01     ` fu.wei-QSEj5FYQhm4dnm+yROfE0A
2015-08-24 17:50     ` Thomas Gleixner
2015-08-25 17:19       ` Fu Wei
2015-08-25 19:17         ` Thomas Gleixner
2015-08-25 19:17           ` Thomas Gleixner
2015-08-27 12:02           ` Hanjun Guo
2015-08-27 12:02             ` Hanjun Guo
2015-08-27 12:08             ` Thomas Gleixner
2015-08-27 12:28               ` Hanjun Guo
2015-08-27 12:28                 ` Hanjun Guo
2015-08-27 13:36                 ` Hanjun Guo
2015-08-27 13:40                   ` Thomas Gleixner
2015-08-27 13:40                     ` Thomas Gleixner
2015-08-27 13:51                     ` Fu Wei
2015-09-14 18:05                       ` Marc Zyngier
2015-09-14 18:05                         ` Marc Zyngier
2015-09-30 17:13   ` [PATCH v7 0/8] Watchdog: introduce ARM SBSA watchdog driver Pratyush Anand
2015-09-30 17:22     ` Fu Wei
2016-02-25  6:50 ` [PATCH v2 0/4] arm64,xen: add xen_boot support into grup-mkconfig fu.wei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55637A6D.3080403@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=al.stone@linaro.org \
    --cc=arnd@arndb.de \
    --cc=ashwin.chaugule@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=fu.wei@linaro.org \
    --cc=graeme.gregory@linaro.org \
    --cc=hanjun.guo@linaro.org \
    --cc=jcm@redhat.com \
    --cc=leo.duran@amd.com \
    --cc=linaro-acpi@lists.linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=tekkamanninja@gmail.com \
    --cc=timur@codeaurora.org \
    --cc=vgandhi@codeaurora.org \
    --cc=will.deacon@arm.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.