[PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Hitomi Hasegawa]

 The interrupt is set using pseudo-NMI if it is available. Arm has a
 diagnostic interrupt feature called "Arm Generic Diagnostic Dump and
 Reset device", but the A64FX does not support this feature and instead
 has its own device definition.

Signed-off-by: Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>
---
 MAINTAINERS                      |   5 +
 drivers/soc/Kconfig              |   1 +
 drivers/soc/Makefile             |   1 +
 drivers/soc/fujitsu/Kconfig      |  13 +++
 drivers/soc/fujitsu/Makefile     |   3 +
 drivers/soc/fujitsu/a64fx-diag.c | 157 +++++++++++++++++++++++++++++++
 6 files changed, 180 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile
 create mode 100644 drivers/soc/fujitsu/a64fx-diag.c

diff --git a/MAINTAINERS b/MAINTAINERS
index dd36acc87ce6..e9663fa92a52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -239,6 +239,11 @@ F:	include/trace/events/9p.h
 F:	include/uapi/linux/virtio_9p.h
 F:	net/9p/
 
+A64FX DIAG DRIVER
+M:	Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>
+S:	Supported
+F:	drivers/soc/fujitsu/a64fx-diag.c
+
 A8293 MEDIA DRIVER
 M:	Antti Palosaari <crope@iki.fi>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index e8a30c4c5aec..0405ff0b3be6 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -8,6 +8,7 @@ source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/canaan/Kconfig"
 source "drivers/soc/fsl/Kconfig"
+source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index a05e9fbcd3e0..86596405a39c 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SOC_CANAAN)	+= canaan/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-y				+= fujitsu/
 obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
 obj-y				+= imx/
 obj-y				+= ixp4xx/
