linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs
@ 2022-07-20  6:00 Appana Durga Kedareswara rao
  2022-07-20  6:00 ` [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager Appana Durga Kedareswara rao
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Appana Durga Kedareswara rao @ 2022-07-20  6:00 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git

This patch series adds support for TMR Manager and TMR Inject IPs.
The TMR Manager is responsible for handling the TMR subsystem state,
including fault detection and error recovery. The core is triplicated
in each of the sub-blocks in the TMR subsystem, and provides majority
voting of its internal state.
The TMR Inject core provides functional fault injection by changing
selected MicroBlaze instructions, which provides the possibility to
verify that the TMR subsystem error detection and fault recovery logic
is working properly.

For more details about TMR Manager and Inject IPs please refer PG268[1].
[1]: https://docs.xilinx.com/r/en-US/pg268-tmr/Triple-Modular-Redundancy-TMR-v1.0-LogiCORE-IP-Product-Guide-PG268

This patch series has a dependency on the patch series
https://www.spinics.net/lists/arm-kernel/msg991887.html

Appana Durga Kedareswara rao (4):
  dt-bindings: misc: tmr-manager: Add device-tree binding for TMR
    Manager
  drivers: misc: Add Support for TMR Manager
  dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  drivers: misc: Add Support for TMR Inject IP

 .../testing/sysfs-driver-xilinx-tmr-inject    |  16 ++
 .../testing/sysfs-driver-xilinx-tmr-manager   |  27 ++
 .../bindings/misc/xlnx,tmr-inject.yaml        |  47 ++++
 .../bindings/misc/xlnx,tmr-manager.yaml       |  47 ++++
 MAINTAINERS                                   |  14 +
 drivers/misc/Kconfig                          |  20 ++
 drivers/misc/Makefile                         |   2 +
 drivers/misc/xilinx_tmr_inject.c              | 186 +++++++++++++
 drivers/misc/xilinx_tmr_manager.c             | 253 ++++++++++++++++++
 9 files changed, 612 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
 create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
 create mode 100644 Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
 create mode 100644 Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
 create mode 100644 drivers/misc/xilinx_tmr_inject.c
 create mode 100644 drivers/misc/xilinx_tmr_manager.c

-- 
2.25.1


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

* [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager
  2022-07-20  6:00 [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs Appana Durga Kedareswara rao
@ 2022-07-20  6:00 ` Appana Durga Kedareswara rao
  2022-07-20  6:15   ` Krzysztof Kozlowski
  2022-07-20  6:00 ` [PATCH v2 2/4] drivers: misc: Add Support " Appana Durga Kedareswara rao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Appana Durga Kedareswara rao @ 2022-07-20  6:00 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

Triple Modular Redundancy(TMR) subsystem contains three microblaze cores,
subsystem is fault-tolerant and continues to operate nominally after
encountering an error. Together with the capability to detect and recover
from errors, the implementation ensures the reliability of the entire
subsystem.  TMR Manager is responsible for performing recovery of the
subsystem detects the fault via a break signal it invokes microblaze
software break handler which calls the tmr manager driver api to
update the error count and status.

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
---
Changes for v2:
--> Improved description for xlnx,magic1 property as suggested by Krzysztof.
--> Fixed style issues (indentation of example node title description
etc..)
 .../bindings/misc/xlnx,tmr-manager.yaml       | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml

diff --git a/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
new file mode 100644
index 000000000000..f7e6ac1d5867
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/xlnx,tmr-manager.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Triple Modular Redundancy(TMR) Manager IP
+
+maintainers:
+  - Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
+
+description: |
+  The Triple Modular Redundancy(TMR) Manager is responsible for handling the
+  TMR subsystem state, including fault detection and error recovery. The core
+  is triplicated in each of the sub-blocks in the TMR subsystem, and provides
+  majority voting of its internal state.
+
+properties:
+  compatible:
+    enum:
+      - xlnx,tmr-manager-1.0
+
+  reg:
+    maxItems: 1
+
+  xlnx,magic1:
+    minimum: 0
+    maximum: 255
+    description:
+      Magic byte 1, When configured it allows the controller to perform
+      recovery.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - xlnx,magic1
+
+additionalProperties: false
+
+examples:
+  - |
+    tmr-manager@44a10000 {
+        compatible = "xlnx,tmr-manager-1.0";
+        reg = <0x44a10000 0x10000>;
+        xlnx,magic1 = <0x46>;
+    };
-- 
2.25.1


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

