LinuxLists.cc - [PATCH 0/4] drivers: misc: Add support for TMR Manager and Inject IPs

[permalink] [raw]

Subject: [PATCH 4/4] drivers: misc: Add Support for TMR Inject IP

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.

This commit adds support for fault injection, 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 <[email protected]>
---
.../testing/sysfs-driver-xilinx-tmr-inject | 12 ++
MAINTAINERS | 7 +
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/xilinx_tmr_inject.c | 201 ++++++++++++++++++
5 files changed, 231 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..373a5964cee9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
@@ -0,0 +1,12 @@
+What: /sys/devices/platform/amba_pl/<dev>/inject_err
+Date: June 2022
+Contact: [email protected]
+Description: This control file allows to inject fault using tmr inject.
+ This file is write only.
+
+What: /sys/devices/platform/amba_pl/<dev>/inject_cpuid
+Date: June 2022
+Contact: [email protected]
+Description: This control file allows to configure the CPU identifier
+ to enable fault injection.
+ This file is write only.
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d29109788ed..1446c5aca9e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12969,6 +12969,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 <[email protected]>
+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 <[email protected]>
M: Tudor Ambarus <[email protected]>
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..e458d6d35ae1
--- /dev/null
+++ b/drivers/misc/xilinx_tmr_inject.c
@@ -0,0 +1,201 @@
+// 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
+ * @dev: pointer to device struct
+ * @cr_val: control register value
+ * @magic: Magic hardware configuration value
+ * @err_cnt: error statistics count
+ */
+struct xtmr_inject_dev {
+ void __iomem *regs;
+ struct device *dev;
+ 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_attrs[] = {
+ &dev_attr_inject_err.attr,
+ &dev_attr_inject_cpuid.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(xtmr_inject);
+
+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->dev = &pdev->dev;
+
+ 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;
+ } else if (xtmr_inject->magic > XTMR_INJECT_MAGIC_MAX_VAL) {
+ dev_err(&pdev->dev, "invalid xlnx,magic property value");
+ return err;
+ }
+
+ /* Initialize TMR Inject */
+ xtmr_inject_init(xtmr_inject);
+
+ err = sysfs_create_groups(&xtmr_inject->dev->kobj, xtmr_inject_groups);
+ if (err < 0) {
+ dev_err(&pdev->dev, "unable to create sysfs entries\n");
+ return err;
+ }
+
+ platform_set_drvdata(pdev, xtmr_inject);
+
+ return 0;
+}
+
+static int xtmr_inject_remove(struct platform_device *pdev)
+{
+ sysfs_remove_groups(&pdev->dev.kobj, xtmr_inject_groups);
+
+ 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,
+ },
+ .probe = xtmr_inject_probe,
+ .remove = xtmr_inject_remove,
+};
+module_platform_driver(xtmr_inject_driver);
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx TMR Inject Driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2022-06-28 06:03:12

[permalink] [raw]

Subject: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

This commit adds documentation for Triple Modular Redundancy(TMR) Manager
IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
the TMR subsystem state, including fault detection and error recovery
provides soft error detection, correction and recovery features.

Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
---
.../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++
1 file changed, 48 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..f6cb4d235981
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
@@ -0,0 +1,48 @@
+# 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 TMR Microblaze Device Tree Bindings
+
+maintainers:
+ - Appana Durga Kedareswara rao <[email protected]>
+
+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 number 1, When writing to the control register the first write data
+ byte (bits 7:0) must match this value in order to have any effect on the
+ nominal recovery function.
+ $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

2022-06-28 06:04:45

[permalink] [raw]

