Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1409112 > unrolled thread

[PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer driver

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer driver Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 15:30 +0200
    [PATCH 3/3] ARM64: dts: amlogic: meson-gxbb: Add watchdog node Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 15:30 +0200
    [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 15:30 +0200
      Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver Carlo Caione <carlo@caione.org> - 2016-05-30 16:00 +0200
      Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver Guenter Roeck <linux@roeck-us.net> - 2016-05-30 16:50 +0200
      Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver Guenter Roeck <linux@roeck-us.net> - 2016-05-30 19:00 +0200
    Re: [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer  driver Carlo Caione <carlo@caione.org> - 2016-05-30 15:50 +0200
      Re: [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer  driver Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 16:20 +0200
    [PATCH 2/3] dt-bindings: watchdog: Add Meson GXBB Watchdog bindings Neil Armstrong <narmstrong@baylibre.com> - 2016-05-30 16:20 +0200

#1409112 — [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer driver

Adds support for the Amlogic Meson GXBB SoC Watchdog Timer.
It differs from the meson6/meson8b HW, so need for a separate driver.
The HW provides a divider capable of having a 1ms timebase thus simplifying
the counter update.
The restart call is not provided even if the HW is capable of triggering a
system reset immediately because of the PSCI firmare having such functionnality.
The watchdog is not expected to be running at boot time since there is a separate
system-level watchdog running from the SCPI co-processor, but this case would be
handle in a next driver update.

Changes since RFC version at http://lkml.kernel.org/r/1464249112-13658-1-git-send-email-narmstrong@baylibre.com :
- Remove status callback, will re-introduce it later to managed the already-running use case
- Fix registers defines indentation
- Fix space between operators
- Make all callbacks static functions
- Fix bindings with missing clocks attribute
- Do not stop/start watchdog on a timeout setup
- Fix probe device configuration

Neil Armstrong (3):
  watchdog: Add Meson GXBB Watchdog Driver
  dt-bindings: watchdog: Add Meson GXBB Watchdog bindings
  ARM64: dts: amlogic: meson-gxbb: Add watchdog node

 .../bindings/watchdog/meson-gxbb-wdt.txt           |  16 ++
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |   6 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/meson_gxbb_wdt.c                  | 277 +++++++++++++++++++++
 4 files changed, 300 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/meson-gxbb-wdt.txt
 create mode 100644 drivers/watchdog/meson_gxbb_wdt.c

-- 
2.7.0

[toc] | [next] | [standalone]

#1409113 — [PATCH 3/3] ARM64: dts: amlogic: meson-gxbb: Add watchdog node

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 832815d..bcca82f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -136,6 +136,12 @@
 				clocks = <&xtal>;
 				status = "disabled";
 			};
+
+			watchdog@98d0 {
+				compatible = "amlogic,meson-gxbb-wdt";
+				reg = <0x0 0x098d0 0x0 0x10>;
+				clocks = <&xtal>;
+			};
 		};
 
 		gic: interrupt-controller@c4301000 {
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1409115 — [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver

Add watchdog specific driver for Amlogic Meson GXBB SoC.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/watchdog/Makefile         |   1 +
 drivers/watchdog/meson_gxbb_wdt.c | 277 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 278 insertions(+)
 create mode 100644 drivers/watchdog/meson_gxbb_wdt.c

diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 9bde095..7679d93 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o
 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_MESON_WATCHDOG) += meson_gxbb_wdt.o
 obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
 obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
 obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
new file mode 100644
index 0000000..9619c5e
--- /dev/null
+++ b/drivers/watchdog/meson_gxbb_wdt.c
@@ -0,0 +1,277 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * BSD LICENSE
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define DEFAULT_TIMEOUT	10	/* seconds */
+
+#define GXBB_WDT_CTRL_REG			0x0
+#define GXBB_WDT_CTRL1_REG			0x4
+#define GXBB_WDT_TCNT_REG			0x8
+#define GXBB_WDT_RSET_REG			0xc
+
+#define GXBB_WDT_CTRL_EE_RESET_NOW		BIT(26)
+
+#define GXBB_WDT_CTRL_CLKDIV_EN			BIT(25)
+#define GXBB_WDT_CTRL_CLK_EN			BIT(24)
+#define GXBB_WDT_CTRL_IRQ_EN			BIT(23)
+#define GXBB_WDT_CTRL_EE_RESET			BIT(21)
+#define GXBB_WDT_CTRL_XTAL_SEL			(0)
+#define GXBB_WDT_CTRL_CLK81_SEL			BIT(19)
+#define GXBB_WDT_CTRL_EN			BIT(18)
+#define GXBB_WDT_CTRL_DIV_MASK			(BIT(18) - 1)
+
+#define GXBB_WDT_CTRL1_GPIO_PULSE		BIT(17)
+#define GXBB_WDT_CTRL1_GPIO_POL_RESET_0		BIT(16)
+#define GXBB_WDT_CTRL1_GPIO_POL_RESET_1		(0)
+#define GXBB_WDT_CTRL1_GPIO_PULSE_CNT		(BIT(16) - 1)
+
+#define GXBB_WDT_TCNT_SETUP_MASK		(BIT(16) - 1)
+#define GXBB_WDT_TCNT_CNT_SHIFT			16
+
+struct meson_gxbb_wdt {
+	void __iomem *reg_base;
+	struct watchdog_device wdt_dev;
+	struct clk *clk;
+};
+
+static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+	writel(readl(data->reg_base + GXBB_WDT_CTRL_REG) | GXBB_WDT_CTRL_EN,
+		data->reg_base + GXBB_WDT_CTRL_REG);
+
+	return 0;
+}
+
+static int meson_gxbb_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+	writel(readl(data->reg_base + GXBB_WDT_CTRL_REG) & ~GXBB_WDT_CTRL_EN,
+		data->reg_base + GXBB_WDT_CTRL_REG);
+
+	return 0;
+}
+
+static int meson_gxbb_wdt_ping(struct watchdog_device *wdt_dev)
+{
+	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, data->reg_base + GXBB_WDT_RSET_REG);
+
+	return 0;
+}
+
+static int meson_gxbb_wdt_get_timeout(struct meson_gxbb_wdt *data)
+{
+	return (readl(data->reg_base + GXBB_WDT_TCNT_REG) &
+			GXBB_WDT_TCNT_SETUP_MASK) / 1000;
+}
+
+static int meson_gxbb_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				      unsigned int timeout)
+{
+	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
+
+	meson_gxbb_wdt_ping(wdt_dev);
+
+	writel(timeout * 1000, data->reg_base + GXBB_WDT_TCNT_REG);
+
+	return 0;
+}
+
+static unsigned int meson_gxbb_wdt_get_timeleft(struct watchdog_device *wdt_dev)
+{
+	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
+	unsigned long reg;
+
+	reg = readl(data->reg_base + GXBB_WDT_TCNT_REG);
+
+	return ((reg >> GXBB_WDT_TCNT_CNT_SHIFT)-
+			(reg & GXBB_WDT_TCNT_SETUP_MASK)) / 1000;
+}
+
+static const struct watchdog_ops meson_gxbb_wdt_ops = {
+	.start = meson_gxbb_wdt_start,
+	.stop = meson_gxbb_wdt_stop,
+	.ping = meson_gxbb_wdt_ping,
+	.set_timeout = meson_gxbb_wdt_set_timeout,
+	.get_timeleft = meson_gxbb_wdt_get_timeleft,
+};
+
+static const struct watchdog_info meson_gxbb_wdt_info = {
+	.identity = "meson-gxbb-wdt",
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+};
+
+static int __maybe_unused meson_gxbb_wdt_resume(struct device *dev)
+{
+	struct meson_gxbb_wdt *data = dev_get_drvdata(dev);
+
+	if (watchdog_active(&data->wdt_dev))
+		meson_gxbb_wdt_start(&data->wdt_dev);
+
+	return 0;
+}
+
+static int __maybe_unused meson_gxbb_wdt_suspend(struct device *dev)
+{
+	struct meson_gxbb_wdt *data = dev_get_drvdata(dev);
+
+	if (watchdog_active(&data->wdt_dev))
+		meson_gxbb_wdt_stop(&data->wdt_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops meson_gxbb_wdt_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(meson_gxbb_wdt_suspend, meson_gxbb_wdt_resume)
+};
+
+static const struct of_device_id meson_gxbb_wdt_dt_ids[] = {
+	 { .compatible = "amlogic,meson-gxbb-wdt", },
+	 { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_gxbb_wdt_dt_ids);
+
+static int meson_gxbb_wdt_probe(struct platform_device *pdev)
+{
+	struct meson_gxbb_wdt *data;
+	struct resource *res;
+	int ret;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->reg_base))
+		return PTR_ERR(data->reg_base);
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk))
+		return PTR_ERR(data->clk);
+
+	clk_prepare_enable(data->clk);
+
+	platform_set_drvdata(pdev, data);
+
+	data->wdt_dev.parent = &pdev->dev;
+	data->wdt_dev.info = &meson_gxbb_wdt_info;
+	data->wdt_dev.ops = &meson_gxbb_wdt_ops;
+	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
+	data->wdt_dev.min_timeout = 1;
+	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
+	watchdog_set_drvdata(&data->wdt_dev, data);
+
+	/* Setup with 1ms timebase */
+	writel(((clk_get_rate(data->clk) / 1000) & GXBB_WDT_CTRL_DIV_MASK) |
+		GXBB_WDT_CTRL_EE_RESET |
+		GXBB_WDT_CTRL_CLK_EN |
+		GXBB_WDT_CTRL_CLKDIV_EN,
+		data->reg_base + GXBB_WDT_CTRL_REG);
+
+	meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
+
+	ret = watchdog_register_device(&data->wdt_dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int meson_gxbb_wdt_remove(struct platform_device *pdev)
+{
+	struct meson_gxbb_wdt *data = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&data->wdt_dev);
+
+	return 0;
+}
+
+static void meson_gxbb_wdt_shutdown(struct platform_device *pdev)
+{
+	struct meson_gxbb_wdt *data = platform_get_drvdata(pdev);
+
+	meson_gxbb_wdt_stop(&data->wdt_dev);
+}
+
+static struct platform_driver meson_gxbb_wdt_driver = {
+	.probe	= meson_gxbb_wdt_probe,
+	.remove	= meson_gxbb_wdt_remove,
+	.shutdown = meson_gxbb_wdt_shutdown,
+	.driver = {
+		.name = "meson-gxbb-wdt",
+		.pm = &meson_gxbb_wdt_pm_ops,
+		.of_match_table	= meson_gxbb_wdt_dt_ids,
+	},
+};
+
+module_platform_driver(meson_gxbb_wdt_driver);
+
+MODULE_ALIAS("platform:meson-gxbb-wdt");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic Meson GXBB Watchdog timer driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1409143 — Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver

On 30/05/16 15:29, Neil Armstrong wrote:
> Add watchdog specific driver for Amlogic Meson GXBB SoC.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> +

> +#define GXBB_WDT_CTRL1_REG			0x4
...
> +#define GXBB_WDT_CTRL_EE_RESET_NOW		BIT(26)
...
> +#define GXBB_WDT_CTRL_IRQ_EN			BIT(23)
...
> +#define GXBB_WDT_CTRL_XTAL_SEL			(0)
...
> +#define GXBB_WDT_CTRL_CLK81_SEL			BIT(19)
...
> +#define GXBB_WDT_CTRL1_GPIO_PULSE		BIT(17)
...
> +#define GXBB_WDT_CTRL1_GPIO_POL_RESET_0		BIT(16)
> +#define GXBB_WDT_CTRL1_GPIO_POL_RESET_1		(0)
> +#define GXBB_WDT_CTRL1_GPIO_PULSE_CNT		(BIT(16) - 1)

nit: all these defines are not used at all in the driver. You can remove them

Thanks!


-- 
Carlo Caione

[toc] | [prev] | [next] | [standalone]

#1409183 — Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver

On 05/30/2016 06:29 AM, Neil Armstrong wrote:
> Add watchdog specific driver for Amlogic Meson GXBB SoC.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>   drivers/watchdog/Makefile         |   1 +
>   drivers/watchdog/meson_gxbb_wdt.c | 277 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 278 insertions(+)
>   create mode 100644 drivers/watchdog/meson_gxbb_wdt.c
>
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9bde095..7679d93 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o
>   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_MESON_WATCHDOG) += meson_gxbb_wdt.o