diff --git a/drivers/soc/fujitsu/Kconfig b/drivers/soc/fujitsu/Kconfig
new file mode 100644
index 000000000000..b41cdac67637
--- /dev/null
+++ b/drivers/soc/fujitsu/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "fujitsu SoC drivers"
+
+config A64FX_DIAG
+	bool "A64FX diag driver"
+	depends on ARM64
+	help
+	  Say Y here if you want to enable diag interrupt on A64FX.
+	  This driver uses pseudo-NMI if available.
+
+	  If unsure, say N.
+
+endmenu
diff --git a/drivers/soc/fujitsu/Makefile b/drivers/soc/fujitsu/Makefile
new file mode 100644
index 000000000000..945bc1c14ad0
--- /dev/null
+++ b/drivers/soc/fujitsu/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_A64FX_DIAG)	+= a64fx-diag.o
diff --git a/drivers/soc/fujitsu/a64fx-diag.c b/drivers/soc/fujitsu/a64fx-diag.c
new file mode 100644
index 000000000000..3e74e391017b
--- /dev/null
+++ b/drivers/soc/fujitsu/a64fx-diag.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A64FX diag driver.
+ */
+
+#include <linux/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define A64FX_DIAG_IRQ 1
+#define BMC_DIAG_INTERRUPT_STATUS_OFFSET (0x0044)
+#define BMC_INTERRUPT_STATUS_MASK ((1U) << 31)
+#define BMC_DIAG_INTERRUPT_ENABLE_OFFSET (0x0040)
+#define BMC_INTERRUPT_ENABLE_MASK ((1U) << 31)
+
+struct a64fx_diag_priv {
+	int irq;
+	void __iomem *mmsc_reg_base;
+	bool has_nmi;
+};
+
+static irqreturn_t a64fx_panic_interrupt_nmi(int irq, void *dev_id)
+{
+	nmi_panic(NULL, "a64fx_panic_interrupt: interrupt received\n");
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t a64fx_panic_interrupt_irq(int irq, void *dev_id)
+{
+	panic("a64fx_panic_interrupt: interrupt received\n");
+
+	return IRQ_HANDLED;
+}
+
+static void a64fx_diag_interrupt_clear(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_status_reg_addr;
+
+	diag_status_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_STATUS_OFFSET;
+	mmsc = readl_relaxed(diag_status_reg_addr);
+	if (mmsc & BMC_INTERRUPT_STATUS_MASK)
+		writel_relaxed(BMC_INTERRUPT_STATUS_MASK, (void *)diag_status_reg_addr);
+}
+
+static void a64fx_diag_interrupt_enable(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_enable_reg_addr;
+
+	diag_enable_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_ENABLE_OFFSET;
+	mmsc = readl_relaxed(diag_enable_reg_addr);
+	if (!(mmsc & BMC_INTERRUPT_ENABLE_MASK)) {
+		mmsc |= BMC_INTERRUPT_STATUS_MASK;
+		writel_relaxed(mmsc, (void *)diag_enable_reg_addr);
+	}
+}
+
+static void a64fx_diag_interrupt_disable(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_enable_reg_addr;
+
+	diag_enable_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_ENABLE_OFFSET;
+	mmsc = readl_relaxed(diag_enable_reg_addr);
+	if (mmsc & BMC_INTERRUPT_ENABLE_MASK) {
+		mmsc &= ~BMC_INTERRUPT_ENABLE_MASK;
+		writel_relaxed(mmsc, (void *)diag_enable_reg_addr);
+	}
+}
+
+static int a64fx_diag_probe(struct platform_device *pdev)
+{
+	int ret;
+	unsigned long irq_flags;
+	struct device *dev = &pdev->dev;
+	struct a64fx_diag_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(struct a64fx_diag_priv), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
+
+	priv->mmsc_reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->mmsc_reg_base))
+		return PTR_ERR(priv->mmsc_reg_base);
+
+	priv->irq = platform_get_irq(pdev, A64FX_DIAG_IRQ);
+	if (priv->irq < 0)
+		return priv->irq;
+
+	platform_set_drvdata(pdev, priv);
+
+	a64fx_diag_interrupt_clear(priv);
+	a64fx_diag_interrupt_enable(priv);
+
+	irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
+		   IRQF_NO_THREAD;
+	ret = request_nmi(priv->irq, &a64fx_panic_interrupt_nmi, irq_flags,
+			"a64fx_diag_nmi", NULL);
+	if (ret) {
+		ret = request_irq(priv->irq, &a64fx_panic_interrupt_irq,
+				irq_flags, "a64fx_diag_irq", NULL);
+		if (ret) {
+			dev_err(dev, "cannot register IRQ %d\n", ret);
+			return ret;
+		}
+		enable_irq(priv->irq);
+		priv->has_nmi = false;
+		dev_info(dev, "registered for IRQ %d\n", priv->irq);
+	} else {
+		enable_nmi(priv->irq);
+		priv->has_nmi = true;
+		dev_info(dev, "registered for NMI %d\n", priv->irq);
+	}
+
+	return 0;
+}
+
+static int __exit a64fx_diag_remove(struct platform_device *pdev)
+{
+	struct a64fx_diag_priv *priv = platform_get_drvdata(pdev);
+
+	a64fx_diag_interrupt_disable(priv);
+	a64fx_diag_interrupt_clear(priv);
+
+	if (priv->has_nmi)
+		free_nmi(priv->irq, NULL);
+	else
+		free_irq(priv->irq, NULL);
+
+	return 0;
+}
+
+static const struct acpi_device_id a64fx_diag_acpi_match[] = {
+	{ "FUJI2007", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, a64fx_diag_acpi_match);
+
+
+static struct platform_driver a64fx_diag_driver = {
+	.driver = {
+		.name = "a64fx_diag_driver",
+		.acpi_match_table = ACPI_PTR(a64fx_diag_acpi_match),
+	},
+	.probe = a64fx_diag_probe,
+	.remove = a64fx_diag_remove,
+};
+
+module_platform_driver(a64fx_diag_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>");
+MODULE_DESCRIPTION("A64FX diag driver");
-- 
2.27.0

[PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Hitomi Hasegawa]

 The interrupt is set using pseudo-NMI if it is available. Arm has a
 diagnostic interrupt feature called "Arm Generic Diagnostic Dump and
 Reset device", but the A64FX does not support this feature and instead
 has its own device definition.

Signed-off-by: Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>
---
 MAINTAINERS                      |   5 +
 drivers/soc/Kconfig              |   1 +
 drivers/soc/Makefile             |   1 +
 drivers/soc/fujitsu/Kconfig      |  13 +++
 drivers/soc/fujitsu/Makefile     |   3 +
 drivers/soc/fujitsu/a64fx-diag.c | 157 +++++++++++++++++++++++++++++++
 6 files changed, 180 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile
 create mode 100644 drivers/soc/fujitsu/a64fx-diag.c

diff --git a/MAINTAINERS b/MAINTAINERS
index dd36acc87ce6..e9663fa92a52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -239,6 +239,11 @@ F:	include/trace/events/9p.h
 F:	include/uapi/linux/virtio_9p.h
 F:	net/9p/
 
+A64FX DIAG DRIVER
+M:	Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>
+S:	Supported
+F:	drivers/soc/fujitsu/a64fx-diag.c
+
 A8293 MEDIA DRIVER
 M:	Antti Palosaari <crope@iki.fi>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index e8a30c4c5aec..0405ff0b3be6 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -8,6 +8,7 @@ source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/canaan/Kconfig"
 source "drivers/soc/fsl/Kconfig"
+source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index a05e9fbcd3e0..86596405a39c 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SOC_CANAAN)	+= canaan/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-y				+= fujitsu/
 obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
 obj-y				+= imx/
 obj-y				+= ixp4xx/
diff --git a/drivers/soc/fujitsu/Kconfig b/drivers/soc/fujitsu/Kconfig
new file mode 100644
index 000000000000..b41cdac67637
--- /dev/null
+++ b/drivers/soc/fujitsu/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "fujitsu SoC drivers"
+
+config A64FX_DIAG
+	bool "A64FX diag driver"
+	depends on ARM64
+	help
+	  Say Y here if you want to enable diag interrupt on A64FX.
+	  This driver uses pseudo-NMI if available.
+
+	  If unsure, say N.
+
+endmenu
diff --git a/drivers/soc/fujitsu/Makefile b/drivers/soc/fujitsu/Makefile
new file mode 100644
index 000000000000..945bc1c14ad0
--- /dev/null
+++ b/drivers/soc/fujitsu/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_A64FX_DIAG)	+= a64fx-diag.o
diff --git a/drivers/soc/fujitsu/a64fx-diag.c b/drivers/soc/fujitsu/a64fx-diag.c
new file mode 100644
index 000000000000..3e74e391017b
--- /dev/null
+++ b/drivers/soc/fujitsu/a64fx-diag.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A64FX diag driver.
+ */
+
+#include <linux/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define A64FX_DIAG_IRQ 1
+#define BMC_DIAG_INTERRUPT_STATUS_OFFSET (0x0044)
+#define BMC_INTERRUPT_STATUS_MASK ((1U) << 31)
+#define BMC_DIAG_INTERRUPT_ENABLE_OFFSET (0x0040)
+#define BMC_INTERRUPT_ENABLE_MASK ((1U) << 31)
+
+struct a64fx_diag_priv {
+	int irq;
+	void __iomem *mmsc_reg_base;
+	bool has_nmi;
+};
+
+static irqreturn_t a64fx_panic_interrupt_nmi(int irq, void *dev_id)
+{
+	nmi_panic(NULL, "a64fx_panic_interrupt: interrupt received\n");
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t a64fx_panic_interrupt_irq(int irq, void *dev_id)
+{
+	panic("a64fx_panic_interrupt: interrupt received\n");
+
+	return IRQ_HANDLED;
+}
+
+static void a64fx_diag_interrupt_clear(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_status_reg_addr;
+
+	diag_status_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_STATUS_OFFSET;
+	mmsc = readl_relaxed(diag_status_reg_addr);
+	if (mmsc & BMC_INTERRUPT_STATUS_MASK)
+		writel_relaxed(BMC_INTERRUPT_STATUS_MASK, (void *)diag_status_reg_addr);
+}
+
+static void a64fx_diag_interrupt_enable(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_enable_reg_addr;
+
+	diag_enable_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_ENABLE_OFFSET;
+	mmsc = readl_relaxed(diag_enable_reg_addr);
+	if (!(mmsc & BMC_INTERRUPT_ENABLE_MASK)) {
+		mmsc |= BMC_INTERRUPT_STATUS_MASK;
+		writel_relaxed(mmsc, (void *)diag_enable_reg_addr);
+	}
+}
+
+static void a64fx_diag_interrupt_disable(struct a64fx_diag_priv *priv)
+{
+	u32 mmsc;
+	const void __iomem *diag_enable_reg_addr;
+
+	diag_enable_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_ENABLE_OFFSET;
+	mmsc = readl_relaxed(diag_enable_reg_addr);
+	if (mmsc & BMC_INTERRUPT_ENABLE_MASK) {
+		mmsc &= ~BMC_INTERRUPT_ENABLE_MASK;
+		writel_relaxed(mmsc, (void *)diag_enable_reg_addr);
+	}
+}
+
+static int a64fx_diag_probe(struct platform_device *pdev)
+{
+	int ret;
+	unsigned long irq_flags;
+	struct device *dev = &pdev->dev;
+	struct a64fx_diag_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(struct a64fx_diag_priv), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
+
+	priv->mmsc_reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->mmsc_reg_base))
+		return PTR_ERR(priv->mmsc_reg_base);
+
+	priv->irq = platform_get_irq(pdev, A64FX_DIAG_IRQ);
+	if (priv->irq < 0)
+		return priv->irq;
+
+	platform_set_drvdata(pdev, priv);
+
+	a64fx_diag_interrupt_clear(priv);
+	a64fx_diag_interrupt_enable(priv);
+
+	irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
+		   IRQF_NO_THREAD;
+	ret = request_nmi(priv->irq, &a64fx_panic_interrupt_nmi, irq_flags,
+			"a64fx_diag_nmi", NULL);
+	if (ret) {
+		ret = request_irq(priv->irq, &a64fx_panic_interrupt_irq,
+				irq_flags, "a64fx_diag_irq", NULL);
+		if (ret) {
+			dev_err(dev, "cannot register IRQ %d\n", ret);
+			return ret;
+		}
+		enable_irq(priv->irq);
+		priv->has_nmi = false;
+		dev_info(dev, "registered for IRQ %d\n", priv->irq);
+	} else {
+		enable_nmi(priv->irq);
+		priv->has_nmi = true;
+		dev_info(dev, "registered for NMI %d\n", priv->irq);
+	}
+
+	return 0;
+}
+
+static int __exit a64fx_diag_remove(struct platform_device *pdev)
+{
+	struct a64fx_diag_priv *priv = platform_get_drvdata(pdev);
+
+	a64fx_diag_interrupt_disable(priv);
+	a64fx_diag_interrupt_clear(priv);
+
+	if (priv->has_nmi)
+		free_nmi(priv->irq, NULL);
+	else
+		free_irq(priv->irq, NULL);
+
+	return 0;
+}
+
+static const struct acpi_device_id a64fx_diag_acpi_match[] = {
+	{ "FUJI2007", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, a64fx_diag_acpi_match);
+
+
+static struct platform_driver a64fx_diag_driver = {
+	.driver = {
+		.name = "a64fx_diag_driver",
+		.acpi_match_table = ACPI_PTR(a64fx_diag_acpi_match),
+	},
+	.probe = a64fx_diag_probe,
+	.remove = a64fx_diag_remove,
+};
+
+module_platform_driver(a64fx_diag_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com>");
+MODULE_DESCRIPTION("A64FX diag driver");
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Arnd Bergmann]

On Fri, Feb 18, 2022 at 10:20 AM Hitomi Hasegawa
<hasegawa-hitomi@fujitsu.com> wrote:
> +
> +static irqreturn_t a64fx_panic_interrupt_nmi(int irq, void *dev_id)
> +{
> +       nmi_panic(NULL, "a64fx_panic_interrupt: interrupt received\n");
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t a64fx_panic_interrupt_irq(int irq, void *dev_id)
> +{
> +       panic("a64fx_panic_interrupt: interrupt received\n");
> +
> +       return IRQ_HANDLED;
> +}

Is panic() the best action for this? I'm not familiar with this feature, but
it sounds like handle_sysrq() might be more appropriate. How does a
user trigger the event, and is there any sideband data that can be
sent along with the interrupt?

> +static void a64fx_diag_interrupt_clear(struct a64fx_diag_priv *priv)
> +{
> +       u32 mmsc;
> +       const void __iomem *diag_status_reg_addr;
> +
> +       diag_status_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_STATUS_OFFSET;
> +       mmsc = readl_relaxed(diag_status_reg_addr);
> +       if (mmsc & BMC_INTERRUPT_STATUS_MASK)
> +               writel_relaxed(BMC_INTERRUPT_STATUS_MASK, (void *)diag_status_reg_addr);
> +}

Normal driver code should use readl()/writel(), not
readl_relaxed()/writel_relaxed().
If you do need the relaxed versions, please add a comment explaining why,
otherwise change the code here and below.

       Arnd

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Arnd Bergmann]

On Fri, Feb 18, 2022 at 10:20 AM Hitomi Hasegawa
<hasegawa-hitomi@fujitsu.com> wrote:
> +
> +static irqreturn_t a64fx_panic_interrupt_nmi(int irq, void *dev_id)
> +{
> +       nmi_panic(NULL, "a64fx_panic_interrupt: interrupt received\n");
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t a64fx_panic_interrupt_irq(int irq, void *dev_id)
> +{
> +       panic("a64fx_panic_interrupt: interrupt received\n");
> +
> +       return IRQ_HANDLED;
> +}

Is panic() the best action for this? I'm not familiar with this feature, but
it sounds like handle_sysrq() might be more appropriate. How does a
user trigger the event, and is there any sideband data that can be
sent along with the interrupt?

> +static void a64fx_diag_interrupt_clear(struct a64fx_diag_priv *priv)
> +{
> +       u32 mmsc;
> +       const void __iomem *diag_status_reg_addr;
> +
> +       diag_status_reg_addr = priv->mmsc_reg_base + BMC_DIAG_INTERRUPT_STATUS_OFFSET;
> +       mmsc = readl_relaxed(diag_status_reg_addr);
> +       if (mmsc & BMC_INTERRUPT_STATUS_MASK)
> +               writel_relaxed(BMC_INTERRUPT_STATUS_MASK, (void *)diag_status_reg_addr);
> +}

Normal driver code should use readl()/writel(), not
readl_relaxed()/writel_relaxed().
If you do need the relaxed versions, please add a comment explaining why,
otherwise change the code here and below.

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Arnd.

> Is panic() the best action for this? I'm not familiar with this feature, but
> it sounds like handle_sysrq() might be more appropriate. How does a
> user trigger the event, and is there any sideband data that can be
> sent along with the interrupt?

This event is triggered by sending "$ ipmitool chassis power diag" to the BMC.
It does not send any data when it interrupts.
Use sysrq can enable to the user to control enable/disable, it seems better
to use handle_sysrq().

> Normal driver code should use readl()/writel(), not
> readl_relaxed()/writel_relaxed().
> If you do need the relaxed versions, please add a comment explaining why,
> otherwise change the code here and below.

Okay, I will correct it as indicated.

Thank you for pointing out the problem. I will release v2 which fixes
the problem.

Hitomi Hasegawa

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Arnd.

> Is panic() the best action for this? I'm not familiar with this feature, but
> it sounds like handle_sysrq() might be more appropriate. How does a
> user trigger the event, and is there any sideband data that can be
> sent along with the interrupt?

This event is triggered by sending "$ ipmitool chassis power diag" to the BMC.
It does not send any data when it interrupts.
Use sysrq can enable to the user to control enable/disable, it seems better
to use handle_sysrq().

> Normal driver code should use readl()/writel(), not
> readl_relaxed()/writel_relaxed().
> If you do need the relaxed versions, please add a comment explaining why,
> otherwise change the code here and below.

Okay, I will correct it as indicated.

Thank you for pointing out the problem. I will release v2 which fixes
the problem.

Hitomi Hasegawa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Greg and Jiri,

I'm trying to implement a feature for A64FX that will panic when
a diagnostic interrupt is received.
Arnd suggested that handle_sysrq() should be used
but I'm wonderingif it's ok to call it in NMI.
Would you please let me know if we can call handle_sysrq() in NMI?

Thank you.
Hitomi Hasegawa

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Greg and Jiri,

I'm trying to implement a feature for A64FX that will panic when
a diagnostic interrupt is received.
Arnd suggested that handle_sysrq() should be used
but I'm wonderingif it's ok to call it in NMI.
Would you please let me know if we can call handle_sysrq() in NMI?

Thank you.
Hitomi Hasegawa
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[gregkh]

On Fri, Feb 25, 2022 at 10:35:10AM +0000, hasegawa-hitomi@fujitsu.com wrote:
> Hi Greg and Jiri,
> 
> I'm trying to implement a feature for A64FX that will panic when
> a diagnostic interrupt is received.
> Arnd suggested that handle_sysrq() should be used
> but I'm wonderingif it's ok to call it in NMI.
> Would you please let me know if we can call handle_sysrq() in NMI?

I doubt it, but try it and see!

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[gregkh]

On Fri, Feb 25, 2022 at 10:35:10AM +0000, hasegawa-hitomi@fujitsu.com wrote:
> Hi Greg and Jiri,
> 
> I'm trying to implement a feature for A64FX that will panic when
> a diagnostic interrupt is received.
> Arnd suggested that handle_sysrq() should be used
> but I'm wonderingif it's ok to call it in NMI.
> Would you please let me know if we can call handle_sysrq() in NMI?

I doubt it, but try it and see!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Arnd Bergmann]

On Fri, Feb 25, 2022 at 11:35 AM hasegawa-hitomi@fujitsu.com
<hasegawa-hitomi@fujitsu.com> wrote:
>
> Hi Greg and Jiri,
>
> I'm trying to implement a feature for A64FX that will panic when
> a diagnostic interrupt is received.
> Arnd suggested that handle_sysrq() should be used
> but I'm wonderingif it's ok to call it in NMI.
> Would you please let me know if we can call handle_sysrq() in NMI?

I see some work from Sumit Garg to make it possible, but this was never
merged, so presumably it is not safe:

https://lore.kernel.org/linux-arm-kernel/CAFA6WYOWHgmYYt=KGXDh2hKiuy_rQbJfi279ev0+s-Qh7L21kA@mail.gmail.com/t/#m2b5006f08581448020eb24566927a104d0b95c44

Sumit has worked on some related areas as well, and may have additional
ideas for you.

        Arnd

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Arnd Bergmann]

