Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933926AbbKSQZ4 (ORCPT ); Thu, 19 Nov 2015 11:25:56 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:48828 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933201AbbKSQZx (ORCPT ); Thu, 19 Nov 2015 11:25:53 -0500 Subject: Re: [PATCH v2 2/2] watchdog: add support for Sigma Designs SMP86xx/SMP87xx To: Mans Rullgard , Wim Van Sebroeck , linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org References: <1447869343-25168-1-git-send-email-mans@mansr.com> <1447869343-25168-2-git-send-email-mans@mansr.com> From: Guenter Roeck Message-ID: <564DF807.3040608@roeck-us.net> Date: Thu, 19 Nov 2015 08:25:43 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1447869343-25168-2-git-send-email-mans@mansr.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8666 Lines: 294 On 11/18/2015 09:55 AM, Mans Rullgard wrote: > This adds support for the Sigma Designs SMP86xx/SMP87xx family built-in > watchdog. > > Signed-off-by: Mans Rullgard > --- > Changes: > - combine start and ping functions > - use watchdog_init_timeout() > - calculate max timeout from clock rate > - add get_timeleft callback > - check if already running on startup > - add explanatory comments > - improve kconfig text > --- > drivers/watchdog/Kconfig | 10 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/tangox_wdt.c | 213 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 224 insertions(+) > create mode 100644 drivers/watchdog/tangox_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 7a8a6c6..f43ff7a 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -135,6 +135,16 @@ config MENF21BMC_WATCHDOG > This driver can also be built as a module. If so the module > will be called menf21bmc_wdt. > > +config TANGOX_WATCHDOG > + tristate "Sigma Designs SMP86xx/SMP87xx watchdog" > + select WATCHDOG_CORE > + depends on ARCH_TANGOX || COMPILE_TEST > + help > + Support for the watchdog in Sigma Designs SMP86xx (tango3) > + and SMP87xx (tango4) family chips. > + > + This driver can be built as a module. The module name is tangox_wdt. > + > config WM831X_WATCHDOG > tristate "WM831x watchdog" > depends on MFD_WM831X > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 53d4827..46cb387 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -187,6 +187,7 @@ obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o > obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o > obj-$(CONFIG_DA9063_WATCHDOG) += da9063_wdt.o > obj-$(CONFIG_GPIO_WATCHDOG) += gpio_wdt.o > +obj-$(CONFIG_TANGOX_WATCHDOG) += tangox_wdt.o > obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o > obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o > obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o > diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c > new file mode 100644 > index 0000000..0b458ff > --- /dev/null > +++ b/drivers/watchdog/tangox_wdt.c > @@ -0,0 +1,213 @@ > +/* > + * Copyright (C) 2015 Mans Rullgard > + * SMP86xx/SMP87xx Watchdog driver > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DEFAULT_TIMEOUT 30 > + > +static bool nowayout = WATCHDOG_NOWAYOUT; > +module_param(nowayout, bool, 0); > +MODULE_PARM_DESC(nowayout, > + "Watchdog cannot be stopped once started (default=" > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > + > +static unsigned int timeout; > +module_param(timeout, int, 0); > +MODULE_PARM_DESC(timeout, "Watchdog timeout"); > + > +/* > + * Counter counts down from programmed value. Reset asserts when > + * the counter reaches 1. > + */ > +#define WD_COUNTER 0 > + > +#define WD_CONFIG 4 > +#define WD_CONFIG_XTAL_IN BIT(0) > +#define WD_CONFIG_DISABLE BIT(31) > + Please include linux/bitops.h when using BIT macros. > +struct tangox_wdt_device { > + struct watchdog_device wdt; > + void __iomem *base; > + unsigned long clk_rate; > + struct clk *clk; > + struct notifier_block restart; > +}; > + > +static int tangox_wdt_set_timeout(struct watchdog_device *wdt, > + unsigned int new_timeout) > +{ > + wdt->timeout = new_timeout; > + > + return 0; > +} > + > +static int tangox_wdt_start(struct watchdog_device *wdt) > +{ > + struct tangox_wdt_device *dev = watchdog_get_drvdata(wdt); > + u32 ticks; > + > + ticks = 1 + wdt->timeout * dev->clk_rate; > + writel(ticks, dev->base + WD_COUNTER); > + > + return 0; > +} > + > +static int tangox_wdt_stop(struct watchdog_device *wdt) > +{ > + struct tangox_wdt_device *dev = watchdog_get_drvdata(wdt); > + > + writel(0, dev->base + WD_COUNTER); > + > + return 0; > +} > + > +static unsigned int tangox_wdt_get_timeleft(struct watchdog_device *wdt) > +{ > + struct tangox_wdt_device *dev = watchdog_get_drvdata(wdt); > + u32 count; > + > + count = readl(dev->base + WD_COUNTER); > + > + if (!count) > + return 0; > + > + return (count - 1) / dev->clk_rate; > +} > + > +static const struct watchdog_info tangox_wdt_info = { > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, > + .identity = "tangox watchdog", > +}; > + > +static const struct watchdog_ops tangox_wdt_ops = { > + .start = tangox_wdt_start, > + .stop = tangox_wdt_stop, > + .set_timeout = tangox_wdt_set_timeout, > + .get_timeleft = tangox_wdt_get_timeleft, > +}; > + > +static int tangox_wdt_restart(struct notifier_block *nb, unsigned long action, > + void *data) > +{ > + struct tangox_wdt_device *dev = > + container_of(nb, struct tangox_wdt_device, restart); > + > + writel(1, dev->base + WD_COUNTER); > + > + return NOTIFY_DONE; > +} > + > +static int tangox_wdt_probe(struct platform_device *pdev) > +{ > + struct tangox_wdt_device *dev; > + struct resource *res; > + int err; > + > + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); > + if (!dev) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + dev->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(dev->base)) > + return PTR_ERR(dev->base); > + > + dev->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(dev->clk)) > + return PTR_ERR(dev->clk); > + > + err = clk_prepare_enable(dev->clk); > + if (err) > + return err; > + > + dev->clk_rate = clk_get_rate(dev->clk); > + > + dev->wdt.parent = &pdev->dev; > + dev->wdt.info = &tangox_wdt_info; > + dev->wdt.ops = &tangox_wdt_ops; > + dev->wdt.timeout = DEFAULT_TIMEOUT; > + dev->wdt.min_timeout = 1; > + dev->wdt.max_timeout = (U32_MAX - 1) / dev->clk_rate; > + > + watchdog_init_timeout(&dev->wdt, timeout, &pdev->dev); > + watchdog_set_nowayout(&dev->wdt, nowayout); > + watchdog_set_drvdata(&dev->wdt, dev); > + > + writel(WD_CONFIG_XTAL_IN, dev->base + WD_CONFIG); What happens if the DISABLE bit was previously set (assuming that DISABLE means that the watchdog is disabled) ? Concern I guess would be the combination of DISABLE being set and WD_COUNTER at a value != 0 (say, 1). That might effectively auto-enable the watchdog or even reset the system. > + > + /* > + * Mark as active and restart with configured timeout if > + * already running. > + */ > + if (readl(dev->base + WD_COUNTER)) { > + set_bit(WDOG_ACTIVE, &dev->wdt.status); > + tangox_wdt_start(&dev->wdt); > + } > + > + err = watchdog_register_device(&dev->wdt); > + if (err) > + return err; Needs clk_disable_unprepare(). > + > + platform_set_drvdata(pdev, dev); > + > + dev->restart.notifier_call = tangox_wdt_restart; > + dev->restart.priority = 128; > + err = register_restart_handler(&dev->restart); > + if (err) > + dev_warn(&pdev->dev, "failed to register restart handler\n"); > + > + dev_info(dev->wdt.dev, "SMP86xx/SMP87xx watchdog registered\n"); > + > + return 0; > +} > + > +static int tangox_wdt_remove(struct platform_device *pdev) > +{ > + struct tangox_wdt_device *dev = platform_get_drvdata(pdev); > + > + tangox_wdt_stop(&dev->wdt); > + clk_disable_unprepare(dev->clk); > + > + unregister_restart_handler(&dev->restart); > + watchdog_unregister_device(&dev->wdt); > + > + return 0; > +} > + > +static const struct of_device_id tangox_wdt_dt_ids[] = { > + { .compatible = "sigma,smp8642-wdt" }, > + { .compatible = "sigma,smp8759-wdt" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, tangox_wdt_dt_ids); > + > +static struct platform_driver tangox_wdt_driver = { > + .probe = tangox_wdt_probe, > + .remove = tangox_wdt_remove, > + .driver = { > + .name = "tangox-wdt", > + .of_match_table = tangox_wdt_dt_ids, > + }, > +}; > + > +module_platform_driver(tangox_wdt_driver); > + > +MODULE_AUTHOR("Mans Rullgard "); > +MODULE_DESCRIPTION("SMP86xx/SMP87xx Watchdog driver"); > +MODULE_LICENSE("GPL"); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/