No extra Kconfig entry ? Why ?

[toc] | [prev] | [next] | [standalone]

#1409294 — Re: [PATCH 1/3] watchdog: Add Meson GXBB Watchdog Driver

On 05/30/2016 06:29 AM, Neil Armstrong wrote:
> Add watchdog specific driver for Amlogic Meson GXBB SoC.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>   drivers/watchdog/Makefile         |   1 +
>   drivers/watchdog/meson_gxbb_wdt.c | 277 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 278 insertions(+)
>   create mode 100644 drivers/watchdog/meson_gxbb_wdt.c
>
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9bde095..7679d93 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o
>   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_MESON_WATCHDOG) += meson_gxbb_wdt.o

I would really prefer a separate configuration option.

>   obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
>   obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
>   obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> new file mode 100644
> index 0000000..9619c5e
> --- /dev/null
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -0,0 +1,277 @@
> +/*
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + * redistributing this file, you may do so under either license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright (c) 2016 BayLibre, SAS.
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + * The full GNU General Public License is included in this distribution
> + * in the file called COPYING.
> + *
> + * BSD LICENSE
> + *
> + * Copyright (c) 2016 BayLibre, SAS.
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + *   * Redistributions of source code must retain the above copyright
> + *     notice, this list of conditions and the following disclaimer.
> + *   * Redistributions in binary form must reproduce the above copyright
> + *     notice, this list of conditions and the following disclaimer in
> + *     the documentation and/or other materials provided with the
> + *     distribution.
> + *   * Neither the name of Intel Corporation nor the names of its
> + *     contributors may be used to endorse or promote products derived
> + *     from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>

Please order include files alphabetically; that simplifies later changes
and helps finding duplicates.

> +
> +#define DEFAULT_TIMEOUT	10	/* seconds */
> +

