* [PATCH v3 0/2] Add WDT driver for Toshiba Visconti ARM SoC
@ 2020-09-20 5:18 Nobuhiro Iwamatsu
2020-09-20 5:18 ` [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device Nobuhiro Iwamatsu
2020-09-20 5:18 ` [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver Nobuhiro Iwamatsu
0 siblings, 2 replies; 7+ messages in thread
From: Nobuhiro Iwamatsu @ 2020-09-20 5:18 UTC (permalink / raw)
To: Rob Herring, Wim Van Sebroeck, Guenter Roeck
Cc: punit1.agrawal, yuji2.ishikawa, devicetree, linux-arm-kernel,
linux-watchdog, Nobuhiro Iwamatsu
Hi,
This is the PWM driver for Toshiba's ARM SoC, Visconti[0].
This has not yet been included in Linux kernel, but patches have been posted [1]
and some patches have been applied [2].
Since this is a SoC driver, this cannot work by itself, but I have confirmed that it
works with the patch series sent as [1] with DT setting.
Best regards,
Nobuhiro
[0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2020-September/599678.html
[2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2020-September/600578.html
watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device
v2 - > v3:
- no update
v1 - > v2:
- no update
watchdog: Add Toshiba Visconti watchdog driver
v1 - > v3:
- Fix unnecessary split lines
- Fix negative value check and handling for visconti_wdt_get_timeleft()
v1 - > v2:
- Sort incclude alphabetically.
- Add negative value check and handling for visconti_wdt_get_timeleft()
- Use dev_err_probe() with devm_clk_get()
Nobuhiro Iwamatsu (2):
watchdog: bindings: Add binding documentation for Toshiba Visconti
watchdog device
watchdog: Add Toshiba Visconti watchdog driver
.../watchdog/toshiba,visconti-wdt.yaml | 49 +++++
drivers/watchdog/Kconfig | 8 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/visconti_wdt.c | 191 ++++++++++++++++++
4 files changed, 249 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
create mode 100644 drivers/watchdog/visconti_wdt.c
--
2.27.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device
2020-09-20 5:18 [PATCH v3 0/2] Add WDT driver for Toshiba Visconti ARM SoC Nobuhiro Iwamatsu
@ 2020-09-20 5:18 ` Nobuhiro Iwamatsu
2020-09-21 7:27 ` Punit Agrawal
2020-09-20 5:18 ` [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver Nobuhiro Iwamatsu
1 sibling, 1 reply; 7+ messages in thread
From: Nobuhiro Iwamatsu @ 2020-09-20 5:18 UTC (permalink / raw)
To: Rob Herring, Wim Van Sebroeck, Guenter Roeck
Cc: punit1.agrawal, yuji2.ishikawa, devicetree, linux-arm-kernel,
linux-watchdog, Nobuhiro Iwamatsu
Add documentation for the binding of Toshiba Visconti SoC's watchdog.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
.../watchdog/toshiba,visconti-wdt.yaml | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
new file mode 100644
index 000000000000..721e38fa5a0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Toshiba Electronic Devices & Storage Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/watchdog/toshiba,visconti-wdt.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Toshiba Visconti SoCs PIUWDT Watchdog timer
+
+maintainers:
+ - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+allOf:
+ - $ref: watchdog.yaml#
+
+properties:
+ compatible:
+ enum:
+ - toshiba,visconti-wdt
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ wdt_clk: wdt-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <150000000>;
+ #clock-cells = <0>;
+ };
+
+ watchdog@28330000 {
+ compatible = "toshiba,visconti-wdt";
+ reg = <0 0x28330000 0 0x1000>;
+ clocks = <&wdt_clk>;
+ };
+ };
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver
2020-09-20 5:18 [PATCH v3 0/2] Add WDT driver for Toshiba Visconti ARM SoC Nobuhiro Iwamatsu
2020-09-20 5:18 ` [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device Nobuhiro Iwamatsu
@ 2020-09-20 5:18 ` Nobuhiro Iwamatsu
2020-09-21 7:25 ` Punit Agrawal
1 sibling, 1 reply; 7+ messages in thread
From: Nobuhiro Iwamatsu @ 2020-09-20 5:18 UTC (permalink / raw)
To: Rob Herring, Wim Van Sebroeck, Guenter Roeck
Cc: punit1.agrawal, yuji2.ishikawa, devicetree, linux-arm-kernel,
linux-watchdog, Nobuhiro Iwamatsu
Add the watchdog driver for Toshiba Visconti series.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/visconti_wdt.c | 191 ++++++++++++++++++++++++++++++++
3 files changed, 200 insertions(+)
create mode 100644 drivers/watchdog/visconti_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index ab7aad5a1e69..0cb078ce5e9d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1004,6 +1004,14 @@ config PM8916_WATCHDOG
Say Y here to include support watchdog timer embedded into the
pm8916 module.
+config VISCONTI_WATCHDOG
+ tristate "Toshiba Visconti series watchdog support"
+ depends on ARCH_VISCONTI || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ Say Y here to include support for the watchdog timer in Toshiba
+ Visconti SoCs.
+
# X86 (i386 + ia64 + x86_64) Architecture
config ACQUIRE_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 97bed1d3d97c..a7747e76fd29 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o
obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o
+obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o
# X86 (i386 + ia64 + x86_64) Architecture
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
diff --git a/drivers/watchdog/visconti_wdt.c b/drivers/watchdog/visconti_wdt.c
new file mode 100644
index 000000000000..4a67b9fe9220
--- /dev/null
+++ b/drivers/watchdog/visconti_wdt.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 TOSHIBA CORPORATION
+ * Copyright (c) 2020 Toshiba Electronic Devices & Storage Corporation
+ * Copyright (c) 2020 Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#define WDT_CNT 0x00
+#define WDT_MIN 0x04
+#define WDT_MAX 0x08
+#define WDT_CTL 0x0c
+#define WDT_CMD 0x10
+#define WDT_CMD_CLEAR 0x4352
+#define WDT_CMD_START_STOP 0x5354
+#define WDT_DIV 0x30
+
+#define VISCONTI_WDT_FREQ 2000000 /* 2MHz */
+#define WDT_DEFAULT_TIMEOUT 10U /* in seconds */
+
+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)")");
+
+struct visconti_wdt_priv {
+ struct watchdog_device wdev;
+ void __iomem *base;
+ u32 div;
+};
+
+static int visconti_wdt_start(struct watchdog_device *wdev)
+{
+ struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+ u32 timeout = wdev->timeout * VISCONTI_WDT_FREQ;
+
+ writel(priv->div, priv->base + WDT_DIV);
+ writel(0, priv->base + WDT_MIN);
+ writel(timeout, priv->base + WDT_MAX);
+ writel(0, priv->base + WDT_CTL);
+ writel(WDT_CMD_START_STOP, priv->base + WDT_CMD);
+
+ return 0;
+}
+
+static int visconti_wdt_stop(struct watchdog_device *wdev)
+{
+ struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+
+ writel(1, priv->base + WDT_CTL);
+ writel(WDT_CMD_START_STOP, priv->base + WDT_CMD);
+
+ return 0;
+}
+
+static int visconti_wdt_ping(struct watchdog_device *wdd)
+{
+ struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+ writel(WDT_CMD_CLEAR, priv->base + WDT_CMD);
+
+ return 0;
+}
+
+static unsigned int visconti_wdt_get_timeleft(struct watchdog_device *wdev)
+{
+ struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+ u32 timeout = wdev->timeout * VISCONTI_WDT_FREQ;
+ u32 cnt = readl(priv->base + WDT_CNT);
+
+ if (timeout <= cnt)
+ return 0;
+ timeout -= cnt;
+
+ return timeout / VISCONTI_WDT_FREQ;
+}
+
+static int visconti_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
+{
+ u32 val;
+ struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+
+ wdev->timeout = timeout;
+ val = wdev->timeout * VISCONTI_WDT_FREQ;
+
+ /* Clear counter before setting timeout because WDT expires */
+ writel(WDT_CMD_CLEAR, priv->base + WDT_CMD);
+ writel(val, priv->base + WDT_MAX);
+
+ return 0;
+}
+
+static const struct watchdog_info visconti_wdt_info = {
+ .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
+ .identity = "Visconti Watchdog",
+};
+
+static const struct watchdog_ops visconti_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = visconti_wdt_start,
+ .stop = visconti_wdt_stop,
+ .ping = visconti_wdt_ping,
+ .get_timeleft = visconti_wdt_get_timeleft,
+ .set_timeout = visconti_wdt_set_timeout,
+};
+
+static int visconti_wdt_probe(struct platform_device *pdev)
+{
+ struct watchdog_device *wdev;
+ struct visconti_wdt_priv *priv;
+ struct device *dev = &pdev->dev;
+ struct clk *clk;
+ int ret;
+ unsigned long clk_freq;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk), "Could not get clock\n");
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(dev, "Could not enable clock\n");
+ return ret;
+ }
+
+ clk_freq = clk_get_rate(clk);
+ if (!clk_freq) {
+ clk_disable_unprepare(clk);
+ dev_err(dev, "Could not get clock rate\n");
+ return -EINVAL;
+ }
+
+ priv->div = clk_freq / VISCONTI_WDT_FREQ;
+
+ /* Initialize struct watchdog_device. */
+ wdev = &priv->wdev;
+ wdev->info = &visconti_wdt_info;
+ wdev->ops = &visconti_wdt_ops;
+ wdev->parent = dev;
+ wdev->min_timeout = 1;
+ wdev->max_timeout = 0xffffffff / VISCONTI_WDT_FREQ;
+ wdev->timeout = min(wdev->max_timeout, WDT_DEFAULT_TIMEOUT);
+
+ watchdog_set_drvdata(wdev, priv);
+ watchdog_set_nowayout(wdev, nowayout);
+ watchdog_stop_on_unregister(wdev);
+
+ /* This overrides the default timeout only if DT configuration was found */
+ ret = watchdog_init_timeout(wdev, 0, dev);
+ if (ret)
+ dev_warn(dev, "Specified timeout value invalid, using default\n");
+
+ return devm_watchdog_register_device(dev, wdev);
+}
+
+static const struct of_device_id visconti_wdt_of_match[] = {
+ { .compatible = "toshiba,visconti-wdt", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, visconti_wdt_of_match);
+
+static struct platform_driver visconti_wdt_driver = {
+ .driver = {
+ .name = "visconti_wdt",
+ .of_match_table = visconti_wdt_of_match,
+ },
+ .probe = visconti_wdt_probe,
+};
+module_platform_driver(visconti_wdt_driver);
+
+MODULE_DESCRIPTION("TOSHIBA Visconti Watchdog Driver");
+MODULE_AUTHOR("TOSHIBA ELECTRONIC DEVICES & STORAGE CORPORATION");
+MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp");
+MODULE_LICENSE("GPL v2");
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver
2020-09-20 5:18 ` [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver Nobuhiro Iwamatsu
@ 2020-09-21 7:25 ` Punit Agrawal
2020-09-21 8:46 ` Nobuhiro Iwamatsu
0 siblings, 1 reply; 7+ messages in thread
From: Punit Agrawal @ 2020-09-21 7:25 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Wim Van Sebroeck, Guenter Roeck, devicetree,
linux-watchdog, yuji2.ishikawa, linux-arm-kernel
Iwamatsu-san,
Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> writes:
> Add the watchdog driver for Toshiba Visconti series.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
> drivers/watchdog/Kconfig | 8 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/visconti_wdt.c | 191 ++++++++++++++++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/watchdog/visconti_wdt.c
>
[...]
> diff --git a/drivers/watchdog/visconti_wdt.c b/drivers/watchdog/visconti_wdt.c
> new file mode 100644
> index 000000000000..4a67b9fe9220
> --- /dev/null
> +++ b/drivers/watchdog/visconti_wdt.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 TOSHIBA CORPORATION
> + * Copyright (c) 2020 Toshiba Electronic Devices & Storage Corporation
> + * Copyright (c) 2020 Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
interrupt.h doesn't seem to be used. It can be dropped.
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +#define WDT_CNT 0x00
> +#define WDT_MIN 0x04
> +#define WDT_MAX 0x08
> +#define WDT_CTL 0x0c
> +#define WDT_CMD 0x10
> +#define WDT_CMD_CLEAR 0x4352
> +#define WDT_CMD_START_STOP 0x5354
> +#define WDT_DIV 0x30
> +
> +#define VISCONTI_WDT_FREQ 2000000 /* 2MHz */
> +#define WDT_DEFAULT_TIMEOUT 10U /* in seconds */
> +
> +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)")");
> +
> +struct visconti_wdt_priv {
> + struct watchdog_device wdev;
> + void __iomem *base;
> + u32 div;
> +};
> +
> +static int visconti_wdt_start(struct watchdog_device *wdev)
> +{
> + struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> + u32 timeout = wdev->timeout * VISCONTI_WDT_FREQ;
> +
> + writel(priv->div, priv->base + WDT_DIV);
> + writel(0, priv->base + WDT_MIN);
> + writel(timeout, priv->base + WDT_MAX);
> + writel(0, priv->base + WDT_CTL);
> + writel(WDT_CMD_START_STOP, priv->base + WDT_CMD);
> +
> + return 0;
> +}
> +
> +static int visconti_wdt_stop(struct watchdog_device *wdev)
> +{
> + struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +
> + writel(1, priv->base + WDT_CTL);
> + writel(WDT_CMD_START_STOP, priv->base + WDT_CMD);
> +
> + return 0;
> +}
> +
> +static int visconti_wdt_ping(struct watchdog_device *wdd)
> +{
> + struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdd);
> +
> + writel(WDT_CMD_CLEAR, priv->base + WDT_CMD);
> +
> + return 0;
> +}
> +
> +static unsigned int visconti_wdt_get_timeleft(struct watchdog_device *wdev)
> +{
> + struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> + u32 timeout = wdev->timeout * VISCONTI_WDT_FREQ;
> + u32 cnt = readl(priv->base + WDT_CNT);
> +
> + if (timeout <= cnt)
> + return 0;
> + timeout -= cnt;
> +
> + return timeout / VISCONTI_WDT_FREQ;
> +}
> +
> +static int visconti_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
> +{
> + u32 val;
> + struct visconti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +
> + wdev->timeout = timeout;
> + val = wdev->timeout * VISCONTI_WDT_FREQ;
> +
> + /* Clear counter before setting timeout because WDT expires */
> + writel(WDT_CMD_CLEAR, priv->base + WDT_CMD);
> + writel(val, priv->base + WDT_MAX);
> +
> + return 0;
> +}
> +
> +static const struct watchdog_info visconti_wdt_info = {
> + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
> + .identity = "Visconti Watchdog",
> +};
> +
> +static const struct watchdog_ops visconti_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = visconti_wdt_start,
> + .stop = visconti_wdt_stop,
> + .ping = visconti_wdt_ping,
> + .get_timeleft = visconti_wdt_get_timeleft,
> + .set_timeout = visconti_wdt_set_timeout,
> +};
> +
> +static int visconti_wdt_probe(struct platform_device *pdev)
> +{
> + struct watchdog_device *wdev;
> + struct visconti_wdt_priv *priv;
> + struct device *dev = &pdev->dev;
> + struct clk *clk;
> + int ret;
> + unsigned long clk_freq;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk), "Could not get clock\n");
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(dev, "Could not enable clock\n");
> + return ret;
> + }
> +
> + clk_freq = clk_get_rate(clk);
> + if (!clk_freq) {
> + clk_disable_unprepare(clk);
> + dev_err(dev, "Could not get clock rate\n");
> + return -EINVAL;
> + }
> +
> + priv->div = clk_freq / VISCONTI_WDT_FREQ;
> +
> + /* Initialize struct watchdog_device. */
> + wdev = &priv->wdev;
> + wdev->info = &visconti_wdt_info;
> + wdev->ops = &visconti_wdt_ops;
> + wdev->parent = dev;
> + wdev->min_timeout = 1;
> + wdev->max_timeout = 0xffffffff / VISCONTI_WDT_FREQ;
> + wdev->timeout = min(wdev->max_timeout, WDT_DEFAULT_TIMEOUT);
> +
> + watchdog_set_drvdata(wdev, priv);
> + watchdog_set_nowayout(wdev, nowayout);
> + watchdog_stop_on_unregister(wdev);
> +
> + /* This overrides the default timeout only if DT configuration was found */
> + ret = watchdog_init_timeout(wdev, 0, dev);
> + if (ret)
> + dev_warn(dev, "Specified timeout value invalid, using default\n");
> +
> + return devm_watchdog_register_device(dev, wdev);
> +}
> +
> +static const struct of_device_id visconti_wdt_of_match[] = {
> + { .compatible = "toshiba,visconti-wdt", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, visconti_wdt_of_match);
> +
> +static struct platform_driver visconti_wdt_driver = {
> + .driver = {
> + .name = "visconti_wdt",
> + .of_match_table = visconti_wdt_of_match,
> + },
> + .probe = visconti_wdt_probe,
> +};
> +module_platform_driver(visconti_wdt_driver);
> +
> +MODULE_DESCRIPTION("TOSHIBA Visconti Watchdog Driver");
> +MODULE_AUTHOR("TOSHIBA ELECTRONIC DEVICES & STORAGE CORPORATION");
The MODULE_AUTHOR() macro is usually used to represent the driver
authors. Not sure about using it for the organization name which is
already included in the copyright notice. Please drop it or replace it
with the author name / email.
> +MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp");
> +MODULE_LICENSE("GPL v2");
Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
Thanks,
Punit
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device
2020-09-20 5:18 ` [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device Nobuhiro Iwamatsu
@ 2020-09-21 7:27 ` Punit Agrawal
2020-09-21 8:43 ` Nobuhiro Iwamatsu
0 siblings, 1 reply; 7+ messages in thread
From: Punit Agrawal @ 2020-09-21 7:27 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Wim Van Sebroeck, Guenter Roeck, devicetree,
linux-watchdog, yuji2.ishikawa, linux-arm-kernel
Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> writes:
> Add documentation for the binding of Toshiba Visconti SoC's watchdog.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
> .../watchdog/toshiba,visconti-wdt.yaml | 49 +++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
>
> diff --git a/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
> new file mode 100644
> index 000000000000..721e38fa5a0f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Toshiba Electronic Devices & Storage Corporation
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/watchdog/toshiba,visconti-wdt.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Toshiba Visconti SoCs PIUWDT Watchdog timer
> +
> +maintainers:
> + - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> +
> +allOf:
> + - $ref: watchdog.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - toshiba,visconti-wdt
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> +
> +examples:
> + - |
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + wdt_clk: wdt-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <150000000>;
> + #clock-cells = <0>;
> + };
> +
> + watchdog@28330000 {
> + compatible = "toshiba,visconti-wdt";
> + reg = <0 0x28330000 0 0x1000>;
> + clocks = <&wdt_clk>;
> + };
I was going to suggest adding the "timeout-sec" property as described in
watchdog.yaml but both code and usage seems to suggest it is optional.
> + };
So,
Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device
2020-09-21 7:27 ` Punit Agrawal
@ 2020-09-21 8:43 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 7+ messages in thread
From: Nobuhiro Iwamatsu @ 2020-09-21 8:43 UTC (permalink / raw)
To: Punit Agrawal
Cc: Rob Herring, Wim Van Sebroeck, Guenter Roeck, devicetree,
linux-watchdog, yuji2.ishikawa, linux-arm-kernel
Hi,
Thanks for you review.
On Mon, Sep 21, 2020 at 04:27:25PM +0900, Punit Agrawal wrote:
> Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> writes:
>
> > Add documentation for the binding of Toshiba Visconti SoC's watchdog.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > ---
> > .../watchdog/toshiba,visconti-wdt.yaml | 49 +++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
> > new file mode 100644
> > index 000000000000..721e38fa5a0f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
> > @@ -0,0 +1,49 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2020 Toshiba Electronic Devices & Storage Corporation
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/watchdog/toshiba,visconti-wdt.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Toshiba Visconti SoCs PIUWDT Watchdog timer
> > +
> > +maintainers:
> > + - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > +
> > +allOf:
> > + - $ref: watchdog.yaml#
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - toshiba,visconti-wdt
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > +
> > +examples:
> > + - |
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + wdt_clk: wdt-clk {
> > + compatible = "fixed-clock";
> > + clock-frequency = <150000000>;
> > + #clock-cells = <0>;
> > + };
> > +
> > + watchdog@28330000 {
> > + compatible = "toshiba,visconti-wdt";
> > + reg = <0 0x28330000 0 0x1000>;
> > + clocks = <&wdt_clk>;
> > + };
>
> I was going to suggest adding the "timeout-sec" property as described in
> watchdog.yaml but both code and usage seems to suggest it is optional.
>
I see. I will add about timeout-sec property.
> > + };
>
> So,
>
> Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>
Thanks!
> Thanks.
>
Best regards,
Nobuhiro
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver
2020-09-21 7:25 ` Punit Agrawal
@ 2020-09-21 8:46 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 7+ messages in thread
From: Nobuhiro Iwamatsu @ 2020-09-21 8:46 UTC (permalink / raw)
To: Punit Agrawal
Cc: Rob Herring, Wim Van Sebroeck, Guenter Roeck, devicetree,
linux-watchdog, yuji2.ishikawa, linux-arm-kernel
Hi,
Thanks for your review.
On Mon, Sep 21, 2020 at 04:25:44PM +0900, Punit Agrawal wrote:
> Iwamatsu-san,
>
> Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> writes:
>
> > Add the watchdog driver for Toshiba Visconti series.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > ---
> > drivers/watchdog/Kconfig | 8 ++
> > drivers/watchdog/Makefile | 1 +
> > drivers/watchdog/visconti_wdt.c | 191 ++++++++++++++++++++++++++++++++
> > 3 files changed, 200 insertions(+)
> > create mode 100644 drivers/watchdog/visconti_wdt.c
> >
>
> [...]
>
> > diff --git a/drivers/watchdog/visconti_wdt.c b/drivers/watchdog/visconti_wdt.c
> > new file mode 100644
> > index 000000000000..4a67b9fe9220
> > --- /dev/null
> > +++ b/drivers/watchdog/visconti_wdt.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 TOSHIBA CORPORATION
> > + * Copyright (c) 2020 Toshiba Electronic Devices & Storage Corporation
> > + * Copyright (c) 2020 Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
>
> interrupt.h doesn't seem to be used. It can be dropped.
>
Right. I will remove this.
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define WDT_CNT 0x00
> > +#define WDT_MIN 0x04
> > +#define WDT_MAX 0x08
> > +#define WDT_CTL 0x0c
> > +#define WDT_CMD 0x10
> > +#define WDT_CMD_CLEAR 0x4352
> > +#define WDT_CMD_START_STOP 0x5354
> > +#define WDT_DIV 0x30
> > +
> > +#define VISCONTI_WDT_FREQ 2000000 /* 2MHz */
> > +#define WDT_DEFAULT_TIMEOUT 10U /* in seconds */
> > +
> > +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)")");
<snip>
> > +module_platform_driver(visconti_wdt_driver);
> > +
> > +MODULE_DESCRIPTION("TOSHIBA Visconti Watchdog Driver");
> > +MODULE_AUTHOR("TOSHIBA ELECTRONIC DEVICES & STORAGE CORPORATION");
>
> The MODULE_AUTHOR() macro is usually used to represent the driver
> authors. Not sure about using it for the organization name which is
> already included in the copyright notice. Please drop it or replace it
> with the author name / email.
Well, it's unnecessary for MODULE_AUTHOR.
I will drop this.
>
> > +MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp");
> > +MODULE_LICENSE("GPL v2");
>
> Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>
Thanks!
> Thanks,
> Punit
>
Best regards,
Nobuhiro
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-21 8:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-20 5:18 [PATCH v3 0/2] Add WDT driver for Toshiba Visconti ARM SoC Nobuhiro Iwamatsu
2020-09-20 5:18 ` [PATCH v3 1/2] watchdog: bindings: Add binding documentation for Toshiba Visconti watchdog device Nobuhiro Iwamatsu
2020-09-21 7:27 ` Punit Agrawal
2020-09-21 8:43 ` Nobuhiro Iwamatsu
2020-09-20 5:18 ` [PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver Nobuhiro Iwamatsu
2020-09-21 7:25 ` Punit Agrawal
2020-09-21 8:46 ` Nobuhiro Iwamatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).