All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver
@ 2023-03-24 14:56 Bharat Bhushan
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bharat Bhushan @ 2023-03-24 14:56 UTC (permalink / raw)
  To: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog,
	devicetree, linux-kernel
  Cc: Bharat Bhushan

Add binding documentation for the Marvell octeonTX2
GTI watchdog driver.

Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
---
 .../watchdog/marvel-octeontx2-wdt.yaml        | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
new file mode 100644
index 000000000000..586b3c1bd780
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/marvel-octeontx2-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell OcteonTX2 GTI watchdog
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+maintainers:
+  - Bharat Bhushan <bbhushan2@marvell.com>
+
+properties:
+  compatible:
+    enum:
+      - mrvl,octeontx2-gti-wdt
+
+  reg:
+    maxItems: 2
+
+  interrupts:
+    minItems: 1
+    maxItems: 36
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    watch-dog@802000040000 {
+      compatible = "mrvl,octeontx2-gti-wdt";
+      reg = <0x8020 0x40000 0x0 0x20000>;
+      interrupts = <0 38 1>, /* Core-0 */
+                   <0 39 1>; /* Core-1 */
+    };
+
+...
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 14:56 [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver Bharat Bhushan
@ 2023-03-24 14:56 ` Bharat Bhushan
  2023-03-24 21:04   ` Thomas Weißschuh
                     ` (2 more replies)
  2023-03-24 20:17 ` [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 " Rob Herring
  2023-03-24 20:34 ` Rob Herring
  2 siblings, 3 replies; 10+ messages in thread
From: Bharat Bhushan @ 2023-03-24 14:56 UTC (permalink / raw)
  To: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog,
	devicetree, linux-kernel
  Cc: Bharat Bhushan

GTI hardware supports per-core watchdog timer which are programmed
in "interrupt + del3t + reset mode" and del3t traps are not enabled.
This driver uses ARM64 pseudo-nmi interrupt support.
GTI watchdog exception flow is:
 - 1st timer expiration generates pseudo-nmi interrupt.
   NMI exception handler dumps register/context state on all cpu's.
 - 2nd timer expiration is ignored

 - On 3rd timer expiration will trigger a system-wide core reset.

Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
---
 drivers/watchdog/Kconfig                  |   9 +
 drivers/watchdog/Makefile                 |   1 +
 drivers/watchdog/octeontx2_gti_watchdog.c | 352 ++++++++++++++++++++++
 3 files changed, 362 insertions(+)
 create mode 100644 drivers/watchdog/octeontx2_gti_watchdog.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f0872970daf9..9607d36645f6 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2212,4 +2212,13 @@ config KEEMBAY_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called keembay_wdt.
 
+config OCTEON_GTI_WATCHDOG
+	tristate "OCTEONTX2 GTI Watchdog driver"
+	depends on ARM64
+	help
+	 OCTEONTX2 GTI hardware supports per-core watchdog timer which
+	 are programmed in "interrupt + del3t + reset mode" and del3t
+	 traps are not enabled.
+	 This driver uses ARM64 pseudo-nmi interrupt support.
+
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 9cbf6580f16c..11af3db62fec 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -230,3 +230,4 @@ obj-$(CONFIG_MENZ069_WATCHDOG) += menz69_wdt.o
 obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
 obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
 obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
+obj-$(CONFIG_OCTEON_GTI_WATCHDOG) += octeontx2_gti_watchdog.o
diff --git a/drivers/watchdog/octeontx2_gti_watchdog.c b/drivers/watchdog/octeontx2_gti_watchdog.c
new file mode 100644
index 000000000000..766b7d41defe
--- /dev/null
+++ b/drivers/watchdog/octeontx2_gti_watchdog.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell GTI Watchdog driver
+ *
+ * Copyright (C) 2023 Marvell International Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/sched/debug.h>
+
+#include <asm/arch_timer.h>
+
+/* GTI CWD Watchdog Registers */
+#define GTI_CWD_WDOG(cpu)		(0x8 * cpu)
+#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST	(0x3)
+#define GTI_CWD_WDOG_MODE_MASK		(0x3)
+#define GTI_CWD_WDOG_LEN_SHIFT		(4)
+#define GTI_CWD_WDOG_CNT_SHIFT		(20)
+
+/* GTI Per-core Watchdog Interrupt Register */
+#define GTI_CWD_INT			0x200
+
+/* GTI Per-core Watchdog Interrupt Enable Clear Register */
+#define GTI_CWD_INT_ENA_CLR		0x210
+
+/* GTI Per-core Watchdog Interrupt Enable Set Register */
+#define GTI_CWD_INT_ENA_SET		0x218
+
+/* GTI Per-core Watchdog Poke Registers */
+#define GTI_CWD_POKE(cpu)		(0x10000 + 0x8 * cpu)
+
+struct octeontx2_gti_wdt_percpu_priv {
+	struct watchdog_device wdev;
+	int irq;
+};
+
+struct octeontx2_gti_wdt_priv {
+	void __iomem *base;
+	u64 clock_freq;
+	int is_nmi;
+	struct octeontx2_gti_wdt_percpu_priv __percpu *percpu_priv;
+};
+
+static int octeontx2_gti_wdt_get_cpuid(struct watchdog_device *wdev)
+{
+	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
+		if (&percpu_priv->wdev == wdev)
+			return cpu;
+	}
+
+	return -1;
+}
+
+void octeontx2_gti_wdt_callback_other_cpus(void *unused)
+{
+	struct pt_regs *regs = get_irq_regs();
+
+	pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
+
+	if (regs)
+		show_regs(regs);
+	else
+		dump_stack();
+}
+
+static irqreturn_t octeontx2_gti_wdt_interrupt(int irq, void *data)
+{
+	struct octeontx2_gti_wdt_priv *priv = (struct octeontx2_gti_wdt_priv *)data;
+	int cpu = smp_processor_id();
+
+	/* Clear interrupt to fire again if delayed poke happens */
+	writeq(1 << cpu, priv->base + GTI_CWD_INT);
+	dump_stack();
+
+	for_each_online_cpu(cpu) {
+		if (cpu == raw_smp_processor_id())
+			continue;
+
+		smp_call_function_single(cpu,
+					 octeontx2_gti_wdt_callback_other_cpus,
+					 NULL, 1);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int octeontx2_gti_wdt_ping(struct watchdog_device *wdev)
+{
+	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
+
+	if (cpu < 0)
+		return -EINVAL;
+
+	writeq(1, priv->base + GTI_CWD_POKE(cpu));
+	return 0;
+}
+
+static int octeontx2_gti_wdt_start(struct watchdog_device *wdev)
+{
+	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
+	u64 regval;
+
+	if (cpu < 0)
+		return -EINVAL;
+
+	set_bit(WDOG_HW_RUNNING, &wdev->status);
+
+	/* Clear any pending interrupt */
+	writeq(1 << cpu, priv->base + GTI_CWD_INT);
+
+	/* Enable Interrupt */
+	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_SET);
+
+	/* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */
+	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
+	regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST;
+	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
+
+	return 0;
+}
+
+static int octeontx2_gti_wdt_stop(struct watchdog_device *wdev)
+{
+	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	u64 regval;
+	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
+
+	if (cpu < 0)
+		return -EINVAL;
+
+	/* Disable Interrupt */
+	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_CLR);
+
+	/* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */
+	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
+	regval &= ~GTI_CWD_WDOG_MODE_MASK;
+	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
+
+	return 0;
+}
+
+static int octeontx2_gti_wdt_settimeout(struct watchdog_device *wdev,
+					unsigned int timeout)
+{
+	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
+	u64 timeout_wdog, regval;
+
+	if (cpu < 0)
+		return -EINVAL;
+
+	/* Update new timeout */
+	wdev->timeout = timeout;
+
+	/* Get clock cycles from timeout in second */
+	timeout_wdog = (u64)timeout * priv->clock_freq;
+
+	/* Watchdog counts in 1024 cycle steps */
+	timeout_wdog = timeout_wdog >> 10;
+
+	/*
+	 * Hardware allows programming of upper 16-bits of 24-bits cycles
+	 * Round up and use upper 16-bits only.
+	 * Set max if timeout more than h/w supported
+	 */
+	timeout_wdog = (timeout_wdog + 0xff) >> 8;
+	if (timeout_wdog >= 0x10000)
+		timeout_wdog = 0xffff;
+
+	/*
+	 * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits
+	 * GTI_CWD_WDOG.CNT, need addition shift of 8.
+	 */
+	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
+	regval &= GTI_CWD_WDOG_MODE_MASK;
+	regval |= ((timeout_wdog) << (GTI_CWD_WDOG_CNT_SHIFT + 8)) |
+		   (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT);
+	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
+	return 0;
+}
+
+static const struct watchdog_info octeontx2_gti_wdt_ident = {
+	.identity = "OcteonTX2 GTI watchdog",
+	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
+			  WDIOF_CARDRESET,
+};
+
+static const struct watchdog_ops octeontx2_gti_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = octeontx2_gti_wdt_start,
+	.stop = octeontx2_gti_wdt_stop,
+	.ping = octeontx2_gti_wdt_ping,
+	.set_timeout = octeontx2_gti_wdt_settimeout,
+};
+
+static void octeontx2_gti_wdt_free_irqs(struct octeontx2_gti_wdt_priv *priv)
+{
+	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
+	int irq, cpu = 0;
+
+	for_each_online_cpu(cpu) {
+		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
+		irq = percpu_priv->irq;
+		if (irq) {
+			if (priv->is_nmi) {
+				disable_nmi_nosync(irq);
+				free_nmi(irq, priv);
+			} else {
+				disable_irq_nosync(irq);
+				free_irq(irq, priv);
+			}
+
+			percpu_priv->irq = 0;
+		}
+	}
+}
+
+static int octeontx2_gti_wdt_probe(struct platform_device *pdev)
+{
+	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
+	struct octeontx2_gti_wdt_priv *priv;
+	struct device *dev = &pdev->dev;
+	struct watchdog_device *wdog_dev;
+	unsigned long irq_flags;
+	int irq, cpu, num_irqs;
+	int err;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->percpu_priv = devm_alloc_percpu(&pdev->dev, *priv->percpu_priv);
+	if (!priv->percpu_priv)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
+			      "reg property not valid/found\n");
+
+	num_irqs = platform_irq_count(pdev);
+	if (num_irqs < 0)
+		return dev_err_probe(dev, num_irqs, "GTI CWD no IRQs\n");
+
+	if (num_irqs < num_online_cpus())
+		return dev_err_probe(dev, -EINVAL, "IRQs (%d) < CPUs (%d)\n",
+				     num_irqs, num_online_cpus());
+
+	priv->clock_freq = arch_timer_get_cntfrq();
+
+	for_each_online_cpu(cpu) {
+		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
+		wdog_dev = &percpu_priv->wdev;
+		wdog_dev->info = &octeontx2_gti_wdt_ident,
+		wdog_dev->ops = &octeontx2_gti_wdt_ops,
+		wdog_dev->parent = dev;
+		wdog_dev->min_timeout = 1;
+		wdog_dev->max_timeout = 16;
+		wdog_dev->max_hw_heartbeat_ms = 16000;
+		wdog_dev->timeout = 8;
+
+		irq = platform_get_irq(pdev, cpu);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "IRQ resource not found\n");
+			err = -ENODEV;
+			goto out;
+		}
+
+		err = irq_force_affinity(irq, cpumask_of(cpu));
+		if (err) {
+			pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n", irq, cpu);
+			goto out;
+		}
+
+		irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
+			    IRQF_NO_THREAD;
+		err = request_nmi(irq, octeontx2_gti_wdt_interrupt, irq_flags,
+				  pdev->name, priv);
+		if (err) {
+			err = request_irq(irq, octeontx2_gti_wdt_interrupt, irq_flags,
+					  pdev->name, priv);
+			if (err) {
+				dev_err(dev, "cannot register interrupt handler %d\n", err);
+				goto out;
+			}
+			enable_irq(irq);
+		} else {
+			priv->is_nmi = 1;
+			enable_nmi(irq);
+		}
+
+		percpu_priv->irq = irq;
+		watchdog_set_drvdata(wdog_dev, priv);
+		platform_set_drvdata(pdev, priv);
+		watchdog_init_timeout(wdog_dev, wdog_dev->timeout, dev);
+		octeontx2_gti_wdt_settimeout(wdog_dev, wdog_dev->timeout);
+		watchdog_stop_on_reboot(wdog_dev);
+		watchdog_stop_on_unregister(wdog_dev);
+
+		err = devm_watchdog_register_device(dev, wdog_dev);
+		if (unlikely(err))
+			goto out;
+		dev_info(dev, "Watchdog enabled (timeout=%d sec)", wdog_dev->timeout);
+	}
+	return 0;
+
+out:
+	octeontx2_gti_wdt_free_irqs(priv);
+	return err;
+}
+
+static int octeontx2_gti_wdt_remove(struct platform_device *pdev)
+{
+	struct octeontx2_gti_wdt_priv *priv = platform_get_drvdata(pdev);
+
+	octeontx2_gti_wdt_free_irqs(priv);
+	return 0;
+}
+
+static const struct of_device_id octeontx2_gti_wdt_of_match[] = {
+	{ .compatible = "mrvl,octeontx2-gti-wdt", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, octeontx2_gti_wdt_of_match);
+
+static struct platform_driver octeontx2_gti_wdt_driver = {
+	.driver = {
+		.name = "octeontx2-gti-wdt",
+		.of_match_table = octeontx2_gti_wdt_of_match,
+	},
+	.probe = octeontx2_gti_wdt_probe,
+	.remove = octeontx2_gti_wdt_remove,
+};
+module_platform_driver(octeontx2_gti_wdt_driver);
+
+MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>");
+MODULE_DESCRIPTION("OcteonTX2 GTI per cpu watchdog driver");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver
  2023-03-24 14:56 [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver Bharat Bhushan
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
@ 2023-03-24 20:17 ` Rob Herring
  2023-03-24 20:34 ` Rob Herring
  2 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2023-03-24 20:17 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: linux-kernel, robh+dt, linux, krzysztof.kozlowski+dt, devicetree,
	linux-watchdog, wim


On Fri, 24 Mar 2023 20:26:51 +0530, Bharat Bhushan wrote:
> Add binding documentation for the Marvell octeonTX2
> GTI watchdog driver.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> ---
>  .../watchdog/marvel-octeontx2-wdt.yaml        | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml:28:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.example.dtb: watch-dog@802000040000: $nodename:0: 'watch-dog@802000040000' does not match '^(timer|watchdog)(@.*|-[0-9a-f])?$'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230324145652.19221-1-bbhushan2@marvell.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver
  2023-03-24 14:56 [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver Bharat Bhushan
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
  2023-03-24 20:17 ` [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 " Rob Herring
@ 2023-03-24 20:34 ` Rob Herring
  2023-04-13 10:14   ` [EXT] " Bharat Bhushan
  2 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2023-03-24 20:34 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: wim, linux, krzysztof.kozlowski+dt, linux-watchdog, devicetree,
	linux-kernel

On Fri, Mar 24, 2023 at 08:26:51PM +0530, Bharat Bhushan wrote:
> Add binding documentation for the Marvell octeonTX2
> GTI watchdog driver.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> ---
>  .../watchdog/marvel-octeontx2-wdt.yaml        | 43 +++++++++++++++++++

The comics?

Use compatible string for filename.

>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> new file mode 100644
> index 000000000000..586b3c1bd780
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/marvel-octeontx2-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell OcteonTX2 GTI watchdog
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"

Drop quotes

> +
> +maintainers:
> +  - Bharat Bhushan <bbhushan2@marvell.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mrvl,octeontx2-gti-wdt

'mrvl' is deprecated. Use 'marvell'

> +
> +  reg:
> +    maxItems: 2

Need to define what each entry is.

> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 36

Need to define what each entry is. How does the h/w have a variable 
number of interrupts?

> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    watch-dog@802000040000 {

watchdog@...

> +      compatible = "mrvl,octeontx2-gti-wdt";
> +      reg = <0x8020 0x40000 0x0 0x20000>;
> +      interrupts = <0 38 1>, /* Core-0 */
> +                   <0 39 1>; /* Core-1 */
> +    };
> +
> +...
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
@ 2023-03-24 21:04   ` Thomas Weißschuh
  2023-04-13 10:20     ` [EXT] " Bharat Bhushan
  2023-03-24 22:03   ` Guenter Roeck
  2023-03-26  8:40   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Thomas Weißschuh @ 2023-03-24 21:04 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog,
	devicetree, linux-kernel

On 2023-03-24 20:26:52+0530, Bharat Bhushan wrote:
> GTI hardware supports per-core watchdog timer which are programmed
> in "interrupt + del3t + reset mode" and del3t traps are not enabled.
> This driver uses ARM64 pseudo-nmi interrupt support.
> GTI watchdog exception flow is:
>  - 1st timer expiration generates pseudo-nmi interrupt.
>    NMI exception handler dumps register/context state on all cpu's.
>  - 2nd timer expiration is ignored
> 
>  - On 3rd timer expiration will trigger a system-wide core reset.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> ---
>  drivers/watchdog/Kconfig                  |   9 +
>  drivers/watchdog/Makefile                 |   1 +
>  drivers/watchdog/octeontx2_gti_watchdog.c | 352 ++++++++++++++++++++++
>  3 files changed, 362 insertions(+)
>  create mode 100644 drivers/watchdog/octeontx2_gti_watchdog.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index f0872970daf9..9607d36645f6 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -2212,4 +2212,13 @@ config KEEMBAY_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called keembay_wdt.
>  
> +config OCTEON_GTI_WATCHDOG
> +	tristate "OCTEONTX2 GTI Watchdog driver"
> +	depends on ARM64
> +	help
> +	 OCTEONTX2 GTI hardware supports per-core watchdog timer which
> +	 are programmed in "interrupt + del3t + reset mode" and del3t
> +	 traps are not enabled.
> +	 This driver uses ARM64 pseudo-nmi interrupt support.
> +
>  endif # WATCHDOG
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9cbf6580f16c..11af3db62fec 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -230,3 +230,4 @@ obj-$(CONFIG_MENZ069_WATCHDOG) += menz69_wdt.o
>  obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
>  obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
>  obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
> +obj-$(CONFIG_OCTEON_GTI_WATCHDOG) += octeontx2_gti_watchdog.o
> diff --git a/drivers/watchdog/octeontx2_gti_watchdog.c b/drivers/watchdog/octeontx2_gti_watchdog.c
> new file mode 100644
> index 000000000000..766b7d41defe
> --- /dev/null
> +++ b/drivers/watchdog/octeontx2_gti_watchdog.c
> @@ -0,0 +1,352 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Marvell GTI Watchdog driver
> + *
> + * Copyright (C) 2023 Marvell International Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/sched/debug.h>
> +
> +#include <asm/arch_timer.h>
> +
> +/* GTI CWD Watchdog Registers */
> +#define GTI_CWD_WDOG(cpu)		(0x8 * cpu)
> +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST	(0x3)
> +#define GTI_CWD_WDOG_MODE_MASK		(0x3)
> +#define GTI_CWD_WDOG_LEN_SHIFT		(4)
> +#define GTI_CWD_WDOG_CNT_SHIFT		(20)
> +
> +/* GTI Per-core Watchdog Interrupt Register */
> +#define GTI_CWD_INT			0x200
> +
> +/* GTI Per-core Watchdog Interrupt Enable Clear Register */
> +#define GTI_CWD_INT_ENA_CLR		0x210
> +
> +/* GTI Per-core Watchdog Interrupt Enable Set Register */
> +#define GTI_CWD_INT_ENA_SET		0x218
> +
> +/* GTI Per-core Watchdog Poke Registers */
> +#define GTI_CWD_POKE(cpu)		(0x10000 + 0x8 * cpu)
> +
> +struct octeontx2_gti_wdt_percpu_priv {
> +	struct watchdog_device wdev;
> +	int irq;
> +};
> +
> +struct octeontx2_gti_wdt_priv {
> +	void __iomem *base;
> +	u64 clock_freq;
> +	int is_nmi;
> +	struct octeontx2_gti_wdt_percpu_priv __percpu *percpu_priv;
> +};
> +
> +static int octeontx2_gti_wdt_get_cpuid(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		if (&percpu_priv->wdev == wdev)
> +			return cpu;
> +	}
> +
> +	return -1;
> +}
> +
> +void octeontx2_gti_wdt_callback_other_cpus(void *unused)

Could be static.

> +{
> +	struct pt_regs *regs = get_irq_regs();
> +
> +	pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
> +
> +	if (regs)
> +		show_regs(regs);
> +	else
> +		dump_stack();
> +}
> +
> +static irqreturn_t octeontx2_gti_wdt_interrupt(int irq, void *data)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = (struct octeontx2_gti_wdt_priv *)data;
> +	int cpu = smp_processor_id();
> +
> +	/* Clear interrupt to fire again if delayed poke happens */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> +	dump_stack();
> +
> +	for_each_online_cpu(cpu) {
> +		if (cpu == raw_smp_processor_id())
> +			continue;
> +
> +		smp_call_function_single(cpu,
> +					 octeontx2_gti_wdt_callback_other_cpus,
> +					 NULL, 1);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int octeontx2_gti_wdt_ping(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	writeq(1, priv->base + GTI_CWD_POKE(cpu));
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_start(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +	u64 regval;
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	set_bit(WDOG_HW_RUNNING, &wdev->status);
> +
> +	/* Clear any pending interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> +
> +	/* Enable Interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_SET);
> +
> +	/* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST;
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_stop(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	u64 regval;
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	/* Disable Interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_CLR);
> +
> +	/* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval &= ~GTI_CWD_WDOG_MODE_MASK;
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_settimeout(struct watchdog_device *wdev,
> +					unsigned int timeout)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +	u64 timeout_wdog, regval;
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	/* Update new timeout */
> +	wdev->timeout = timeout;
> +
> +	/* Get clock cycles from timeout in second */
> +	timeout_wdog = (u64)timeout * priv->clock_freq;
> +
> +	/* Watchdog counts in 1024 cycle steps */
> +	timeout_wdog = timeout_wdog >> 10;
> +
> +	/*
> +	 * Hardware allows programming of upper 16-bits of 24-bits cycles
> +	 * Round up and use upper 16-bits only.
> +	 * Set max if timeout more than h/w supported
> +	 */
> +	timeout_wdog = (timeout_wdog + 0xff) >> 8;
> +	if (timeout_wdog >= 0x10000)
> +		timeout_wdog = 0xffff;
> +
> +	/*
> +	 * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits
> +	 * GTI_CWD_WDOG.CNT, need addition shift of 8.
> +	 */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval &= GTI_CWD_WDOG_MODE_MASK;
> +	regval |= ((timeout_wdog) << (GTI_CWD_WDOG_CNT_SHIFT + 8)) |
> +		   (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT);
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +	return 0;
> +}
> +
> +static const struct watchdog_info octeontx2_gti_wdt_ident = {
> +	.identity = "OcteonTX2 GTI watchdog",
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> +			  WDIOF_CARDRESET,

Weird alignment.

> +};
> +
> +static const struct watchdog_ops octeontx2_gti_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = octeontx2_gti_wdt_start,
> +	.stop = octeontx2_gti_wdt_stop,
> +	.ping = octeontx2_gti_wdt_ping,
> +	.set_timeout = octeontx2_gti_wdt_settimeout,
> +};
> +
> +static void octeontx2_gti_wdt_free_irqs(struct octeontx2_gti_wdt_priv *priv)
> +{
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	int irq, cpu = 0;
> +
> +	for_each_online_cpu(cpu) {
> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		irq = percpu_priv->irq;
> +		if (irq) {
> +			if (priv->is_nmi) {
> +				disable_nmi_nosync(irq);
> +				free_nmi(irq, priv);
> +			} else {
> +				disable_irq_nosync(irq);
> +				free_irq(irq, priv);
> +			}
> +
> +			percpu_priv->irq = 0;
> +		}
> +	}
> +}
> +
> +static int octeontx2_gti_wdt_probe(struct platform_device *pdev)
> +{
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	struct octeontx2_gti_wdt_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	struct watchdog_device *wdog_dev;
> +	unsigned long irq_flags;
> +	int irq, cpu, num_irqs;
> +	int err;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->percpu_priv = devm_alloc_percpu(&pdev->dev, *priv->percpu_priv);
> +	if (!priv->percpu_priv)
> +		return -ENOMEM;
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
> +			      "reg property not valid/found\n");
> +
> +	num_irqs = platform_irq_count(pdev);
> +	if (num_irqs < 0)
> +		return dev_err_probe(dev, num_irqs, "GTI CWD no IRQs\n");
> +
> +	if (num_irqs < num_online_cpus())
> +		return dev_err_probe(dev, -EINVAL, "IRQs (%d) < CPUs (%d)\n",
> +				     num_irqs, num_online_cpus());
> +
> +	priv->clock_freq = arch_timer_get_cntfrq();
> +
> +	for_each_online_cpu(cpu) {
> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		wdog_dev = &percpu_priv->wdev;
> +		wdog_dev->info = &octeontx2_gti_wdt_ident,
> +		wdog_dev->ops = &octeontx2_gti_wdt_ops,
> +		wdog_dev->parent = dev;
> +		wdog_dev->min_timeout = 1;
> +		wdog_dev->max_timeout = 16;
> +		wdog_dev->max_hw_heartbeat_ms = 16000;
> +		wdog_dev->timeout = 8;
> +
> +		irq = platform_get_irq(pdev, cpu);
> +		if (irq < 0) {
> +			dev_err(&pdev->dev, "IRQ resource not found\n");
> +			err = -ENODEV;
> +			goto out;
> +		}
> +
> +		err = irq_force_affinity(irq, cpumask_of(cpu));
> +		if (err) {
> +			pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n", irq, cpu);
> +			goto out;
> +		}
> +
> +		irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
> +			    IRQF_NO_THREAD;
> +		err = request_nmi(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> +				  pdev->name, priv);
> +		if (err) {
> +			err = request_irq(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> +					  pdev->name, priv);
> +			if (err) {
> +				dev_err(dev, "cannot register interrupt handler %d\n", err);
> +				goto out;
> +			}
> +			enable_irq(irq);
> +		} else {
> +			priv->is_nmi = 1;
> +			enable_nmi(irq);
> +		}
> +
> +		percpu_priv->irq = irq;
> +		watchdog_set_drvdata(wdog_dev, priv);
> +		platform_set_drvdata(pdev, priv);
> +		watchdog_init_timeout(wdog_dev, wdog_dev->timeout, dev);
> +		octeontx2_gti_wdt_settimeout(wdog_dev, wdog_dev->timeout);
> +		watchdog_stop_on_reboot(wdog_dev);
> +		watchdog_stop_on_unregister(wdog_dev);
> +
> +		err = devm_watchdog_register_device(dev, wdog_dev);
> +		if (unlikely(err))

This unlikely() should really not be needed.

> +			goto out;
> +		dev_info(dev, "Watchdog enabled (timeout=%d sec)", wdog_dev->timeout);
> +	}
> +	return 0;
> +
> +out:
> +	octeontx2_gti_wdt_free_irqs(priv);
> +	return err;
> +}
> +
> +static int octeontx2_gti_wdt_remove(struct platform_device *pdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = platform_get_drvdata(pdev);
> +
> +	octeontx2_gti_wdt_free_irqs(priv);
> +	return 0;
> +}
> +
> +static const struct of_device_id octeontx2_gti_wdt_of_match[] = {
> +	{ .compatible = "mrvl,octeontx2-gti-wdt", },
> +	{ },

No trailing comma for end-of-array marker.

> +};
> +MODULE_DEVICE_TABLE(of, octeontx2_gti_wdt_of_match);
> +
> +static struct platform_driver octeontx2_gti_wdt_driver = {
> +	.driver = {
> +		.name = "octeontx2-gti-wdt",
> +		.of_match_table = octeontx2_gti_wdt_of_match,
> +	},
> +	.probe = octeontx2_gti_wdt_probe,
> +	.remove = octeontx2_gti_wdt_remove,
> +};
> +module_platform_driver(octeontx2_gti_wdt_driver);
> +
> +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>");
> +MODULE_DESCRIPTION("OcteonTX2 GTI per cpu watchdog driver");

No MODULE_LICENSE() ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
  2023-03-24 21:04   ` Thomas Weißschuh
@ 2023-03-24 22:03   ` Guenter Roeck
  2023-04-13 10:22     ` [EXT] " Bharat Bhushan
  2023-03-26  8:40   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2023-03-24 22:03 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: wim, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree,
	linux-kernel

On Fri, Mar 24, 2023 at 08:26:52PM +0530, Bharat Bhushan wrote:
> GTI hardware supports per-core watchdog timer which are programmed
> in "interrupt + del3t + reset mode" and del3t traps are not enabled.
> This driver uses ARM64 pseudo-nmi interrupt support.
> GTI watchdog exception flow is:
>  - 1st timer expiration generates pseudo-nmi interrupt.
>    NMI exception handler dumps register/context state on all cpu's.
>  - 2nd timer expiration is ignored
> 
>  - On 3rd timer expiration will trigger a system-wide core reset.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> ---
>  drivers/watchdog/Kconfig                  |   9 +
>  drivers/watchdog/Makefile                 |   1 +
>  drivers/watchdog/octeontx2_gti_watchdog.c | 352 ++++++++++++++++++++++
>  3 files changed, 362 insertions(+)
>  create mode 100644 drivers/watchdog/octeontx2_gti_watchdog.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index f0872970daf9..9607d36645f6 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -2212,4 +2212,13 @@ config KEEMBAY_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called keembay_wdt.
>  
> +config OCTEON_GTI_WATCHDOG
> +	tristate "OCTEONTX2 GTI Watchdog driver"
> +	depends on ARM64
> +	help
> +	 OCTEONTX2 GTI hardware supports per-core watchdog timer which
> +	 are programmed in "interrupt + del3t + reset mode" and del3t
> +	 traps are not enabled.
> +	 This driver uses ARM64 pseudo-nmi interrupt support.
> +
>  endif # WATCHDOG
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9cbf6580f16c..11af3db62fec 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -230,3 +230,4 @@ obj-$(CONFIG_MENZ069_WATCHDOG) += menz69_wdt.o
>  obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
>  obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
>  obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
> +obj-$(CONFIG_OCTEON_GTI_WATCHDOG) += octeontx2_gti_watchdog.o
> diff --git a/drivers/watchdog/octeontx2_gti_watchdog.c b/drivers/watchdog/octeontx2_gti_watchdog.c
> new file mode 100644
> index 000000000000..766b7d41defe
> --- /dev/null
> +++ b/drivers/watchdog/octeontx2_gti_watchdog.c
> @@ -0,0 +1,352 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Marvell GTI Watchdog driver
> + *
> + * Copyright (C) 2023 Marvell International Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/sched/debug.h>
> +
> +#include <asm/arch_timer.h>
> +
> +/* GTI CWD Watchdog Registers */
> +#define GTI_CWD_WDOG(cpu)		(0x8 * cpu)
> +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST	(0x3)
> +#define GTI_CWD_WDOG_MODE_MASK		(0x3)
> +#define GTI_CWD_WDOG_LEN_SHIFT		(4)
> +#define GTI_CWD_WDOG_CNT_SHIFT		(20)
> +
> +/* GTI Per-core Watchdog Interrupt Register */
> +#define GTI_CWD_INT			0x200
> +
> +/* GTI Per-core Watchdog Interrupt Enable Clear Register */
> +#define GTI_CWD_INT_ENA_CLR		0x210
> +
> +/* GTI Per-core Watchdog Interrupt Enable Set Register */
> +#define GTI_CWD_INT_ENA_SET		0x218
> +
> +/* GTI Per-core Watchdog Poke Registers */
> +#define GTI_CWD_POKE(cpu)		(0x10000 + 0x8 * cpu)
> +
> +struct octeontx2_gti_wdt_percpu_priv {
> +	struct watchdog_device wdev;
> +	int irq;
> +};
> +
> +struct octeontx2_gti_wdt_priv {
> +	void __iomem *base;
> +	u64 clock_freq;
> +	int is_nmi;
> +	struct octeontx2_gti_wdt_percpu_priv __percpu *percpu_priv;
> +};
> +
> +static int octeontx2_gti_wdt_get_cpuid(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		if (&percpu_priv->wdev == wdev)
> +			return cpu;
> +	}
> +
> +	return -1;
> +}
> +
> +void octeontx2_gti_wdt_callback_other_cpus(void *unused)
> +{
> +	struct pt_regs *regs = get_irq_regs();
> +
> +	pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
> +
> +	if (regs)
> +		show_regs(regs);
> +	else
> +		dump_stack();
> +}
> +
> +static irqreturn_t octeontx2_gti_wdt_interrupt(int irq, void *data)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = (struct octeontx2_gti_wdt_priv *)data;
> +	int cpu = smp_processor_id();
> +
> +	/* Clear interrupt to fire again if delayed poke happens */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> +	dump_stack();
> +
> +	for_each_online_cpu(cpu) {
> +		if (cpu == raw_smp_processor_id())
> +			continue;
> +
> +		smp_call_function_single(cpu,
> +					 octeontx2_gti_wdt_callback_other_cpus,
> +					 NULL, 1);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int octeontx2_gti_wdt_ping(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	writeq(1, priv->base + GTI_CWD_POKE(cpu));
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_start(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +	u64 regval;
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	set_bit(WDOG_HW_RUNNING, &wdev->status);
> +
> +	/* Clear any pending interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> +
> +	/* Enable Interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_SET);
> +
> +	/* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST;
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_stop(struct watchdog_device *wdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	u64 regval;
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	/* Disable Interrupt */
> +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_CLR);
> +
> +	/* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval &= ~GTI_CWD_WDOG_MODE_MASK;
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +
> +	return 0;
> +}
> +
> +static int octeontx2_gti_wdt_settimeout(struct watchdog_device *wdev,
> +					unsigned int timeout)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> +	u64 timeout_wdog, regval;
> +
> +	if (cpu < 0)
> +		return -EINVAL;
> +
> +	/* Update new timeout */
> +	wdev->timeout = timeout;
> +
> +	/* Get clock cycles from timeout in second */
> +	timeout_wdog = (u64)timeout * priv->clock_freq;
> +
> +	/* Watchdog counts in 1024 cycle steps */
> +	timeout_wdog = timeout_wdog >> 10;
> +
> +	/*
> +	 * Hardware allows programming of upper 16-bits of 24-bits cycles
> +	 * Round up and use upper 16-bits only.
> +	 * Set max if timeout more than h/w supported
> +	 */
> +	timeout_wdog = (timeout_wdog + 0xff) >> 8;
> +	if (timeout_wdog >= 0x10000)
> +		timeout_wdog = 0xffff;
> +
> +	/*
> +	 * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits
> +	 * GTI_CWD_WDOG.CNT, need addition shift of 8.
> +	 */
> +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> +	regval &= GTI_CWD_WDOG_MODE_MASK;
> +	regval |= ((timeout_wdog) << (GTI_CWD_WDOG_CNT_SHIFT + 8)) |
> +		   (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT);
> +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> +	return 0;
> +}
> +
> +static const struct watchdog_info octeontx2_gti_wdt_ident = {
> +	.identity = "OcteonTX2 GTI watchdog",
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> +			  WDIOF_CARDRESET,
> +};
> +
> +static const struct watchdog_ops octeontx2_gti_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = octeontx2_gti_wdt_start,
> +	.stop = octeontx2_gti_wdt_stop,
> +	.ping = octeontx2_gti_wdt_ping,
> +	.set_timeout = octeontx2_gti_wdt_settimeout,
> +};
> +
> +static void octeontx2_gti_wdt_free_irqs(struct octeontx2_gti_wdt_priv *priv)
> +{
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	int irq, cpu = 0;
> +
> +	for_each_online_cpu(cpu) {
> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		irq = percpu_priv->irq;
> +		if (irq) {
> +			if (priv->is_nmi) {
> +				disable_nmi_nosync(irq);
> +				free_nmi(irq, priv);
> +			} else {
> +				disable_irq_nosync(irq);
> +				free_irq(irq, priv);
> +			}
> +
> +			percpu_priv->irq = 0;
> +		}
> +	}
> +}
> +
> +static int octeontx2_gti_wdt_probe(struct platform_device *pdev)
> +{
> +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> +	struct octeontx2_gti_wdt_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	struct watchdog_device *wdog_dev;
> +	unsigned long irq_flags;
> +	int irq, cpu, num_irqs;
> +	int err;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->percpu_priv = devm_alloc_percpu(&pdev->dev, *priv->percpu_priv);
> +	if (!priv->percpu_priv)
> +		return -ENOMEM;
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
> +			      "reg property not valid/found\n");
> +
> +	num_irqs = platform_irq_count(pdev);
> +	if (num_irqs < 0)
> +		return dev_err_probe(dev, num_irqs, "GTI CWD no IRQs\n");
> +
> +	if (num_irqs < num_online_cpus())
> +		return dev_err_probe(dev, -EINVAL, "IRQs (%d) < CPUs (%d)\n",
> +				     num_irqs, num_online_cpus());
> +
> +	priv->clock_freq = arch_timer_get_cntfrq();
> +
> +	for_each_online_cpu(cpu) {

Watchdogs are supposed to be per system, not per CPU. The Linux kernel has
other means to detect hung CPUs, and the watchdog subsystem should be
(ab)used to bypass or replace those methods.

I am not inclined to accept this patch.

Guenter

> +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> +		wdog_dev = &percpu_priv->wdev;
> +		wdog_dev->info = &octeontx2_gti_wdt_ident,
> +		wdog_dev->ops = &octeontx2_gti_wdt_ops,
> +		wdog_dev->parent = dev;
> +		wdog_dev->min_timeout = 1;
> +		wdog_dev->max_timeout = 16;
> +		wdog_dev->max_hw_heartbeat_ms = 16000;
> +		wdog_dev->timeout = 8;
> +
> +		irq = platform_get_irq(pdev, cpu);
> +		if (irq < 0) {
> +			dev_err(&pdev->dev, "IRQ resource not found\n");
> +			err = -ENODEV;
> +			goto out;
> +		}
> +
> +		err = irq_force_affinity(irq, cpumask_of(cpu));
> +		if (err) {
> +			pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n", irq, cpu);
> +			goto out;
> +		}
> +
> +		irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
> +			    IRQF_NO_THREAD;
> +		err = request_nmi(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> +				  pdev->name, priv);
> +		if (err) {
> +			err = request_irq(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> +					  pdev->name, priv);
> +			if (err) {
> +				dev_err(dev, "cannot register interrupt handler %d\n", err);
> +				goto out;
> +			}
> +			enable_irq(irq);
> +		} else {
> +			priv->is_nmi = 1;
> +			enable_nmi(irq);
> +		}
> +
> +		percpu_priv->irq = irq;
> +		watchdog_set_drvdata(wdog_dev, priv);
> +		platform_set_drvdata(pdev, priv);
> +		watchdog_init_timeout(wdog_dev, wdog_dev->timeout, dev);
> +		octeontx2_gti_wdt_settimeout(wdog_dev, wdog_dev->timeout);
> +		watchdog_stop_on_reboot(wdog_dev);
> +		watchdog_stop_on_unregister(wdog_dev);
> +
> +		err = devm_watchdog_register_device(dev, wdog_dev);
> +		if (unlikely(err))
> +			goto out;
> +		dev_info(dev, "Watchdog enabled (timeout=%d sec)", wdog_dev->timeout);
> +	}
> +	return 0;
> +
> +out:
> +	octeontx2_gti_wdt_free_irqs(priv);
> +	return err;
> +}
> +
> +static int octeontx2_gti_wdt_remove(struct platform_device *pdev)
> +{
> +	struct octeontx2_gti_wdt_priv *priv = platform_get_drvdata(pdev);
> +
> +	octeontx2_gti_wdt_free_irqs(priv);
> +	return 0;
> +}
> +
> +static const struct of_device_id octeontx2_gti_wdt_of_match[] = {
> +	{ .compatible = "mrvl,octeontx2-gti-wdt", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, octeontx2_gti_wdt_of_match);
> +
> +static struct platform_driver octeontx2_gti_wdt_driver = {
> +	.driver = {
> +		.name = "octeontx2-gti-wdt",
> +		.of_match_table = octeontx2_gti_wdt_of_match,
> +	},
> +	.probe = octeontx2_gti_wdt_probe,
> +	.remove = octeontx2_gti_wdt_remove,
> +};
> +module_platform_driver(octeontx2_gti_wdt_driver);
> +
> +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>");
> +MODULE_DESCRIPTION("OcteonTX2 GTI per cpu watchdog driver");
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
  2023-03-24 21:04   ` Thomas Weißschuh
  2023-03-24 22:03   ` Guenter Roeck
@ 2023-03-26  8:40   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2023-03-26  8:40 UTC (permalink / raw)
  To: Bharat Bhushan, wim, linux, robh+dt, krzysztof.kozlowski+dt,
	linux-watchdog, devicetree, linux-kernel
  Cc: llvm, oe-kbuild-all, Bharat Bhushan

Hi Bharat,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on groeck-staging/hwmon-next linus/master v6.3-rc3 next-20230324]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bharat-Bhushan/Watchdog-octeontx2-Add-Pseudo-NMI-GTI-watchdog-driver/20230324-225820
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230324145652.19221-2-bbhushan2%40marvell.com
patch subject: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20230326/202303261656.JoHKPpgU-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/b703845ae954bff5a3ae14669043889a8989adde
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Bharat-Bhushan/Watchdog-octeontx2-Add-Pseudo-NMI-GTI-watchdog-driver/20230324-225820
        git checkout b703845ae954bff5a3ae14669043889a8989adde
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/infiniband/hw/bnxt_re/ drivers/watchdog/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303261656.JoHKPpgU-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/watchdog/octeontx2_gti_watchdog.c:67:6: warning: no previous prototype for function 'octeontx2_gti_wdt_callback_other_cpus' [-Wmissing-prototypes]
   void octeontx2_gti_wdt_callback_other_cpus(void *unused)
        ^
   drivers/watchdog/octeontx2_gti_watchdog.c:67:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void octeontx2_gti_wdt_callback_other_cpus(void *unused)
   ^
   static 
   1 warning generated.


vim +/octeontx2_gti_wdt_callback_other_cpus +67 drivers/watchdog/octeontx2_gti_watchdog.c

    66	
  > 67	void octeontx2_gti_wdt_callback_other_cpus(void *unused)
    68	{
    69		struct pt_regs *regs = get_irq_regs();
    70	
    71		pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
    72	
    73		if (regs)
    74			show_regs(regs);
    75		else
    76			dump_stack();
    77	}
    78	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver
  2023-03-24 20:34 ` Rob Herring
@ 2023-04-13 10:14   ` Bharat Bhushan
  0 siblings, 0 replies; 10+ messages in thread
From: Bharat Bhushan @ 2023-04-13 10:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: wim, linux, krzysztof.kozlowski+dt, linux-watchdog, devicetree,
	linux-kernel

Please see inline

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Saturday, March 25, 2023 2:05 AM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: wim@linux-watchdog.org; linux@roeck-us.net;
> krzysztof.kozlowski+dt@linaro.org; linux-watchdog@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI
> watchdog driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, Mar 24, 2023 at 08:26:51PM +0530, Bharat Bhushan wrote:
> > Add binding documentation for the Marvell octeonTX2 GTI watchdog
> > driver.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> >  .../watchdog/marvel-octeontx2-wdt.yaml        | 43 +++++++++++++++++++
> 
> The comics?
> 
> Use compatible string for filename.

Sorry for late reply, just returned from vacation. Thanks for review.

Will fix all these issues. Also run "make dt_binding_check".

Thanks
-Bharat

> 
> >  1 file changed, 43 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> > b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.yaml
> > new file mode 100644
> > index 000000000000..586b3c1bd780
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/marvel-octeontx2-wdt.
> > +++ yaml
> > @@ -0,0 +1,43 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_sc
> > +hemas_watchdog_marvel-2Docteontx2-2Dwdt.yaml-
> 23&d=DwIBAg&c=nKjWec2b6R
> >
> +0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=
> Ut_moFc_
> >
> +jI4JG2ZlRAaLIL9HLPOI2Thz6WmALCu5SENmtKrGa0M4tN0m5TP0eBw5&s=J3P7a
> QnYY3
> > +pKqoGp1-eiSsINlof1pAh3u-aLn4tfrLk&e=
> > +$schema:
> > +https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_me
> > +ta-2Dschemas_core.yaml-
> 23&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWsw
> >
> +Pe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=Ut_moFc_jI4JG2ZlRAaLIL9HL
> POI2T
> >
> +hz6WmALCu5SENmtKrGa0M4tN0m5TP0eBw5&s=GdtkUc3_hrJV0gvS1Eviv1mxf
> KMLIv7Y
> > +IohR76Dtkmc&e=
> > +
> > +title: Marvell OcteonTX2 GTI watchdog
> > +
> > +allOf:
> > +  - $ref: "watchdog.yaml#"
> 
> Drop quotes
> 
> > +
> > +maintainers:
> > +  - Bharat Bhushan <bbhushan2@marvell.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mrvl,octeontx2-gti-wdt
> 
> 'mrvl' is deprecated. Use 'marvell'
> 
> > +
> > +  reg:
> > +    maxItems: 2
> 
> Need to define what each entry is.
> 
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 36
> 
> Need to define what each entry is. How does the h/w have a variable number of
> interrupts?
> 
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    watch-dog@802000040000 {
> 
> watchdog@...
> 
> > +      compatible = "mrvl,octeontx2-gti-wdt";
> > +      reg = <0x8020 0x40000 0x0 0x20000>;
> > +      interrupts = <0 38 1>, /* Core-0 */
> > +                   <0 39 1>; /* Core-1 */
> > +    };
> > +
> > +...
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 21:04   ` Thomas Weißschuh
@ 2023-04-13 10:20     ` Bharat Bhushan
  0 siblings, 0 replies; 10+ messages in thread
From: Bharat Bhushan @ 2023-04-13 10:20 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: wim, linux, robh+dt, krzysztof.kozlowski+dt, linux-watchdog,
	devicetree, linux-kernel

Please see inline

> -----Original Message-----
> From: Thomas Weißschuh <thomas@t-8ch.de>
> Sent: Saturday, March 25, 2023 2:35 AM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: wim@linux-watchdog.org; linux@roeck-us.net; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; linux-watchdog@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI
> watchdog driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 2023-03-24 20:26:52+0530, Bharat Bhushan wrote:
> > GTI hardware supports per-core watchdog timer which are programmed in
> > "interrupt + del3t + reset mode" and del3t traps are not enabled.
> > This driver uses ARM64 pseudo-nmi interrupt support.
> > GTI watchdog exception flow is:
> >  - 1st timer expiration generates pseudo-nmi interrupt.
> >    NMI exception handler dumps register/context state on all cpu's.
> >  - 2nd timer expiration is ignored
> >
> >  - On 3rd timer expiration will trigger a system-wide core reset.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> >  drivers/watchdog/Kconfig                  |   9 +
> >  drivers/watchdog/Makefile                 |   1 +
> >  drivers/watchdog/octeontx2_gti_watchdog.c | 352
> > ++++++++++++++++++++++
> >  3 files changed, 362 insertions(+)
> >  create mode 100644 drivers/watchdog/octeontx2_gti_watchdog.c
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
> > f0872970daf9..9607d36645f6 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -2212,4 +2212,13 @@ config KEEMBAY_WATCHDOG
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called keembay_wdt.
> >
> > +config OCTEON_GTI_WATCHDOG
> > +	tristate "OCTEONTX2 GTI Watchdog driver"
> > +	depends on ARM64
> > +	help
> > +	 OCTEONTX2 GTI hardware supports per-core watchdog timer which
> > +	 are programmed in "interrupt + del3t + reset mode" and del3t
> > +	 traps are not enabled.
> > +	 This driver uses ARM64 pseudo-nmi interrupt support.
> > +
> >  endif # WATCHDOG
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index 9cbf6580f16c..11af3db62fec 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -230,3 +230,4 @@ obj-$(CONFIG_MENZ069_WATCHDOG) +=
> menz69_wdt.o
> >  obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
> >  obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
> >  obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
> > +obj-$(CONFIG_OCTEON_GTI_WATCHDOG) += octeontx2_gti_watchdog.o
> > diff --git a/drivers/watchdog/octeontx2_gti_watchdog.c
> > b/drivers/watchdog/octeontx2_gti_watchdog.c
> > new file mode 100644
> > index 000000000000..766b7d41defe
> > --- /dev/null
> > +++ b/drivers/watchdog/octeontx2_gti_watchdog.c
> > @@ -0,0 +1,352 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Marvell GTI Watchdog driver
> > + *
> > + * Copyright (C) 2023 Marvell International Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/cpu.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/sched/debug.h>
> > +
> > +#include <asm/arch_timer.h>
> > +
> > +/* GTI CWD Watchdog Registers */
> > +#define GTI_CWD_WDOG(cpu)		(0x8 * cpu)
> > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST	(0x3)
> > +#define GTI_CWD_WDOG_MODE_MASK		(0x3)
> > +#define GTI_CWD_WDOG_LEN_SHIFT		(4)
> > +#define GTI_CWD_WDOG_CNT_SHIFT		(20)
> > +
> > +/* GTI Per-core Watchdog Interrupt Register */
> > +#define GTI_CWD_INT			0x200
> > +
> > +/* GTI Per-core Watchdog Interrupt Enable Clear Register */
> > +#define GTI_CWD_INT_ENA_CLR		0x210
> > +
> > +/* GTI Per-core Watchdog Interrupt Enable Set Register */
> > +#define GTI_CWD_INT_ENA_SET		0x218
> > +
> > +/* GTI Per-core Watchdog Poke Registers */
> > +#define GTI_CWD_POKE(cpu)		(0x10000 + 0x8 * cpu)
> > +
> > +struct octeontx2_gti_wdt_percpu_priv {
> > +	struct watchdog_device wdev;
> > +	int irq;
> > +};
> > +
> > +struct octeontx2_gti_wdt_priv {
> > +	void __iomem *base;
> > +	u64 clock_freq;
> > +	int is_nmi;
> > +	struct octeontx2_gti_wdt_percpu_priv __percpu *percpu_priv; };
> > +
> > +static int octeontx2_gti_wdt_get_cpuid(struct watchdog_device *wdev)
> > +{
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	int cpu;
> > +
> > +	for_each_online_cpu(cpu) {
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		if (&percpu_priv->wdev == wdev)
> > +			return cpu;
> > +	}
> > +
> > +	return -1;
> > +}
> > +
> > +void octeontx2_gti_wdt_callback_other_cpus(void *unused)
> 
> Could be static.

First, sorry for late reply, just came back from vacation.

Will fix,

> 
> > +{
> > +	struct pt_regs *regs = get_irq_regs();
> > +
> > +	pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
> > +
> > +	if (regs)
> > +		show_regs(regs);
> > +	else
> > +		dump_stack();
> > +}
> > +
> > +static irqreturn_t octeontx2_gti_wdt_interrupt(int irq, void *data) {
> > +	struct octeontx2_gti_wdt_priv *priv = (struct octeontx2_gti_wdt_priv
> *)data;
> > +	int cpu = smp_processor_id();
> > +
> > +	/* Clear interrupt to fire again if delayed poke happens */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> > +	dump_stack();
> > +
> > +	for_each_online_cpu(cpu) {
> > +		if (cpu == raw_smp_processor_id())
> > +			continue;
> > +
> > +		smp_call_function_single(cpu,
> > +
> octeontx2_gti_wdt_callback_other_cpus,
> > +					 NULL, 1);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int octeontx2_gti_wdt_ping(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	writeq(1, priv->base + GTI_CWD_POKE(cpu));
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_start(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +	u64 regval;
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	set_bit(WDOG_HW_RUNNING, &wdev->status);
> > +
> > +	/* Clear any pending interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> > +
> > +	/* Enable Interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_SET);
> > +
> > +	/* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST;
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_stop(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	u64 regval;
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	/* Disable Interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_CLR);
> > +
> > +	/* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval &= ~GTI_CWD_WDOG_MODE_MASK;
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_settimeout(struct watchdog_device *wdev,
> > +					unsigned int timeout)
> > +{
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +	u64 timeout_wdog, regval;
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	/* Update new timeout */
> > +	wdev->timeout = timeout;
> > +
> > +	/* Get clock cycles from timeout in second */
> > +	timeout_wdog = (u64)timeout * priv->clock_freq;
> > +
> > +	/* Watchdog counts in 1024 cycle steps */
> > +	timeout_wdog = timeout_wdog >> 10;
> > +
> > +	/*
> > +	 * Hardware allows programming of upper 16-bits of 24-bits cycles
> > +	 * Round up and use upper 16-bits only.
> > +	 * Set max if timeout more than h/w supported
> > +	 */
> > +	timeout_wdog = (timeout_wdog + 0xff) >> 8;
> > +	if (timeout_wdog >= 0x10000)
> > +		timeout_wdog = 0xffff;
> > +
> > +	/*
> > +	 * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits
> > +	 * GTI_CWD_WDOG.CNT, need addition shift of 8.
> > +	 */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval &= GTI_CWD_WDOG_MODE_MASK;
> > +	regval |= ((timeout_wdog) << (GTI_CWD_WDOG_CNT_SHIFT + 8)) |
> > +		   (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT);
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +	return 0;
> > +}
> > +
> > +static const struct watchdog_info octeontx2_gti_wdt_ident = {
> > +	.identity = "OcteonTX2 GTI watchdog",
> > +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> WDIOF_MAGICCLOSE |
> > +			  WDIOF_CARDRESET,
> 
> Weird alignment.

Do not know why it is showing like this, in code it looks good.

> 
> > +};
> > +
> > +static const struct watchdog_ops octeontx2_gti_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = octeontx2_gti_wdt_start,
> > +	.stop = octeontx2_gti_wdt_stop,
> > +	.ping = octeontx2_gti_wdt_ping,
> > +	.set_timeout = octeontx2_gti_wdt_settimeout, };
> > +
> > +static void octeontx2_gti_wdt_free_irqs(struct octeontx2_gti_wdt_priv
> > +*priv) {
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	int irq, cpu = 0;
> > +
> > +	for_each_online_cpu(cpu) {
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		irq = percpu_priv->irq;
> > +		if (irq) {
> > +			if (priv->is_nmi) {
> > +				disable_nmi_nosync(irq);
> > +				free_nmi(irq, priv);
> > +			} else {
> > +				disable_irq_nosync(irq);
> > +				free_irq(irq, priv);
> > +			}
> > +
> > +			percpu_priv->irq = 0;
> > +		}
> > +	}
> > +}
> > +
> > +static int octeontx2_gti_wdt_probe(struct platform_device *pdev) {
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	struct octeontx2_gti_wdt_priv *priv;
> > +	struct device *dev = &pdev->dev;
> > +	struct watchdog_device *wdog_dev;
> > +	unsigned long irq_flags;
> > +	int irq, cpu, num_irqs;
> > +	int err;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->percpu_priv = devm_alloc_percpu(&pdev->dev, *priv-
> >percpu_priv);
> > +	if (!priv->percpu_priv)
> > +		return -ENOMEM;
> > +
> > +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(priv->base))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
> > +			      "reg property not valid/found\n");
> > +
> > +	num_irqs = platform_irq_count(pdev);
> > +	if (num_irqs < 0)
> > +		return dev_err_probe(dev, num_irqs, "GTI CWD no IRQs\n");
> > +
> > +	if (num_irqs < num_online_cpus())
> > +		return dev_err_probe(dev, -EINVAL, "IRQs (%d) < CPUs (%d)\n",
> > +				     num_irqs, num_online_cpus());
> > +
> > +	priv->clock_freq = arch_timer_get_cntfrq();
> > +
> > +	for_each_online_cpu(cpu) {
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		wdog_dev = &percpu_priv->wdev;
> > +		wdog_dev->info = &octeontx2_gti_wdt_ident,
> > +		wdog_dev->ops = &octeontx2_gti_wdt_ops,
> > +		wdog_dev->parent = dev;
> > +		wdog_dev->min_timeout = 1;
> > +		wdog_dev->max_timeout = 16;
> > +		wdog_dev->max_hw_heartbeat_ms = 16000;
> > +		wdog_dev->timeout = 8;
> > +
> > +		irq = platform_get_irq(pdev, cpu);
> > +		if (irq < 0) {
> > +			dev_err(&pdev->dev, "IRQ resource not found\n");
> > +			err = -ENODEV;
> > +			goto out;
> > +		}
> > +
> > +		err = irq_force_affinity(irq, cpumask_of(cpu));
> > +		if (err) {
> > +			pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n",
> irq, cpu);
> > +			goto out;
> > +		}
> > +
> > +		irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> IRQF_NO_AUTOEN |
> > +			    IRQF_NO_THREAD;
> > +		err = request_nmi(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> > +				  pdev->name, priv);
> > +		if (err) {
> > +			err = request_irq(irq, octeontx2_gti_wdt_interrupt,
> irq_flags,
> > +					  pdev->name, priv);
> > +			if (err) {
> > +				dev_err(dev, "cannot register interrupt handler
> %d\n", err);
> > +				goto out;
> > +			}
> > +			enable_irq(irq);
> > +		} else {
> > +			priv->is_nmi = 1;
> > +			enable_nmi(irq);
> > +		}
> > +
> > +		percpu_priv->irq = irq;
> > +		watchdog_set_drvdata(wdog_dev, priv);
> > +		platform_set_drvdata(pdev, priv);
> > +		watchdog_init_timeout(wdog_dev, wdog_dev->timeout, dev);
> > +		octeontx2_gti_wdt_settimeout(wdog_dev, wdog_dev-
> >timeout);
> > +		watchdog_stop_on_reboot(wdog_dev);
> > +		watchdog_stop_on_unregister(wdog_dev);
> > +
> > +		err = devm_watchdog_register_device(dev, wdog_dev);
> > +		if (unlikely(err))
> 
> This unlikely() should really not be needed.

Will remove.

> 
> > +			goto out;
> > +		dev_info(dev, "Watchdog enabled (timeout=%d sec)",
> wdog_dev->timeout);
> > +	}
> > +	return 0;
> > +
> > +out:
> > +	octeontx2_gti_wdt_free_irqs(priv);
> > +	return err;
> > +}
> > +
> > +static int octeontx2_gti_wdt_remove(struct platform_device *pdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = platform_get_drvdata(pdev);
> > +
> > +	octeontx2_gti_wdt_free_irqs(priv);
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id octeontx2_gti_wdt_of_match[] = {
> > +	{ .compatible = "mrvl,octeontx2-gti-wdt", },
> > +	{ },
> 
> No trailing comma for end-of-array marker.

Will fix.

Thanks
-Bharat

> 
> > +};
> > +MODULE_DEVICE_TABLE(of, octeontx2_gti_wdt_of_match);
> > +
> > +static struct platform_driver octeontx2_gti_wdt_driver = {
> > +	.driver = {
> > +		.name = "octeontx2-gti-wdt",
> > +		.of_match_table = octeontx2_gti_wdt_of_match,
> > +	},
> > +	.probe = octeontx2_gti_wdt_probe,
> > +	.remove = octeontx2_gti_wdt_remove,
> > +};
> > +module_platform_driver(octeontx2_gti_wdt_driver);
> > +
> > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>");
> > +MODULE_DESCRIPTION("OcteonTX2 GTI per cpu watchdog driver");
> 
> No MODULE_LICENSE() ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI watchdog driver
  2023-03-24 22:03   ` Guenter Roeck
@ 2023-04-13 10:22     ` Bharat Bhushan
  0 siblings, 0 replies; 10+ messages in thread
From: Bharat Bhushan @ 2023-04-13 10:22 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: wim, robh+dt, krzysztof.kozlowski+dt, linux-watchdog, devicetree,
	linux-kernel

Please see inline

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Saturday, March 25, 2023 3:33 AM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: wim@linux-watchdog.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; linux-watchdog@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI GTI
> watchdog driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, Mar 24, 2023 at 08:26:52PM +0530, Bharat Bhushan wrote:
> > GTI hardware supports per-core watchdog timer which are programmed in
> > "interrupt + del3t + reset mode" and del3t traps are not enabled.
> > This driver uses ARM64 pseudo-nmi interrupt support.
> > GTI watchdog exception flow is:
> >  - 1st timer expiration generates pseudo-nmi interrupt.
> >    NMI exception handler dumps register/context state on all cpu's.
> >  - 2nd timer expiration is ignored
> >
> >  - On 3rd timer expiration will trigger a system-wide core reset.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> >  drivers/watchdog/Kconfig                  |   9 +
> >  drivers/watchdog/Makefile                 |   1 +
> >  drivers/watchdog/octeontx2_gti_watchdog.c | 352
> > ++++++++++++++++++++++
> >  3 files changed, 362 insertions(+)
> >  create mode 100644 drivers/watchdog/octeontx2_gti_watchdog.c
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
> > f0872970daf9..9607d36645f6 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -2212,4 +2212,13 @@ config KEEMBAY_WATCHDOG
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called keembay_wdt.
> >
> > +config OCTEON_GTI_WATCHDOG
> > +	tristate "OCTEONTX2 GTI Watchdog driver"
> > +	depends on ARM64
> > +	help
> > +	 OCTEONTX2 GTI hardware supports per-core watchdog timer which
> > +	 are programmed in "interrupt + del3t + reset mode" and del3t
> > +	 traps are not enabled.
> > +	 This driver uses ARM64 pseudo-nmi interrupt support.
> > +
> >  endif # WATCHDOG
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index 9cbf6580f16c..11af3db62fec 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -230,3 +230,4 @@ obj-$(CONFIG_MENZ069_WATCHDOG) +=
> menz69_wdt.o
> >  obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
> >  obj-$(CONFIG_STPMIC1_WATCHDOG) += stpmic1_wdt.o
> >  obj-$(CONFIG_SL28CPLD_WATCHDOG) += sl28cpld_wdt.o
> > +obj-$(CONFIG_OCTEON_GTI_WATCHDOG) += octeontx2_gti_watchdog.o
> > diff --git a/drivers/watchdog/octeontx2_gti_watchdog.c
> > b/drivers/watchdog/octeontx2_gti_watchdog.c
> > new file mode 100644
> > index 000000000000..766b7d41defe
> > --- /dev/null
> > +++ b/drivers/watchdog/octeontx2_gti_watchdog.c
> > @@ -0,0 +1,352 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Marvell GTI Watchdog driver
> > + *
> > + * Copyright (C) 2023 Marvell International Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/cpu.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/sched/debug.h>
> > +
> > +#include <asm/arch_timer.h>
> > +
> > +/* GTI CWD Watchdog Registers */
> > +#define GTI_CWD_WDOG(cpu)		(0x8 * cpu)
> > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST	(0x3)
> > +#define GTI_CWD_WDOG_MODE_MASK		(0x3)
> > +#define GTI_CWD_WDOG_LEN_SHIFT		(4)
> > +#define GTI_CWD_WDOG_CNT_SHIFT		(20)
> > +
> > +/* GTI Per-core Watchdog Interrupt Register */
> > +#define GTI_CWD_INT			0x200
> > +
> > +/* GTI Per-core Watchdog Interrupt Enable Clear Register */
> > +#define GTI_CWD_INT_ENA_CLR		0x210
> > +
> > +/* GTI Per-core Watchdog Interrupt Enable Set Register */
> > +#define GTI_CWD_INT_ENA_SET		0x218
> > +
> > +/* GTI Per-core Watchdog Poke Registers */
> > +#define GTI_CWD_POKE(cpu)		(0x10000 + 0x8 * cpu)
> > +
> > +struct octeontx2_gti_wdt_percpu_priv {
> > +	struct watchdog_device wdev;
> > +	int irq;
> > +};
> > +
> > +struct octeontx2_gti_wdt_priv {
> > +	void __iomem *base;
> > +	u64 clock_freq;
> > +	int is_nmi;
> > +	struct octeontx2_gti_wdt_percpu_priv __percpu *percpu_priv; };
> > +
> > +static int octeontx2_gti_wdt_get_cpuid(struct watchdog_device *wdev)
> > +{
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	int cpu;
> > +
> > +	for_each_online_cpu(cpu) {
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		if (&percpu_priv->wdev == wdev)
> > +			return cpu;
> > +	}
> > +
> > +	return -1;
> > +}
> > +
> > +void octeontx2_gti_wdt_callback_other_cpus(void *unused) {
> > +	struct pt_regs *regs = get_irq_regs();
> > +
> > +	pr_emerg("GTI Watchdog CPU:%d\n", raw_smp_processor_id());
> > +
> > +	if (regs)
> > +		show_regs(regs);
> > +	else
> > +		dump_stack();
> > +}
> > +
> > +static irqreturn_t octeontx2_gti_wdt_interrupt(int irq, void *data) {
> > +	struct octeontx2_gti_wdt_priv *priv = (struct octeontx2_gti_wdt_priv
> *)data;
> > +	int cpu = smp_processor_id();
> > +
> > +	/* Clear interrupt to fire again if delayed poke happens */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> > +	dump_stack();
> > +
> > +	for_each_online_cpu(cpu) {
> > +		if (cpu == raw_smp_processor_id())
> > +			continue;
> > +
> > +		smp_call_function_single(cpu,
> > +
> octeontx2_gti_wdt_callback_other_cpus,
> > +					 NULL, 1);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int octeontx2_gti_wdt_ping(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	writeq(1, priv->base + GTI_CWD_POKE(cpu));
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_start(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +	u64 regval;
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	set_bit(WDOG_HW_RUNNING, &wdev->status);
> > +
> > +	/* Clear any pending interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT);
> > +
> > +	/* Enable Interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_SET);
> > +
> > +	/* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST;
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_stop(struct watchdog_device *wdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	u64 regval;
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	/* Disable Interrupt */
> > +	writeq(1 << cpu, priv->base + GTI_CWD_INT_ENA_CLR);
> > +
> > +	/* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval &= ~GTI_CWD_WDOG_MODE_MASK;
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +
> > +	return 0;
> > +}
> > +
> > +static int octeontx2_gti_wdt_settimeout(struct watchdog_device *wdev,
> > +					unsigned int timeout)
> > +{
> > +	struct octeontx2_gti_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > +	int cpu = octeontx2_gti_wdt_get_cpuid(wdev);
> > +	u64 timeout_wdog, regval;
> > +
> > +	if (cpu < 0)
> > +		return -EINVAL;
> > +
> > +	/* Update new timeout */
> > +	wdev->timeout = timeout;
> > +
> > +	/* Get clock cycles from timeout in second */
> > +	timeout_wdog = (u64)timeout * priv->clock_freq;
> > +
> > +	/* Watchdog counts in 1024 cycle steps */
> > +	timeout_wdog = timeout_wdog >> 10;
> > +
> > +	/*
> > +	 * Hardware allows programming of upper 16-bits of 24-bits cycles
> > +	 * Round up and use upper 16-bits only.
> > +	 * Set max if timeout more than h/w supported
> > +	 */
> > +	timeout_wdog = (timeout_wdog + 0xff) >> 8;
> > +	if (timeout_wdog >= 0x10000)
> > +		timeout_wdog = 0xffff;
> > +
> > +	/*
> > +	 * GTI_CWD_WDOG.LEN have only upper 16-bits of 24-bits
> > +	 * GTI_CWD_WDOG.CNT, need addition shift of 8.
> > +	 */
> > +	regval = readq(priv->base + GTI_CWD_WDOG(cpu));
> > +	regval &= GTI_CWD_WDOG_MODE_MASK;
> > +	regval |= ((timeout_wdog) << (GTI_CWD_WDOG_CNT_SHIFT + 8)) |
> > +		   (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT);
> > +	writeq(regval, priv->base + GTI_CWD_WDOG(cpu));
> > +	return 0;
> > +}
> > +
> > +static const struct watchdog_info octeontx2_gti_wdt_ident = {
> > +	.identity = "OcteonTX2 GTI watchdog",
> > +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> WDIOF_MAGICCLOSE |
> > +			  WDIOF_CARDRESET,
> > +};
> > +
> > +static const struct watchdog_ops octeontx2_gti_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = octeontx2_gti_wdt_start,
> > +	.stop = octeontx2_gti_wdt_stop,
> > +	.ping = octeontx2_gti_wdt_ping,
> > +	.set_timeout = octeontx2_gti_wdt_settimeout, };
> > +
> > +static void octeontx2_gti_wdt_free_irqs(struct octeontx2_gti_wdt_priv
> > +*priv) {
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	int irq, cpu = 0;
> > +
> > +	for_each_online_cpu(cpu) {
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		irq = percpu_priv->irq;
> > +		if (irq) {
> > +			if (priv->is_nmi) {
> > +				disable_nmi_nosync(irq);
> > +				free_nmi(irq, priv);
> > +			} else {
> > +				disable_irq_nosync(irq);
> > +				free_irq(irq, priv);
> > +			}
> > +
> > +			percpu_priv->irq = 0;
> > +		}
> > +	}
> > +}
> > +
> > +static int octeontx2_gti_wdt_probe(struct platform_device *pdev) {
> > +	struct octeontx2_gti_wdt_percpu_priv *percpu_priv;
> > +	struct octeontx2_gti_wdt_priv *priv;
> > +	struct device *dev = &pdev->dev;
> > +	struct watchdog_device *wdog_dev;
> > +	unsigned long irq_flags;
> > +	int irq, cpu, num_irqs;
> > +	int err;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->percpu_priv = devm_alloc_percpu(&pdev->dev, *priv-
> >percpu_priv);
> > +	if (!priv->percpu_priv)
> > +		return -ENOMEM;
> > +
> > +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(priv->base))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
> > +			      "reg property not valid/found\n");
> > +
> > +	num_irqs = platform_irq_count(pdev);
> > +	if (num_irqs < 0)
> > +		return dev_err_probe(dev, num_irqs, "GTI CWD no IRQs\n");
> > +
> > +	if (num_irqs < num_online_cpus())
> > +		return dev_err_probe(dev, -EINVAL, "IRQs (%d) < CPUs (%d)\n",
> > +				     num_irqs, num_online_cpus());
> > +
> > +	priv->clock_freq = arch_timer_get_cntfrq();
> > +
> > +	for_each_online_cpu(cpu) {
> 
> Watchdogs are supposed to be per system, not per CPU. The Linux kernel has
> other means to detect hung CPUs, and the watchdog subsystem should be
> (ab)used to bypass or replace those methods.

Sorry for late reply, just returned from vacation.

Okay, will remove the per core watchdog and submit next patch for global watchdog.

Thanks
-Bharat

> 
> I am not inclined to accept this patch.
> 
> Guenter
> 
> > +		percpu_priv = per_cpu_ptr(priv->percpu_priv, cpu);
> > +		wdog_dev = &percpu_priv->wdev;
> > +		wdog_dev->info = &octeontx2_gti_wdt_ident,
> > +		wdog_dev->ops = &octeontx2_gti_wdt_ops,
> > +		wdog_dev->parent = dev;
> > +		wdog_dev->min_timeout = 1;
> > +		wdog_dev->max_timeout = 16;
> > +		wdog_dev->max_hw_heartbeat_ms = 16000;
> > +		wdog_dev->timeout = 8;
> > +
> > +		irq = platform_get_irq(pdev, cpu);
> > +		if (irq < 0) {
> > +			dev_err(&pdev->dev, "IRQ resource not found\n");
> > +			err = -ENODEV;
> > +			goto out;
> > +		}
> > +
> > +		err = irq_force_affinity(irq, cpumask_of(cpu));
> > +		if (err) {
> > +			pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n",
> irq, cpu);
> > +			goto out;
> > +		}
> > +
> > +		irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> IRQF_NO_AUTOEN |
> > +			    IRQF_NO_THREAD;
> > +		err = request_nmi(irq, octeontx2_gti_wdt_interrupt, irq_flags,
> > +				  pdev->name, priv);
> > +		if (err) {
> > +			err = request_irq(irq, octeontx2_gti_wdt_interrupt,
> irq_flags,
> > +					  pdev->name, priv);
> > +			if (err) {
> > +				dev_err(dev, "cannot register interrupt handler
> %d\n", err);
> > +				goto out;
> > +			}
> > +			enable_irq(irq);
> > +		} else {
> > +			priv->is_nmi = 1;
> > +			enable_nmi(irq);
> > +		}
> > +
> > +		percpu_priv->irq = irq;
> > +		watchdog_set_drvdata(wdog_dev, priv);
> > +		platform_set_drvdata(pdev, priv);
> > +		watchdog_init_timeout(wdog_dev, wdog_dev->timeout, dev);
> > +		octeontx2_gti_wdt_settimeout(wdog_dev, wdog_dev-
> >timeout);
> > +		watchdog_stop_on_reboot(wdog_dev);
> > +		watchdog_stop_on_unregister(wdog_dev);
> > +
> > +		err = devm_watchdog_register_device(dev, wdog_dev);
> > +		if (unlikely(err))
> > +			goto out;
> > +		dev_info(dev, "Watchdog enabled (timeout=%d sec)",
> wdog_dev->timeout);
> > +	}
> > +	return 0;
> > +
> > +out:
> > +	octeontx2_gti_wdt_free_irqs(priv);
> > +	return err;
> > +}
> > +
> > +static int octeontx2_gti_wdt_remove(struct platform_device *pdev) {
> > +	struct octeontx2_gti_wdt_priv *priv = platform_get_drvdata(pdev);
> > +
> > +	octeontx2_gti_wdt_free_irqs(priv);
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id octeontx2_gti_wdt_of_match[] = {
> > +	{ .compatible = "mrvl,octeontx2-gti-wdt", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, octeontx2_gti_wdt_of_match);
> > +
> > +static struct platform_driver octeontx2_gti_wdt_driver = {
> > +	.driver = {
> > +		.name = "octeontx2-gti-wdt",
> > +		.of_match_table = octeontx2_gti_wdt_of_match,
> > +	},
> > +	.probe = octeontx2_gti_wdt_probe,
> > +	.remove = octeontx2_gti_wdt_remove,
> > +};
> > +module_platform_driver(octeontx2_gti_wdt_driver);
> > +
> > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>");
> > +MODULE_DESCRIPTION("OcteonTX2 GTI per cpu watchdog driver");
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-04-13 10:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 14:56 [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 GTI watchdog driver Bharat Bhushan
2023-03-24 14:56 ` [PATCH 2/2] Watchdog: octeontx2: Add Pseudo-NMI " Bharat Bhushan
2023-03-24 21:04   ` Thomas Weißschuh
2023-04-13 10:20     ` [EXT] " Bharat Bhushan
2023-03-24 22:03   ` Guenter Roeck
2023-04-13 10:22     ` [EXT] " Bharat Bhushan
2023-03-26  8:40   ` kernel test robot
2023-03-24 20:17 ` [PATCH 1/2] dt-bindings: watchdog: marvell octeonTX2 " Rob Herring
2023-03-24 20:34 ` Rob Herring
2023-04-13 10:14   ` [EXT] " Bharat Bhushan

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.