Sure you want the default timeout to be that low ?

> +#define GXBB_WDT_CTRL_REG			0x0
> +#define GXBB_WDT_CTRL1_REG			0x4
> +#define GXBB_WDT_TCNT_REG			0x8
> +#define GXBB_WDT_RSET_REG			0xc
> +
> +#define GXBB_WDT_CTRL_EE_RESET_NOW		BIT(26)
> +
> +#define GXBB_WDT_CTRL_CLKDIV_EN			BIT(25)
> +#define GXBB_WDT_CTRL_CLK_EN			BIT(24)
> +#define GXBB_WDT_CTRL_IRQ_EN			BIT(23)
> +#define GXBB_WDT_CTRL_EE_RESET			BIT(21)
> +#define GXBB_WDT_CTRL_XTAL_SEL			(0)
> +#define GXBB_WDT_CTRL_CLK81_SEL			BIT(19)
> +#define GXBB_WDT_CTRL_EN			BIT(18)
> +#define GXBB_WDT_CTRL_DIV_MASK			(BIT(18) - 1)
> +
> +#define GXBB_WDT_CTRL1_GPIO_PULSE		BIT(17)
> +#define GXBB_WDT_CTRL1_GPIO_POL_RESET_0		BIT(16)
> +#define GXBB_WDT_CTRL1_GPIO_POL_RESET_1		(0)
> +#define GXBB_WDT_CTRL1_GPIO_PULSE_CNT		(BIT(16) - 1)
> +
> +#define GXBB_WDT_TCNT_SETUP_MASK		(BIT(16) - 1)
> +#define GXBB_WDT_TCNT_CNT_SHIFT			16
> +
> +struct meson_gxbb_wdt {
> +	void __iomem *reg_base;
> +	struct watchdog_device wdt_dev;
> +	struct clk *clk;
> +};
> +
> +static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(readl(data->reg_base + GXBB_WDT_CTRL_REG) | GXBB_WDT_CTRL_EN,
> +		data->reg_base + GXBB_WDT_CTRL_REG);
> +
> +	return 0;
> +}
> +
> +static int meson_gxbb_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(readl(data->reg_base + GXBB_WDT_CTRL_REG) & ~GXBB_WDT_CTRL_EN,
> +		data->reg_base + GXBB_WDT_CTRL_REG);

Please align continuation lines with '('.

> +
> +	return 0;
> +}
> +
> +static int meson_gxbb_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> +	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(0, data->reg_base + GXBB_WDT_RSET_REG);
> +
> +	return 0;
> +}
> +
> +static int meson_gxbb_wdt_get_timeout(struct meson_gxbb_wdt *data)
> +{
> +	return (readl(data->reg_base + GXBB_WDT_TCNT_REG) &
> +			GXBB_WDT_TCNT_SETUP_MASK) / 1000;
> +}
> +
Unused function ?

> +static int meson_gxbb_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				      unsigned int timeout)
> +{
> +	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> +
> +	meson_gxbb_wdt_ping(wdt_dev);

Is this necessary ? The infrastructure calls it after calling this function.

> +
> +	writel(timeout * 1000, data->reg_base + GXBB_WDT_TCNT_REG);

	wdt_dev->timeout = timeout;

Also, timeout may be larger than the maximum supported by the hardware,
so you need to ensure that the value your write is not larger than
GXBB_WDT_TCNT_SETUP_MASK.

> +
> +	return 0;
> +}
> +
> +static unsigned int meson_gxbb_wdt_get_timeleft(struct watchdog_device *wdt_dev)
> +{
> +	struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> +	unsigned long reg;
> +
> +	reg = readl(data->reg_base + GXBB_WDT_TCNT_REG);
> +
> +	return ((reg >> GXBB_WDT_TCNT_CNT_SHIFT)-

Space between operators, please.

> +			(reg & GXBB_WDT_TCNT_SETUP_MASK)) / 1000;
> +}
> +
> +static const struct watchdog_ops meson_gxbb_wdt_ops = {
> +	.start = meson_gxbb_wdt_start,
> +	.stop = meson_gxbb_wdt_stop,
> +	.ping = meson_gxbb_wdt_ping,
> +	.set_timeout = meson_gxbb_wdt_set_timeout,
> +	.get_timeleft = meson_gxbb_wdt_get_timeleft,
> +};
> +
> +static const struct watchdog_info meson_gxbb_wdt_info = {
> +	.identity = "meson-gxbb-wdt",

It is common here to provide a user readable string, which would be something
like "Meson GXBB Watchdog".

> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +};
> +
> +static int __maybe_unused meson_gxbb_wdt_resume(struct device *dev)
> +{
> +	struct meson_gxbb_wdt *data = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&data->wdt_dev))
> +		meson_gxbb_wdt_start(&data->wdt_dev);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused meson_gxbb_wdt_suspend(struct device *dev)
> +{
> +	struct meson_gxbb_wdt *data = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(&data->wdt_dev))
> +		meson_gxbb_wdt_stop(&data->wdt_dev);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops meson_gxbb_wdt_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(meson_gxbb_wdt_suspend, meson_gxbb_wdt_resume)
> +};
> +
> +static const struct of_device_id meson_gxbb_wdt_dt_ids[] = {
> +	 { .compatible = "amlogic,meson-gxbb-wdt", },
> +	 { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, meson_gxbb_wdt_dt_ids);
> +
> +static int meson_gxbb_wdt_probe(struct platform_device *pdev)
> +{
> +	struct meson_gxbb_wdt *data;
> +	struct resource *res;
> +	int ret;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->reg_base))
> +		return PTR_ERR(data->reg_base);
> +
> +	data->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(data->clk))
> +		return PTR_ERR(data->clk);
> +
> +	clk_prepare_enable(data->clk);
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	data->wdt_dev.parent = &pdev->dev;
> +	data->wdt_dev.info = &meson_gxbb_wdt_info;
> +	data->wdt_dev.ops = &meson_gxbb_wdt_ops;
> +	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
> +	data->wdt_dev.min_timeout = 1;
> +	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> +	watchdog_set_drvdata(&data->wdt_dev, data);
> +
> +	/* Setup with 1ms timebase */
> +	writel(((clk_get_rate(data->clk) / 1000) & GXBB_WDT_CTRL_DIV_MASK) |
> +		GXBB_WDT_CTRL_EE_RESET |
> +		GXBB_WDT_CTRL_CLK_EN |
> +		GXBB_WDT_CTRL_CLKDIV_EN,
> +		data->reg_base + GXBB_WDT_CTRL_REG);
> +
> +	meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
> +
> +	ret = watchdog_register_device(&data->wdt_dev);
> +	if (ret)
> +		return ret;

clk_disable_unprepare() ?