On Fri, Feb 25, 2022 at 11:35 AM hasegawa-hitomi@fujitsu.com
<hasegawa-hitomi@fujitsu.com> wrote:
>
> Hi Greg and Jiri,
>
> I'm trying to implement a feature for A64FX that will panic when
> a diagnostic interrupt is received.
> Arnd suggested that handle_sysrq() should be used
> but I'm wonderingif it's ok to call it in NMI.
> Would you please let me know if we can call handle_sysrq() in NMI?

I see some work from Sumit Garg to make it possible, but this was never
merged, so presumably it is not safe:

https://lore.kernel.org/linux-arm-kernel/CAFA6WYOWHgmYYt=KGXDh2hKiuy_rQbJfi279ev0+s-Qh7L21kA@mail.gmail.com/t/#m2b5006f08581448020eb24566927a104d0b95c44

Sumit has worked on some related areas as well, and may have additional
ideas for you.

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Sumit Garg]

Hi Arnd, Hitomi,

On Fri, 25 Feb 2022 at 16:50, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Feb 25, 2022 at 11:35 AM hasegawa-hitomi@fujitsu.com
> <hasegawa-hitomi@fujitsu.com> wrote:
> >
> > Hi Greg and Jiri,
> >
> > I'm trying to implement a feature for A64FX that will panic when
> > a diagnostic interrupt is received.
> > Arnd suggested that handle_sysrq() should be used
> > but I'm wonderingif it's ok to call it in NMI.
> > Would you please let me know if we can call handle_sysrq() in NMI?
>
> I see some work from Sumit Garg to make it possible, but this was never
> merged, so presumably it is not safe:
>
> https://lore.kernel.org/linux-arm-kernel/CAFA6WYOWHgmYYt=KGXDh2hKiuy_rQbJfi279ev0+s-Qh7L21kA@mail.gmail.com/t/#m2b5006f08581448020eb24566927a104d0b95c44
>