* [PATCH v2 2/4] drivers: misc: Add Support for TMR Manager
  2022-07-20  6:00 [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs Appana Durga Kedareswara rao
  2022-07-20  6:00 ` [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager Appana Durga Kedareswara rao
@ 2022-07-20  6:00 ` Appana Durga Kedareswara rao
  2022-07-28 14:27   ` Greg KH
  2022-07-20  6:00 ` [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject Appana Durga Kedareswara rao
  2022-07-20  6:00 ` [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP Appana Durga Kedareswara rao
  3 siblings, 1 reply; 19+ messages in thread
From: Appana Durga Kedareswara rao @ 2022-07-20  6:00 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

Triple Modular Redundancy(TMR) subsystem contains three microblaze cores,
subsystem is fault-tolerant and continues to operate nominally after
encountering an error. Together with the capability to detect and recover
from errors, the implementation ensures the reliability of the entire
subsystem.  TMR Manager is responsible for performing recovery of the
subsystem detects the fault via a break signal it invokes microblaze
software break handler which calls the tmr manager driver api to
update the error count and status, added support for fault detection
feature via sysfs interface.

Usage:
To know the hardware status:
cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
To know the break handler count(Error count):
cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
---
Changes for v2:
--> Added Examples for sysfs entries
--> Removed uneeded struct dev from the driver private structure
--> Fixed style issues (Used resource_size_t instead of uintptr_t)
--> Updated driver to use sysfs_emit() API instead of sprintf() API
--> Added error checks wherever applicable.
--> Fixed sysfs registration.
 .../testing/sysfs-driver-xilinx-tmr-manager   |  27 ++
 MAINTAINERS                                   |   7 +
 drivers/misc/Kconfig                          |  10 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/xilinx_tmr_manager.c             | 253 ++++++++++++++++++
 5 files changed, 298 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
 create mode 100644 drivers/misc/xilinx_tmr_manager.c

diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
new file mode 100644
index 000000000000..fc5fe7e22b09
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
@@ -0,0 +1,27 @@
+What:		/sys/devices/platform/amba_pl/<dev>/status
+Date:		June 2022
+Contact:	appana.durga.rao@xilinx.com
+Description:	This control file provides the status of the tmr manager
+		useful for getting the status of fault.
+		This file cannot be written.
+		Example:
+		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
+		  Lockstep mismatch between processor 1 and 2
+		  Lockstep mismatch between processor 2 and 3
+
+What:		/sys/devices/platform/amba_pl/<dev>/errcnt
+Date:		June 2022
+Contact:	appana.durga.rao@xilinx.com
+Description:	This control file provides the fault detection count.
+		This file cannot be written.
+		Example:
+		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt
+		  1
+
+What:		/sys/devices/platform/amba_pl/<dev>/dis_block_break
+Date:		June 2022
+Contact:	appana.durga.rao@xilinx.com
+Description:	This control file enables the break signal.
+		This file is write only.
+		Example:
+		# echo 1 > /sys/devices/platform/amba_pl/44a10000.tmr_manager/dis_block_break
diff --git a/MAINTAINERS b/MAINTAINERS
index 651616ed8ae2..732fd9ae7d9f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13080,6 +13080,13 @@ W:	http://www.monstr.eu/fdt/
 T:	git git://git.monstr.eu/linux-2.6-microblaze.git
 F:	arch/microblaze/
 
+MICROBLAZE TMR MANAGER
+M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
+S:	Supported
+F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
+F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
+F:	drivers/misc/xilinx_tmr_manager.c
+
 MICROCHIP AT91 DMA DRIVERS
 M:	Ludovic Desroches <ludovic.desroches@microchip.com>
 M:	Tudor Ambarus <tudor.ambarus@microchip.com>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 41d2bb0ae23a..555ae2e33b91 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -483,6 +483,16 @@ config OPEN_DICE
 
 	  If unsure, say N.
 
+config TMR_MANAGER
+	bool "Select TMR Manager"
+	depends on MICROBLAZE && MB_MANAGER
+	help
+	  This option enables the driver developed for TMR Manager. The Triple
+	  Modular Redundancy(TMR) manager provides support for fault detection
+	  via sysfs interface.
+
+	  Say N here unless you know what you are doing.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 70e800e9127f..28b9803f909b 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
 obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
 obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
 obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
+obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
diff --git a/drivers/misc/xilinx_tmr_manager.c b/drivers/misc/xilinx_tmr_manager.c
new file mode 100644
index 000000000000..dbeca18c409f
--- /dev/null
+++ b/drivers/misc/xilinx_tmr_manager.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx TMR Subsystem.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Description:
+ * This driver is developed for TMR Manager,The Triple Modular Redundancy(TMR)
+ * Manager is responsible for handling the TMR subsystem state, including
+ * fault detection and error recovery. The core is triplicated in each of
+ * the sub-blocks in the TMR subsystem, and provides majority voting of
+ * its internal state provides soft error detection, correction and
+ * recovery. Error detection feature is provided through sysfs
+ * entries which allow the user to observer the TMR microblaze
+ * status.
+ */
+
+#include <asm/xilinx_mb_manager.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+/* TMR Manager Register offsets */
+#define XTMR_MANAGER_CR_OFFSET		0x0
+#define XTMR_MANAGER_FFR_OFFSET		0x4
+#define XTMR_MANAGER_CMR0_OFFSET	0x8
+#define XTMR_MANAGER_CMR1_OFFSET	0xC
+#define XTMR_MANAGER_BDIR_OFFSET	0x10
+#define XTMR_MANAGER_SEMIMR_OFFSET	0x1C
+
+/* Register Bitmasks/shifts */
+#define XTMR_MANAGER_CR_MAGIC1_MASK	GENMASK(7, 0)
+#define XTMR_MANAGER_CR_MAGIC2_MASK	GENMASK(15, 8)
+#define XTMR_MANAGER_CR_RIR_MASK	BIT(16)
+#define XTMR_MANAGER_FFR_LM12_MASK	BIT(0)
+#define XTMR_MANAGER_FFR_LM13_MASK	BIT(1)
+#define XTMR_MANAGER_FFR_LM23_MASK	BIT(2)
+
+#define XTMR_MANAGER_CR_MAGIC2_SHIFT	4
+#define XTMR_MANAGER_CR_RIR_SHIFT	16
+#define XTMR_MANAGER_CR_BB_SHIFT	18
+
+#define XTMR_MANAGER_MAGIC1_MAX_VAL	255
+
+/**
+ * struct xtmr_manager_dev - Driver data for TMR Manager
+ * @regs: device physical base address
+ * @cr_val: control register value
+ * @magic1: Magic 1 hardware configuration value
+ * @err_cnt: error statistics count
+ * @phys_baseaddr: Physical base address
+ */
+struct xtmr_manager_dev {
+	void __iomem *regs;
+	u32 cr_val;
+	u32 magic1;
+	u32 err_cnt;
+	resource_size_t phys_baseaddr;
+};
+
+/* IO accessors */
+static inline void xtmr_manager_write(struct xtmr_manager_dev *xtmr_manager,
+				      u32 addr, u32 value)
+{
+	iowrite32(value, xtmr_manager->regs + addr);
+}
+
+static inline u32 xtmr_manager_read(struct xtmr_manager_dev *xtmr_manager,
+				    u32 addr)
+{
+	return ioread32(xtmr_manager->regs + addr);
+}
+
+static void xmb_manager_reset_handler(struct xtmr_manager_dev *xtmr_manager)
+{
+	/* Clear the FFR Register contents as a part of recovery process. */
+	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_FFR_OFFSET, 0);
+}
+
+static void xmb_manager_update_errcnt(struct xtmr_manager_dev *xtmr_manager)
+{
+	xtmr_manager->err_cnt++;
+}
+
+static ssize_t errcnt_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%x\n", xtmr_manager->err_cnt);
+}
+static DEVICE_ATTR_RO(errcnt);
+
+static ssize_t status_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
+	size_t ffr;
+	int len = 0;
+
+	ffr = xtmr_manager_read(xtmr_manager, XTMR_MANAGER_FFR_OFFSET);
+	if ((ffr & XTMR_MANAGER_FFR_LM12_MASK) == XTMR_MANAGER_FFR_LM12_MASK) {
+		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
+		len += sysfs_emit_at(buf, len, "processor 1 and 2\n");
+	}
+
+	if ((ffr & XTMR_MANAGER_FFR_LM13_MASK) == XTMR_MANAGER_FFR_LM13_MASK) {
+		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
+		len += sysfs_emit_at(buf, len, "processor 1 and 3\n");
+	}
+
+	if ((ffr & XTMR_MANAGER_FFR_LM23_MASK) == XTMR_MANAGER_FFR_LM23_MASK) {
+		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
+		len += sysfs_emit_at(buf, len, "processor 2 and 3\n");
+	}
+
+	return len;
+}
+static DEVICE_ATTR_RO(status);
+
+static ssize_t dis_block_break_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t size)
+{
+	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
+	int ret;
+	long value;
+
+	ret = kstrtoul(buf, 16, &value);
+	if (ret)
+		return ret;
+
+	if (value > 1)
+		return -EINVAL;
+
+	/* unblock the break signal*/
+	xtmr_manager->cr_val &= ~(1 << XTMR_MANAGER_CR_BB_SHIFT);
+	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_CR_OFFSET,
+			   xtmr_manager->cr_val);
+	return size;
+}
+static DEVICE_ATTR_WO(dis_block_break);
+
+static struct attribute *xtmr_manager_dev_attrs[] = {
+	&dev_attr_dis_block_break.attr,
+	&dev_attr_status.attr,
+	&dev_attr_errcnt.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(xtmr_manager_dev);
+
+static void xtmr_manager_init(struct xtmr_manager_dev *xtmr_manager)
+{
+	/* Clear the SEM interrupt mask register to disable the interrupt */
+	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_SEMIMR_OFFSET, 0);
+
+	/* Allow recovery reset by default */
+	xtmr_manager->cr_val = (1 << XTMR_MANAGER_CR_RIR_SHIFT) |
+				xtmr_manager->magic1;
+	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_CR_OFFSET,
+			   xtmr_manager->cr_val);
+	/*
+	 * Configure Break Delay Initialization Register to zero so that
+	 * break occurs immediately
+	 */
+	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_BDIR_OFFSET, 0);
+
+	/*
+	 * To come out of break handler need to block the break signal
+	 * in the tmr manager, update the xtmr_manager cr_val for the same
+	 */
+	xtmr_manager->cr_val |= (1 << XTMR_MANAGER_CR_BB_SHIFT);
+
+	/*
+	 * When the break vector gets asserted because of error injection,
+	 * the break signal must be blocked before exiting from the
+	 * break handler, Below api updates the TMR manager address and
+	 * control register and error counter callback arguments,
+	 * which will be used by the break handler to block the
+	 * break and call the callback function.
+	 */
+	xmb_manager_register(xtmr_manager->phys_baseaddr, xtmr_manager->cr_val,
+			     (void *)xmb_manager_update_errcnt,
+			     xtmr_manager, (void *)xmb_manager_reset_handler);
+}
+
+/**
+ * xtmr_manager_probe - Driver probe function
+ * @pdev: Pointer to the platform_device structure
+ *
+ * This is the driver probe routine. It does all the memory
+ * allocation and creates sysfs entries for the device.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int xtmr_manager_probe(struct platform_device *pdev)
+{
+	struct xtmr_manager_dev *xtmr_manager;
+	struct resource *res;
+	int err;
+
+	xtmr_manager = devm_kzalloc(&pdev->dev, sizeof(*xtmr_manager),
+				    GFP_KERNEL);
+	if (!xtmr_manager)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	xtmr_manager->regs =  devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(xtmr_manager->regs))
+		return PTR_ERR(xtmr_manager->regs);
+
+	xtmr_manager->phys_baseaddr = res->start;
+
+	err = of_property_read_u32(pdev->dev.of_node, "xlnx,magic1",
+				   &xtmr_manager->magic1);
+	if (err < 0) {
+		dev_err(&pdev->dev, "unable to read xlnx,magic1 property");
+		return err;
+	}
+
+	if (xtmr_manager->magic1 > XTMR_MANAGER_MAGIC1_MAX_VAL) {
+		dev_err(&pdev->dev, "invalid xlnx,magic1 property value");
+		return -EINVAL;
+	}
+
+	/* Initialize TMR Manager */
+	xtmr_manager_init(xtmr_manager);
+
+	platform_set_drvdata(pdev, xtmr_manager);
+
+	return 0;
+}
+
+static const struct of_device_id xtmr_manager_of_match[] = {
+	{
+		.compatible = "xlnx,tmr-manager-1.0",
+	},
+	{ /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, xtmr_manager_of_match);
+
+static struct platform_driver xtmr_manager_driver = {
+	.driver = {
+		.name = "xilinx-tmr_manager",
+		.of_match_table = xtmr_manager_of_match,
+		.dev_groups = xtmr_manager_dev_groups,
+	},
+	.probe = xtmr_manager_probe,
+};
+module_platform_driver(xtmr_manager_driver);
+
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx TMR Manager Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20  6:00 [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs Appana Durga Kedareswara rao
  2022-07-20  6:00 ` [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager Appana Durga Kedareswara rao
  2022-07-20  6:00 ` [PATCH v2 2/4] drivers: misc: Add Support " Appana Durga Kedareswara rao
@ 2022-07-20  6:00 ` Appana Durga Kedareswara rao
  2022-07-20  6:15   ` Krzysztof Kozlowski
  2022-07-20  6:00 ` [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP Appana Durga Kedareswara rao
  3 siblings, 1 reply; 19+ messages in thread
From: Appana Durga Kedareswara rao @ 2022-07-20  6:00 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

The Triple Modular Redundancy(TMR) Inject core provides functional fault
injection by changing selected MicroBlaze instructions, which provides the
possibility to verify that the TMR subsystem error detection and fault
recovery logic is working properly.

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
---
Changes for v2:
--> Improved description for xlnx,magic property as suggested by Krzysztof.
--> Fixed style issues (indentation of example node title description
etc..)
 .../bindings/misc/xlnx,tmr-inject.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml

diff --git a/Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml b/Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
new file mode 100644
index 000000000000..cb14ea73fb4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/xlnx,tmr-inject.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Triple Modular Redundancy(TMR) Inject IP
+
+maintainers:
+  - Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
+
+description: |
+  The Triple Modular Redundancy(TMR) Inject core provides functional fault
+  injection by changing selected MicroBlaze instructions, which provides the
+  possibility to verify that the TMR subsystem error detection and fault
+  recovery logic is working properly.
+
+properties:
+  compatible:
+    enum:
+      - xlnx,tmr-inject-1.0
+
+  reg:
+    maxItems: 1
+
+  xlnx,magic:
+    minimum: 0
+    maximum: 255
+    description: |
+      Magic number, When configured it allows the controller to perform
+      recovery.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - xlnx,magic
+
+additionalProperties: false
+
+examples:
+  - |
+    fault-inject@44a30000 {
+        compatible = "xlnx,tmr-inject-1.0";
+        reg = <0x44a10000 0x10000>;
+        xlnx,magic = <0x46>;
+    };
-- 
2.25.1


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

* [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP
  2022-07-20  6:00 [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs Appana Durga Kedareswara rao
                   ` (2 preceding siblings ...)
  2022-07-20  6:00 ` [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject Appana Durga Kedareswara rao
@ 2022-07-20  6:00 ` Appana Durga Kedareswara rao
  2022-07-28 14:24   ` Greg KH
  3 siblings, 1 reply; 19+ messages in thread
From: Appana Durga Kedareswara rao @ 2022-07-20  6:00 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

The Triple Modular Redundancy(TMR) provides functional fault injection by
changing selected MicroBlaze instructions, which provides the possibility
to verify that the TMR subsystem error detection and fault recovery logic
is working properly, provided sysfs entries which allow the user to inject
a fault.

Usage:
echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
---
Changes for v2:
--> Added Examples for sysfs entries
--> Removed uneeded struct dev from the driver private structure
--> Updated driver to use sysfs_emit() API instead of sprintf() API
--> Added error checks wherever applicable.
--> Fixed sysfs registration.
 .../testing/sysfs-driver-xilinx-tmr-inject    |  16 ++
 MAINTAINERS                                   |   7 +
 drivers/misc/Kconfig                          |  10 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/xilinx_tmr_inject.c              | 186 ++++++++++++++++++
 5 files changed, 220 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
 create mode 100644 drivers/misc/xilinx_tmr_inject.c

diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
new file mode 100644
index 000000000000..d274b30ee24c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
@@ -0,0 +1,16 @@
+What:		/sys/devices/platform/amba_pl/<dev>/inject_err
+Date:		June 2022
+Contact:	appana.durga.rao@xilinx.com
+Description:	This control file allows to inject fault using tmr inject.
+		This file is write only.
+		Example:
+		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
+
+What:		/sys/devices/platform/amba_pl/<dev>/inject_cpuid
+Date:		June 2022
+Contact:	appana.durga.rao@xilinx.com
+Description:	This control file allows to configure the CPU identifier
+		to enable fault injection.
+		This file is write only.
+		Example:
+		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid
diff --git a/MAINTAINERS b/MAINTAINERS
index 732fd9ae7d9f..c903b45c204a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13087,6 +13087,13 @@ F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
 F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
 F:	drivers/misc/xilinx_tmr_manager.c
 
+MICROBLAZE TMR INJECT
+M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
+S:	Supported
+F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
+F:	Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
+F:	drivers/misc/xilinx_tmr_inject.c
+
 MICROCHIP AT91 DMA DRIVERS
 M:	Ludovic Desroches <ludovic.desroches@microchip.com>
 M:	Tudor Ambarus <tudor.ambarus@microchip.com>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 555ae2e33b91..0989c36f3051 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -493,6 +493,16 @@ config TMR_MANAGER
 
 	  Say N here unless you know what you are doing.
 
+config TMR_INJECT
+	bool "Select TMR Inject"
+	depends on TMR_MANAGER
+	help
+	  This option enables the driver developed for TMR Inject.
+	  The Triple Modular Redundancy(TMR) Inject provides
+	  fault injection.
+
+	  Say N here unless you know what you are doing.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 28b9803f909b..e9d0a709e207 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
 obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
 obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
 obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
+obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
new file mode 100644
index 000000000000..930d89e90b61
--- /dev/null
+++ b/drivers/misc/xilinx_tmr_inject.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Xilinx TMR Inject IP.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Description:
+ * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
+ * Inject provides fault injection.
+ * Fault injection and detection features are provided through sysfs entries
+ * which allow the user to generate a fault.
+ */
+
+#include <asm/xilinx_mb_manager.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+/* TMR Inject Register offsets */
+#define XTMR_INJECT_CR_OFFSET		0x0
+#define XTMR_INJECT_AIR_OFFSET		0x4
+#define XTMR_INJECT_IIR_OFFSET		0xC
+#define XTMR_INJECT_EAIR_OFFSET		0x10
+#define XTMR_INJECT_ERR_OFFSET		0x204
+
+/* Register Bitmasks/shifts */
+#define XTMR_INJECT_CR_CPUID_SHIFT	8
+#define XTMR_INJECT_CR_IE_SHIFT		10
+#define XTMR_INJECT_IIR_ADDR_MASK	GENMASK(31, 16)
+
+#define XTMR_INJECT_MAGIC_MAX_VAL	255
+
+/**
+ * struct xtmr_inject_dev - Driver data for TMR Inject
+ * @regs: device physical base address
+ * @cr_val: control register value
+ * @magic: Magic hardware configuration value
+ * @err_cnt: error statistics count
+ */
+struct xtmr_inject_dev {
+	void __iomem *regs;
+	u32 cr_val;
+	u32 magic;
+	u32 err_cnt;
+};
+
+/* IO accessors */
+static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
+				     u32 addr, u32 value)
+{
+	iowrite32(value, xtmr_inject->regs + addr);
+}
+
+static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
+				   u32 addr)
+{
+	return ioread32(xtmr_inject->regs + addr);
+}
+
+static ssize_t inject_err_store(struct device *dev,
+				struct device_attribute *attr, const char *buf,
+				size_t size)
+{
+	int ret;
+	long value;
+
+	ret = kstrtoul(buf, 16, &value);
+	if (ret)
+		return ret;
+
+	if (value > 1)
+		return -EINVAL;
+
+	xmb_inject_err();
+
+	return size;
+}
+static DEVICE_ATTR_WO(inject_err);
+
+static ssize_t inject_cpuid_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size)
+{
+	struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
+	int ret;
+	long value;
+
+	ret = kstrtoul(buf, 0, &value);
+	if (ret)
+		return ret;
+
+	if (value > 3)
+		return -EINVAL;
+
+	xtmr_inject->cr_val |= (value << XTMR_INJECT_CR_CPUID_SHIFT);
+	xtmr_inject_write(xtmr_inject, XTMR_INJECT_CR_OFFSET,
+			  xtmr_inject->cr_val);
+
+	return size;
+}
+static DEVICE_ATTR_WO(inject_cpuid);
+
+static struct attribute *xtmr_inject_dev_attrs[] = {
+	&dev_attr_inject_err.attr,
+	&dev_attr_inject_cpuid.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(xtmr_inject_dev);
+
+static void xtmr_inject_init(struct xtmr_inject_dev *xtmr_inject)
+{
+	/* Allow fault injection */
+	xtmr_inject->cr_val = xtmr_inject->magic |
+				(1 << XTMR_INJECT_CR_IE_SHIFT) |
+				(1 << XTMR_INJECT_CR_CPUID_SHIFT);
+	xtmr_inject_write(xtmr_inject, XTMR_INJECT_CR_OFFSET,
+			  xtmr_inject->cr_val);
+	/* Initialize the address inject and instruction inject registers */
+	xtmr_inject_write(xtmr_inject, XTMR_INJECT_AIR_OFFSET,
+			  XMB_INJECT_ERR_OFFSET);
+	xtmr_inject_write(xtmr_inject, XTMR_INJECT_IIR_OFFSET,
+			  XMB_INJECT_ERR_OFFSET & XTMR_INJECT_IIR_ADDR_MASK);
+}
+
+/**
+ * xtmr_inject_probe - Driver probe function
+ * @pdev: Pointer to the platform_device structure
+ *
+ * This is the driver probe routine. It does all the memory
+ * allocation and creates sysfs entries for the device.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int xtmr_inject_probe(struct platform_device *pdev)
+{
+	struct xtmr_inject_dev *xtmr_inject;
+	int err;
+
+	xtmr_inject = devm_kzalloc(&pdev->dev, sizeof(*xtmr_inject),
+				   GFP_KERNEL);
+	if (!xtmr_inject)
+		return -ENOMEM;
+
+	xtmr_inject->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(xtmr_inject->regs))
+		return PTR_ERR(xtmr_inject->regs);
+
+	err = of_property_read_u32(pdev->dev.of_node, "xlnx,magic",
+				   &xtmr_inject->magic);
+	if (err < 0) {
+		dev_err(&pdev->dev, "unable to read xlnx,magic property");
+		return err;
+	}
+
+	if (xtmr_inject->magic > XTMR_INJECT_MAGIC_MAX_VAL) {
+		dev_err(&pdev->dev, "invalid xlnx,magic property value");
+		return -EINVAL;
+	}
+
+	/* Initialize TMR Inject */
+	xtmr_inject_init(xtmr_inject);
+
+	platform_set_drvdata(pdev, xtmr_inject);
+
+	return 0;
+}
+
+static const struct of_device_id xtmr_inject_of_match[] = {
+	{
+		.compatible = "xlnx,tmr-inject-1.0",
+	},
+	{ /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, xtmr_inject_of_match);
+
+static struct platform_driver xtmr_inject_driver = {
+	.driver = {
+		.name = "xilinx-tmr_inject",
+		.of_match_table = xtmr_inject_of_match,
+		.dev_groups = xtmr_inject_dev_groups,
+	},
+	.probe = xtmr_inject_probe,
+};
+module_platform_driver(xtmr_inject_driver);
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx TMR Inject Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* Re: [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager
  2022-07-20  6:00 ` [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager Appana Durga Kedareswara rao
@ 2022-07-20  6:15   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-20  6:15 UTC (permalink / raw)
  To: Appana Durga Kedareswara rao, robh+dt, krzysztof.kozlowski+dt,
	michal.simek, derek.kiernan, dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> Triple Modular Redundancy(TMR) subsystem contains three microblaze cores,
> subsystem is fault-tolerant and continues to operate nominally after
> encountering an error. Together with the capability to detect and recover
> from errors, the implementation ensures the reliability of the entire
> subsystem.  TMR Manager is responsible for performing recovery of the
> subsystem detects the fault via a break signal it invokes microblaze
> software break handler which calls the tmr manager driver api to
> update the error count and status.
> 
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

Use one identity, not two.

> ---
> Changes for v2:
> --> Improved description for xlnx,magic1 property as suggested by Krzysztof.
> --> Fixed style issues (indentation of example node title description
> etc..)
>  .../bindings/misc/xlnx,tmr-manager.yaml       | 47 +++++++++++++++++++

It's a pity it was put into misc, but since we did not reach consensus
during last discussion and there is no fault-handler-like subsystem yet,
we might move it there later:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20  6:00 ` [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject Appana Durga Kedareswara rao
@ 2022-07-20  6:15   ` Krzysztof Kozlowski
  2022-07-20  8:26     ` Michal Simek
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-20  6:15 UTC (permalink / raw)
  To: Appana Durga Kedareswara rao, robh+dt, krzysztof.kozlowski+dt,
	michal.simek, derek.kiernan, dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> The Triple Modular Redundancy(TMR) Inject core provides functional fault
> injection by changing selected MicroBlaze instructions, which provides the
> possibility to verify that the TMR subsystem error detection and fault
> recovery logic is working properly.
> 
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

Keep only one SoB.

> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20  6:15   ` Krzysztof Kozlowski
@ 2022-07-20  8:26     ` Michal Simek
  2022-07-20  9:35       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Michal Simek @ 2022-07-20  8:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Appana Durga Kedareswara rao, robh+dt,
	krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao



On 7/20/22 08:15, Krzysztof Kozlowski wrote:
> On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>
>> The Triple Modular Redundancy(TMR) Inject core provides functional fault
>> injection by changing selected MicroBlaze instructions, which provides the
>> possibility to verify that the TMR subsystem error detection and fault
>> recovery logic is working properly.
>>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> Keep only one SoB.

nit: First of all it is from xilinx.com that's why xilinx.com should be the first.

Just for my understanding about guidance here.
Code was developed by Xilinx before acquisition with AMD. And because it was 
picked from vendor tree origin xilinx.com was there to keep origin author there. 
And upstreaming is done by new company. I can't see nothing wrong on keeping 
both emails there but that's why my opinion. Definitely not a problem to remove 
one of them but wanted to make sure that we do it properly for all our submissions.

Thanks,
Michal

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20  8:26     ` Michal Simek
@ 2022-07-20  9:35       ` Krzysztof Kozlowski
  2022-07-20 12:09         ` Michal Simek
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-20  9:35 UTC (permalink / raw)
  To: Michal Simek, Appana Durga Kedareswara rao, robh+dt,
	krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

On 20/07/2022 10:26, Michal Simek wrote:
> 
> 
> On 7/20/22 08:15, Krzysztof Kozlowski wrote:
>> On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
>>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>>
>>> The Triple Modular Redundancy(TMR) Inject core provides functional fault
>>> injection by changing selected MicroBlaze instructions, which provides the
>>> possibility to verify that the TMR subsystem error detection and fault
>>> recovery logic is working properly.
>>>
>>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>
>> Keep only one SoB.
> 
> nit: First of all it is from xilinx.com that's why xilinx.com should be the first.
> 
> Just for my understanding about guidance here.
> Code was developed by Xilinx before acquisition with AMD. And because it was 
> picked from vendor tree origin xilinx.com was there to keep origin author there. 
> And upstreaming is done by new company. I can't see nothing wrong on keeping 
> both emails there but that's why my opinion. Definitely not a problem to remove 
> one of them but wanted to make sure that we do it properly for all our submissions.

It's the same person. No need for two SoBs from the same person. Since
AMD acquired Xilinx, it holds all copyrights thus @amd.com person does
not have to include previous SoB. He/She/They has the permission from
employer to submit it. The second SoB is just redundant - brings no
actual information. Otherwise please tell me which piece of DCO the
additional SoB adds/solves (comparing to single SoB - @amd.com)?

Similarly when you change jobs while resending your patch - you do not
add new SoB but just keep SoB from @previous-company.com.


Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20  9:35       ` Krzysztof Kozlowski
@ 2022-07-20 12:09         ` Michal Simek
  2022-07-20 12:21           ` Krzysztof Kozlowski
  2022-07-20 13:21           ` Rao, Appana Durga Kedareswara
  0 siblings, 2 replies; 19+ messages in thread
From: Michal Simek @ 2022-07-20 12:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Appana Durga Kedareswara rao, robh+dt,
	krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao



On 7/20/22 11:35, Krzysztof Kozlowski wrote:
> On 20/07/2022 10:26, Michal Simek wrote:
>>
>>
>> On 7/20/22 08:15, Krzysztof Kozlowski wrote:
>>> On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
>>>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>>>
>>>> The Triple Modular Redundancy(TMR) Inject core provides functional fault
>>>> injection by changing selected MicroBlaze instructions, which provides the
>>>> possibility to verify that the TMR subsystem error detection and fault
>>>> recovery logic is working properly.
>>>>
>>>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>>>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>>
>>> Keep only one SoB.
>>
>> nit: First of all it is from xilinx.com that's why xilinx.com should be the first.
>>
>> Just for my understanding about guidance here.
>> Code was developed by Xilinx before acquisition with AMD. And because it was
>> picked from vendor tree origin xilinx.com was there to keep origin author there.
>> And upstreaming is done by new company. I can't see nothing wrong on keeping
>> both emails there but that's why my opinion. Definitely not a problem to remove
>> one of them but wanted to make sure that we do it properly for all our submissions.
> 
> It's the same person. No need for two SoBs from the same person. Since
> AMD acquired Xilinx, it holds all copyrights thus @amd.com person does
> not have to include previous SoB. He/She/They has the permission from
> employer to submit it. The second SoB is just redundant - brings no
> actual information. Otherwise please tell me which piece of DCO the
> additional SoB adds/solves (comparing to single SoB - @amd.com)?

ok. It means enough to choose one now. I am aware about some IT issues in 
progress that's why that patches can come from xilinx.com or amd.com for some 
time time.
Kedar: please just choose one.


> Similarly when you change jobs while resending your patch - you do not
> add new SoB but just keep SoB from @previous-company.com.

IMHO That would be more questionable when you create changes in origin series 
and new employer pays you to do the work.

If it is 3rd party company picking series where upstreaming is not finished you 
will expect that 3rd party will add their sob lines there too.

Thanks,
Michal

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20 12:09         ` Michal Simek
@ 2022-07-20 12:21           ` Krzysztof Kozlowski
  2022-07-20 12:35             ` Michal Simek
  2022-07-20 13:21           ` Rao, Appana Durga Kedareswara
  1 sibling, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-20 12:21 UTC (permalink / raw)
  To: Michal Simek, Appana Durga Kedareswara rao, robh+dt,
	krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

On 20/07/2022 14:09, Michal Simek wrote:
>> Similarly when you change jobs while resending your patch - you do not
>> add new SoB but just keep SoB from @previous-company.com.
> 
> IMHO That would be more questionable when you create changes in origin series 
> and new employer pays you to do the work.

True, when changes happen in between, it gets tricky.

> If it is 3rd party company picking series where upstreaming is not finished you 
> will expect that 3rd party will add their sob lines there too.

Of course, because this is different person. The SoB comes always from a
person, although he might represent company.

Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20 12:21           ` Krzysztof Kozlowski
@ 2022-07-20 12:35             ` Michal Simek
  0 siblings, 0 replies; 19+ messages in thread
From: Michal Simek @ 2022-07-20 12:35 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Appana Durga Kedareswara rao, robh+dt,
	krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, gregkh
  Cc: appanad, devicetree, linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao



On 7/20/22 14:21, Krzysztof Kozlowski wrote:
> On 20/07/2022 14:09, Michal Simek wrote:
>>> Similarly when you change jobs while resending your patch - you do not
>>> add new SoB but just keep SoB from @previous-company.com.
>>
>> IMHO That would be more questionable when you create changes in origin series
>> and new employer pays you to do the work.
> 
> True, when changes happen in between, it gets tricky.
> 
>> If it is 3rd party company picking series where upstreaming is not finished you
>> will expect that 3rd party will add their sob lines there too.
> 
> Of course, because this is different person. The SoB comes always from a
> person, although he might represent company.

Good chat.

Cheers,
Michal




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

* Re: [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject
  2022-07-20 12:09         ` Michal Simek
  2022-07-20 12:21           ` Krzysztof Kozlowski
@ 2022-07-20 13:21           ` Rao, Appana Durga Kedareswara
  1 sibling, 0 replies; 19+ messages in thread
From: Rao, Appana Durga Kedareswara @ 2022-07-20 13:21 UTC (permalink / raw)
  To: Simek, Michal, Krzysztof Kozlowski, Rao,
	Appana Durga Kedareswara, robh+dt, krzysztof.kozlowski+dt,
	michal.simek, derek.kiernan, dragan.cvetic, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-arm-kernel, git (AMD-Xilinx),
	git, Appana Durga Kedareswara rao

Hi,

Thanks for the review Krzysztof

On 20/07/22 5:39 pm, Michal Simek wrote:
> 
> 
> On 7/20/22 11:35, Krzysztof Kozlowski wrote:
>> On 20/07/2022 10:26, Michal Simek wrote:
>>>
>>>
>>> On 7/20/22 08:15, Krzysztof Kozlowski wrote:
>>>> On 20/07/2022 08:00, Appana Durga Kedareswara rao wrote:
>>>>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>>>>
>>>>> The Triple Modular Redundancy(TMR) Inject core provides functional 
>>>>> fault
>>>>> injection by changing selected MicroBlaze instructions, which 
>>>>> provides the
>>>>> possibility to verify that the TMR subsystem error detection and fault
>>>>> recovery logic is working properly.
>>>>>
>>>>> Signed-off-by: Appana Durga Kedareswara rao 
>>>>> <appana.durga.kedareswara.rao@amd.com>
>>>>> Signed-off-by: Appana Durga Kedareswara rao 
>>>>> <appana.durga.rao@xilinx.com>
>>>>
>>>> Keep only one SoB.
Surce Krzysztof will take care of it next patch series onwards.
>>>
>>> nit: First of all it is from xilinx.com that's why xilinx.com should 
>>> be the first.
>>>
>>> Just for my understanding about guidance here.
>>> Code was developed by Xilinx before acquisition with AMD. And because 
>>> it was
>>> picked from vendor tree origin xilinx.com was there to keep origin 
>>> author there.
>>> And upstreaming is done by new company. I can't see nothing wrong on 
>>> keeping
>>> both emails there but that's why my opinion. Definitely not a problem 
>>> to remove
>>> one of them but wanted to make sure that we do it properly for all 
>>> our submissions.
>>
>> It's the same person. No need for two SoBs from the same person. Since
>> AMD acquired Xilinx, it holds all copyrights thus @amd.com person does
>> not have to include previous SoB. He/She/They has the permission from
>> employer to submit it. The second SoB is just redundant - brings no
>> actual information. Otherwise please tell me which piece of DCO the
>> additional SoB adds/solves (comparing to single SoB - @amd.com)?
> 
> ok. It means enough to choose one now. I am aware about some IT issues 
> in progress that's why that patches can come from xilinx.com or amd.com 
> for some time time.
> Kedar: please just choose one.
> 

Sure Michal

Regards,
Kedar.
> 
>> Similarly when you change jobs while resending your patch - you do not
>> add new SoB but just keep SoB from @previous-company.com.
> 
> IMHO That would be more questionable when you create changes in origin 
> series and new employer pays you to do the work.
> 
> If it is 3rd party company picking series where upstreaming is not 
> finished you will expect that 3rd party will add their sob lines there too.
> 
> Thanks,
> Michal


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

* Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP
  2022-07-20  6:00 ` [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP Appana Durga Kedareswara rao
@ 2022-07-28 14:24   ` Greg KH
  2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2022-07-28 14:24 UTC (permalink / raw)
  To: Appana Durga Kedareswara rao
  Cc: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, appanad, devicetree, linux-kernel,
	linux-arm-kernel, git, git, Appana Durga Kedareswara rao

On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> The Triple Modular Redundancy(TMR) provides functional fault injection by
> changing selected MicroBlaze instructions, which provides the possibility
> to verify that the TMR subsystem error detection and fault recovery logic
> is working properly, provided sysfs entries which allow the user to inject
> a fault.

We already have a fault-injection api, why are you not using that?

> 
> Usage:
> echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
> 
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>

Odd, just one is needed.

> ---
> Changes for v2:
> --> Added Examples for sysfs entries
> --> Removed uneeded struct dev from the driver private structure
> --> Updated driver to use sysfs_emit() API instead of sprintf() API
> --> Added error checks wherever applicable.
> --> Fixed sysfs registration.
>  .../testing/sysfs-driver-xilinx-tmr-inject    |  16 ++
>  MAINTAINERS                                   |   7 +
>  drivers/misc/Kconfig                          |  10 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/xilinx_tmr_inject.c              | 186 ++++++++++++++++++
>  5 files changed, 220 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>  create mode 100644 drivers/misc/xilinx_tmr_inject.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> new file mode 100644
> index 000000000000..d274b30ee24c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> @@ -0,0 +1,16 @@
> +What:		/sys/devices/platform/amba_pl/<dev>/inject_err
> +Date:		June 2022

It's not June anymore, even when you sent this patch :(

> +Contact:	appana.durga.rao@xilinx.com
> +Description:	This control file allows to inject fault using tmr inject.
> +		This file is write only.
> +		Example:
> +		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
> +
> +What:		/sys/devices/platform/amba_pl/<dev>/inject_cpuid
> +Date:		June 2022
> +Contact:	appana.durga.rao@xilinx.com
> +Description:	This control file allows to configure the CPU identifier
> +		to enable fault injection.
> +		This file is write only.
> +		Example:
> +		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid

What errors and faults happen?  Where is that documented?  What happens
when you write to these sysfs files?  Does the system crash?  Why would
you want to use them ever?



> diff --git a/MAINTAINERS b/MAINTAINERS
> index 732fd9ae7d9f..c903b45c204a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13087,6 +13087,13 @@ F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>  F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>  F:	drivers/misc/xilinx_tmr_manager.c
>  
> +MICROBLAZE TMR INJECT
> +M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> +S:	Supported
> +F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> +F:	Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
> +F:	drivers/misc/xilinx_tmr_inject.c
> +
>  MICROCHIP AT91 DMA DRIVERS
>  M:	Ludovic Desroches <ludovic.desroches@microchip.com>
>  M:	Tudor Ambarus <tudor.ambarus@microchip.com>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 555ae2e33b91..0989c36f3051 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -493,6 +493,16 @@ config TMR_MANAGER
>  
>  	  Say N here unless you know what you are doing.
>  
> +config TMR_INJECT
> +	bool "Select TMR Inject"
> +	depends on TMR_MANAGER
> +	help
> +	  This option enables the driver developed for TMR Inject.
> +	  The Triple Modular Redundancy(TMR) Inject provides
> +	  fault injection.
> +
> +	  Say N here unless you know what you are doing.

Why can't this be a module?



> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 28b9803f909b..e9d0a709e207 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
>  obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
>  obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
>  obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
> +obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
> diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
> new file mode 100644
> index 000000000000..930d89e90b61
> --- /dev/null
> +++ b/drivers/misc/xilinx_tmr_inject.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Xilinx TMR Inject IP.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Description:
> + * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
> + * Inject provides fault injection.
> + * Fault injection and detection features are provided through sysfs entries
> + * which allow the user to generate a fault.
> + */
> +
> +#include <asm/xilinx_mb_manager.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +/* TMR Inject Register offsets */
> +#define XTMR_INJECT_CR_OFFSET		0x0
> +#define XTMR_INJECT_AIR_OFFSET		0x4
> +#define XTMR_INJECT_IIR_OFFSET		0xC
> +#define XTMR_INJECT_EAIR_OFFSET		0x10
> +#define XTMR_INJECT_ERR_OFFSET		0x204
> +
> +/* Register Bitmasks/shifts */
> +#define XTMR_INJECT_CR_CPUID_SHIFT	8
> +#define XTMR_INJECT_CR_IE_SHIFT		10
> +#define XTMR_INJECT_IIR_ADDR_MASK	GENMASK(31, 16)
> +
> +#define XTMR_INJECT_MAGIC_MAX_VAL	255
> +
> +/**
> + * struct xtmr_inject_dev - Driver data for TMR Inject
> + * @regs: device physical base address
> + * @cr_val: control register value
> + * @magic: Magic hardware configuration value
> + * @err_cnt: error statistics count
> + */
> +struct xtmr_inject_dev {
> +	void __iomem *regs;
> +	u32 cr_val;
> +	u32 magic;
> +	u32 err_cnt;
> +};
> +
> +/* IO accessors */
> +static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
> +				     u32 addr, u32 value)
> +{
> +	iowrite32(value, xtmr_inject->regs + addr);
> +}
> +
> +static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
> +				   u32 addr)
> +{
> +	return ioread32(xtmr_inject->regs + addr);
> +}
> +
> +static ssize_t inject_err_store(struct device *dev,
> +				struct device_attribute *attr, const char *buf,
> +				size_t size)
> +{
> +	int ret;
> +	long value;
> +
> +	ret = kstrtoul(buf, 16, &value);
> +	if (ret)
> +		return ret;
> +
> +	if (value > 1)
> +		return -EINVAL;

That does not match your documentation :(


> +
> +	xmb_inject_err();
> +
> +	return size;
> +}
> +static DEVICE_ATTR_WO(inject_err);
> +
> +static ssize_t inject_cpuid_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t size)
> +{
> +	struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
> +	int ret;
> +	long value;
> +
> +	ret = kstrtoul(buf, 0, &value);
> +	if (ret)
> +		return ret;
> +
> +	if (value > 3)
> +		return -EINVAL;

Again, does not match the documentation at all.

confused,

greg k-h

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

* Re: [PATCH v2 2/4] drivers: misc: Add Support for TMR Manager
  2022-07-20  6:00 ` [PATCH v2 2/4] drivers: misc: Add Support " Appana Durga Kedareswara rao
@ 2022-07-28 14:27   ` Greg KH
  2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2022-07-28 14:27 UTC (permalink / raw)
  To: Appana Durga Kedareswara rao
  Cc: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, appanad, devicetree, linux-kernel,
	linux-arm-kernel, git, git, Appana Durga Kedareswara rao

On Wed, Jul 20, 2022 at 11:30:14AM +0530, Appana Durga Kedareswara rao wrote:
> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> Triple Modular Redundancy(TMR) subsystem contains three microblaze cores,
> subsystem is fault-tolerant and continues to operate nominally after
> encountering an error. Together with the capability to detect and recover
> from errors, the implementation ensures the reliability of the entire
> subsystem.  TMR Manager is responsible for performing recovery of the
> subsystem detects the fault via a break signal it invokes microblaze
> software break handler which calls the tmr manager driver api to
> update the error count and status, added support for fault detection
> feature via sysfs interface.
> 
> Usage:
> To know the hardware status:
> cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
> To know the break handler count(Error count):
> cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt
> 
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> ---
> Changes for v2:
> --> Added Examples for sysfs entries
> --> Removed uneeded struct dev from the driver private structure
> --> Fixed style issues (Used resource_size_t instead of uintptr_t)
> --> Updated driver to use sysfs_emit() API instead of sprintf() API
> --> Added error checks wherever applicable.
> --> Fixed sysfs registration.
>  .../testing/sysfs-driver-xilinx-tmr-manager   |  27 ++
>  MAINTAINERS                                   |   7 +
>  drivers/misc/Kconfig                          |  10 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/xilinx_tmr_manager.c             | 253 ++++++++++++++++++
>  5 files changed, 298 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>  create mode 100644 drivers/misc/xilinx_tmr_manager.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
> new file mode 100644
> index 000000000000..fc5fe7e22b09
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
> @@ -0,0 +1,27 @@
> +What:		/sys/devices/platform/amba_pl/<dev>/status
> +Date:		June 2022
> +Contact:	appana.durga.rao@xilinx.com
> +Description:	This control file provides the status of the tmr manager
> +		useful for getting the status of fault.
> +		This file cannot be written.
> +		Example:
> +		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
> +		  Lockstep mismatch between processor 1 and 2
> +		  Lockstep mismatch between processor 2 and 3

Why a whole long string?

And this should only be 1 line, not multiple lines.  If it's multiple
lines, this is NOT ok for a sysfs file.

> +
> +What:		/sys/devices/platform/amba_pl/<dev>/errcnt
> +Date:		June 2022
> +Contact:	appana.durga.rao@xilinx.com
> +Description:	This control file provides the fault detection count.
> +		This file cannot be written.
> +		Example:
> +		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt
> +		  1
> +
> +What:		/sys/devices/platform/amba_pl/<dev>/dis_block_break
> +Date:		June 2022
> +Contact:	appana.durga.rao@xilinx.com
> +Description:	This control file enables the break signal.
> +		This file is write only.
> +		Example:
> +		# echo 1 > /sys/devices/platform/amba_pl/44a10000.tmr_manager/dis_block_break
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 651616ed8ae2..732fd9ae7d9f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13080,6 +13080,13 @@ W:	http://www.monstr.eu/fdt/
>  T:	git git://git.monstr.eu/linux-2.6-microblaze.git
>  F:	arch/microblaze/
>  
> +MICROBLAZE TMR MANAGER
> +M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
> +S:	Supported
> +F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
> +F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
> +F:	drivers/misc/xilinx_tmr_manager.c
> +
>  MICROCHIP AT91 DMA DRIVERS
>  M:	Ludovic Desroches <ludovic.desroches@microchip.com>
>  M:	Tudor Ambarus <tudor.ambarus@microchip.com>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 41d2bb0ae23a..555ae2e33b91 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -483,6 +483,16 @@ config OPEN_DICE
>  
>  	  If unsure, say N.
>  
> +config TMR_MANAGER
> +	bool "Select TMR Manager"
> +	depends on MICROBLAZE && MB_MANAGER
> +	help
> +	  This option enables the driver developed for TMR Manager. The Triple
> +	  Modular Redundancy(TMR) manager provides support for fault detection
> +	  via sysfs interface.
> +
> +	  Say N here unless you know what you are doing.

Not a module?

> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 70e800e9127f..28b9803f909b 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
>  obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
>  obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
>  obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
> +obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
> diff --git a/drivers/misc/xilinx_tmr_manager.c b/drivers/misc/xilinx_tmr_manager.c
> new file mode 100644
> index 000000000000..dbeca18c409f
> --- /dev/null
> +++ b/drivers/misc/xilinx_tmr_manager.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx TMR Subsystem.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Description:
> + * This driver is developed for TMR Manager,The Triple Modular Redundancy(TMR)
> + * Manager is responsible for handling the TMR subsystem state, including
> + * fault detection and error recovery. The core is triplicated in each of
> + * the sub-blocks in the TMR subsystem, and provides majority voting of
> + * its internal state provides soft error detection, correction and
> + * recovery. Error detection feature is provided through sysfs
> + * entries which allow the user to observer the TMR microblaze
> + * status.
> + */
> +
> +#include <asm/xilinx_mb_manager.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +/* TMR Manager Register offsets */
> +#define XTMR_MANAGER_CR_OFFSET		0x0
> +#define XTMR_MANAGER_FFR_OFFSET		0x4
> +#define XTMR_MANAGER_CMR0_OFFSET	0x8
> +#define XTMR_MANAGER_CMR1_OFFSET	0xC
> +#define XTMR_MANAGER_BDIR_OFFSET	0x10
> +#define XTMR_MANAGER_SEMIMR_OFFSET	0x1C
> +
> +/* Register Bitmasks/shifts */
> +#define XTMR_MANAGER_CR_MAGIC1_MASK	GENMASK(7, 0)
> +#define XTMR_MANAGER_CR_MAGIC2_MASK	GENMASK(15, 8)
> +#define XTMR_MANAGER_CR_RIR_MASK	BIT(16)
> +#define XTMR_MANAGER_FFR_LM12_MASK	BIT(0)
> +#define XTMR_MANAGER_FFR_LM13_MASK	BIT(1)
> +#define XTMR_MANAGER_FFR_LM23_MASK	BIT(2)
> +
> +#define XTMR_MANAGER_CR_MAGIC2_SHIFT	4
> +#define XTMR_MANAGER_CR_RIR_SHIFT	16
> +#define XTMR_MANAGER_CR_BB_SHIFT	18
> +
> +#define XTMR_MANAGER_MAGIC1_MAX_VAL	255
> +
> +/**
> + * struct xtmr_manager_dev - Driver data for TMR Manager
> + * @regs: device physical base address
> + * @cr_val: control register value
> + * @magic1: Magic 1 hardware configuration value
> + * @err_cnt: error statistics count
> + * @phys_baseaddr: Physical base address
> + */
> +struct xtmr_manager_dev {
> +	void __iomem *regs;
> +	u32 cr_val;
> +	u32 magic1;
> +	u32 err_cnt;
> +	resource_size_t phys_baseaddr;
> +};
> +
> +/* IO accessors */
> +static inline void xtmr_manager_write(struct xtmr_manager_dev *xtmr_manager,
> +				      u32 addr, u32 value)
> +{
> +	iowrite32(value, xtmr_manager->regs + addr);
> +}
> +
> +static inline u32 xtmr_manager_read(struct xtmr_manager_dev *xtmr_manager,
> +				    u32 addr)
> +{
> +	return ioread32(xtmr_manager->regs + addr);
> +}
> +
> +static void xmb_manager_reset_handler(struct xtmr_manager_dev *xtmr_manager)
> +{
> +	/* Clear the FFR Register contents as a part of recovery process. */
> +	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_FFR_OFFSET, 0);
> +}
> +
> +static void xmb_manager_update_errcnt(struct xtmr_manager_dev *xtmr_manager)
> +{
> +	xtmr_manager->err_cnt++;
> +}
> +
> +static ssize_t errcnt_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%x\n", xtmr_manager->err_cnt);
> +}
> +static DEVICE_ATTR_RO(errcnt);
> +
> +static ssize_t status_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
> +	size_t ffr;
> +	int len = 0;
> +
> +	ffr = xtmr_manager_read(xtmr_manager, XTMR_MANAGER_FFR_OFFSET);
> +	if ((ffr & XTMR_MANAGER_FFR_LM12_MASK) == XTMR_MANAGER_FFR_LM12_MASK) {
> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
> +		len += sysfs_emit_at(buf, len, "processor 1 and 2\n");

You can write a full string all at once, no need to call this twice.

> +	}
> +
> +	if ((ffr & XTMR_MANAGER_FFR_LM13_MASK) == XTMR_MANAGER_FFR_LM13_MASK) {
> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
> +		len += sysfs_emit_at(buf, len, "processor 1 and 3\n");
> +	}
> +
> +	if ((ffr & XTMR_MANAGER_FFR_LM23_MASK) == XTMR_MANAGER_FFR_LM23_MASK) {
> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
> +		len += sysfs_emit_at(buf, len, "processor 2 and 3\n");
> +	}

As said above, multiple lines is not ok, you need to fix up this api.

Perhaps 3 files, one for eacy type of mismatch and a simple 0/1 value
returned in them?


> +
> +	return len;
> +}
> +static DEVICE_ATTR_RO(status);
> +
> +static ssize_t dis_block_break_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t size)
> +{
> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
> +	int ret;
> +	long value;
> +
> +	ret = kstrtoul(buf, 16, &value);
> +	if (ret)
> +		return ret;
> +
> +	if (value > 1)
> +		return -EINVAL;

Why is 1 magic?

And we have a sysfs function to read a 0/1/Y/N/y/n value, please use
that.

thanks,

greg k-h

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

* Re: [PATCH v2 2/4] drivers: misc: Add Support for TMR Manager
  2022-07-28 14:27   ` Greg KH
@ 2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
  0 siblings, 0 replies; 19+ messages in thread
From: Rao, Appana Durga Kedareswara @ 2022-07-29  8:13 UTC (permalink / raw)
  To: Greg KH, Appana Durga Kedareswara rao
  Cc: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, devicetree, linux-kernel, linux-arm-kernel,
	git, git, Appana Durga Kedareswara rao

Hi Greg,

Thanks for the review.

On 28/07/22 7:57 pm, Greg KH wrote:
> On Wed, Jul 20, 2022 at 11:30:14AM +0530, Appana Durga Kedareswara rao wrote:
>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>
>> Triple Modular Redundancy(TMR) subsystem contains three microblaze cores,
>> subsystem is fault-tolerant and continues to operate nominally after
>> encountering an error. Together with the capability to detect and recover
>> from errors, the implementation ensures the reliability of the entire
>> subsystem.  TMR Manager is responsible for performing recovery of the
>> subsystem detects the fault via a break signal it invokes microblaze
>> software break handler which calls the tmr manager driver api to
>> update the error count and status, added support for fault detection
>> feature via sysfs interface.
>>
>> Usage:
>> To know the hardware status:
>> cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
>> To know the break handler count(Error count):
>> cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt
>>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>> ---
>> Changes for v2:
>> --> Added Examples for sysfs entries
>> --> Removed uneeded struct dev from the driver private structure
>> --> Fixed style issues (Used resource_size_t instead of uintptr_t)
>> --> Updated driver to use sysfs_emit() API instead of sprintf() API
>> --> Added error checks wherever applicable.
>> --> Fixed sysfs registration.
>>   .../testing/sysfs-driver-xilinx-tmr-manager   |  27 ++
>>   MAINTAINERS                                   |   7 +
>>   drivers/misc/Kconfig                          |  10 +
>>   drivers/misc/Makefile                         |   1 +
>>   drivers/misc/xilinx_tmr_manager.c             | 253 ++++++++++++++++++
>>   5 files changed, 298 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>>   create mode 100644 drivers/misc/xilinx_tmr_manager.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>> new file mode 100644
>> index 000000000000..fc5fe7e22b09
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>> @@ -0,0 +1,27 @@
>> +What:		/sys/devices/platform/amba_pl/<dev>/status
>> +Date:		June 2022
>> +Contact:	appana.durga.rao@xilinx.com
>> +Description:	This control file provides the status of the tmr manager
>> +		useful for getting the status of fault.
>> +		This file cannot be written.
>> +		Example:
>> +		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/status
>> +		  Lockstep mismatch between processor 1 and 2
>> +		  Lockstep mismatch between processor 2 and 3
> 
> Why a whole long string?
To let user know about exact status of the TMR Subsystem(Manager) status

> 
> And this should only be 1 line, not multiple lines.  If it's multiple
> lines, this is NOT ok for a sysfs file.

Sure will fix in next version.
> 
>> +
>> +What:		/sys/devices/platform/amba_pl/<dev>/errcnt
>> +Date:		June 2022
>> +Contact:	appana.durga.rao@xilinx.com
>> +Description:	This control file provides the fault detection count.
>> +		This file cannot be written.
>> +		Example:
>> +		# cat /sys/devices/platform/amba_pl/44a10000.tmr_manager/errcnt
>> +		  1
>> +
>> +What:		/sys/devices/platform/amba_pl/<dev>/dis_block_break
>> +Date:		June 2022
>> +Contact:	appana.durga.rao@xilinx.com
>> +Description:	This control file enables the break signal.
>> +		This file is write only.
>> +		Example:
>> +		# echo 1 > /sys/devices/platform/amba_pl/44a10000.tmr_manager/dis_block_break
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 651616ed8ae2..732fd9ae7d9f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13080,6 +13080,13 @@ W:	http://www.monstr.eu/fdt/
>>   T:	git git://git.monstr.eu/linux-2.6-microblaze.git
>>   F:	arch/microblaze/
>>   
>> +MICROBLAZE TMR MANAGER
>> +M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>> +S:	Supported
>> +F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>> +F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>> +F:	drivers/misc/xilinx_tmr_manager.c
>> +
>>   MICROCHIP AT91 DMA DRIVERS
>>   M:	Ludovic Desroches <ludovic.desroches@microchip.com>
>>   M:	Tudor Ambarus <tudor.ambarus@microchip.com>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 41d2bb0ae23a..555ae2e33b91 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -483,6 +483,16 @@ config OPEN_DICE
>>   
>>   	  If unsure, say N.
>>   
>> +config TMR_MANAGER
>> +	bool "Select TMR Manager"
>> +	depends on MICROBLAZE && MB_MANAGER
>> +	help
>> +	  This option enables the driver developed for TMR Manager. The Triple
>> +	  Modular Redundancy(TMR) manager provides support for fault detection
>> +	  via sysfs interface.
>> +
>> +	  Say N here unless you know what you are doing.
> 
> Not a module?
> 

Will fix in next version.
>> +
>>   source "drivers/misc/c2port/Kconfig"
>>   source "drivers/misc/eeprom/Kconfig"
>>   source "drivers/misc/cb710/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 70e800e9127f..28b9803f909b 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -60,3 +60,4 @@ obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
>>   obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
>>   obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
>>   obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
>> +obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
>> diff --git a/drivers/misc/xilinx_tmr_manager.c b/drivers/misc/xilinx_tmr_manager.c
>> new file mode 100644
>> index 000000000000..dbeca18c409f
>> --- /dev/null
>> +++ b/drivers/misc/xilinx_tmr_manager.c
>> @@ -0,0 +1,253 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx TMR Subsystem.
>> + *
>> + * Copyright (C) 2022 Xilinx, Inc.
>> + *
>> + * Description:
>> + * This driver is developed for TMR Manager,The Triple Modular Redundancy(TMR)
>> + * Manager is responsible for handling the TMR subsystem state, including
>> + * fault detection and error recovery. The core is triplicated in each of
>> + * the sub-blocks in the TMR subsystem, and provides majority voting of
>> + * its internal state provides soft error detection, correction and
>> + * recovery. Error detection feature is provided through sysfs
>> + * entries which allow the user to observer the TMR microblaze
>> + * status.
>> + */
>> +
>> +#include <asm/xilinx_mb_manager.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +
>> +/* TMR Manager Register offsets */
>> +#define XTMR_MANAGER_CR_OFFSET		0x0
>> +#define XTMR_MANAGER_FFR_OFFSET		0x4
>> +#define XTMR_MANAGER_CMR0_OFFSET	0x8
>> +#define XTMR_MANAGER_CMR1_OFFSET	0xC
>> +#define XTMR_MANAGER_BDIR_OFFSET	0x10
>> +#define XTMR_MANAGER_SEMIMR_OFFSET	0x1C
>> +
>> +/* Register Bitmasks/shifts */
>> +#define XTMR_MANAGER_CR_MAGIC1_MASK	GENMASK(7, 0)
>> +#define XTMR_MANAGER_CR_MAGIC2_MASK	GENMASK(15, 8)
>> +#define XTMR_MANAGER_CR_RIR_MASK	BIT(16)
>> +#define XTMR_MANAGER_FFR_LM12_MASK	BIT(0)
>> +#define XTMR_MANAGER_FFR_LM13_MASK	BIT(1)
>> +#define XTMR_MANAGER_FFR_LM23_MASK	BIT(2)
>> +
>> +#define XTMR_MANAGER_CR_MAGIC2_SHIFT	4
>> +#define XTMR_MANAGER_CR_RIR_SHIFT	16
>> +#define XTMR_MANAGER_CR_BB_SHIFT	18
>> +
>> +#define XTMR_MANAGER_MAGIC1_MAX_VAL	255
>> +
>> +/**
>> + * struct xtmr_manager_dev - Driver data for TMR Manager
>> + * @regs: device physical base address
>> + * @cr_val: control register value
>> + * @magic1: Magic 1 hardware configuration value
>> + * @err_cnt: error statistics count
>> + * @phys_baseaddr: Physical base address
>> + */
>> +struct xtmr_manager_dev {
>> +	void __iomem *regs;
>> +	u32 cr_val;
>> +	u32 magic1;
>> +	u32 err_cnt;
>> +	resource_size_t phys_baseaddr;
>> +};
>> +
>> +/* IO accessors */
>> +static inline void xtmr_manager_write(struct xtmr_manager_dev *xtmr_manager,
>> +				      u32 addr, u32 value)
>> +{
>> +	iowrite32(value, xtmr_manager->regs + addr);
>> +}
>> +
>> +static inline u32 xtmr_manager_read(struct xtmr_manager_dev *xtmr_manager,
>> +				    u32 addr)
>> +{
>> +	return ioread32(xtmr_manager->regs + addr);
>> +}
>> +
>> +static void xmb_manager_reset_handler(struct xtmr_manager_dev *xtmr_manager)
>> +{
>> +	/* Clear the FFR Register contents as a part of recovery process. */
>> +	xtmr_manager_write(xtmr_manager, XTMR_MANAGER_FFR_OFFSET, 0);
>> +}
>> +
>> +static void xmb_manager_update_errcnt(struct xtmr_manager_dev *xtmr_manager)
>> +{
>> +	xtmr_manager->err_cnt++;
>> +}
>> +
>> +static ssize_t errcnt_show(struct device *dev, struct device_attribute *attr,
>> +			   char *buf)
>> +{
>> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
>> +
>> +	return sysfs_emit(buf, "%x\n", xtmr_manager->err_cnt);
>> +}
>> +static DEVICE_ATTR_RO(errcnt);
>> +
>> +static ssize_t status_show(struct device *dev, struct device_attribute *attr,
>> +			   char *buf)
>> +{
>> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
>> +	size_t ffr;
>> +	int len = 0;
>> +
>> +	ffr = xtmr_manager_read(xtmr_manager, XTMR_MANAGER_FFR_OFFSET);
>> +	if ((ffr & XTMR_MANAGER_FFR_LM12_MASK) == XTMR_MANAGER_FFR_LM12_MASK) {
>> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
>> +		len += sysfs_emit_at(buf, len, "processor 1 and 2\n");
> 
> You can write a full string all at once, no need to call this twice.
> 

Sure will fix in next version.
>> +	}
>> +
>> +	if ((ffr & XTMR_MANAGER_FFR_LM13_MASK) == XTMR_MANAGER_FFR_LM13_MASK) {
>> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
>> +		len += sysfs_emit_at(buf, len, "processor 1 and 3\n");
>> +	}
>> +
>> +	if ((ffr & XTMR_MANAGER_FFR_LM23_MASK) == XTMR_MANAGER_FFR_LM23_MASK) {
>> +		len += sysfs_emit_at(buf, len, "Lockstep mismatch between ");
>> +		len += sysfs_emit_at(buf, len, "processor 2 and 3\n");
>> +	}
> 
> As said above, multiple lines is not ok, you need to fix up this api.
> 
> Perhaps 3 files, one for eacy type of mismatch and a simple 0/1 value
> returned in them?
> 

Okay will fix in next version.

> 
>> +
>> +	return len;
>> +}
>> +static DEVICE_ATTR_RO(status);
>> +
>> +static ssize_t dis_block_break_store(struct device *dev,
>> +				     struct device_attribute *attr,
>> +				     const char *buf, size_t size)
>> +{
>> +	struct xtmr_manager_dev *xtmr_manager = dev_get_drvdata(dev);
>> +	int ret;
>> +	long value;
>> +
>> +	ret = kstrtoul(buf, 16, &value);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (value > 1)
>> +		return -EINVAL;
> 
> Why is 1 magic?
> 
> And we have a sysfs function to read a 0/1/Y/N/y/n value, please use
> that.

Sure will fix in next version.

Regards,
Kedar.
> 
> thanks,
> 
> greg k-h

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

* Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP
  2022-07-28 14:24   ` Greg KH
@ 2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
  2022-07-29  9:02       ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Rao, Appana Durga Kedareswara @ 2022-07-29  8:13 UTC (permalink / raw)
  To: Greg KH, Appana Durga Kedareswara rao
  Cc: robh+dt, krzysztof.kozlowski+dt, michal.simek, derek.kiernan,
	dragan.cvetic, arnd, devicetree, linux-kernel, linux-arm-kernel,
	git, git, Appana Durga Kedareswara rao

Hi Greg,

Thanks for the review.

On 28/07/22 7:54 pm, Greg KH wrote:
> On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>
>> The Triple Modular Redundancy(TMR) provides functional fault injection by
>> changing selected MicroBlaze instructions, which provides the possibility
>> to verify that the TMR subsystem error detection and fault recovery logic
>> is working properly, provided sysfs entries which allow the user to inject
>> a fault.
> 
> We already have a fault-injection api, why are you not using that?
> 

Inorder to inject the error using TMR inject IP, The API
which injects the error should be executed from Processor LMB,
below sysfs entry calls microblaze core API xmb_inject_err()
which switches the processor to real mode and injects the error,
Please find the code corresponds to xmb_inject_err() API here: 
https://www.spinics.net/lists/arm-kernel/msg991888.html


>>
>> Usage:
>> echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
>>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> 
> Odd, just one is needed.

Sure will fix in next version.
> 
>> ---
>> Changes for v2:
>> --> Added Examples for sysfs entries
>> --> Removed uneeded struct dev from the driver private structure
>> --> Updated driver to use sysfs_emit() API instead of sprintf() API
>> --> Added error checks wherever applicable.
>> --> Fixed sysfs registration.
>>   .../testing/sysfs-driver-xilinx-tmr-inject    |  16 ++
>>   MAINTAINERS                                   |   7 +
>>   drivers/misc/Kconfig                          |  10 +
>>   drivers/misc/Makefile                         |   1 +
>>   drivers/misc/xilinx_tmr_inject.c              | 186 ++++++++++++++++++
>>   5 files changed, 220 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>>   create mode 100644 drivers/misc/xilinx_tmr_inject.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> new file mode 100644
>> index 000000000000..d274b30ee24c
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> @@ -0,0 +1,16 @@
>> +What:		/sys/devices/platform/amba_pl/<dev>/inject_err
>> +Date:		June 2022
> 
> It's not June anymore, even when you sent this patch :(

Will fix in next version
> 
>> +Contact:	appana.durga.rao@xilinx.com
>> +Description:	This control file allows to inject fault using tmr inject.
>> +		This file is write only.
>> +		Example:
>> +		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
>> +
>> +What:		/sys/devices/platform/amba_pl/<dev>/inject_cpuid
>> +Date:		June 2022
>> +Contact:	appana.durga.rao@xilinx.com
>> +Description:	This control file allows to configure the CPU identifier
>> +		to enable fault injection.
>> +		This file is write only.
>> +		Example:
>> +		# echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid
> 
> What errors and faults happen?  Where is that documented?  What happens
> when you write to these sysfs files?  Does the system crash?  Why would
> you want to use them ever?
> 
> 

TMR subsystem has 3 Microblaze processor cores, by default driver is 
configured to inject the error at processor core-0,  This sysfs entry 
provides a mechanism to inject the fault at the user-specified core.

> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 732fd9ae7d9f..c903b45c204a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13087,6 +13087,13 @@ F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>>   F:	Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>>   F:	drivers/misc/xilinx_tmr_manager.c
>>   
>> +MICROBLAZE TMR INJECT
>> +M:	Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
>> +S:	Supported
>> +F:	Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> +F:	Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
>> +F:	drivers/misc/xilinx_tmr_inject.c
>> +
>>   MICROCHIP AT91 DMA DRIVERS
>>   M:	Ludovic Desroches <ludovic.desroches@microchip.com>
>>   M:	Tudor Ambarus <tudor.ambarus@microchip.com>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 555ae2e33b91..0989c36f3051 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -493,6 +493,16 @@ config TMR_MANAGER
>>   
>>   	  Say N here unless you know what you are doing.
>>   
>> +config TMR_INJECT
>> +	bool "Select TMR Inject"
>> +	depends on TMR_MANAGER
>> +	help
>> +	  This option enables the driver developed for TMR Inject.
>> +	  The Triple Modular Redundancy(TMR) Inject provides
>> +	  fault injection.
>> +
>> +	  Say N here unless you know what you are doing.
> 
> Why can't this be a module?
> 
> 

We can use this driver as a module will fix in next version.

> 
>> +
>>   source "drivers/misc/c2port/Kconfig"
>>   source "drivers/misc/eeprom/Kconfig"
>>   source "drivers/misc/cb710/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 28b9803f909b..e9d0a709e207 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
>>   obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
>>   obj-$(CONFIG_OPEN_DICE)		+= open-dice.o
>>   obj-$(CONFIG_TMR_MANAGER)	+= xilinx_tmr_manager.o
>> +obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
>> diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
>> new file mode 100644
>> index 000000000000..930d89e90b61
>> --- /dev/null
>> +++ b/drivers/misc/xilinx_tmr_inject.c
>> @@ -0,0 +1,186 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Xilinx TMR Inject IP.
>> + *
>> + * Copyright (C) 2022 Xilinx, Inc.
>> + *
>> + * Description:
>> + * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
>> + * Inject provides fault injection.
>> + * Fault injection and detection features are provided through sysfs entries
>> + * which allow the user to generate a fault.
>> + */
>> +
>> +#include <asm/xilinx_mb_manager.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +
>> +/* TMR Inject Register offsets */
>> +#define XTMR_INJECT_CR_OFFSET		0x0
>> +#define XTMR_INJECT_AIR_OFFSET		0x4
>> +#define XTMR_INJECT_IIR_OFFSET		0xC
>> +#define XTMR_INJECT_EAIR_OFFSET		0x10
>> +#define XTMR_INJECT_ERR_OFFSET		0x204
>> +
>> +/* Register Bitmasks/shifts */
>> +#define XTMR_INJECT_CR_CPUID_SHIFT	8
>> +#define XTMR_INJECT_CR_IE_SHIFT		10
>> +#define XTMR_INJECT_IIR_ADDR_MASK	GENMASK(31, 16)
>> +
>> +#define XTMR_INJECT_MAGIC_MAX_VAL	255
>> +
>> +/**
>> + * struct xtmr_inject_dev - Driver data for TMR Inject
>> + * @regs: device physical base address
>> + * @cr_val: control register value
>> + * @magic: Magic hardware configuration value
>> + * @err_cnt: error statistics count
>> + */
>> +struct xtmr_inject_dev {
>> +	void __iomem *regs;
>> +	u32 cr_val;
>> +	u32 magic;
>> +	u32 err_cnt;
>> +};
>> +
>> +/* IO accessors */
>> +static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
>> +				     u32 addr, u32 value)
>> +{
>> +	iowrite32(value, xtmr_inject->regs + addr);
>> +}
>> +
>> +static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
>> +				   u32 addr)
>> +{
>> +	return ioread32(xtmr_inject->regs + addr);
>> +}
>> +
>> +static ssize_t inject_err_store(struct device *dev,
>> +				struct device_attribute *attr, const char *buf,
>> +				size_t size)
>> +{
>> +	int ret;
>> +	long value;
>> +
>> +	ret = kstrtoul(buf, 16, &value);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (value > 1)
>> +		return -EINVAL;
> 
> That does not match your documentation :(
> 

Will fix the documentation in next version

> 
>> +
>> +	xmb_inject_err();
>> +
>> +	return size;
>> +}
>> +static DEVICE_ATTR_WO(inject_err);
>> +
>> +static ssize_t inject_cpuid_store(struct device *dev,
>> +				  struct device_attribute *attr,
>> +				  const char *buf, size_t size)
>> +{
>> +	struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
>> +	int ret;
>> +	long value;
>> +
>> +	ret = kstrtoul(buf, 0, &value);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (value > 3)
>> +		return -EINVAL;
> 
> Again, does not match the documentation at all.
> 

Will fix the documentation in next version


> confused,

sorry for the improper sysfs documentation will fix in next version.

Regards,
Kedar.
> 
> greg k-h

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

* Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP
  2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
@ 2022-07-29  9:02       ` Greg KH
  2022-08-03  6:35         ` Rao, Appana Durga Kedareswara
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2022-07-29  9:02 UTC (permalink / raw)
  To: Rao, Appana Durga Kedareswara
  Cc: Appana Durga Kedareswara rao, robh+dt, krzysztof.kozlowski+dt,
	michal.simek, derek.kiernan, dragan.cvetic, arnd, devicetree,
	linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

On Fri, Jul 29, 2022 at 01:43:47PM +0530, Rao, Appana Durga Kedareswara wrote:
> Hi Greg,
> 
> Thanks for the review.
> 
> On 28/07/22 7:54 pm, Greg KH wrote:
> > On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
> > > From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> > > 
> > > The Triple Modular Redundancy(TMR) provides functional fault injection by
> > > changing selected MicroBlaze instructions, which provides the possibility
> > > to verify that the TMR subsystem error detection and fault recovery logic
> > > is working properly, provided sysfs entries which allow the user to inject
> > > a fault.
> > 
> > We already have a fault-injection api, why are you not using that?
> > 
> 
> Inorder to inject the error using TMR inject IP, The API
> which injects the error should be executed from Processor LMB,
> below sysfs entry calls microblaze core API xmb_inject_err()
> which switches the processor to real mode and injects the error,
> Please find the code corresponds to xmb_inject_err() API here:
> https://www.spinics.net/lists/arm-kernel/msg991888.html

You did not answer the question.  Why are you not using the in-kernel
fault injection userspace api and why have you created your own?

Also, please use lore.kernel.org links when possible.

thanks,

greg k-h

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

* Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP
  2022-07-29  9:02       ` Greg KH
@ 2022-08-03  6:35         ` Rao, Appana Durga Kedareswara
  0 siblings, 0 replies; 19+ messages in thread
From: Rao, Appana Durga Kedareswara @ 2022-08-03  6:35 UTC (permalink / raw)
  To: Greg KH
  Cc: Appana Durga Kedareswara rao, robh+dt, krzysztof.kozlowski+dt,
	michal.simek, derek.kiernan, dragan.cvetic, arnd, devicetree,
	linux-kernel, linux-arm-kernel, git, git,
	Appana Durga Kedareswara rao

Hi Greg,

On 29/07/22 2:32 pm, Greg KH wrote:
> On Fri, Jul 29, 2022 at 01:43:47PM +0530, Rao, Appana Durga Kedareswara wrote:
>> Hi Greg,
>>
>> Thanks for the review.
>>
>> On 28/07/22 7:54 pm, Greg KH wrote:
>>> On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
>>>> From: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
>>>>
>>>> The Triple Modular Redundancy(TMR) provides functional fault injection by
>>>> changing selected MicroBlaze instructions, which provides the possibility
>>>> to verify that the TMR subsystem error detection and fault recovery logic
>>>> is working properly, provided sysfs entries which allow the user to inject
>>>> a fault.
>>>
>>> We already have a fault-injection api, why are you not using that?
>>>
>>
>> Inorder to inject the error using TMR inject IP, The API
>> which injects the error should be executed from Processor LMB,
>> below sysfs entry calls microblaze core API xmb_inject_err()
>> which switches the processor to real mode and injects the error,
>> Please find the code corresponds to xmb_inject_err() API here:
>> https://www.spinics.net/lists/arm-kernel/msg991888.html
> 
> You did not answer the question.  Why are you not using the in-kernel
> fault injection userspace api and why have you created your own?

will update the driver to use fault injection user space API in next 
version.

> 
> Also, please use lore.kernel.org links when possible.

Sure

Regards,
Kedar.
> 
> thanks,
> 
> greg k-h

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

end of thread, other threads:[~2022-08-03  6:36 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20  6:00 [PATCH v2 0/4] drivers: misc: Add support for TMR Manager and Inject IPs Appana Durga Kedareswara rao
2022-07-20  6:00 ` [PATCH v2 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager Appana Durga Kedareswara rao
2022-07-20  6:15   ` Krzysztof Kozlowski
2022-07-20  6:00 ` [PATCH v2 2/4] drivers: misc: Add Support " Appana Durga Kedareswara rao
2022-07-28 14:27   ` Greg KH
2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
2022-07-20  6:00 ` [PATCH v2 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject Appana Durga Kedareswara rao
2022-07-20  6:15   ` Krzysztof Kozlowski
2022-07-20  8:26     ` Michal Simek
2022-07-20  9:35       ` Krzysztof Kozlowski
2022-07-20 12:09         ` Michal Simek
2022-07-20 12:21           ` Krzysztof Kozlowski
2022-07-20 12:35             ` Michal Simek
2022-07-20 13:21           ` Rao, Appana Durga Kedareswara
2022-07-20  6:00 ` [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP Appana Durga Kedareswara rao
2022-07-28 14:24   ` Greg KH
2022-07-29  8:13     ` Rao, Appana Durga Kedareswara
2022-07-29  9:02       ` Greg KH
2022-08-03  6:35         ` Rao, Appana Durga Kedareswara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).