> +
> +	return 0;
> +}
> +
> +static int meson_gxbb_wdt_remove(struct platform_device *pdev)
> +{
> +	struct meson_gxbb_wdt *data = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(&data->wdt_dev);
> +

clk_disable_unprepare() ?

> +	return 0;
> +}
> +
> +static void meson_gxbb_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct meson_gxbb_wdt *data = platform_get_drvdata(pdev);
> +
> +	meson_gxbb_wdt_stop(&data->wdt_dev);
> +}
> +
> +static struct platform_driver meson_gxbb_wdt_driver = {
> +	.probe	= meson_gxbb_wdt_probe,
> +	.remove	= meson_gxbb_wdt_remove,
> +	.shutdown = meson_gxbb_wdt_shutdown,
> +	.driver = {
> +		.name = "meson-gxbb-wdt",
> +		.pm = &meson_gxbb_wdt_pm_ops,
> +		.of_match_table	= meson_gxbb_wdt_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(meson_gxbb_wdt_driver);
> +
> +MODULE_ALIAS("platform:meson-gxbb-wdt");
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_DESCRIPTION("Amlogic Meson GXBB Watchdog timer driver");
> +MODULE_LICENSE("Dual BSD/GPL");
>

[toc] | [prev] | [next] | [standalone]

#1409141 — Re: [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer driver

On 30/05/16 15:29, Neil Armstrong wrote:
> Adds support for the Amlogic Meson GXBB SoC Watchdog Timer.
> It differs from the meson6/meson8b HW, so need for a separate driver.
> The HW provides a divider capable of having a 1ms timebase thus simplifying
> the counter update.
> The restart call is not provided even if the HW is capable of triggering a
> system reset immediately because of the PSCI firmare having such functionnality.
> The watchdog is not expected to be running at boot time since there is a separate
> system-level watchdog running from the SCPI co-processor, but this case would be
> handle in a next driver update.
> 
> Changes since RFC version at http://lkml.kernel.org/r/1464249112-13658-1-git-send-email-narmstrong@baylibre.com :
> - Remove status callback, will re-introduce it later to managed the already-running use case
> - Fix registers defines indentation
> - Fix space between operators
> - Make all callbacks static functions
> - Fix bindings with missing clocks attribute
> - Do not stop/start watchdog on a timeout setup
> - Fix probe device configuration
> 
> Neil Armstrong (3):
>   watchdog: Add Meson GXBB Watchdog Driver
>   dt-bindings: watchdog: Add Meson GXBB Watchdog bindings
>   ARM64: dts: amlogic: meson-gxbb: Add watchdog node

I'm missing [PATCH 2/3]

-- 
Carlo Caione

[toc] | [prev] | [next] | [standalone]

#1409151 — Re: [PATCH 0/3] watchdog: Add Amlogic Meson GXBB Watchdog Timer driver

On 05/30/2016 03:45 PM, Carlo Caione wrote:
> On 30/05/16 15:29, Neil Armstrong wrote:
>> Adds support for the Amlogic Meson GXBB SoC Watchdog Timer.
>> It differs from the meson6/meson8b HW, so need for a separate driver.
>> The HW provides a divider capable of having a 1ms timebase thus simplifying
>> the counter update.
>> The restart call is not provided even if the HW is capable of triggering a
>> system reset immediately because of the PSCI firmare having such functionnality.
>> The watchdog is not expected to be running at boot time since there is a separate
>> system-level watchdog running from the SCPI co-processor, but this case would be
>> handle in a next driver update.
>>
>> Changes since RFC version at http://lkml.kernel.org/r/1464249112-13658-1-git-send-email-narmstrong@baylibre.com :
>> - Remove status callback, will re-introduce it later to managed the already-running use case
>> - Fix registers defines indentation
>> - Fix space between operators
>> - Make all callbacks static functions
>> - Fix bindings with missing clocks attribute
>> - Do not stop/start watchdog on a timeout setup
>> - Fix probe device configuration
>>
>> Neil Armstrong (3):
>>   watchdog: Add Meson GXBB Watchdog Driver
>>   dt-bindings: watchdog: Add Meson GXBB Watchdog bindings
>>   ARM64: dts: amlogic: meson-gxbb: Add watchdog node
> 
> I'm missing [PATCH 2/3]
> 
Sorry, fixed here :

[toc] | [prev] | [next] | [standalone]

#1409155 — [PATCH 2/3] dt-bindings: watchdog: Add Meson GXBB Watchdog bindings

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/watchdog/meson-gxbb-wdt.txt      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/meson-gxbb-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/meson-gxbb-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson-gxbb-wdt.txt
new file mode 100644
index 0000000..c7fe36f
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/meson-gxbb-wdt.txt
@@ -0,0 +1,16 @@
+Meson GXBB SoCs Watchdog timer
+
+Required properties:
+
+- compatible : should be "amlogic,meson-gxbb-wdt"
+- reg : Specifies base physical address and size of the registers.
+- clocks : Should be a phandle to the Watchdog clock source, for GXBB the xtal
+	   is the default clock source.
+
+Example:
+
+wdt: watchdog@98d0 {
+	compatible = "amlogic,meson-gxbb-wdt";
+	reg = <0 0x98d0 0x0 0x10>;
+	clocks = <&xtal>;
+};
-- 
2.7.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web