2022-09-07 11:31:16

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v4 0/2] edac: Add support for Xilinx ZynqMP OCM EDAC

Add binding and driver for Xilinx ZynqMP OCM controller.

changes in v4:
-> 2/2 - Replaced \n\r with \n.

changes in v3:
-> 1/2 - Moved the binding from edac to memory-controllers directory.
-> 1/2 - Changed the file name to match with the compatible.
-> 1/2 - Used additionalProperties instead of unevaluatedProperties.
-> 1/2 - Used macro instead of constant value.

changes in v2:
-> 1/2 - Used define for interrupt flag.
-> 1/2 - Updated the description and title.
-> 2/2 - Removed Kernel doc for probe and remove.
-> 2/2 - Used COMPILE_TEST, used wrapper for get and ioremap resource.
-> 2/2 - Fixed few comments related to static variable declaration
and print statements.


Sai Krishna Potthuri (1):
edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

Shubhrajyoti Datta (1):
dt-bindings: edac: Add bindings for Xilinx ZynqMP OCM

.../xlnx,zynqmp-ocmc-1.0.yaml | 45 ++
MAINTAINERS | 7 +
drivers/edac/Kconfig | 9 +
drivers/edac/Makefile | 1 +
drivers/edac/zynqmp_ocm_edac.c | 622 ++++++++++++++++++
5 files changed, 684 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml
create mode 100644 drivers/edac/zynqmp_ocm_edac.c

--
2.17.1


2022-09-07 12:00:28

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v4 1/2] dt-bindings: edac: Add bindings for Xilinx ZynqMP OCM

From: Shubhrajyoti Datta <[email protected]>

Add bindings for Xilinx ZynqMP OCM controller.

Signed-off-by: Shubhrajyoti Datta <[email protected]>
Signed-off-by: Sai Krishna Potthuri <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../xlnx,zynqmp-ocmc-1.0.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml b/Documentation/devicetree/bindings/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml
new file mode 100644
index 000000000000..ca9fc747bf4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/xlnx,zynqmp-ocmc-1.0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Zynqmp OCM(On-Chip Memory) Controller
+
+maintainers:
+ - Shubhrajyoti Datta <[email protected]>
+ - Sai Krishna Potthuri <[email protected]>
+
+description: |
+ The OCM supports 64-bit wide ECC functionality to detect multi-bit errors
+ and recover from a single-bit memory fault.On a write, if all bytes are
+ being written, the ECC is generated and written into the ECC RAM along with
+ the write-data that is written into the data RAM. If one or more bytes are
+ not written, then the read operation results in an correctable error or
+ uncorrectable error.
+
+properties:
+ compatible:
+ const: xlnx,zynqmp-ocmc-1.0
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ memory-controller@ff960000 {
+ compatible = "xlnx,zynqmp-ocmc-1.0";
+ reg = <0xff960000 0x1000>;
+ interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+ };
--
2.17.1

2022-09-07 12:29:31

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

Add EDAC support for Xilinx ZynqMP OCM Controller, this driver
reports CE and UE errors based on the interrupts, and also creates ue/ce
sysfs entries for error injection.

Signed-off-by: Sai Krishna Potthuri <[email protected]>
Signed-off-by: Shubhrajyoti Datta <[email protected]>
---
MAINTAINERS | 7 +
drivers/edac/Kconfig | 9 +
drivers/edac/Makefile | 1 +
drivers/edac/zynqmp_ocm_edac.c | 622 +++++++++++++++++++++++++++++++++
4 files changed, 639 insertions(+)
create mode 100644 drivers/edac/zynqmp_ocm_edac.c

diff --git a/MAINTAINERS b/MAINTAINERS
index edc96cdb85e8..cd4c6c90bca3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21692,6 +21692,13 @@ F: Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
F: drivers/dma/xilinx/xilinx_dpdma.c
F: include/dt-bindings/dma/xlnx-zynqmp-dpdma.h

+XILINX ZYNQMP OCM EDAC DRIVER
+M: Shubhrajyoti Datta <[email protected]>
+M: Sai Krishna Potthuri <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/edac/xlnx,zynqmp-ocmc.yaml
+F: drivers/edac/zynqmp_ocm_edac.c
+
XILINX ZYNQMP PSGTR PHY DRIVER
M: Anurag Kumar Vulisha <[email protected]>
M: Laurent Pinchart <[email protected]>
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 58ab63642e72..08adedfd42b5 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -539,4 +539,13 @@ config EDAC_DMC520
Support for error detection and correction on the
SoCs with ARM DMC-520 DRAM controller.

+config EDAC_ZYNQMP_OCM
+ tristate "Xilinx ZynqMP OCM Controller"
+ depends on ARCH_ZYNQMP || COMPILE_TEST
+ help
+ Support for error detection and correction on the xilinx ZynqMP OCM
+ controller.
+ This driver can also be built as a module. If so, the module
+ will be called zynqmp_ocm_edac.
+
endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 2d1641a27a28..634c1cec1588 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_EDAC_QCOM) += qcom_edac.o
obj-$(CONFIG_EDAC_ASPEED) += aspeed_edac.o
obj-$(CONFIG_EDAC_BLUEFIELD) += bluefield_edac.o
obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o
+obj-$(CONFIG_EDAC_ZYNQMP_OCM) += zynqmp_ocm_edac.o
diff --git a/drivers/edac/zynqmp_ocm_edac.c b/drivers/edac/zynqmp_ocm_edac.c
new file mode 100644
index 000000000000..a8da94f3e067
--- /dev/null
+++ b/drivers/edac/zynqmp_ocm_edac.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx ZynqMP OCM ECC Driver
+ *
+ * Copyright (C) 2022 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "edac_module.h"
+
+#define ZYNQMP_OCM_EDAC_MSG_SIZE 256
+
+#define ZYNQMP_OCM_EDAC_STRING "zynqmp_ocm"
+
+/* Controller registers */
+#define CTRL_OFST 0x0
+#define OCM_ISR_OFST 0x04
+#define OCM_IMR_OFST 0x08
+#define OCM_IEN_OFST 0x0C
+#define OCM_IDS_OFST 0x10
+
+/* ECC control register */
+#define ECC_CTRL_OFST 0x14
+
+/* Correctable error info registers */
+#define CE_FFA_OFST 0x1C
+#define CE_FFD0_OFST 0x20
+#define CE_FFD1_OFST 0x24
+#define CE_FFD2_OFST 0x28
+#define CE_FFD3_OFST 0x2C
+#define CE_FFE_OFST 0x30
+
+/* Uncorrectable error info registers */
+#define UE_FFA_OFST 0x34
+#define UE_FFD0_OFST 0x38
+#define UE_FFD1_OFST 0x3C
+#define UE_FFD2_OFST 0x40
+#define UE_FFD3_OFST 0x44
+#define UE_FFE_OFST 0x48
+
+/* ECC control register bit field definitions */
+#define ECC_CTRL_CLR_CE_ERR 0x40
+#define ECC_CTRL_CLR_UE_ERR 0x80
+
+/* Fault injection data and count registers */
+#define OCM_FID0_OFST 0x4C
+#define OCM_FID1_OFST 0x50
+#define OCM_FID2_OFST 0x54
+#define OCM_FID3_OFST 0x58
+#define OCM_FIC_OFST 0x74
+
+#define UE_MAX_BITPOS_LOWER 31
+#define UE_MIN_BITPOS_UPPER 32
+#define UE_MAX_BITPOS_UPPER 63
+
+/* Interrupt masks */
+#define OCM_CEINTR_MASK BIT(6)
+#define OCM_UEINTR_MASK BIT(7)
+#define OCM_ECC_ENABLE_MASK BIT(0)
+#define OCM_CEUE_MASK GENMASK(7, 6)
+
+#define OCM_FICOUNT_MASK GENMASK(23, 0)
+#define OCM_BASEVAL 0xFFFC0000
+#define EDAC_DEVICE "ZynqMP-OCM"
+
+/**
+ * struct ecc_error_info - ECC error log information
+ * @addr: Fault generated at this address
+ * @data0: Generated fault data
+ * @data1: Generated fault data
+ */
+struct ecc_error_info {
+ u32 addr;
+ u32 data0;
+ u32 data1;
+};
+
+/**
+ * struct zynqmp_ocm_ecc_status - ECC status information to report
+ * @ce_cnt: Correctable error count
+ * @ue_cnt: Uncorrectable error count
+ * @ceinfo: Correctable error log information
+ * @ueinfo: Uncorrectable error log information
+ */
+struct zynqmp_ocm_ecc_status {
+ u32 ce_cnt;
+ u32 ue_cnt;
+ struct ecc_error_info ceinfo;
+ struct ecc_error_info ueinfo;
+};
+
+/**
+ * struct zynqmp_ocm_edac_priv - DDR memory controller private instance data
+ * @baseaddr: Base address of the DDR controller
+ * @message: Buffer for framing the event specific info
+ * @stat: ECC status information
+ * @p_data: Pointer to platform data
+ * @ce_cnt: Correctable Error count
+ * @ue_cnt: Uncorrectable Error count
+ * @ce_bitpos: Bit position for Correctable Error
+ * @ue_bitpos0: First bit position for Uncorrectable Error
+ * @ue_bitpos1: Second bit position for Uncorrectable Error
+ */
+struct zynqmp_ocm_edac_priv {
+ void __iomem *baseaddr;
+ char message[ZYNQMP_OCM_EDAC_MSG_SIZE];
+ struct zynqmp_ocm_ecc_status stat;
+ const struct zynqmp_ocm_platform_data *p_data;
+ u32 ce_cnt;
+ u32 ue_cnt;
+ u8 ce_bitpos;
+ u8 ue_bitpos0;
+ u8 ue_bitpos1;
+};
+
+/**
+ * zynqmp_ocm_edac_geterror_info - Get the current ecc error info
+ * @base: Pointer to the base address of the ddr memory controller
+ * @p: Pointer to the ocm ecc status structure
+ * @mask: Status register mask value
+ *
+ * Determines there is any ecc error or not
+ *
+ */
+static void zynqmp_ocm_edac_geterror_info(void __iomem *base,
+ struct zynqmp_ocm_ecc_status *p, int mask)
+{
+ if (mask & OCM_CEINTR_MASK) {
+ p->ce_cnt++;
+ p->ceinfo.data0 = readl(base + CE_FFD0_OFST);
+ p->ceinfo.data1 = readl(base + CE_FFD1_OFST);
+ p->ceinfo.addr = (OCM_BASEVAL | readl(base + CE_FFA_OFST));
+ writel(ECC_CTRL_CLR_CE_ERR, base + OCM_ISR_OFST);
+ } else if (mask & OCM_UEINTR_MASK) {
+ p->ue_cnt++;
+ p->ueinfo.data0 = readl(base + UE_FFD0_OFST);
+ p->ueinfo.data1 = readl(base + UE_FFD1_OFST);
+ p->ueinfo.addr = (OCM_BASEVAL | readl(base + UE_FFA_OFST));
+ writel(ECC_CTRL_CLR_UE_ERR, base + OCM_ISR_OFST);
+ }
+}
+
+/**
+ * zynqmp_ocm_edac_handle_error - Handle controller error types CE and UE
+ * @dci: Pointer to the edac device controller instance
+ * @p: Pointer to the ocm ecc status structure
+ *
+ * Handles the controller ECC correctable and uncorrectable error.
+ */
+static void zynqmp_ocm_edac_handle_error(struct edac_device_ctl_info *dci,
+ struct zynqmp_ocm_ecc_status *p)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+ struct ecc_error_info *pinf;
+
+ if (p->ce_cnt) {
+ pinf = &p->ceinfo;
+ snprintf(priv->message, ZYNQMP_OCM_EDAC_MSG_SIZE,
+ "\nOCM ECC error type :%s\n"
+ "Addr: [0x%X]\nFault Data[31:0]: [0x%X]\n"
+ "Fault Data[63:32]: [0x%X]",
+ "CE", pinf->addr, pinf->data0, pinf->data1);
+ edac_device_handle_ce(dci, 0, 0, priv->message);
+ }
+
+ if (p->ue_cnt) {
+ pinf = &p->ueinfo;
+ snprintf(priv->message, ZYNQMP_OCM_EDAC_MSG_SIZE,
+ "\nOCM ECC error type :%s\n"
+ "Addr: [0x%X]\nFault Data[31:0]: [0x%X]\n"
+ "Fault Data[63:32]: [0x%X]",
+ "UE", pinf->addr, pinf->data0, pinf->data1);
+ edac_device_handle_ue(dci, 0, 0, priv->message);
+ }
+
+ memset(p, 0, sizeof(*p));
+}
+
+/**
+ * zynqmp_ocm_edac_intr_handler - isr routine
+ * @irq: irq number
+ * @dev_id: device id pointer
+ *
+ * This is the ISR routine called by edac core interrupt thread.
+ * Used to check and post ECC errors.
+ *
+ * Return: IRQ_NONE, if interrupt not set or IRQ_HANDLED otherwise
+ */
+static irqreturn_t zynqmp_ocm_edac_intr_handler(int irq, void *dev_id)
+{
+ struct edac_device_ctl_info *dci = dev_id;
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+ int regval;
+
+ regval = readl(priv->baseaddr + OCM_ISR_OFST);
+ if (!(regval & OCM_CEUE_MASK))
+ return IRQ_NONE;
+
+ zynqmp_ocm_edac_geterror_info(priv->baseaddr,
+ &priv->stat, regval);
+
+ priv->ce_cnt += priv->stat.ce_cnt;
+ priv->ue_cnt += priv->stat.ue_cnt;
+ zynqmp_ocm_edac_handle_error(dci, &priv->stat);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * zynqmp_ocm_edac_get_eccstate - Return the controller ecc status
+ * @base: Pointer to the ddr memory controller base address
+ *
+ * Get the ECC enable/disable status for the controller
+ *
+ * Return: ecc status 0/1.
+ */
+static bool zynqmp_ocm_edac_get_eccstate(void __iomem *base)
+{
+ return readl(base + ECC_CTRL_OFST) & OCM_ECC_ENABLE_MASK;
+}
+
+/**
+ * zynqmp_ocm_edac_inject_fault_count_show - Shows fault injection count
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ *
+ * Shows the fault injection count, once the counter reaches
+ * zero, it injects errors
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_fault_count_show(struct edac_device_ctl_info *dci,
+ char *data)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ return sprintf(data, "FIC: 0x%x\n",
+ readl(priv->baseaddr + OCM_FIC_OFST));
+}
+
+/**
+ * zynqmp_ocm_edac_inject_fault_count_store - write fi count
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ * @count: read the size bytes from buffer
+ *
+ * Update the fault injection count register, once the counter reaches
+ * zero, it injects errors
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_fault_count_store(struct edac_device_ctl_info *dci,
+ const char *data, size_t count)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+ u32 ficount;
+
+ if (!data)
+ return -EFAULT;
+
+ if (kstrtouint(data, 0, &ficount))
+ return -EINVAL;
+
+ ficount &= OCM_FICOUNT_MASK;
+ writel(ficount, priv->baseaddr + OCM_FIC_OFST);
+
+ return count;
+}
+
+/**
+ * zynqmp_ocm_edac_inject_cebitpos_show - Shows CE bit position
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ *
+ * Shows the Correctable error bit position,
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_cebitpos_show(struct edac_device_ctl_info
+ *dci, char *data)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ if (priv->ce_bitpos <= UE_MAX_BITPOS_LOWER)
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID0_OFST))));
+
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID1_OFST))));
+}
+
+/**
+ * zynqmp_ocm_edac_inject_cebitpos_store - Set CE bit position
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ * @count: read the size bytes from buffer
+ *
+ * Set any one bit to inject CE error
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_cebitpos_store(struct edac_device_ctl_info *dci,
+ const char *data, size_t count)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ if (!data)
+ return -EFAULT;
+
+ if (kstrtou8(data, 0, &priv->ce_bitpos))
+ return -EINVAL;
+
+ if (priv->ce_bitpos <= UE_MAX_BITPOS_LOWER) {
+ writel(1 << priv->ce_bitpos, priv->baseaddr + OCM_FID0_OFST);
+ writel(0, priv->baseaddr + OCM_FID1_OFST);
+ } else if (priv->ce_bitpos <= UE_MAX_BITPOS_UPPER) {
+ writel(1 << (priv->ce_bitpos - UE_MIN_BITPOS_UPPER),
+ priv->baseaddr + OCM_FID1_OFST);
+ writel(0, priv->baseaddr + OCM_FID0_OFST);
+ } else {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Bit number > 64 is not valid\n");
+ }
+
+ return count;
+}
+
+/**
+ * zynqmp_ocm_edac_inject_uebitpos0_show - Shows UE bit postion0
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ *
+ * Shows the one of bit position for UE error
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_uebitpos0_show(struct edac_device_ctl_info *dci,
+ char *data)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ if (priv->ue_bitpos0 <= UE_MAX_BITPOS_LOWER)
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID0_OFST))));
+
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID1_OFST))));
+}
+
+/**
+ * zynqmp_ocm_edac_inject_uebitpos0_store - set UE bit position0
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ * @count: read the size bytes from buffer
+ *
+ * Set the first bit position for UE Error generation,we need to configure
+ * any two bit positions to inject UE Error
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_uebitpos0_store(struct edac_device_ctl_info *dci,
+ const char *data, size_t count)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ if (!data)
+ return -EFAULT;
+
+ if (kstrtou8(data, 0, &priv->ue_bitpos0))
+ return -EINVAL;
+
+ if (priv->ue_bitpos0 <= UE_MAX_BITPOS_LOWER)
+ writel(1 << priv->ue_bitpos0, priv->baseaddr + OCM_FID0_OFST);
+ else if (priv->ue_bitpos0 <= UE_MAX_BITPOS_UPPER)
+ writel(1 << (priv->ue_bitpos0 - UE_MIN_BITPOS_UPPER),
+ priv->baseaddr + OCM_FID1_OFST);
+ else
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Bit position > 64 is not valid\n");
+ edac_printk(KERN_INFO, EDAC_DEVICE,
+ "Set another bit position for UE\n");
+
+ return count;
+}
+
+/**
+ * zynqmp_ocm_edac_inject_uebitpos1_show - Shows UE bit postion1
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ *
+ * Shows the second bit position configured for UE error
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_uebitpos1_show(struct edac_device_ctl_info *dci,
+ char *data)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ if (priv->ue_bitpos1 <= UE_MAX_BITPOS_LOWER)
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID0_OFST))));
+
+ return sprintf(data, "Fault Injection Data Reg: [0x%x]\n",
+ ((readl(priv->baseaddr + OCM_FID1_OFST))));
+}
+
+/**
+ * zynqmp_ocm_edac_inject_uebitpos1_store - Set UE second bit position
+ * @dci: Pointer to the edac device struct
+ * @data: Pointer to user data
+ * @count: read the size bytes from buffer
+ *
+ * Set the second bit position for UE Error generation,we need to configure
+ * any two bit positions to inject UE Error
+ * Return: Number of bytes copied on success else error code.
+ */
+static ssize_t zynqmp_ocm_edac_inject_uebitpos1_store(struct edac_device_ctl_info *dci,
+ const char *data, size_t count)
+{
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+ u32 mask;
+
+ if (!data)
+ return -EFAULT;
+
+ if (kstrtou8(data, 0, &priv->ue_bitpos1))
+ return -EINVAL;
+
+ if (priv->ue_bitpos0 == priv->ue_bitpos1) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Bit positions should not be equal\n");
+ return -EINVAL;
+ }
+
+ /*
+ * If both bit positions are referring to 32 bit data, then configure
+ * only FID0 register or if it is 64 bit data, then configure only
+ * FID1 register.
+ */
+ if (priv->ue_bitpos0 <= UE_MAX_BITPOS_LOWER &&
+ priv->ue_bitpos1 <= UE_MAX_BITPOS_LOWER) {
+ mask = (1 << priv->ue_bitpos0);
+ mask |= (1 << priv->ue_bitpos1);
+ writel(mask, priv->baseaddr + OCM_FID0_OFST);
+ writel(0, priv->baseaddr + OCM_FID1_OFST);
+ } else if ((priv->ue_bitpos0 >= UE_MIN_BITPOS_UPPER &&
+ priv->ue_bitpos0 <= UE_MAX_BITPOS_UPPER) &&
+ (priv->ue_bitpos1 >= UE_MIN_BITPOS_UPPER &&
+ priv->ue_bitpos1 <= UE_MAX_BITPOS_UPPER)) {
+ mask = (1 << (priv->ue_bitpos0 - UE_MIN_BITPOS_UPPER));
+ mask |= (1 << (priv->ue_bitpos1 - UE_MIN_BITPOS_UPPER));
+ writel(mask, priv->baseaddr + OCM_FID1_OFST);
+ writel(0, priv->baseaddr + OCM_FID0_OFST);
+ }
+
+ /*
+ * If one bit position is referring a bit in 32 bit data and other in
+ * 64 bit data, just configure FID0/FID1 based on uebitpos1.
+ */
+ if (priv->ue_bitpos0 <= UE_MAX_BITPOS_LOWER &&
+ (priv->ue_bitpos1 >= UE_MIN_BITPOS_UPPER &&
+ priv->ue_bitpos1 <= UE_MAX_BITPOS_UPPER)) {
+ writel(1 << (priv->ue_bitpos1 - UE_MIN_BITPOS_UPPER),
+ priv->baseaddr + OCM_FID1_OFST);
+ } else if ((priv->ue_bitpos0 >= UE_MIN_BITPOS_UPPER &&
+ priv->ue_bitpos0 <= UE_MAX_BITPOS_UPPER) &&
+ (priv->ue_bitpos1 <= UE_MAX_BITPOS_LOWER)) {
+ writel(1 << priv->ue_bitpos1,
+ priv->baseaddr + OCM_FID0_OFST);
+ } else {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Bit position > 64 is not valid, Valid bits:[63:0]\n");
+ }
+
+ edac_printk(KERN_INFO, EDAC_DEVICE,
+ "UE at Bit Position0: %d Bit Position1: %d\n",
+ priv->ue_bitpos0, priv->ue_bitpos1);
+
+ return count;
+}
+
+static struct edac_dev_sysfs_attribute zynqmp_ocm_edac_sysfs_attributes[] = {
+ {
+ .attr = {
+ .name = "inject_cebitpos",
+ .mode = (0644)
+ },
+ .show = zynqmp_ocm_edac_inject_cebitpos_show,
+ .store = zynqmp_ocm_edac_inject_cebitpos_store},
+ {
+ .attr = {
+ .name = "inject_uebitpos0",
+ .mode = (0644)
+ },
+ .show = zynqmp_ocm_edac_inject_uebitpos0_show,
+ .store = zynqmp_ocm_edac_inject_uebitpos0_store},
+ {
+ .attr = {
+ .name = "inject_uebitpos1",
+ .mode = (0644)
+ },
+ .show = zynqmp_ocm_edac_inject_uebitpos1_show,
+ .store = zynqmp_ocm_edac_inject_uebitpos1_store},
+ {
+ .attr = {
+ .name = "inject_fault_count",
+ .mode = (0644)
+ },
+ .show = zynqmp_ocm_edac_inject_fault_count_show,
+ .store = zynqmp_ocm_edac_inject_fault_count_store},
+ /* End of list */
+ {
+ .attr = {.name = NULL}
+ }
+};
+
+/**
+ * zynqmp_set_ocm_edac_sysfs_attributes - create sysfs attributes
+ * @edac_dev: Pointer to the edac device struct
+ *
+ * Creates sysfs entries for error injection
+ */
+static void zynqmp_set_ocm_edac_sysfs_attributes(struct edac_device_ctl_info
+ *edac_dev)
+{
+ edac_dev->sysfs_attributes = zynqmp_ocm_edac_sysfs_attributes;
+}
+
+static int zynqmp_ocm_edac_probe(struct platform_device *pdev)
+{
+ struct zynqmp_ocm_edac_priv *priv;
+ struct edac_device_ctl_info *dci;
+ void __iomem *baseaddr;
+ struct resource *res;
+ int irq, ret;
+
+ baseaddr = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(baseaddr))
+ return PTR_ERR(baseaddr);
+
+ if (!zynqmp_ocm_edac_get_eccstate(baseaddr)) {
+ edac_printk(KERN_INFO, EDAC_DEVICE,
+ "ECC not enabled\n");
+ return -ENXIO;
+ }
+
+ dci = edac_device_alloc_ctl_info(sizeof(*priv), ZYNQMP_OCM_EDAC_STRING,
+ 1, ZYNQMP_OCM_EDAC_STRING, 1, 0, NULL, 0,
+ edac_device_alloc_index());
+ if (!dci)
+ return -ENOMEM;
+
+ priv = dci->pvt_info;
+ platform_set_drvdata(pdev, dci);
+ dci->dev = &pdev->dev;
+ priv->baseaddr = baseaddr;
+ dci->mod_name = pdev->dev.driver->name;
+ dci->ctl_name = ZYNQMP_OCM_EDAC_STRING;
+ dci->dev_name = dev_name(&pdev->dev);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ ret = irq;
+ goto free_dev_ctl;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq,
+ zynqmp_ocm_edac_intr_handler,
+ 0, dev_name(&pdev->dev), dci);
+ if (ret) {
+ edac_printk(KERN_ERR, EDAC_DEVICE, "Failed to request Irq\n");
+ goto free_dev_ctl;
+ }
+
+ writel(OCM_CEUE_MASK, priv->baseaddr + OCM_IEN_OFST);
+
+ zynqmp_set_ocm_edac_sysfs_attributes(dci);
+ ret = edac_device_add_device(dci);
+ if (ret)
+ goto free_dev_ctl;
+
+ return 0;
+
+free_dev_ctl:
+ edac_device_free_ctl_info(dci);
+
+ return ret;
+}
+
+static int zynqmp_ocm_edac_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *dci = platform_get_drvdata(pdev);
+ struct zynqmp_ocm_edac_priv *priv = dci->pvt_info;
+
+ writel(OCM_CEUE_MASK, priv->baseaddr + OCM_IDS_OFST);
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(dci);
+
+ return 0;
+}
+
+static const struct of_device_id zynqmp_ocm_edac_match[] = {
+ { .compatible = "xlnx,zynqmp-ocmc-1.0"},
+ { /* end of table */ }
+};
+
+MODULE_DEVICE_TABLE(of, zynqmp_ocm_edac_match);
+
+static struct platform_driver zynqmp_ocm_edac_driver = {
+ .driver = {
+ .name = "zynqmp-ocm-edac",
+ .of_match_table = zynqmp_ocm_edac_match,
+ },
+ .probe = zynqmp_ocm_edac_probe,
+ .remove = zynqmp_ocm_edac_remove,
+};
+
+module_platform_driver(zynqmp_ocm_edac_driver);
+
+MODULE_AUTHOR("Advanced Micro Devices, Inc");
+MODULE_DESCRIPTION("Xilinx ZynqMP OCM ECC driver");
+MODULE_LICENSE("GPL");
--
2.17.1

2022-09-07 14:39:37

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

On Wed, Sep 07, 2022 at 04:45:31PM +0530, Sai Krishna Potthuri wrote:
> Add EDAC support for Xilinx ZynqMP OCM Controller, this driver
> reports CE and UE errors based on the interrupts, and also creates ue/ce
> sysfs entries for error injection.
>
> Signed-off-by: Sai Krishna Potthuri <[email protected]>
> Signed-off-by: Shubhrajyoti Datta <[email protected]>

I think you folks need to brush up on

Documentation/process/submitting-patches.rst

in order to build your SOB chain properly.

> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 58ab63642e72..08adedfd42b5 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -539,4 +539,13 @@ config EDAC_DMC520
> Support for error detection and correction on the
> SoCs with ARM DMC-520 DRAM controller.
>
> +config EDAC_ZYNQMP_OCM
> + tristate "Xilinx ZynqMP OCM Controller"
> + depends on ARCH_ZYNQMP || COMPILE_TEST

We already have an EDAC driver which depends on ARCH_ZYNQMP - the
Synopsys one. Can this one be made part of it?

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-09-08 04:51:28

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

Hi Sai,

I love your patch! Perhaps something to improve:

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

url: https://github.com/intel-lab-lkp/linux/commits/Sai-Krishna-Potthuri/edac-Add-support-for-Xilinx-ZynqMP-OCM-EDAC/20220907-191854
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/038e95811e4eddd23a098d0faf7a90bbccdb6e52
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sai-Krishna-Potthuri/edac-Add-support-for-Xilinx-ZynqMP-OCM-EDAC/20220907-191854
git checkout 038e95811e4eddd23a098d0faf7a90bbccdb6e52
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/edac/xlnx,zynqmp-ocmc.yaml

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-08 05:51:26

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: RE: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

Hi Boris,

> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: Wednesday, September 7, 2022 7:31 PM
> To: Potthuri, Sai Krishna <[email protected]>
> Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Michal Simek
> <[email protected]>; Mauro Carvalho Chehab
> <[email protected]>; Tony Luck <[email protected]>; James Morse
> <[email protected]>; Robert Richter <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; git (AMD-Xilinx) <[email protected]>; Datta,
> Shubhrajyoti <[email protected]>
> Subject: Re: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for
> ZynqMP OCM
>
> On Wed, Sep 07, 2022 at 04:45:31PM +0530, Sai Krishna Potthuri wrote:
> > Add EDAC support for Xilinx ZynqMP OCM Controller, this driver reports
> > CE and UE errors based on the interrupts, and also creates ue/ce sysfs
> > entries for error injection.
> >
> > Signed-off-by: Sai Krishna Potthuri <[email protected]>
> > Signed-off-by: Shubhrajyoti Datta <[email protected]>
>
> I think you folks need to brush up on
>
> Documentation/process/submitting-patches.rst
>
> in order to build your SOB chain properly.
Got it, i will add "Co-developed-by" tag and send v5.

>
> > diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index
> > 58ab63642e72..08adedfd42b5 100644
> > --- a/drivers/edac/Kconfig
> > +++ b/drivers/edac/Kconfig
> > @@ -539,4 +539,13 @@ config EDAC_DMC520
> > Support for error detection and correction on the
> > SoCs with ARM DMC-520 DRAM controller.
> >
> > +config EDAC_ZYNQMP_OCM
> > + tristate "Xilinx ZynqMP OCM Controller"
> > + depends on ARCH_ZYNQMP || COMPILE_TEST
>
> We already have an EDAC driver which depends on ARCH_ZYNQMP - the
> Synopsys one. Can this one be made part of it?
Synopsys EDAC driver is targeted for DDR Memory Controller and this driver
is for OCM (On Chip Memory) Controller, both are different hardware controllers.

Regards
Sai Krishna

2022-09-08 09:09:24

by Borislav Petkov

[permalink] [raw]
Subject: RE: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

On September 8, 2022 5:39:56 AM UTC, "Potthuri, Sai Krishna" <[email protected]> wrote:
>Synopsys EDAC driver is targeted for DDR Memory Controller and this driver
>is for OCM (On Chip Memory) Controller, both are different hardware controllers.

It would be helpful if the driver help text explained a bit more what hw this driver is for...

Thx.

--
Sent from a small device: formatting sux and brevity is inevitable.

2022-09-08 09:19:59

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: RE: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for ZynqMP OCM

Hi Boris,

> -----Original Message-----
> From: Boris Petkov <[email protected]>
> Sent: Thursday, September 8, 2022 2:06 PM
> To: Potthuri, Sai Krishna <[email protected]>
> Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Mauro Carvalho Chehab
> <[email protected]>; Michal Simek <[email protected]>; git
> (AMD-Xilinx) <[email protected]>; [email protected]; James Morse
> <[email protected]>; Tony Luck <[email protected]>; linux-
> [email protected]; [email protected];
> [email protected]; Robert Richter <[email protected]>; linux-
> [email protected]; Datta, Shubhrajyoti <[email protected]>
> Subject: RE: [PATCH v4 2/2] edac: zynqmp_ocm: Add EDAC support for
> ZynqMP OCM
>
> On September 8, 2022 5:39:56 AM UTC, "Potthuri, Sai Krishna"
> <[email protected]> wrote:
> >Synopsys EDAC driver is targeted for DDR Memory Controller and this
> >driver is for OCM (On Chip Memory) Controller, both are different
> hardware controllers.
>
> It would be helpful if the driver help text explained a bit more what hw this
> driver is for...
Sure, i will add in the next version.

Regards
Sai Krishna