Subject: [PATCH 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject

This commit adds documentation for Triple Modular Redundancy (TMR)
Inject IP.

Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
---
.../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..4daf9539000b
--- /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 TMR Inject Device Tree Bindings
+
+maintainers:
+ - Appana Durga Kedareswara rao <[email protected]>
+
+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 used when injecting faults. The fault inject write data least
+ significant byte (bits 7:0) must match this number to have any effect.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - reg
+ - xlnx,magic
+
+additionalProperties: false
+
+examples:
+ - |
+ tmr-inject@44a30000 {
+ compatible = "xlnx,tmr-inject-1.0";
+ reg = <0x44a10000 0x10000>;
+ xlnx,magic = <0x46>;
+ };
--
2.25.1

2022-06-28 06:05:26

[permalink] [raw]

Subject: [PATCH 2/4] drivers: misc: Add Support for TMR Manager

The Triple Modular Redundancy(TMR) Manager is responsible for handling
the TMR subsystem state, including fault detection and error recovery
provides soft error detection, correction and recovery features.

This commit adds 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 <[email protected]>
---
.../testing/sysfs-driver-xilinx-tmr-manager | 18 ++
MAINTAINERS | 7 +
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/xilinx_tmr_manager.c | 288 ++++++++++++++++++
5 files changed, 324 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..c2f615126201
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
@@ -0,0 +1,18 @@
+What: /sys/devices/platform/amba_pl/<dev>/status
+Date: June 2022
+Contact: [email protected]
+Description: This control file provides the status of the tmr manager
+ useful for getting the status of fault.
+ This file cannot be written.
+
+What: /sys/devices/platform/amba_pl/<dev>/errcnt
+Date: June 2022
+Contact: [email protected]
+Description: This control file provides the fault detection count.
+ This file cannot be written.
+
+What: /sys/devices/platform/amba_pl/<dev>/dis_block_break
+Date: June 2022
+Contact: [email protected]
+Description: This control file enables the break signal.
+ This file is write only.
diff --git a/MAINTAINERS b/MAINTAINERS
index 6cc825857722..3d29109788ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12962,6 +12962,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 <[email protected]>
+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 <[email protected]>
M: Tudor Ambarus <[email protected]>
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..5a4562e4a9d4
--- /dev/null
+++ b/drivers/misc/xilinx_tmr_manager.c
@@ -0,0 +1,288 @@
+// 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
+ * @dev: pointer to device struct
+ * @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;
+ struct device *dev;
+ u32 cr_val;
+ u32 magic1;
+ u32 err_cnt;
+ uintptr_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);
+}
+
+/**
+ * xtmr_manager_unblock_break - unblocks the break signal
+ * @xtmr_manager: Pointer to xtmr_manager_dev structure
+ */
+static void xtmr_manager_unblock_break(struct xtmr_manager_dev *xtmr_manager)
+{
+ xtmr_manager->cr_val &= ~(1 << XTMR_MANAGER_CR_BB_SHIFT);
+ xtmr_manager_write(xtmr_manager, XTMR_MANAGER_CR_OFFSET,
+ xtmr_manager->cr_val);
+}
+
+/**
+ * xmb_manager_reset_handler - clears the ffr register contents
+ * @priv: Private pointer
+ */
+static void xmb_manager_reset_handler(void *priv)
+{
+ struct xtmr_manager_dev *xtmr_manager = (struct xtmr_manager_dev *)priv;
+ /* Clear the FFR Register contents as a part of recovery process. */
+ xtmr_manager_write(xtmr_manager, XTMR_MANAGER_FFR_OFFSET, 0);
+}
+
+/**
+ * xmb_manager_update_errcnt - update the error inject count
+ * @priv: Private pointer
+ */
+static void xmb_manager_update_errcnt(void *priv)
+{
+ struct xtmr_manager_dev *xtmr_manager = (struct xtmr_manager_dev *)priv;
+
+ 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 sprintf(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 += sprintf(buf + len,
+ "Lockstep mismatch between processor 1 and 2\n");
+ }
+
+ if ((ffr & XTMR_MANAGER_FFR_LM13_MASK) == XTMR_MANAGER_FFR_LM13_MASK) {
+ len += sprintf(buf + len,
+ "Lockstep mismatch between processor 1 and 3\n");
+ }
+
+ if ((ffr & XTMR_MANAGER_FFR_LM23_MASK) == XTMR_MANAGER_FFR_LM23_MASK) {
+ len += sprintf(buf + len,
+ "Lockstep mismatch between 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;
+
+ xtmr_manager_unblock_break(xtmr_manager);
+ return size;
+}
+static DEVICE_ATTR_WO(dis_block_break);
+
+static struct attribute *xtmr_manager_attrs[] = {
+ &dev_attr_dis_block_break.attr,
+ &dev_attr_status.attr,
+ &dev_attr_errcnt.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(xtmr_manager);
+
+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,
+ xmb_manager_update_errcnt,
+ xtmr_manager, 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;
+
+ xtmr_manager->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ xtmr_manager->regs = devm_ioremap_resource(xtmr_manager->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;
+ } else if (xtmr_manager->magic1 > XTMR_MANAGER_MAGIC1_MAX_VAL) {
+ dev_err(&pdev->dev, "invalid xlnx,magic1 property value");
+ return err;
+ }
+
+ /* Initialize TMR Manager */
+ xtmr_manager_init(xtmr_manager);
+
+ err = sysfs_create_groups(&xtmr_manager->dev->kobj,
+ xtmr_manager_groups);
+ if (err < 0) {
+ dev_err(&pdev->dev, "unable to create sysfs entries\n");
+ return err;
+ }
+
+ platform_set_drvdata(pdev, xtmr_manager);
+
+ return 0;
+}
+
+static int xtmr_manager_remove(struct platform_device *pdev)
+{
+ sysfs_remove_groups(&pdev->dev.kobj, xtmr_manager_groups);
+
+ 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,
+ },
+ .probe = xtmr_manager_probe,
+ .remove = xtmr_manager_remove,
+};
+module_platform_driver(xtmr_manager_driver);
+
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx TMR Manager Driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2022-06-29 10:27:18

[permalink] [raw]

Subject: Re: [PATCH 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject

On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
> This commit adds documentation for Triple Modular Redundancy (TMR)

No "This commit"

https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> Inject IP.
>
> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
> ---
> .../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..4daf9539000b
> --- /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 TMR Inject Device Tree Bindings

Same comments.

> +
> +maintainers:
> + - Appana Durga Kedareswara rao <[email protected]>
> +
> +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 used when injecting faults. The fault inject write data least
> + significant byte (bits 7:0) must match this number to have any effect.

And why exactly register value is a property of DT?

> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> + - compatible
> + - reg
> + - xlnx,magic
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + tmr-inject@44a30000 {

fault-inject?

> + compatible = "xlnx,tmr-inject-1.0";
> + reg = <0x44a10000 0x10000>;
> + xlnx,magic = <0x46>;
> + };

Best regards,
Krzysztof

2022-06-29 10:38:13

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
> This commit adds documentation for Triple Modular Redundancy(TMR) Manager
> IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
> the TMR subsystem state, including fault detection and error recovery
> provides soft error detection, correction and recovery features.
>
> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
> ---
> .../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++

This is not a misc device. Find appropriate subsystem for it. It's not
EDAC, right?

> 1 file changed, 48 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..f6cb4d235981
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
> @@ -0,0 +1,48 @@
> +# 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 TMR Microblaze Device Tree Bindings

s/Device Tree Bindings//

> +
> +maintainers:
> + - Appana Durga Kedareswara rao <[email protected]>
> +
> +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 number 1, When writing to the control register the first write data
> + byte (bits 7:0) must match this value in order to have any effect on the
> + nominal recovery function.

Register values are usually not in the scope of bindings. We describe
here hardware, not programming model, although in case of soft cores
maybe it's a bit different.

Anyway you need to describe the actual feature, not OS behavior.

> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> + - compatible
> + - reg
> + - xlnx,magic1
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + tmr-manager@44a10000 {

Depending on what is this, you should use appropriate generic node name.

> + compatible = "xlnx,tmr-manager-1.0";
> + reg = <0x44a10000 0x10000>;
> + xlnx,magic1 = <0x46>;
> + };

Best regards,
Krzysztof

2022-06-29 11:01:33

[permalink] [raw]

Subject: Re: [PATCH 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject

On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:

(...)

> + - compatible
> + - reg
> + - xlnx,magic
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + tmr-inject@44a30000 {
> + compatible = "xlnx,tmr-inject-1.0";

Ah, and one more - wrong indentation. Use 4 spaces for DTS example.

Best regards,
Krzysztof

2022-06-29 11:40:51

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 6/29/22 12:07, Krzysztof Kozlowski wrote:
> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>> This commit adds documentation for Triple Modular Redundancy(TMR) Manager
>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
>> the TMR subsystem state, including fault detection and error recovery
>> provides soft error detection, correction and recovery features.
>>
>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>> ---
>> .../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++
>
> This is not a misc device. Find appropriate subsystem for it. It's not
> EDAC, right?

We were thinking where to put it but it is not EDAC driver.
If you have better suggestion for subsystem please let us know.

>
>> 1 file changed, 48 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..f6cb4d235981
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>> @@ -0,0 +1,48 @@
>> +# 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 TMR Microblaze Device Tree Bindings
>
> s/Device Tree Bindings//
>
>> +
>> +maintainers:
>> + - Appana Durga Kedareswara rao <[email protected]>
>> +
>> +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 number 1, When writing to the control register the first write data
>> + byte (bits 7:0) must match this value in order to have any effect on the
>> + nominal recovery function.
>
> Register values are usually not in the scope of bindings. We describe
> here hardware, not programming model, although in case of soft cores
> maybe it's a bit different.

you need to setup this value for every instance in design tools and it is
directly present in HW. It means this value is really describing HW.

Thanks,
Michal

2022-06-29 11:52:10

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 29/06/2022 13:23, Michal Simek wrote:
>
>
> On 6/29/22 12:07, Krzysztof Kozlowski wrote:
>> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>>> This commit adds documentation for Triple Modular Redundancy(TMR) Manager
>>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
>>> the TMR subsystem state, including fault detection and error recovery
>>> provides soft error detection, correction and recovery features.
>>>
>>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>>> ---
>>> .../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++
>>
>> This is not a misc device. Find appropriate subsystem for it. It's not
>> EDAC, right?
>
> We were thinking where to put it but it is not EDAC driver.
> If you have better suggestion for subsystem please let us know.

I don't know what's the device about. The description does not help:

"TMR Manager is responsible for TMR subsystem state..."

>
>>
>>> 1 file changed, 48 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..f6cb4d235981
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>>> @@ -0,0 +1,48 @@
>>> +# 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 TMR Microblaze Device Tree Bindings
>>
>> s/Device Tree Bindings//
>>
>>> +
>>> +maintainers:
>>> + - Appana Durga Kedareswara rao <[email protected]>
>>> +
>>> +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 number 1, When writing to the control register the first write data
>>> + byte (bits 7:0) must match this value in order to have any effect on the
>>> + nominal recovery function.
>>
>> Register values are usually not in the scope of bindings. We describe
>> here hardware, not programming model, although in case of soft cores
>> maybe it's a bit different.
>
> you need to setup this value for every instance in design tools and it is
> directly present in HW. It means this value is really describing HW.

OK, thanks for clarification.

Best regards,
Krzysztof

2022-06-29 11:54:09

[permalink] [raw]

Subject: Re: [PATCH 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject

On 6/29/22 12:08, Krzysztof Kozlowski wrote:
> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>> This commit adds documentation for Triple Modular Redundancy (TMR)
>
> No "This commit"
>
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
>> Inject IP.
>>
>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>> ---
>> .../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..4daf9539000b
>> --- /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 TMR Inject Device Tree Bindings
>
> Same comments.
>
>> +
>> +maintainers:
>> + - Appana Durga Kedareswara rao <[email protected]>
>> +
>> +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 used when injecting faults. The fault inject write data least
>> + significant byte (bits 7:0) must match this number to have any effect.
>
> And why exactly register value is a property of DT?
>
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - xlnx,magic
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + tmr-inject@44a30000 {
>
> fault-inject?

no problem with it but I think it would be good to extend 2.2.2 chapter in
devicetree specification with these generic names.

Thanks,
Michal

2022-06-29 12:26:52

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 6/29/22 13:45, Krzysztof Kozlowski wrote:
> On 29/06/2022 13:23, Michal Simek wrote:
>>
>>
>> On 6/29/22 12:07, Krzysztof Kozlowski wrote:
>>> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>>>> This commit adds documentation for Triple Modular Redundancy(TMR) Manager
>>>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
>>>> the TMR subsystem state, including fault detection and error recovery
>>>> provides soft error detection, correction and recovery features.
>>>>
>>>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>>>> ---
>>>> .../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++
>>>
>>> This is not a misc device. Find appropriate subsystem for it. It's not
>>> EDAC, right?
>>
>> We were thinking where to put it but it is not EDAC driver.
>> If you have better suggestion for subsystem please let us know.
>
> I don't know what's the device about. The description does not help:
>
> "TMR Manager is responsible for TMR subsystem state..."

ok. let's improve commit message in v2.

TMR - triple module redundancy.

You design the system with one CPU which is default microblaze configuration
with interrupt controller, timer and other IPs.

And then say I want to do it triple redundant with all that voting, etc.
If you want to get all details you can take a look at this guide

https://www.xilinx.com/content/dam/xilinx/support/documents/ip_documentation/tmr/v1_0/pg268-tmr.pdf

In short TMR manager is servicing all that 3 cores and making sure that they are
all running in sync. If not it has capability recover the system. It means cpu
gets to break handler (it is the part of microblaze series) and it restarts all
cpus.

And TMR inject driver is module which is capable to inject error to internal
memory to cause the exception to exercise that recovery code.

Kedar: Feel free to correct me or add more details.

Thanks,
Michal

2022-06-29 12:52:41

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

Hi Krzysztof,

Thanks for the review, please find response inline.

On 29/06/22 3:37 pm, Krzysztof Kozlowski wrote:
> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>> This commit adds documentation for Triple Modular Redundancy(TMR) Manager
>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for handling
>> the TMR subsystem state, including fault detection and error recovery
>> provides soft error detection, correction and recovery features.
>>
>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>> ---
>> .../bindings/misc/xlnx,tmr-manager.yaml | 48 +++++++++++++++++++
>
> This is not a misc device. Find appropriate subsystem for it. It's not
> EDAC, right?
>
>> 1 file changed, 48 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..f6cb4d235981
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>> @@ -0,0 +1,48 @@
>> +# 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 TMR Microblaze Device Tree Bindings
>
> s/Device Tree Bindings//

Sure will fix in v2.
>
>> +
>> +maintainers:
>> + - Appana Durga Kedareswara rao <[email protected]>
>> +
>> +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 number 1, When writing to the control register the first write data
>> + byte (bits 7:0) must match this value in order to have any effect on the
>> + nominal recovery function.
>
> Register values are usually not in the scope of bindings. We describe
> here hardware, not programming model, although in case of soft cores
> maybe it's a bit different.
>
> Anyway you need to describe the actual feature, not OS behavior.

Like Michal said for every instance of the hardware
this value can be configured at vivado tool level.

>
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - xlnx,magic1
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + tmr-manager@44a10000 {
>
> Depending on what is this, you should use appropriate generic node name.

Sure will fix in v2...

Regards,
Kedar.
>
>> + compatible = "xlnx,tmr-manager-1.0";
>> + reg = <0x44a10000 0x10000>;
>> + xlnx,magic1 = <0x46>;
>> + };
>
>
> Best regards,
> Krzysztof

2022-06-29 13:15:07

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

Hi,

On 29/06/22 5:29 pm, Michal Simek wrote:
>
>
> On 6/29/22 13:45, Krzysztof Kozlowski wrote:
>> On 29/06/2022 13:23, Michal Simek wrote:
>>>
>>>
>>> On 6/29/22 12:07, Krzysztof Kozlowski wrote:
>>>> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>>>>> This commit adds documentation for Triple Modular Redundancy(TMR)
>>>>> Manager
>>>>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for
>>>>> handling
>>>>> the TMR subsystem state, including fault detection and error recovery
>>>>> provides soft error detection, correction and recovery features.
>>>>>
>>>>> Signed-off-by: Appana Durga Kedareswara rao
>>>>> <[email protected]>
>>>>> ---
>>>>> .../bindings/misc/xlnx,tmr-manager.yaml | 48
>>>>> +++++++++++++++++++
>>>>
>>>> This is not a misc device. Find appropriate subsystem for it. It's not
>>>> EDAC, right?
>>>
>>> We were thinking where to put it but it is not EDAC driver.
>>> If you have better suggestion for subsystem please let us know.
>>
>> I don't know what's the device about. The description does not help:
>>
>> "TMR Manager is responsible for TMR subsystem state..."
>
> ok. let's improve commit message in v2.

Sure will improve the commit message in v2.
>
> TMR - triple module redundancy.
>
> You design the system with one CPU which is default microblaze
> configuration with interrupt controller, timer and other IPs.
>
> And then say I want to do it triple redundant with all that voting, etc.
> If you want to get all details you can take a look at this guide
>
> https://www.xilinx.com/content/dam/xilinx/support/documents/ip_documentation/tmr/v1_0/pg268-tmr.pdf
>
>
> In short TMR manager is servicing all that 3 cores and making sure that
> they are all running in sync. If not it has capability recover the
> system. It means cpu gets to break handler (it is the part of microblaze
> series) and it restarts all cpus.
>
> And TMR inject driver is module which is capable to inject error to
> internal memory to cause the exception to exercise that recovery code.
>
> Kedar: Feel free to correct me or add more details.

Thanks Michal for the detailed explanation.

The Triple Modular Redundancy(TMR) subsystem has three Microblaze
processor instances, If any one of the Microblaze processors goes to an
unknown state due to fault injection break handler will get called,
which in turn calls the tmr manager driver API to perform recovery.
like Michal said TMR inject driver is capable of inject error to
internal memory to cause fault in one the Microblaze processor

@Krzysztof : please let me know if more information required about
this TMR subsystem will provide.

Regards,
Kedar.

>
> Thanks,
> Michal

2022-06-29 13:24:53

[permalink] [raw]

Subject: Re: [PATCH 3/4] dt-bindings: misc: tmr-inject: Add device-tree binding for TMR Inject

Hi,

On 29/06/22 3:39 pm, Krzysztof Kozlowski wrote:
> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>
> (...)
>
>> + - compatible
>> + - reg
>> + - xlnx,magic
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + tmr-inject@44a30000 {
>> + compatible = "xlnx,tmr-inject-1.0";
>
> Ah, and one more - wrong indentation. Use 4 spaces for DTS example.

Sure will fix in v2.

Regards,
Kedar.

>
>
> Best regards,
> Krzysztof

2022-06-30 10:09:58

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 29/06/2022 14:37, Rao, Appana Durga Kedareswara wrote:
> Hi,
>
>
> On 29/06/22 5:29 pm, Michal Simek wrote:
>>
>>
>> On 6/29/22 13:45, Krzysztof Kozlowski wrote:
>>> On 29/06/2022 13:23, Michal Simek wrote:
>>>>
>>>>
>>>> On 6/29/22 12:07, Krzysztof Kozlowski wrote:
>>>>> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>>>>>> This commit adds documentation for Triple Modular Redundancy(TMR)
>>>>>> Manager
>>>>>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for
>>>>>> handling
>>>>>> the TMR subsystem state, including fault detection and error recovery
>>>>>> provides soft error detection, correction and recovery features.
>>>>>>
>>>>>> Signed-off-by: Appana Durga Kedareswara rao
>>>>>> <[email protected]>
>>>>>> ---
>>>>>> .../bindings/misc/xlnx,tmr-manager.yaml | 48
>>>>>> +++++++++++++++++++
>>>>>
>>>>> This is not a misc device. Find appropriate subsystem for it. It's not
>>>>> EDAC, right?
>>>>
>>>> We were thinking where to put it but it is not EDAC driver.
>>>> If you have better suggestion for subsystem please let us know.
>>>
>>> I don't know what's the device about. The description does not help:
>>>
>>> "TMR Manager is responsible for TMR subsystem state..."
>>
>> ok. let's improve commit message in v2.
>
> Sure will improve the commit message in v2.
>>
>> TMR - triple module redundancy.
>>
>> You design the system with one CPU which is default microblaze
>> configuration with interrupt controller, timer and other IPs.
>>
>> And then say I want to do it triple redundant with all that voting, etc.
>> If you want to get all details you can take a look at this guide
>>
>> https://www.xilinx.com/content/dam/xilinx/support/documents/ip_documentation/tmr/v1_0/pg268-tmr.pdf
>>
>>
>> In short TMR manager is servicing all that 3 cores and making sure that
>> they are all running in sync. If not it has capability recover the
>> system. It means cpu gets to break handler (it is the part of microblaze
>> series) and it restarts all cpus.
>>
>> And TMR inject driver is module which is capable to inject error to
>> internal memory to cause the exception to exercise that recovery code.
>>
>> Kedar: Feel free to correct me or add more details.
>
> Thanks Michal for the detailed explanation.
>
> The Triple Modular Redundancy(TMR) subsystem has three Microblaze
> processor instances, If any one of the Microblaze processors goes to an
> unknown state due to fault injection break handler will get called,
> which in turn calls the tmr manager driver API to perform recovery.
> like Michal said TMR inject driver is capable of inject error to
> internal memory to cause fault in one the Microblaze processor
>
> @Krzysztof : please let me know if more information required about
> this TMR subsystem will provide.

Some features sound like watchdog. If it was ARM, I would suggest to put
it under "soc". Is a term System-on-Chip applicable to Microblaze? Other
option is to store it under microblaze (although for ARM and RISC-V this
is actually discouraged in favor of soc).

Best regards,
Krzysztof

2022-06-30 11:45:58

[permalink] [raw]

Subject: Re: [PATCH 1/4] dt-bindings: misc: tmr-manager: Add device-tree binding for TMR Manager

On 6/30/22 12:07, Krzysztof Kozlowski wrote:
> On 29/06/2022 14:37, Rao, Appana Durga Kedareswara wrote:
>> Hi,
>>
>>
>> On 29/06/22 5:29 pm, Michal Simek wrote:
>>>
>>>
>>> On 6/29/22 13:45, Krzysztof Kozlowski wrote:
>>>> On 29/06/2022 13:23, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 6/29/22 12:07, Krzysztof Kozlowski wrote:
>>>>>> On 28/06/2022 07:43, Appana Durga Kedareswara rao wrote:
>>>>>>> This commit adds documentation for Triple Modular Redundancy(TMR)
>>>>>>> Manager
>>>>>>> IP. The Triple Modular Redundancy(TMR) Manager is responsible for
>>>>>>> handling
>>>>>>> the TMR subsystem state, including fault detection and error recovery
>>>>>>> provides soft error detection, correction and recovery features.
>>>>>>>
>>>>>>> Signed-off-by: Appana Durga Kedareswara rao
>>>>>>> <[email protected]>
>>>>>>> ---
>>>>>>> .../bindings/misc/xlnx,tmr-manager.yaml | 48
>>>>>>> +++++++++++++++++++
>>>>>>
>>>>>> This is not a misc device. Find appropriate subsystem for it. It's not
>>>>>> EDAC, right?
>>>>>
>>>>> We were thinking where to put it but it is not EDAC driver.
>>>>> If you have better suggestion for subsystem please let us know.
>>>>
>>>> I don't know what's the device about. The description does not help:
>>>>
>>>> "TMR Manager is responsible for TMR subsystem state..."
>>>
>>> ok. let's improve commit message in v2.
>>
>> Sure will improve the commit message in v2.
>>>
>>> TMR - triple module redundancy.
>>>
>>> You design the system with one CPU which is default microblaze
>>> configuration with interrupt controller, timer and other IPs.
>>>
>>> And then say I want to do it triple redundant with all that voting, etc.
>>> If you want to get all details you can take a look at this guide
>>>
>>> https://www.xilinx.com/content/dam/xilinx/support/documents/ip_documentation/tmr/v1_0/pg268-tmr.pdf
>>>
>>>
>>> In short TMR manager is servicing all that 3 cores and making sure that
>>> they are all running in sync. If not it has capability recover the
>>> system. It means cpu gets to break handler (it is the part of microblaze
>>> series) and it restarts all cpus.
>>>
>>> And TMR inject driver is module which is capable to inject error to
>>> internal memory to cause the exception to exercise that recovery code.
>>>
>>> Kedar: Feel free to correct me or add more details.
>>
>> Thanks Michal for the detailed explanation.
>>
>> The Triple Modular Redundancy(TMR) subsystem has three Microblaze
>> processor instances, If any one of the Microblaze processors goes to an
>> unknown state due to fault injection break handler will get called,
>> which in turn calls the tmr manager driver API to perform recovery.
>> like Michal said TMR inject driver is capable of inject error to
>> internal memory to cause fault in one the Microblaze processor
>>
>> @Krzysztof : please let me know if more information required about
>> this TMR subsystem will provide.
>
> Some features sound like watchdog.

watchdog needs to be keep alive which is not the case here. These systems are
designed for safety or space applications and I am quite sure that there are
going to be couple or regular watchdogs wired too.

> If it was ARM, I would suggest to put
> it under "soc". Is a term System-on-Chip applicable to Microblaze?

I have never seen microblaze in connection to SOC. You can make SOC based on
microblaze cpu (using hard cores) but in most cases microblaze as soft cpu is
loaded to fpga or to programmable logic.

> Other
> option is to store it under microblaze (although for ARM and RISC-V this
> is actually discouraged in favor of soc).

Exactly. That was my main concern too that adding to microblaze is likely not
the best location that's why we wanted to add it to different location.

Thanks,
Michal

2022-07-01 08:47:31

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH 2/4] drivers: misc: Add Support for TMR Manager

On Tue, Jun 28, 2022 at 11:13:36AM +0530, Appana Durga Kedareswara rao wrote:
> The Triple Modular Redundancy(TMR) Manager is responsible for handling
> the TMR subsystem state, including fault detection and error recovery
> provides soft error detection, correction and recovery features.
>
> This commit adds 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 <[email protected]>
> ---
> .../testing/sysfs-driver-xilinx-tmr-manager | 18 ++
> MAINTAINERS | 7 +
> drivers/misc/Kconfig | 10 +
> drivers/misc/Makefile | 1 +
> drivers/misc/xilinx_tmr_manager.c | 288 ++++++++++++++++++
> 5 files changed, 324 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..c2f615126201
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
> @@ -0,0 +1,18 @@
> +What: /sys/devices/platform/amba_pl/<dev>/status
> +Date: June 2022
> +Contact: [email protected]
> +Description: This control file provides the status of the tmr manager
> + useful for getting the status of fault.
> + This file cannot be written.

Examples of what the data in this file will be?

> +
> +What: /sys/devices/platform/amba_pl/<dev>/errcnt
> +Date: June 2022
> +Contact: [email protected]
> +Description: This control file provides the fault detection count.
> + This file cannot be written.

Again, example output?

> +
> +What: /sys/devices/platform/amba_pl/<dev>/dis_block_break
> +Date: June 2022
> +Contact: [email protected]
> +Description: This control file enables the break signal.
> + This file is write only.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6cc825857722..3d29109788ed 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12962,6 +12962,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 <[email protected]>
> +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 <[email protected]>
> M: Tudor Ambarus <[email protected]>
> 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..5a4562e4a9d4
> --- /dev/null
> +++ b/drivers/misc/xilinx_tmr_manager.c
> @@ -0,0 +1,288 @@
> +// 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
> + * @dev: pointer to device struct
> + * @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;
> + struct device *dev;

Is this the parent of this device? Why not make a new device? The
interaction here isn't well defined, what controls the lifespan of this
structure?

> + u32 cr_val;
> + u32 magic1;
> + u32 err_cnt;
> + uintptr_t phys_baseaddr;

This should be resource_size_t, not uintptr_t, right?

> +};
> +
> +/* 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);
> +}
> +
> +/**
> + * xtmr_manager_unblock_break - unblocks the break signal
> + * @xtmr_manager: Pointer to xtmr_manager_dev structure
> + */
> +static void xtmr_manager_unblock_break(struct xtmr_manager_dev *xtmr_manager)
> +{
> + xtmr_manager->cr_val &= ~(1 << XTMR_MANAGER_CR_BB_SHIFT);
> + xtmr_manager_write(xtmr_manager, XTMR_MANAGER_CR_OFFSET,
> + xtmr_manager->cr_val);
> +}
> +
> +/**
> + * xmb_manager_reset_handler - clears the ffr register contents
> + * @priv: Private pointer
> + */
> +static void xmb_manager_reset_handler(void *priv)
> +{
> + struct xtmr_manager_dev *xtmr_manager = (struct xtmr_manager_dev *)priv;
> + /* Clear the FFR Register contents as a part of recovery process. */
> + xtmr_manager_write(xtmr_manager, XTMR_MANAGER_FFR_OFFSET, 0);

Did you run checkpatch on this code? Please do so.

> +}
> +
> +/**

No need for kernel doc formatting for static functions, right?

> + * xmb_manager_update_errcnt - update the error inject count
> + * @priv: Private pointer
> + */
> +static void xmb_manager_update_errcnt(void *priv)

Use a real pointer, not a void one please. We want type safe functions.

> +{
> + struct xtmr_manager_dev *xtmr_manager = (struct xtmr_manager_dev *)priv;

And even if you did have a real pointer, you never need to cast from a
void *.

> +
> + 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 sprintf(buf, "%x\n", xtmr_manager->err_cnt);

sysfs_emit() here and everywhere please.

> +}
> +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 += sprintf(buf + len,
> + "Lockstep mismatch between processor 1 and 2\n");
> + }
> +
> + if ((ffr & XTMR_MANAGER_FFR_LM13_MASK) == XTMR_MANAGER_FFR_LM13_MASK) {
> + len += sprintf(buf + len,
> + "Lockstep mismatch between processor 1 and 3\n");
> + }
> +
> + if ((ffr & XTMR_MANAGER_FFR_LM23_MASK) == XTMR_MANAGER_FFR_LM23_MASK) {
> + len += sprintf(buf + len,
> + "Lockstep mismatch between processor 2 and 3\n");

That feels like a lot more than "one bit of information", who is going
to parse this and do something with this information?

> + }
> +
> + 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;
> +
> + xtmr_manager_unblock_break(xtmr_manager);

No error checking?

> + return size;
> +}
> +static DEVICE_ATTR_WO(dis_block_break);
> +
> +static struct attribute *xtmr_manager_attrs[] = {
> + &dev_attr_dis_block_break.attr,
> + &dev_attr_status.attr,
> + &dev_attr_errcnt.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(xtmr_manager);
> +
> +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,
> + xmb_manager_update_errcnt,
> + xtmr_manager, xmb_manager_reset_handler);

No error handling?

> +}
> +
> +/**
> + * 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;
> +
> + xtmr_manager->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + xtmr_manager->regs = devm_ioremap_resource(xtmr_manager->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;
> + } else if (xtmr_manager->magic1 > XTMR_MANAGER_MAGIC1_MAX_VAL) {

No need for else.

> + dev_err(&pdev->dev, "invalid xlnx,magic1 property value");
> + return err;

but err is not an error value here, right?

> + }
> +
> + /* Initialize TMR Manager */
> + xtmr_manager_init(xtmr_manager);
> +
> + err = sysfs_create_groups(&xtmr_manager->dev->kobj,
> + xtmr_manager_groups);

You just raced with userspace and lost. NEVER deal with raw kobjects,
that's a huge sign that you are doing something wrong in a driver.

Please use the proper default groups pointer for your driver to do this
correctly so that the driver core will handle this for you.

> + if (err < 0) {
> + dev_err(&pdev->dev, "unable to create sysfs entries\n");
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, xtmr_manager);
> +
> + return 0;
> +}
> +
> +static int xtmr_manager_remove(struct platform_device *pdev)
> +{
> + sysfs_remove_groups(&pdev->dev.kobj, xtmr_manager_groups);

Again, never touch a kobject in a driver.

thanks,

greg k-h

2022-07-04 04:44:36