Yeah it had some tricky bits but it wasn't the reason for the work
being stalled. Actually it turned out to be cumbersome to allow all
8250 UARTs to be driven from NMI interrupts as well. But given this
special diagnostic interrupt on A64FX turned as pseudo NMI, it
shouldn't be that hard to test it with this [1] minimal patch.

[1] https://lkml.org/lkml/2022/2/28/83

-Sumit

> Sumit has worked on some related areas as well, and may have additional
> ideas for you.
>
>         Arnd

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[Sumit Garg]

Hi Arnd, Hitomi,

On Fri, 25 Feb 2022 at 16:50, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Feb 25, 2022 at 11:35 AM hasegawa-hitomi@fujitsu.com
> <hasegawa-hitomi@fujitsu.com> wrote:
> >
> > Hi Greg and Jiri,
> >
> > I'm trying to implement a feature for A64FX that will panic when
> > a diagnostic interrupt is received.
> > Arnd suggested that handle_sysrq() should be used
> > but I'm wonderingif it's ok to call it in NMI.
> > Would you please let me know if we can call handle_sysrq() in NMI?
>
> I see some work from Sumit Garg to make it possible, but this was never
> merged, so presumably it is not safe:
>
> https://lore.kernel.org/linux-arm-kernel/CAFA6WYOWHgmYYt=KGXDh2hKiuy_rQbJfi279ev0+s-Qh7L21kA@mail.gmail.com/t/#m2b5006f08581448020eb24566927a104d0b95c44
>

Yeah it had some tricky bits but it wasn't the reason for the work
being stalled. Actually it turned out to be cumbersome to allow all
8250 UARTs to be driven from NMI interrupts as well. But given this
special diagnostic interrupt on A64FX turned as pseudo NMI, it
shouldn't be that hard to test it with this [1] minimal patch.

[1] https://lkml.org/lkml/2022/2/28/83

-Sumit

> Sumit has worked on some related areas as well, and may have additional
> ideas for you.
>
>         Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Greg, Arnd, and Sumit,

Thank you for your comments and information. I did not know there is 
already a related patch.
I will test Sumit's v2 patch and reply there with the results later. Also, I
will make the v2 patch, based on that patch.

Thank you
Hitomi Hasegawa

Re: [PATCH] soc: fujitsu: Add A64FX diagnostic interrupt driver

[hasegawa-hitomi]

Hi Greg, Arnd, and Sumit,

Thank you for your comments and information. I did not know there is 
already a related patch.
I will test Sumit's v2 patch and reply there with the results later. Also, I
will make the v2 patch, based on that patch.

Thank you
Hitomi Hasegawa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel