linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups
@ 2022-08-22 19:07 Serge Semin
  2022-08-22 19:07 ` [PATCH 01/20] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure Serge Semin
                   ` (19 more replies)
  0 siblings, 20 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, James Morse, Robert Richter, Rob Herring,
	Krzysztof Kozlowski, devicetree, linux-arm-kernel, linux-edac,
	linux-kernel

This patchset is a first one in the series created in the framework of
my Baikal-T1 DDRC-related work:

[1: In-progress] EDAC/mc/synopsys: Various fixes and cleanups
Link: ---you are looking at it---
[2: To be submitted] EDAC/synopsys: Add generic DDRC info and address mapping
Link:
[3: To be submitted] EDAC/synopsys: Add generic resources and Baikal-T1 support
Link:

Note the patchsets above must be merged in the same order as they are
placed in the list in order to prevent conflicts. Nothing prevents them
from being reviewed synchronously though. Any tests are very welcome.
Thanks in advance.

Regarding this series content. It's an initial patchset which
traditionally provides various fixes, cleanups and modifications required
for the more comfortable further features development. The main goal of it
though is to detach the Xilinx Zynq A05 DDRC related code into the
dedicated driver since first it has nothing to do with the Synopsys DW
uMCTL2 DDR controller and second it will be a great deal obstacle on the
way of extending the Synopsys-part functionality.

The series starts with fixes patches, which in short concern the next
aspects: touching the ZynqMP-specific CSRs on the Xilinx ZinqMP platform
only, serializing an access to the ECCCLR register, adding correct memory
devices type detection, setting a correct value to the
mem_ctl_info.scrub_cap field, dropping an erroneous ADDRMAP[4] parsing and
getting back a correct order of the ECC errors info detection procedure.

Afterwards the patchset provides several cleanup patches required for the
more coherent code splitting up (Xilinx Zynq A05 and Synopsys DW uMCTL2)
so the provided modifications would be useful in both drivers. First we
get to replace the platform resource manual IO-remapping with the
devm_platform_ioremap_resource() method call. Secondly we suggest to drop:
internal CE/UE errors counters, local to_mci() macros definition, some
redundant ecc_error_info structure fields and redundant info from the
error message, duplicated dimm->nr_pages debug printout and spaces from
the MEM_TYPE flags declarations. (The later two updates concern the MCI
core part.) Thirdly before splitting up the driver we need to add an
unique MC index allocation infrastructure to the MCI core.  It's required
since after splitting the driver up we'll need to make sure both device
types could be correctly probed on the same platform. Finally the Xilinx
Zynq A05 part of the driver is moved out to a dedicated driver where it
should been originally placed. After that the platform-specific setups API
is removed from the Synopsys DW uMCTL2 DDRC driver since it's no longer
required.

Finally as the cherry on the cake we suggest to unify the DW uMCTL2 DDRC
driver entities naming and replace the open-coded "shift/mask" patter with
the kernel helpers like BIT/GENMASK/FIELD_x in there. It shall
significantly improve the code readability.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Michail Ivanov <Michail.Ivanov@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@xilinx.com>
Cc: Manish Narani <manish.narani@xilinx.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: Robert Richter <rric@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (20):
  EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure
  EDAC/synopsys: Fix generic device type detection procedure
  EDAC/synopsys: Fix mci->scrub_cap field setting
  EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse
  EDAC/synopsys: Fix reading errors count before ECC status
  EDAC/synopsys: Use platform device devm ioremap method
  EDAC/synopsys: Drop internal CE and UE counters
  EDAC/synopsys: Drop local to_mci macro implementation
  EDAC/synopsys: Drop struct ecc_error_info.blknr field
  EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name
  EDAC/synopsys: Drop redundant info from error message
  EDAC/mc: Replace spaces with tabs in memtype flags definition
  EDAC/mc: Drop duplicated dimm->nr_pages debug printout
  EDAC/mc: Init DIMM labels in MC registration method
  EDAC/mc: Add MC unique index allocation procedure
  dt-bindings: memory: snps: Detach Zynq DDRC controller support
  EDAC/synopsys: Detach Zynq DDRC controller support
  EDAC/synopsys: Drop unused platform-specific setup API
  EDAC/synopsys: Unify the driver entities naming
  EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros

 .../snps,dw-umctl2-ddrc.yaml                  |  51 +
 .../memory-controllers/synopsys,ddrc-ecc.yaml |  76 --
 .../xlnx,zynq-ddrc-a05.yaml                   |  38 +
 MAINTAINERS                                   |   3 +
 drivers/edac/Kconfig                          |   9 +-
 drivers/edac/Makefile                         |   1 +
 drivers/edac/edac_mc.c                        | 136 ++-
 drivers/edac/edac_mc.h                        |   4 +
 drivers/edac/synopsys_edac.c                  | 902 ++++++------------
 drivers/edac/zynq_edac.c                      | 504 ++++++++++
 include/linux/edac.h                          |  30 +-
 11 files changed, 1037 insertions(+), 717 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/synopsys,ddrc-ecc.yaml
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml
 create mode 100644 drivers/edac/zynq_edac.c

-- 
2.35.1


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

* [PATCH 01/20] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 02/20] EDAC/synopsys: Fix generic device type detection procedure Serge Semin
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter,
	Dinh Nguyen
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Rob Herring, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-edac, linux-kernel, Borislav Petkov

The generic DW uMCTL2 DDRC v3.x support was added in commit f7824ded4149
("EDAC/synopsys: Add support for version 3 of the Synopsys EDAC DDR"). It
hasn't been done quiet well there with respect to the IRQs handling
procedure. An attempt to fix that was introduced in the recent commit
4bcffe941758 ("EDAC/synopsys: Re-enable the error interrupts on v3 hw").
Alas again it didn't provide quite complete solution.

First of all the commit f7824ded4149 ("EDAC/synopsys: Add support for
version 3 of the Synopsys EDAC DDR") log says that v3.80a "has UE/CE auto
cleared". They aren't in none of the IP-core versions. The IRQ status can
be cleared by means of setting the ECCCLR/ECCCTL register self-cleared
flags 0-3. The pending IRQ clearance is done in the respective
get_error_info() method of the driver. Thus defining a quirk flag with the
"DDR_ECC_INTR_SELF_CLEAR" name was at least very inaccurate if not to say
misleading. So was adding the comments about the "ce/ue bits automatically
cleared".

Second, disabling the being handled IRQ in the handler doesn't make sense
in Linux since the IC line is masked during that procedure anyway. So
disabling the IRQ in one part of the handler and enabling it at the end of
the method is simply redundant. (See, the ZynqMP-specific code with the
QoS IRQ CSR didn't do that originally.)

Finally calling the zynqmp_get_error_info() method concurrently with the
enable_irq()/disable_irq() functions causes the IRQs mask state race
condition. Starting from DW uMCTL2 DDRC IP-core v3.10a [1] the ECCCLR
register has been renamed to ECCCTL and has been equipped with CE/UE IRQs
enable/disable flags [2]. So the CSR now serves for the IRQ status and
control functions used concurrently during the IRQ handling and the IRQ
disabling/enabling. Thus the corresponding critical section must be
protected with the IRQ-safe spin-lock.

So let's fix all the problems noted above. First the
DDR_ECC_INTR_SELF_CLEAR flag is renamed to SYNPS_ZYNQMP_IRQ_REGS. Its
semantic is now the opposite: the quirk means having the ZynqMP IRQ CSRs
available on the platform. Second the DDR_UE_MASK and DDR_CE_MASK macros
are renamed to imply being used in the framework of the ECCCLR/ECCCTL CSRs
accesses. Third all the misleading comments are removed. Finally the
ECC_CLR_OFST register IOs are now protected with the IRQ-safe spin-lock
taken in order to prevent the IRQ status clearance and IRQ enable/disable
race condition.

[1] DesignWare Cores Enhanced Universal DDR Memory and Protocol
Controllers (uMCTL2/uPCTL2), Release Notes, Version 3.91a, October 2020,
p. 27.
[2] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2),
Databook Version 3.91a, October 2020, p.818-819.

Fixes: f7824ded4149 ("EDAC/synopsys: Add support for version 3 of the Synopsys EDAC DDR")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 76 +++++++++++++++++++++++-------------
 1 file changed, 48 insertions(+), 28 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index f7d37c282819..c78fb5781ff9 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -9,6 +9,7 @@
 #include <linux/edac.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -88,7 +89,7 @@
 /* DDR ECC Quirks */
 #define DDR_ECC_INTR_SUPPORT		BIT(0)
 #define DDR_ECC_DATA_POISON_SUPPORT	BIT(1)
-#define DDR_ECC_INTR_SELF_CLEAR		BIT(2)
+#define SYNPS_ZYNQMP_IRQ_REGS		BIT(2)
 
 /* ZynqMP Enhanced DDR memory controller registers that are relevant to ECC */
 /* ECC Configuration Registers */
@@ -133,11 +134,13 @@
 
 #define ECC_ADDRMAP0_OFFSET		0x200
 
-/* Control register bitfield definitions */
+/* ECC control/clear register definitions */
 #define ECC_CTRL_BUSWIDTH_MASK		0x3000
 #define ECC_CTRL_BUSWIDTH_SHIFT		12
 #define ECC_CTRL_CLR_CE_ERRCNT		BIT(2)
 #define ECC_CTRL_CLR_UE_ERRCNT		BIT(3)
+#define ECC_CTRL_EN_CE_IRQ		BIT(8)
+#define ECC_CTRL_EN_UE_IRQ		BIT(9)
 
 /* DDR Control Register width definitions  */
 #define DDRCTL_EWDTH_16			2
@@ -164,10 +167,6 @@
 #define DDR_QOS_IRQ_EN_OFST		0x20208
 #define DDR_QOS_IRQ_DB_OFST		0x2020C
 
-/* DDR QOS Interrupt register definitions */
-#define DDR_UE_MASK			BIT(9)
-#define DDR_CE_MASK			BIT(8)
-
 /* ECC Corrected Error Register Mask and Shifts*/
 #define ECC_CEADDR0_RW_MASK		0x3FFFF
 #define ECC_CEADDR0_RNK_MASK		BIT(24)
@@ -300,6 +299,7 @@ struct synps_ecc_status {
 /**
  * struct synps_edac_priv - DDR memory controller private instance data.
  * @baseaddr:		Base address of the DDR controller.
+ * @lock:		Concurrent CSRs access lock.
  * @message:		Buffer for framing the event specific info.
  * @stat:		ECC status information.
  * @p_data:		Platform data.
@@ -314,6 +314,7 @@ struct synps_ecc_status {
  */
 struct synps_edac_priv {
 	void __iomem *baseaddr;
+	spinlock_t lock;
 	char message[SYNPS_EDAC_MSG_SIZE];
 	struct synps_ecc_status stat;
 	const struct synps_platform_data *p_data;
@@ -409,7 +410,8 @@ static int zynq_get_error_info(struct synps_edac_priv *priv)
 static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 {
 	struct synps_ecc_status *p;
-	u32 regval, clearval = 0;
+	u32 regval, clearval;
+	unsigned long flags;
 	void __iomem *base;
 
 	base = priv->baseaddr;
@@ -452,11 +454,16 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 					ECC_CEADDR1_BNKNR_SHIFT;
 	p->ueinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
 	p->ueinfo.data = readl(base + ECC_UESYND0_OFST);
+
 out:
-	clearval = ECC_CTRL_CLR_CE_ERR | ECC_CTRL_CLR_CE_ERRCNT;
-	clearval |= ECC_CTRL_CLR_UE_ERR | ECC_CTRL_CLR_UE_ERRCNT;
+	spin_lock_irqsave(&priv->lock, flags);
+
+	clearval = readl(base + ECC_CLR_OFST) |
+		   ECC_CTRL_CLR_CE_ERR | ECC_CTRL_CLR_CE_ERRCNT |
+		   ECC_CTRL_CLR_UE_ERR | ECC_CTRL_CLR_UE_ERRCNT;
 	writel(clearval, base + ECC_CLR_OFST);
-	writel(0x0, base + ECC_CLR_OFST);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return 0;
 }
@@ -516,24 +523,42 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 
 static void enable_intr(struct synps_edac_priv *priv)
 {
+	unsigned long flags;
+
 	/* Enable UE/CE Interrupts */
-	if (priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR)
-		writel(DDR_UE_MASK | DDR_CE_MASK,
-		       priv->baseaddr + ECC_CLR_OFST);
-	else
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 		       priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
 
+		return;
+	}
+
+	/* IRQs Enable/Disable feature has been available since v3.10a */
+	spin_lock_irqsave(&priv->lock, flags);
+
+	writel(ECC_CTRL_EN_CE_IRQ | ECC_CTRL_EN_UE_IRQ,
+	       priv->baseaddr + ECC_CLR_OFST);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
 static void disable_intr(struct synps_edac_priv *priv)
 {
+	unsigned long flags;
+
 	/* Disable UE/CE Interrupts */
-	if (priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR)
-		writel(0x0, priv->baseaddr + ECC_CLR_OFST);
-	else
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 		       priv->baseaddr + DDR_QOS_IRQ_DB_OFST);
+
+		return;
+	}
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	writel(0, priv->baseaddr + ECC_CLR_OFST);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
 /**
@@ -553,11 +578,7 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
 	priv = mci->pvt_info;
 	p_data = priv->p_data;
 
-	/*
-	 * v3.0 of the controller has the ce/ue bits cleared automatically,
-	 * so this condition does not apply.
-	 */
-	if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR)) {
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
 		regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
 		if (!(regval & ECC_CE_UE_INTR_MASK))
@@ -574,11 +595,9 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
 
 	edac_dbg(3, "Total error count CE %d UE %d\n",
 		 priv->ce_cnt, priv->ue_cnt);
-	/* v3.0 of the controller does not have this register */
-	if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR))
+
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS)
 		writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
-	else
-		enable_intr(priv);
 
 	return IRQ_HANDLED;
 }
@@ -900,7 +919,7 @@ static const struct synps_platform_data zynqmp_edac_def = {
 	.get_mtype	= zynqmp_get_mtype,
 	.get_dtype	= zynqmp_get_dtype,
 	.get_ecc_state	= zynqmp_get_ecc_state,
-	.quirks         = (DDR_ECC_INTR_SUPPORT
+	.quirks         = (DDR_ECC_INTR_SUPPORT | SYNPS_ZYNQMP_IRQ_REGS
 #ifdef CONFIG_EDAC_DEBUG
 			  | DDR_ECC_DATA_POISON_SUPPORT
 #endif
@@ -912,7 +931,7 @@ static const struct synps_platform_data synopsys_edac_def = {
 	.get_mtype	= zynqmp_get_mtype,
 	.get_dtype	= zynqmp_get_dtype,
 	.get_ecc_state	= zynqmp_get_ecc_state,
-	.quirks         = (DDR_ECC_INTR_SUPPORT | DDR_ECC_INTR_SELF_CLEAR
+	.quirks         = (DDR_ECC_INTR_SUPPORT
 #ifdef CONFIG_EDAC_DEBUG
 			  | DDR_ECC_DATA_POISON_SUPPORT
 #endif
@@ -1360,6 +1379,7 @@ static int mc_probe(struct platform_device *pdev)
 	priv = mci->pvt_info;
 	priv->baseaddr = baseaddr;
 	priv->p_data = p_data;
+	spin_lock_init(&priv->lock);
 
 	mc_init(mci, pdev);
 
-- 
2.35.1


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

* [PATCH 02/20] EDAC/synopsys: Fix generic device type detection procedure
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
  2022-08-22 19:07 ` [PATCH 01/20] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 03/20] EDAC/synopsys: Fix mci->scrub_cap field setting Serge Semin
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter,
	Manish Narani
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Dinh Nguyen,
	Rob Herring, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-edac, linux-kernel, Borislav Petkov

First of all the enum dev_type constants define the Memory devices, i.e.
DRAM chips, DQ-bus width (see the enumberation kdoc for details). So what
is returned from the zynqmp_get_dtype() procedure is definitely wrong.
Secondly the DRAM chips type has nothing to do with the data bus width
specified in the MSTR.data_bus_width CSR field. The later one just
determines the part of the whole DQ-bus used to access the data from the
all DRAM chips. So it doesn't indicate the individual chips type. Thirdly
the DRAM chips type can be determined only in case of the DDR4 protocol by
means of the MSTR.device_config field state (it is supposed to be set by
the system firmware). Finally the DW uMCTL2 DDRC ECC capability doesn't
depend on the memory chips type. Moreover it doesn't depend on the data
bus width in runtime either. The IP-core reference manual says in [1,2]
that the ECC support can't be enabled during the IP-core synthesizes for
the DRAM data bus widths other than 16, 32 or 64. At the same time the
bus width mode (MSTR.data_bus_width) doesn't change the ECC feature
availability. Thus it was wrong to determine the ECC state with respect to
the DQ-bus width mode.

Let's fix all of the mistakes above in the zynqmp_get_dtype() and
zynqmp_get_ecc_state() methods. In accordance with the DW uMCTL2 DDRC
nature the DRAM chips type in most of the cases will be unknown except
when DDR4 protocol is utilized. ECC availability will be determined by the
ECCCFG0.ecc_mode field state only.

[1] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2)
Databook, Version 3.91a, October 2020, p. 421.
[2] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2)
Databook, Version 3.91a, October 2020, p. 633.

Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 56 +++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index c78fb5781ff9..39c63dd1b9be 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -142,7 +142,12 @@
 #define ECC_CTRL_EN_CE_IRQ		BIT(8)
 #define ECC_CTRL_EN_UE_IRQ		BIT(9)
 
-/* DDR Control Register width definitions  */
+/* DDR Master Register 0 definitions */
+#define DDR_MSTR_DEV_CFG_MASK		GENMASK(31, 30)
+#define DDR_MSTR_DEV_X4			0x0
+#define DDR_MSTR_DEV_X8			0x1
+#define DDR_MSTR_DEV_X16		0x2
+#define DDR_MSTR_DEV_X32		0x3
 #define DDRCTL_EWDTH_16			2
 #define DDRCTL_EWDTH_32			1
 #define DDRCTL_EWDTH_64			0
@@ -671,26 +676,25 @@ static enum dev_type zynq_get_dtype(const void __iomem *base)
  */
 static enum dev_type zynqmp_get_dtype(const void __iomem *base)
 {
-	enum dev_type dt;
-	u32 width;
-
-	width = readl(base + CTRL_OFST);
-	width = (width & ECC_CTRL_BUSWIDTH_MASK) >> ECC_CTRL_BUSWIDTH_SHIFT;
-	switch (width) {
-	case DDRCTL_EWDTH_16:
-		dt = DEV_X2;
-		break;
-	case DDRCTL_EWDTH_32:
-		dt = DEV_X4;
-		break;
-	case DDRCTL_EWDTH_64:
-		dt = DEV_X8;
-		break;
-	default:
-		dt = DEV_UNKNOWN;
+	u32 regval;
+
+	regval = readl(base + DDR_MSTR_OFST);
+	if (!(regval & MEM_TYPE_DDR4))
+		return DEV_UNKNOWN;
+
+	regval = FIELD_GET(DDR_MSTR_DEV_CFG_MASK, regval);
+	switch (regval) {
+	case DDR_MSTR_DEV_X4:
+		return DEV_X4;
+	case DDR_MSTR_DEV_X8:
+		return DEV_X8;
+	case DDR_MSTR_DEV_X16:
+		return DEV_X16;
+	case DDR_MSTR_DEV_X32:
+		return DEV_X32;
 	}
 
-	return dt;
+	return DEV_UNKNOWN;
 }
 
 /**
@@ -727,19 +731,11 @@ static bool zynq_get_ecc_state(void __iomem *base)
  */
 static bool zynqmp_get_ecc_state(void __iomem *base)
 {
-	enum dev_type dt;
-	u32 ecctype;
+	u32 regval;
 
-	dt = zynqmp_get_dtype(base);
-	if (dt == DEV_UNKNOWN)
-		return false;
+	regval = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK;
 
-	ecctype = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK;
-	if ((ecctype == SCRUB_MODE_SECDED) &&
-	    ((dt == DEV_X2) || (dt == DEV_X4) || (dt == DEV_X8)))
-		return true;
-
-	return false;
+	return (regval == SCRUB_MODE_SECDED);
 }
 
 /**
-- 
2.35.1


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

* [PATCH 03/20] EDAC/synopsys: Fix mci->scrub_cap field setting
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
  2022-08-22 19:07 ` [PATCH 01/20] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure Serge Semin
  2022-08-22 19:07 ` [PATCH 02/20] EDAC/synopsys: Fix generic device type detection procedure Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 04/20] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse Serge Semin
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter,
	Punnaiah Choudary Kalluri
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Manish Narani, Dinh Nguyen, Rob Herring,
	Krzysztof Kozlowski, devicetree, linux-arm-kernel, linux-edac,
	linux-kernel, Borislav Petkov

The mem_ctl_info.scrub_cap field is supposed to be set with the ECC
scrub-related flags while the driver initializes it with the SCRUB_HW_SRC
flag ID. It's definitely wrong, though hasn't caused any problem since the
structure field isn't used by the EDAC core. Let's fix it anyway by using
the SCRUB_FLAG_HW_SRC macro to initialize the field.

Fixes: ae9b56e3996d ("EDAC, synps: Add EDAC support for zynq ddr ecc controller")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 39c63dd1b9be..b9b68f3ecf52 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -857,7 +857,7 @@ static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 	/* Initialize controller capabilities and configuration */
 	mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR2;
 	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
-	mci->scrub_cap = SCRUB_HW_SRC;
+	mci->scrub_cap = SCRUB_FLAG_HW_SRC;
 	mci->scrub_mode = SCRUB_NONE;
 
 	mci->edac_cap = EDAC_FLAG_SECDED;
-- 
2.35.1


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

* [PATCH 04/20] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (2 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 03/20] EDAC/synopsys: Fix mci->scrub_cap field setting Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 05/20] EDAC/synopsys: Fix reading errors count before ECC status Serge Semin
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter,
	Manish Narani
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Dinh Nguyen,
	Rob Herring, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-edac, linux-kernel, Borislav Petkov

Currently the ADDRMAP4.addrmap_col_b10 field gets to be parsed in case of
the LPDDR3 memory and Quarter DQ bus width mode. It's wrong since that
field is marked as unused for that mode in all the available DW uMCTL2
DDRC releases (up to IP-core v3.91a). Most likely the field parsing has
been added by mistake as a result of the copy-paste from the Half DW bus
width mode part of the same function. Drop it from the driver for good
then.

Fixes: 1a81361f75d8 ("EDAC, synopsys: Add Error Injection support for ZynqMP DDR controller")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index b9b68f3ecf52..873dbc684fe6 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -1238,10 +1238,6 @@ static void setup_column_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 				COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
 				(((addrmap[3] >> 24) & COL_MAX_VAL_MASK) +
 				 COL_B9_BASE);
-			priv->col_shift[13] = ((addrmap[4] &
-				COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
-				((addrmap[4] & COL_MAX_VAL_MASK) +
-				 COL_B10_BASE);
 		} else {
 			priv->col_shift[11] = (((addrmap[3] >> 16) &
 				COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
-- 
2.35.1


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

* [PATCH 05/20] EDAC/synopsys: Fix reading errors count before ECC status
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (3 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 04/20] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 06/20] EDAC/synopsys: Use platform device devm ioremap method Serge Semin
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter,
	Shubhrajyoti Datta
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel, Borislav Petkov

Aside with fixing the errors count CSR usage the commit e2932d1f6f05
("EDAC/synopsys: Read the error count from the correct register") all of
the sudden has also changed the order of the errors status check
procedure. So now the errors handler method first reads the number of CE
and UE and only then makes sure that any of these errors have actually
happened. It doesn't make much sense. Let's fix that by getting back the
procedures order: first check the ECC status, then read the number of
errors.

Fixes: e2932d1f6f05 ("EDAC/synopsys: Read the error count from the correct register")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 873dbc684fe6..14653b799901 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -422,18 +422,18 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 	base = priv->baseaddr;
 	p = &priv->stat;
 
-	regval = readl(base + ECC_ERRCNT_OFST);
-	p->ce_cnt = regval & ECC_ERRCNT_CECNT_MASK;
-	p->ue_cnt = (regval & ECC_ERRCNT_UECNT_MASK) >> ECC_ERRCNT_UECNT_SHIFT;
-	if (!p->ce_cnt)
-		goto ue_err;
-
 	regval = readl(base + ECC_STAT_OFST);
 	if (!regval)
 		return 1;
 
 	p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
 
+	regval = readl(base + ECC_ERRCNT_OFST);
+	p->ce_cnt = regval & ECC_ERRCNT_CECNT_MASK;
+	p->ue_cnt = (regval & ECC_ERRCNT_UECNT_MASK) >> ECC_ERRCNT_UECNT_SHIFT;
+	if (!p->ce_cnt)
+		goto ue_err;
+
 	regval = readl(base + ECC_CEADDR0_OFST);
 	p->ceinfo.row = (regval & ECC_CEADDR0_RW_MASK);
 	regval = readl(base + ECC_CEADDR1_OFST);
-- 
2.35.1


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

* [PATCH 06/20] EDAC/synopsys: Use platform device devm ioremap method
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (4 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 05/20] EDAC/synopsys: Fix reading errors count before ECC status Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 07/20] EDAC/synopsys: Drop internal CE and UE counters Serge Semin
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

DW DDRs CSRs resource descriptor is used by the devm_ioremap_resource()
function invocation only in the driver probe method. Thus we can freely
convert the platform_get_resource() and devm_ioremap_resource() couple to
just a single devm_platform_ioremap_resource() method call.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 14653b799901..f38c326f2cf5 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -1336,11 +1336,9 @@ static int mc_probe(struct platform_device *pdev)
 	struct synps_edac_priv *priv;
 	struct mem_ctl_info *mci;
 	void __iomem *baseaddr;
-	struct resource *res;
 	int rc;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	baseaddr = devm_ioremap_resource(&pdev->dev, res);
+	baseaddr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(baseaddr))
 		return PTR_ERR(baseaddr);
 
-- 
2.35.1


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

* [PATCH 07/20] EDAC/synopsys: Drop internal CE and UE counters
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (5 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 06/20] EDAC/synopsys: Use platform device devm ioremap method Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 08/20] EDAC/synopsys: Drop local to_mci macro implementation Serge Semin
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

First of all they aren't exposed anyhow by the driver. Secondly the EDAC
core already tracks the total amount of correctable and uncorrectable
errors (see mem_ctl_info.{ce_mc,ue_mc} fields usage). Let's drop the
useless internal counters then for good.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index f38c326f2cf5..956de6fb3125 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -308,8 +308,6 @@ struct synps_ecc_status {
  * @message:		Buffer for framing the event specific info.
  * @stat:		ECC status information.
  * @p_data:		Platform data.
- * @ce_cnt:		Correctable Error count.
- * @ue_cnt:		Uncorrectable Error count.
  * @poison_addr:	Data poison address.
  * @row_shift:		Bit shifts for row bit.
  * @col_shift:		Bit shifts for column bit.
@@ -323,8 +321,6 @@ struct synps_edac_priv {
 	char message[SYNPS_EDAC_MSG_SIZE];
 	struct synps_ecc_status stat;
 	const struct synps_platform_data *p_data;
-	u32 ce_cnt;
-	u32 ue_cnt;
 #ifdef CONFIG_EDAC_DEBUG
 	ulong poison_addr;
 	u32 row_shift[18];
@@ -594,12 +590,8 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
 	if (status)
 		return IRQ_NONE;
 
-	priv->ce_cnt += priv->stat.ce_cnt;
-	priv->ue_cnt += priv->stat.ue_cnt;
 	handle_error(mci, &priv->stat);
 
-	edac_dbg(3, "Total error count CE %d UE %d\n",
-		 priv->ce_cnt, priv->ue_cnt);
 
 	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS)
 		writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
-- 
2.35.1


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

* [PATCH 08/20] EDAC/synopsys: Drop local to_mci macro implementation
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (6 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 07/20] EDAC/synopsys: Drop internal CE and UE counters Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 09/20] EDAC/synopsys: Drop struct ecc_error_info.blknr field Serge Semin
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

The to_mci macro was added in commit 1a81361f75d8 ("EDAC, synopsys: Add
Error Injection support for ZynqMP DDR controller") together with the
errors injection debug feature. It turns our the macro with the same
semantic and name has already been defined in the edac_mc.h (former
edac_core.h) header file. No idea why it was needed to have a local
version with the same semantic, but now there is no point in that. Drop
the local implementation for good then.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 956de6fb3125..c51a25a28835 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -948,7 +948,6 @@ static const struct of_device_id synps_edac_match[] = {
 MODULE_DEVICE_TABLE(of, synps_edac_match);
 
 #ifdef CONFIG_EDAC_DEBUG
-#define to_mci(k) container_of(k, struct mem_ctl_info, dev)
 
 /**
  * ddr_poison_setup -	Update poison registers.
-- 
2.35.1


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

* [PATCH 09/20] EDAC/synopsys: Drop struct ecc_error_info.blknr field
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (7 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 08/20] EDAC/synopsys: Drop local to_mci macro implementation Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 10/20] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name Serge Semin
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Even though the ECC(C|U)ADDR1 CSR description indeed says it's a "Block
number" in the DW uMCTL2 DDRC hw reference manuals, the corresponding
register field name (ECC(C|U)ADDR1.ecc_(un)corr_col) and the rest of the
hw documentation refer to the field as the SDRAM address column. Thus
let's use the already available ecc_error_info.col field to read the
column number to and drop the questionable ecc_error_info.blknr field for
good.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index c51a25a28835..843d2717c72b 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -177,7 +177,7 @@
 #define ECC_CEADDR0_RNK_MASK		BIT(24)
 #define ECC_CEADDR1_BNKGRP_MASK		0x3000000
 #define ECC_CEADDR1_BNKNR_MASK		0x70000
-#define ECC_CEADDR1_BLKNR_MASK		0xFFF
+#define ECC_CEADDR1_COL_MASK		0xFFF
 #define ECC_CEADDR1_BNKGRP_SHIFT	24
 #define ECC_CEADDR1_BNKNR_SHIFT		16
 
@@ -275,7 +275,6 @@
  * @bitpos:	Bit position.
  * @data:	Data causing the error.
  * @bankgrpnr:	Bank group number.
- * @blknr:	Block number.
  */
 struct ecc_error_info {
 	u32 row;
@@ -284,7 +283,6 @@ struct ecc_error_info {
 	u32 bitpos;
 	u32 data;
 	u32 bankgrpnr;
-	u32 blknr;
 };
 
 /**
@@ -437,7 +435,7 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 					ECC_CEADDR1_BNKNR_SHIFT;
 	p->ceinfo.bankgrpnr = (regval &	ECC_CEADDR1_BNKGRP_MASK) >>
 					ECC_CEADDR1_BNKGRP_SHIFT;
-	p->ceinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
+	p->ceinfo.col = (regval & ECC_CEADDR1_COL_MASK);
 	p->ceinfo.data = readl(base + ECC_CSYND0_OFST);
 	edac_dbg(2, "ECCCSYN0: 0x%08X ECCCSYN1: 0x%08X ECCCSYN2: 0x%08X\n",
 		 readl(base + ECC_CSYND0_OFST), readl(base + ECC_CSYND1_OFST),
@@ -453,7 +451,7 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 					ECC_CEADDR1_BNKGRP_SHIFT;
 	p->ueinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
 					ECC_CEADDR1_BNKNR_SHIFT;
-	p->ueinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
+	p->ueinfo.col = (regval & ECC_CEADDR1_COL_MASK);
 	p->ueinfo.data = readl(base + ECC_UESYND0_OFST);
 
 out:
@@ -485,10 +483,9 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ceinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Bank %d BankGroup Number %d Block Number %d Bit Position: %d Data: 0x%08x",
-				 "CE", pinf->row, pinf->bank,
-				 pinf->bankgrpnr, pinf->blknr,
-				 pinf->bitpos, pinf->data);
+				 "DDR ECC error type:%s Row %d Col %d Bank %d BankGroup Number %d Bit Position: %d Data: 0x%08x",
+				 "CE", pinf->row, pinf->col, pinf->bank,
+				 pinf->bankgrpnr, pinf->bitpos, pinf->data);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
 				 "DDR ECC error type:%s Row %d Bank %d Col %d Bit Position: %d Data: 0x%08x",
@@ -505,9 +502,9 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ueinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type :%s Row %d Bank %d BankGroup Number %d Block Number %d",
-				 "UE", pinf->row, pinf->bank,
-				 pinf->bankgrpnr, pinf->blknr);
+				 "DDR ECC error type :%s Row %d Col %d Bank %d BankGroup Number %d",
+				 "UE", pinf->row, pinf->col, pinf->bank,
+				 pinf->bankgrpnr);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
 				 "DDR ECC error type :%s Row %d Bank %d Col %d ",
-- 
2.35.1


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

* [PATCH 10/20] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (8 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 09/20] EDAC/synopsys: Drop struct ecc_error_info.blknr field Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 11/20] EDAC/synopsys: Drop redundant info from error message Serge Semin
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

None of the ecc_error_info structure fields has "nr" suffix even though
each of them do re-present some number (row number, column number, bank
number). Let's drop the suffix from the bankgrpnr field name for the sake
of unification. While at it drop the word "Number" from the CE/UE error
messages too since it doesn't give any helpful info there.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 843d2717c72b..63b3d11e3904 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -272,17 +272,17 @@
  * @row:	Row number.
  * @col:	Column number.
  * @bank:	Bank number.
+ * @bankgrp:	Bank group number.
  * @bitpos:	Bit position.
  * @data:	Data causing the error.
- * @bankgrpnr:	Bank group number.
  */
 struct ecc_error_info {
 	u32 row;
 	u32 col;
 	u32 bank;
+	u32 bankgrp;
 	u32 bitpos;
 	u32 data;
-	u32 bankgrpnr;
 };
 
 /**
@@ -433,7 +433,7 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 	regval = readl(base + ECC_CEADDR1_OFST);
 	p->ceinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
 					ECC_CEADDR1_BNKNR_SHIFT;
-	p->ceinfo.bankgrpnr = (regval &	ECC_CEADDR1_BNKGRP_MASK) >>
+	p->ceinfo.bankgrp = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
 					ECC_CEADDR1_BNKGRP_SHIFT;
 	p->ceinfo.col = (regval & ECC_CEADDR1_COL_MASK);
 	p->ceinfo.data = readl(base + ECC_CSYND0_OFST);
@@ -447,7 +447,7 @@ static int zynqmp_get_error_info(struct synps_edac_priv *priv)
 	regval = readl(base + ECC_UEADDR0_OFST);
 	p->ueinfo.row = (regval & ECC_CEADDR0_RW_MASK);
 	regval = readl(base + ECC_UEADDR1_OFST);
-	p->ueinfo.bankgrpnr = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
+	p->ueinfo.bankgrp = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
 					ECC_CEADDR1_BNKGRP_SHIFT;
 	p->ueinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
 					ECC_CEADDR1_BNKNR_SHIFT;
@@ -483,9 +483,9 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ceinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Col %d Bank %d BankGroup Number %d Bit Position: %d Data: 0x%08x",
+				 "DDR ECC error type:%s Row %d Col %d Bank %d Bank Group %d Bit Position: %d Data: 0x%08x",
 				 "CE", pinf->row, pinf->col, pinf->bank,
-				 pinf->bankgrpnr, pinf->bitpos, pinf->data);
+				 pinf->bankgrp, pinf->bitpos, pinf->data);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
 				 "DDR ECC error type:%s Row %d Bank %d Col %d Bit Position: %d Data: 0x%08x",
@@ -502,9 +502,9 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ueinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type :%s Row %d Col %d Bank %d BankGroup Number %d",
+				 "DDR ECC error type :%s Row %d Col %d Bank %d Bank Group %d",
 				 "UE", pinf->row, pinf->col, pinf->bank,
-				 pinf->bankgrpnr);
+				 pinf->bankgrp);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
 				 "DDR ECC error type :%s Row %d Bank %d Col %d ",
-- 
2.35.1


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

* [PATCH 11/20] EDAC/synopsys: Drop redundant info from error message
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (9 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 10/20] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition Serge Semin
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Currently the custom error message is too long so the corresponding log
messages printed in several lines. There is some duplicated/redundant
information we can remove from it. First of all we can shorten it out by
dropping the message prefix "DDR ECC error type:%s". Indeed the text
printed to the console later by the edac_mc_printk() method will contain
the error type and the memory controller id referring to the device
detected the error. So having the same info in the custom part of the
same message is useless especially seeing the string has got too long
already. Secondly referring to the corrected bit as "Bit Position" is
redundant since saying just "Bit" is not that less descriptive. Let's
drop all of these parts of the error message then so to have a shorter
but still informative enough log message.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 63b3d11e3904..379a668295cc 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -483,13 +483,13 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ceinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Col %d Bank %d Bank Group %d Bit Position: %d Data: 0x%08x",
-				 "CE", pinf->row, pinf->col, pinf->bank,
-				 pinf->bankgrp, pinf->bitpos, pinf->data);
+				 "Row %d Col %d Bank %d Bank Group %d Bit %d Data 0x%08x",
+				 pinf->row, pinf->col, pinf->bank, pinf->bankgrp,
+				 pinf->bitpos, pinf->data);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Bank %d Col %d Bit Position: %d Data: 0x%08x",
-				 "CE", pinf->row, pinf->bank, pinf->col,
+				 "Row %d Bank %d Col %d Bit: %d Data: 0x%08x",
+				 pinf->row, pinf->bank, pinf->col,
 				 pinf->bitpos, pinf->data);
 		}
 
@@ -502,13 +502,12 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ueinfo;
 		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type :%s Row %d Col %d Bank %d Bank Group %d",
-				 "UE", pinf->row, pinf->col, pinf->bank,
-				 pinf->bankgrp);
+				 "Row %d Col %d Bank %d Bank Group %d",
+				 pinf->row, pinf->col, pinf->bank, pinf->bankgrp);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type :%s Row %d Bank %d Col %d ",
-				 "UE", pinf->row, pinf->bank, pinf->col);
+				 "Row %d Bank %d Col %d",
+				 pinf->row, pinf->bank, pinf->col);
 		}
 
 		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
-- 
2.35.1


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

* [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (10 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 11/20] EDAC/synopsys: Drop redundant info from error message Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-09-01  6:59   ` Borislav Petkov
  2022-08-22 19:07 ` [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout Serge Semin
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Currently the memory type macros are partly defined with the multiple
spaces between the macro name and its definition. Let's replace the spaces
with tabs as the kernel coding style requires.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 include/linux/edac.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/edac.h b/include/linux/edac.h
index e730b3468719..fa4bda2a70f6 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -231,21 +231,21 @@ enum mem_type {
 #define MEM_FLAG_DDR		BIT(MEM_DDR)
 #define MEM_FLAG_RDDR		BIT(MEM_RDDR)
 #define MEM_FLAG_RMBS		BIT(MEM_RMBS)
-#define MEM_FLAG_DDR2           BIT(MEM_DDR2)
-#define MEM_FLAG_FB_DDR2        BIT(MEM_FB_DDR2)
-#define MEM_FLAG_RDDR2          BIT(MEM_RDDR2)
-#define MEM_FLAG_XDR            BIT(MEM_XDR)
-#define MEM_FLAG_DDR3           BIT(MEM_DDR3)
-#define MEM_FLAG_RDDR3          BIT(MEM_RDDR3)
-#define MEM_FLAG_LPDDR3         BIT(MEM_LPDDR3)
-#define MEM_FLAG_DDR4           BIT(MEM_DDR4)
-#define MEM_FLAG_RDDR4          BIT(MEM_RDDR4)
-#define MEM_FLAG_LRDDR4         BIT(MEM_LRDDR4)
-#define MEM_FLAG_LPDDR4         BIT(MEM_LPDDR4)
-#define MEM_FLAG_DDR5           BIT(MEM_DDR5)
-#define MEM_FLAG_RDDR5          BIT(MEM_RDDR5)
-#define MEM_FLAG_LRDDR5         BIT(MEM_LRDDR5)
-#define MEM_FLAG_NVDIMM         BIT(MEM_NVDIMM)
+#define MEM_FLAG_DDR2		BIT(MEM_DDR2)
+#define MEM_FLAG_FB_DDR2	BIT(MEM_FB_DDR2)
+#define MEM_FLAG_RDDR2		BIT(MEM_RDDR2)
+#define MEM_FLAG_XDR		BIT(MEM_XDR)
+#define MEM_FLAG_DDR3		BIT(MEM_DDR3)
+#define MEM_FLAG_RDDR3		BIT(MEM_RDDR3)
+#define MEM_FLAG_LPDDR3		BIT(MEM_LPDDR3)
+#define MEM_FLAG_DDR4		BIT(MEM_DDR4)
+#define MEM_FLAG_RDDR4		BIT(MEM_RDDR4)
+#define MEM_FLAG_LRDDR4		BIT(MEM_LRDDR4)
+#define MEM_FLAG_LPDDR4		BIT(MEM_LPDDR4)
+#define MEM_FLAG_DDR5		BIT(MEM_DDR5)
+#define MEM_FLAG_RDDR5		BIT(MEM_RDDR5)
+#define MEM_FLAG_LRDDR5		BIT(MEM_LRDDR5)
+#define MEM_FLAG_NVDIMM		BIT(MEM_NVDIMM)
 #define MEM_FLAG_WIO2		BIT(MEM_WIO2)
 #define MEM_FLAG_HBM2		BIT(MEM_HBM2)
 
-- 
2.35.1


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

* [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (11 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-09-01  7:00   ` Borislav Petkov
  2022-08-22 19:07 ` [PATCH 14/20] EDAC/mc: Init DIMM labels in MC registration method Serge Semin
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

The duplicated edac_dbg()-based dimm->nr_pages field was introduced in
commit 6e84d359b2be ("edac_mc: Cleanup per-dimm_info debug messages"). You
can find the duplicated line even in the commit message text:
> [ 1011.380101] EDAC DEBUG: edac_mc_dump_dimm:   dimm->nr_pages = 0x40000
> [ 1011.380103] EDAC DEBUG: edac_mc_dump_dimm:   dimm->grain = 8
> [ 1011.380104] EDAC DEBUG: edac_mc_dump_dimm:   dimm->nr_pages = 0x40000
Let's drop the corresponding edac_dbg() method call then.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/edac_mc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index eb58644bb019..6faeb2ab3960 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -103,7 +103,6 @@ static void edac_mc_dump_dimm(struct dimm_info *dimm)
 	edac_dbg(4, "  dimm->label = '%s'\n", dimm->label);
 	edac_dbg(4, "  dimm->nr_pages = 0x%x\n", dimm->nr_pages);
 	edac_dbg(4, "  dimm->grain = %d\n", dimm->grain);
-	edac_dbg(4, "  dimm->nr_pages = 0x%x\n", dimm->nr_pages);
 }
 
 static void edac_mc_dump_csrow(struct csrow_info *csrow)
-- 
2.35.1


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

* [PATCH 14/20] EDAC/mc: Init DIMM labels in MC registration method
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (12 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 15/20] EDAC/mc: Add MC unique index allocation procedure Serge Semin
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

As a preparation before adding the unique MC index allocation procedure we
need to move the DIMM labels initialization to the memory controller
registration method. It's required because the labels contain the MC index
as the "mc%u" part of the string, which in case of the auto-generated
index isn't available at the moment of the MCI/csrow/dimms descriptor
allocation.

So nothing complex is done here. Just move the labels initialization from
edac_mc_alloc_dimms() to the dedicated method edac_mc_init_labels() and
call it from edac_mc_add_mc_with_groups().

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/edac_mc.c | 48 +++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6faeb2ab3960..24814839d885 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -256,7 +256,6 @@ static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
 	unsigned int pos[EDAC_MAX_LAYERS];
 	unsigned int row, chn, idx;
 	int layer;
-	void *p;
 
 	/*
 	 * Allocate and fill the dimm structs
@@ -271,7 +270,6 @@ static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
 	for (idx = 0; idx < mci->tot_dimms; idx++) {
 		struct dimm_info *dimm;
 		struct rank_info *chan;
-		int n, len;
 
 		chan = mci->csrows[row]->channels[chn];
 
@@ -282,22 +280,9 @@ static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
 		dimm->mci = mci;
 		dimm->idx = idx;
 
-		/*
-		 * Copy DIMM location and initialize it.
-		 */
-		len = sizeof(dimm->label);
-		p = dimm->label;
-		n = scnprintf(p, len, "mc#%u", mci->mc_idx);
-		p += n;
-		len -= n;
-		for (layer = 0; layer < mci->n_layers; layer++) {
-			n = scnprintf(p, len, "%s#%u",
-				      edac_layer_name[mci->layers[layer].type],
-				      pos[layer]);
-			p += n;
-			len -= n;
+		/* Copy DIMM location */
+		for (layer = 0; layer < mci->n_layers; layer++)
 			dimm->location[layer] = pos[layer];
-		}
 
 		/* Link it to the csrows old API data */
 		chan->dimm = dimm;
@@ -510,6 +495,33 @@ void edac_mc_reset_delay_period(unsigned long value)
 
 
 
+/**
+ * edac_mc_init_labels() - Initialize DIMM labels
+ *
+ * @mci: pointer to the mci structure which DIMM labels need to be initialized
+ *
+ * .. note::
+ *	locking model: must be called with the mem_ctls_mutex lock held
+ */
+static void edac_mc_init_labels(struct mem_ctl_info *mci)
+{
+	int n, len, layer;
+	unsigned int idx;
+	char *p;
+
+	for (idx = 0; idx < mci->tot_dimms; idx++) {
+		len = sizeof(mci->dimms[idx]->label);
+		p = mci->dimms[idx]->label;
+
+		n = scnprintf(p, len, "mc#%u", mci->mc_idx);
+		for (layer = 0; layer < mci->n_layers; layer++) {
+			n += scnprintf(p + n, len - n, "%s#%u",
+				      edac_layer_name[mci->layers[layer].type],
+				      mci->dimms[idx]->location[layer]);
+		}
+	}
+}
+
 /* Return 0 on success, 1 on failure.
  * Before calling this function, caller must
  * assign a unique value to mci->mc_idx.
@@ -637,6 +649,8 @@ int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 		goto fail0;
 	}
 
+	edac_mc_init_labels(mci);
+
 	if (add_mc_to_global_list(mci))
 		goto fail0;
 
-- 
2.35.1


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

* [PATCH 15/20] EDAC/mc: Add MC unique index allocation procedure
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (13 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 14/20] EDAC/mc: Init DIMM labels in MC registration method Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support Serge Semin
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

In case of the unique index allocation it's not that optimal to always
rely on the low-level device drivers (platform drivers), because they get
to start to implement either the same design pattern (for instance global
static MC counter) or may end-up with having non-unique index eventually
at runtime. Needless to say that having a generic unique index
allocation/tracking procedure will make code more readable and safer.

The suggested implementation is based on the kernel IDA infrastructure
exposed by the lib/idr.c driver with API described in linux/idr.h header
file. It's used to create an ID resource descriptor "mc_idr", which then
is utilized either to track the custom MC idx specified by EDAC LLDDs or
to allocate the next-free MC idx.

A new special MC index is introduced here. It's defined by the
EDAC_AUTO_MC_NUM macro with a value specifically chosen as the least
probable value used for the real MC index. In case if the EDAC_AUTO_MC_NUM
index is specified by the EDAC LLDD, the MC index will be either retrieved
from the MC device OF-node alias index ("mc[:number:]") or automatically
generated as the next-free MC index found by the ID allocation procedure.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Note the approach implemented here has been partly ported from the SPI
core driver using IDA to track/allocate SPI bus numbers.
Link: https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L2957
---
 drivers/edac/edac_mc.c | 89 +++++++++++++++++++++++++++++++++++++++---
 drivers/edac/edac_mc.h |  4 ++
 2 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 24814839d885..634c41ea7804 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -29,6 +29,9 @@
 #include <linux/edac.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
+#include <linux/idr.h>
+#include <linux/of.h>
+
 #include <asm/page.h>
 #include "edac_mc.h"
 #include "edac_module.h"
@@ -46,6 +49,7 @@ EXPORT_SYMBOL_GPL(edac_op_state);
 /* lock to memory controller's control array */
 static DEFINE_MUTEX(mem_ctls_mutex);
 static LIST_HEAD(mc_devices);
+static DEFINE_IDR(mc_idr);
 
 /*
  * Used to lock EDAC MC to just one module, avoiding two drivers e. g.
@@ -493,7 +497,64 @@ void edac_mc_reset_delay_period(unsigned long value)
 	mutex_unlock(&mem_ctls_mutex);
 }
 
+/**
+ * edac_mc_alloc_id() - Allocate unique Memory Controller identifier
+ *
+ * @mci: pointer to the mci structure to allocate ID for
+ *
+ * Use edac_mc_free_id() to coherently free the MC identifier.
+ *
+ * .. note::
+ *	locking model: must be called with the mem_ctls_mutex lock held
+ *
+ * Returns:
+ *	0 on Success, or an error code on failure
+ */
+static int edac_mc_alloc_id(struct mem_ctl_info *mci)
+{
+	struct device_node *np = dev_of_node(mci->pdev);
+	int ret, min, max;
+
+	if (mci->mc_idx == EDAC_AUTO_MC_NUM) {
+		ret = of_alias_get_id(np, "mc");
+		if (ret >= 0) {
+			min = ret;
+			max = ret + 1;
+		} else {
+			min = of_alias_get_highest_id("mc");
+			if (min >= 0)
+				min++;
+			else
+				min = 0;
+
+			max = 0;
+		}
+	} else {
+		min = mci->mc_idx;
+		max = mci->mc_idx + 1;
+	}
+
+	ret = idr_alloc(&mc_idr, mci, min, max, GFP_KERNEL);
+	if (ret < 0)
+		return ret == -ENOSPC ? -EBUSY : ret;
+
+	mci->mc_idx = ret;
+
+	return 0;
+}
 
+/**
+ * edac_mc_free_id() - Free Memory Controller identifier
+ *
+ * @mci: pointer to the mci structure to free ID from
+ *
+ * .. note::
+ *	locking model: must be called with the mem_ctls_mutex lock held
+ */
+static void edac_mc_free_id(struct mem_ctl_info *mci)
+{
+	idr_remove(&mc_idr, mci->mc_idx);
+}
 
 /**
  * edac_mc_init_labels() - Initialize DIMM labels
@@ -612,7 +673,8 @@ EXPORT_SYMBOL_GPL(edac_get_owner);
 int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 			       const struct attribute_group **groups)
 {
-	int ret = -EINVAL;
+	int ret;
+
 	edac_dbg(0, "\n");
 
 #ifdef CONFIG_EDAC_DEBUG
@@ -649,20 +711,30 @@ int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 		goto fail0;
 	}
 
+	ret = edac_mc_alloc_id(mci);
+	if (ret) {
+		edac_printk(KERN_ERR, EDAC_MC, "failed to allocate MC idx %u\n",
+			    mci->mc_idx);
+		goto fail0;
+	}
+
 	edac_mc_init_labels(mci);
 
-	if (add_mc_to_global_list(mci))
-		goto fail0;
+	if (add_mc_to_global_list(mci)) {
+		ret = -EINVAL;
+		goto fail1;
+	}
 
 	/* set load time so that error rate can be tracked */
 	mci->start_time = jiffies;
 
 	mci->bus = edac_get_sysfs_subsys();
 
-	if (edac_create_sysfs_mci_device(mci, groups)) {
+	ret = edac_create_sysfs_mci_device(mci, groups);
+	if (ret) {
 		edac_mc_printk(mci, KERN_WARNING,
 			"failed to create sysfs device\n");
-		goto fail1;
+		goto fail2;
 	}
 
 	if (mci->edac_check) {
@@ -686,9 +758,12 @@ int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 	mutex_unlock(&mem_ctls_mutex);
 	return 0;
 
-fail1:
+fail2:
 	del_mc_from_global_list(mci);
 
+fail1:
+	edac_mc_free_id(mci);
+
 fail0:
 	mutex_unlock(&mem_ctls_mutex);
 	return ret;
@@ -716,6 +791,8 @@ struct mem_ctl_info *edac_mc_del_mc(struct device *dev)
 	if (del_mc_from_global_list(mci))
 		edac_mc_owner = NULL;
 
+	edac_mc_free_id(mci);
+
 	mutex_unlock(&mem_ctls_mutex);
 
 	if (mci->edac_check)
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 881b00eadf7a..4b6676235b1b 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -23,6 +23,7 @@
 #define _EDAC_MC_H_
 
 #include <linux/kernel.h>
+#include <linux/limits.h>
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -37,6 +38,9 @@
 #include <linux/workqueue.h>
 #include <linux/edac.h>
 
+/* Generate MC identifier automatically */
+#define EDAC_AUTO_MC_NUM	UINT_MAX
+
 #if PAGE_SHIFT < 20
 #define PAGES_TO_MiB(pages)	((pages) >> (20 - PAGE_SHIFT))
 #define MiB_TO_PAGES(mb)	((mb) << (20 - PAGE_SHIFT))
-- 
2.35.1


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

* [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (14 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 15/20] EDAC/mc: Add MC unique index allocation procedure Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-23  8:17   ` Krzysztof Kozlowski
  2022-08-23  8:26   ` Krzysztof Kozlowski
  2022-08-22 19:07 ` [PATCH 17/20] EDAC/synopsys: " Serge Semin
                   ` (3 subsequent siblings)
  19 siblings, 2 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Krzysztof Kozlowski,
	Rob Herring, Manish Narani
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Dinh Nguyen,
	James Morse, Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel, Krzysztof Kozlowski

The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
no any reason to have these controllers described by the same bindings.
Thus let's split them up.

While at it rename the original Synopsys uMCTL2 DT-schema file to a more
descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
description of the device bindings.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
 .../memory-controllers/synopsys,ddrc-ecc.yaml | 76 -------------------
 .../xlnx,zynq-ddrc-a05.yaml                   | 38 ++++++++++
 MAINTAINERS                                   |  2 +
 4 files changed, 91 insertions(+), 76 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/synopsys,ddrc-ecc.yaml
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
new file mode 100644
index 000000000000..787d91d64eee
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/snps,dw-umctl2-ddrc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare Universal Multi-Protocol Memory Controller
+
+maintainers:
+  - Krzysztof Kozlowski <krzk@kernel.org>
+  - Manish Narani <manish.narani@xilinx.com>
+  - Michal Simek <michal.simek@xilinx.com>
+
+description: |
+  Synopsys DesignWare Enhanced uMCTL2 DDR Memory Controller is cappable of
+  working with DDR devices up to (LP)DDR4 protocol. It can be equipped
+  with SEC/DEC ECC feature if DRAM data bus width is either 16-bits or
+  32-bits or 64-bits wide.
+
+  The ZynqMP DDR controller is based on the DW uMCTL2 v2.40a controller.
+  It has an optional SEC/DEC ECC support in 64-bit and 32-bit bus width
+  configurations.
+
+properties:
+  compatible:
+    enum:
+      - snps,ddrc-3.80a
+      - xlnx,zynqmp-ddrc-2.40a
+
+  interrupts:
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    memory-controller@fd070000 {
+      compatible = "xlnx,zynqmp-ddrc-2.40a";
+      reg = <0xfd070000 0x30000>;
+      interrupt-parent = <&gic>;
+      interrupts = <0 112 4>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/memory-controllers/synopsys,ddrc-ecc.yaml b/Documentation/devicetree/bindings/memory-controllers/synopsys,ddrc-ecc.yaml
deleted file mode 100644
index f46e95704f53..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/synopsys,ddrc-ecc.yaml
+++ /dev/null
@@ -1,76 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/memory-controllers/synopsys,ddrc-ecc.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Synopsys IntelliDDR Multi Protocol memory controller
-
-maintainers:
-  - Krzysztof Kozlowski <krzk@kernel.org>
-  - Manish Narani <manish.narani@xilinx.com>
-  - Michal Simek <michal.simek@xilinx.com>
-
-description: |
-  The ZynqMP DDR ECC controller has an optional ECC support in 64-bit and
-  32-bit bus width configurations.
-
-  The Zynq DDR ECC controller has an optional ECC support in half-bus width
-  (16-bit) configuration.
-
-  These both ECC controllers correct single bit ECC errors and detect double bit
-  ECC errors.
-
-properties:
-  compatible:
-    enum:
-      - snps,ddrc-3.80a
-      - xlnx,zynq-ddrc-a05
-      - xlnx,zynqmp-ddrc-2.40a
-
-  interrupts:
-    maxItems: 1
-
-  reg:
-    maxItems: 1
-
-required:
-  - compatible
-  - reg
-
-allOf:
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - snps,ddrc-3.80a
-              - xlnx,zynqmp-ddrc-2.40a
-    then:
-      required:
-        - interrupts
-    else:
-      properties:
-        interrupts: false
-
-additionalProperties: false
-
-examples:
-  - |
-    memory-controller@f8006000 {
-        compatible = "xlnx,zynq-ddrc-a05";
-        reg = <0xf8006000 0x1000>;
-    };
-
-  - |
-    axi {
-        #address-cells = <2>;
-        #size-cells = <2>;
-
-        memory-controller@fd070000 {
-            compatible = "xlnx,zynqmp-ddrc-2.40a";
-            reg = <0x0 0xfd070000 0x0 0x30000>;
-            interrupt-parent = <&gic>;
-            interrupts = <0 112 4>;
-        };
-    };
diff --git a/Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml b/Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml
new file mode 100644
index 000000000000..8f72e2f8588a
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/xlnx,zynq-ddrc-a05.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Zynq A05 DDR Memory Controller
+
+maintainers:
+  - Krzysztof Kozlowski <krzk@kernel.org>
+  - Manish Narani <manish.narani@xilinx.com>
+  - Michal Simek <michal.simek@xilinx.com>
+
+description:
+  The Zynq DDR ECC controller has an optional ECC support in half-bus width
+  (16-bit) configuration. It is cappable of correcting single bit ECC errors
+  and detecting double bit ECC errors.
+
+properties:
+  compatible:
+    const: xlnx,zynq-ddrc-a05
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    memory-controller@f8006000 {
+      compatible = "xlnx,zynq-ddrc-a05";
+      reg = <0xf8006000 0x1000>;
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 9d7f64dc0efe..17f23f810ee4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3086,7 +3086,9 @@ S:	Supported
 W:	http://wiki.xilinx.com
 T:	git https://github.com/Xilinx/linux-xlnx.git
 F:	Documentation/devicetree/bindings/i2c/cdns,i2c-r1p10.yaml
+F:	Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
 F:	Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
+F:	Documentation/devicetree/bindings/memory-controllers/xlnx,zynq-ddrc-a05.yaml
 F:	Documentation/devicetree/bindings/spi/xlnx,zynq-qspi.yaml
 F:	arch/arm/mach-zynq/
 F:	drivers/clocksource/timer-cadence-ttc.c
-- 
2.35.1


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

* [PATCH 17/20] EDAC/synopsys: Detach Zynq DDRC controller support
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (15 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-22 19:07 ` [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

It was a bad idea in the first place to combine two absolutely different
controllers support in a single driver [1]. It caused having an additional
level of abstraction, which obviously have needlessly overcomplicated the
driver and as such caused many problems in the new main controller
features support implementation. The solution looks even more unreasonable
now seeing the justification of having both controllers support in a
single driver hasn't been implemented by the original code author [2].

Anyway since the Zynq A05 DDRC controller has nothing in common with DW
uMCTL2 DDRC (even the CSRs layout is absolutely different, not to say it
doesn't support IRQs), since the EDAC MC core driver now support MC index
auto-generation and since we are going to extend the DW uMCTL2 EDAC driver
functionality (having an incompatible code support will greatly complicate
the resultant driver and the upcoming patches) let's move the Xilinx Zynq
DDRC support to another driver - zynq_edac.c. Basically it means to simply
detach the Zynq-specific callbacks, init/probe/remove methods and move
them in the new driver. The resultant driver will mostly look similar to
the code submitted in the initial commit ae9b56e3996d ("EDAC, synps: Add
EDAC support for zynq ddr ecc controller") except a few fixes added
afterwards. The Zynq-specific code and macros are removed from the
original module of course thus leaving the DWC uMCTL2 DDRC driver purely
synopsys DDRC specific.

Note 1. In order to improve the new driver code readability all the
internal entities have been equipped with the vendor-specific prefix. Thus
the local, global and EDAC-core specific names will be explicitly
distinguishable right from the place they are called.

Note 2. Some of the Zynq-specific macros have been used in the framework
of the DW uMCTL2 DDRC-specific functions. In most of the cases it was
wrong even though hasn't caused any problem since the macro substituted
with correct values (for instance, CTRL_OFST macro usage where
DDR_MSTR_OFST should have been utilized). Anyway since these macros are
now moved to another driver let's fix the places where the macros have
been improperly used.

[1] Link: https://lore.kernel.org/all/808655a9-77eb-4e3a-9781-2b059ad9517b@BN1AFFO11FD020.protection.gbl/
[2] Link: https://lore.kernel.org/all/9dc2a947-d2ab-4f00-8ed3-d2499cb6fdfd@BN1BFFO11FD002.protection.gbl/

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 MAINTAINERS                  |   1 +
 drivers/edac/Kconfig         |   9 +-
 drivers/edac/Makefile        |   1 +
 drivers/edac/synopsys_edac.c | 271 +++----------------
 drivers/edac/zynq_edac.c     | 504 +++++++++++++++++++++++++++++++++++
 5 files changed, 547 insertions(+), 239 deletions(-)
 create mode 100644 drivers/edac/zynq_edac.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 17f23f810ee4..27b6bb41e401 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3094,6 +3094,7 @@ F:	arch/arm/mach-zynq/
 F:	drivers/clocksource/timer-cadence-ttc.c
 F:	drivers/cpuidle/cpuidle-zynq.c
 F:	drivers/edac/synopsys_edac.c
+F:	drivers/edac/zynq_edac.c
 F:	drivers/i2c/busses/i2c-cadence.c
 F:	drivers/i2c/busses/i2c-xiic.c
 F:	drivers/mmc/host/sdhci-of-arasan.c
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 17562cf1fe97..98bcdadf4143 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -486,7 +486,7 @@ config EDAC_ARMADA_XP
 
 config EDAC_SYNOPSYS
 	tristate "Synopsys DDR Memory Controller"
-	depends on ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || ARCH_MXC
+	depends on ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || ARCH_MXC
 	help
 	  Support for error detection and correction on the Synopsys DDR
 	  memory controller.
@@ -541,4 +541,11 @@ config EDAC_DMC520
 	  Support for error detection and correction on the
 	  SoCs with ARM DMC-520 DRAM controller.
 
+config EDAC_ZYNQ
+	tristate "Xilinx Zynq A05 DDR Memory Controller"
+	depends on ARCH_ZYNQ
+	help
+	  Support for error detection and correction on the Xilinx Zynq A05
+	  DDR memory controller.
+
 endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 2d1641a27a28..83e063f53b22 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_ZYNQ)			+= zynq_edac.o
diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 379a668295cc..0cb84f183aed 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -30,68 +30,16 @@
 #define SYNPS_EDAC_MOD_STRING		"synps_edac"
 #define SYNPS_EDAC_MOD_VER		"1"
 
-/* Synopsys DDR memory controller registers that are relevant to ECC */
-#define CTRL_OFST			0x0
-#define T_ZQ_OFST			0xA4
-
-/* ECC control register */
-#define ECC_CTRL_OFST			0xC4
-/* ECC log register */
-#define CE_LOG_OFST			0xC8
-/* ECC address register */
-#define CE_ADDR_OFST			0xCC
-/* ECC data[31:0] register */
-#define CE_DATA_31_0_OFST		0xD0
-
-/* Uncorrectable error info registers */
-#define UE_LOG_OFST			0xDC
-#define UE_ADDR_OFST			0xE0
-#define UE_DATA_31_0_OFST		0xE4
-
-#define STAT_OFST			0xF0
-#define SCRUB_OFST			0xF4
-
-/* Control register bit field definitions */
-#define CTRL_BW_MASK			0xC
-#define CTRL_BW_SHIFT			2
-
-#define DDRCTL_WDTH_16			1
-#define DDRCTL_WDTH_32			0
-
-/* ZQ register bit field definitions */
-#define T_ZQ_DDRMODE_MASK		0x2
-
-/* ECC control register bit field definitions */
-#define ECC_CTRL_CLR_CE_ERR		0x2
-#define ECC_CTRL_CLR_UE_ERR		0x1
-
-/* ECC correctable/uncorrectable error log register definitions */
-#define LOG_VALID			0x1
-#define CE_LOG_BITPOS_MASK		0xFE
-#define CE_LOG_BITPOS_SHIFT		1
-
-/* ECC correctable/uncorrectable error address register definitions */
-#define ADDR_COL_MASK			0xFFF
-#define ADDR_ROW_MASK			0xFFFF000
-#define ADDR_ROW_SHIFT			12
-#define ADDR_BANK_MASK			0x70000000
-#define ADDR_BANK_SHIFT			28
-
-/* ECC statistic register definitions */
-#define STAT_UECNT_MASK			0xFF
-#define STAT_CECNT_MASK			0xFF00
-#define STAT_CECNT_SHIFT		8
-
-/* ECC scrub register definitions */
-#define SCRUB_MODE_MASK			0x7
-#define SCRUB_MODE_SECDED		0x4
-
 /* DDR ECC Quirks */
 #define DDR_ECC_INTR_SUPPORT		BIT(0)
 #define DDR_ECC_DATA_POISON_SUPPORT	BIT(1)
 #define SYNPS_ZYNQMP_IRQ_REGS		BIT(2)
 
-/* ZynqMP Enhanced DDR memory controller registers that are relevant to ECC */
+/* Synopsys DDR memory controller registers that are relevant to ECC */
+
+/* DDRC master0 Register */
+#define DDR_MSTR_OFST			0x0
+
 /* ECC Configuration Registers */
 #define ECC_CFG0_OFST			0x70
 #define ECC_CFG1_OFST			0x74
@@ -132,15 +80,11 @@
 #define ECC_POISON0_OFST		0xB8
 #define ECC_POISON1_OFST		0xBC
 
-#define ECC_ADDRMAP0_OFFSET		0x200
+/* DDR Address map0 Registers */
+#define ECC_ADDRMAP0_OFST		0x200
 
-/* ECC control/clear register definitions */
-#define ECC_CTRL_BUSWIDTH_MASK		0x3000
-#define ECC_CTRL_BUSWIDTH_SHIFT		12
-#define ECC_CTRL_CLR_CE_ERRCNT		BIT(2)
-#define ECC_CTRL_CLR_UE_ERRCNT		BIT(3)
-#define ECC_CTRL_EN_CE_IRQ		BIT(8)
-#define ECC_CTRL_EN_UE_IRQ		BIT(9)
+/* DDR Software control register */
+#define DDR_SWCTL			0x320
 
 /* DDR Master Register 0 definitions */
 #define DDR_MSTR_DEV_CFG_MASK		GENMASK(31, 30)
@@ -148,10 +92,16 @@
 #define DDR_MSTR_DEV_X8			0x1
 #define DDR_MSTR_DEV_X16		0x2
 #define DDR_MSTR_DEV_X32		0x3
+#define DDR_MSTR_BUSWIDTH_MASK		0x3000
+#define DDR_MSTR_BUSWIDTH_SHIFT		12
 #define DDRCTL_EWDTH_16			2
 #define DDRCTL_EWDTH_32			1
 #define DDRCTL_EWDTH_64			0
 
+/* ECC CFG0 register definitions */
+#define ECC_CFG0_MODE_MASK		0x7
+#define ECC_CFG0_MODE_SECDED		0x4
+
 /* ECC status register definitions */
 #define ECC_STAT_UECNT_MASK		0xF0000
 #define ECC_STAT_UECNT_SHIFT		16
@@ -159,6 +109,14 @@
 #define ECC_STAT_CECNT_SHIFT		8
 #define ECC_STAT_BITNUM_MASK		0x7F
 
+/* ECC control/clear register definitions */
+#define ECC_CTRL_CLR_CE_ERR		BIT(0)
+#define ECC_CTRL_CLR_UE_ERR		BIT(1)
+#define ECC_CTRL_CLR_CE_ERRCNT		BIT(2)
+#define ECC_CTRL_CLR_UE_ERRCNT		BIT(3)
+#define ECC_CTRL_EN_CE_IRQ		BIT(8)
+#define ECC_CTRL_EN_UE_IRQ		BIT(9)
+
 /* ECC error count register definitions */
 #define ECC_ERRCNT_UECNT_MASK		0xFFFF0000
 #define ECC_ERRCNT_UECNT_SHIFT		16
@@ -200,21 +158,11 @@
 #define MEM_TYPE_DDR4			0x10
 #define MEM_TYPE_LPDDR4			0x20
 
-/* DDRC Software control register */
-#define DDRC_SWCTL			0x320
-
 /* DDRC ECC CE & UE poison mask */
 #define ECC_CEPOISON_MASK		0x3
 #define ECC_UEPOISON_MASK		0x1
 
-/* DDRC Device config masks */
-#define DDRC_MSTR_CFG_MASK		0xC0000000
-#define DDRC_MSTR_CFG_SHIFT		30
-#define DDRC_MSTR_CFG_X4_MASK		0x0
-#define DDRC_MSTR_CFG_X8_MASK		0x1
-#define DDRC_MSTR_CFG_X16_MASK		0x2
-#define DDRC_MSTR_CFG_X32_MASK		0x3
-
+/* DDRC Device config shifts/masks */
 #define DDR_MAX_ROW_SHIFT		18
 #define DDR_MAX_COL_SHIFT		14
 #define DDR_MAX_BANK_SHIFT		3
@@ -345,61 +293,6 @@ struct synps_platform_data {
 	int quirks;
 };
 
-/**
- * zynq_get_error_info - Get the current ECC error info.
- * @priv:	DDR memory controller private instance data.
- *
- * Return: one if there is no error, otherwise zero.
- */
-static int zynq_get_error_info(struct synps_edac_priv *priv)
-{
-	struct synps_ecc_status *p;
-	u32 regval, clearval = 0;
-	void __iomem *base;
-
-	base = priv->baseaddr;
-	p = &priv->stat;
-
-	regval = readl(base + STAT_OFST);
-	if (!regval)
-		return 1;
-
-	p->ce_cnt = (regval & STAT_CECNT_MASK) >> STAT_CECNT_SHIFT;
-	p->ue_cnt = regval & STAT_UECNT_MASK;
-
-	regval = readl(base + CE_LOG_OFST);
-	if (!(p->ce_cnt && (regval & LOG_VALID)))
-		goto ue_err;
-
-	p->ceinfo.bitpos = (regval & CE_LOG_BITPOS_MASK) >> CE_LOG_BITPOS_SHIFT;
-	regval = readl(base + CE_ADDR_OFST);
-	p->ceinfo.row = (regval & ADDR_ROW_MASK) >> ADDR_ROW_SHIFT;
-	p->ceinfo.col = regval & ADDR_COL_MASK;
-	p->ceinfo.bank = (regval & ADDR_BANK_MASK) >> ADDR_BANK_SHIFT;
-	p->ceinfo.data = readl(base + CE_DATA_31_0_OFST);
-	edac_dbg(3, "CE bit position: %d data: %d\n", p->ceinfo.bitpos,
-		 p->ceinfo.data);
-	clearval = ECC_CTRL_CLR_CE_ERR;
-
-ue_err:
-	regval = readl(base + UE_LOG_OFST);
-	if (!(p->ue_cnt && (regval & LOG_VALID)))
-		goto out;
-
-	regval = readl(base + UE_ADDR_OFST);
-	p->ueinfo.row = (regval & ADDR_ROW_MASK) >> ADDR_ROW_SHIFT;
-	p->ueinfo.col = regval & ADDR_COL_MASK;
-	p->ueinfo.bank = (regval & ADDR_BANK_MASK) >> ADDR_BANK_SHIFT;
-	p->ueinfo.data = readl(base + UE_DATA_31_0_OFST);
-	clearval |= ECC_CTRL_CLR_UE_ERR;
-
-out:
-	writel(clearval, base + ECC_CTRL_OFST);
-	writel(0x0, base + ECC_CTRL_OFST);
-
-	return 0;
-}
-
 /**
  * zynqmp_get_error_info - Get the current ECC error info.
  * @priv:	DDR memory controller private instance data.
@@ -622,37 +515,6 @@ static void check_errors(struct mem_ctl_info *mci)
 		 priv->ce_cnt, priv->ue_cnt);
 }
 
-/**
- * zynq_get_dtype - Return the controller memory width.
- * @base:	DDR memory controller base address.
- *
- * Get the EDAC device type width appropriate for the current controller
- * configuration.
- *
- * Return: a device type width enumeration.
- */
-static enum dev_type zynq_get_dtype(const void __iomem *base)
-{
-	enum dev_type dt;
-	u32 width;
-
-	width = readl(base + CTRL_OFST);
-	width = (width & CTRL_BW_MASK) >> CTRL_BW_SHIFT;
-
-	switch (width) {
-	case DDRCTL_WDTH_16:
-		dt = DEV_X2;
-		break;
-	case DDRCTL_WDTH_32:
-		dt = DEV_X4;
-		break;
-	default:
-		dt = DEV_UNKNOWN;
-	}
-
-	return dt;
-}
-
 /**
  * zynqmp_get_dtype - Return the controller memory width.
  * @base:	DDR memory controller base address.
@@ -685,30 +547,6 @@ static enum dev_type zynqmp_get_dtype(const void __iomem *base)
 	return DEV_UNKNOWN;
 }
 
-/**
- * zynq_get_ecc_state - Return the controller ECC enable/disable status.
- * @base:	DDR memory controller base address.
- *
- * Get the ECC enable/disable status of the controller.
- *
- * Return: true if enabled, otherwise false.
- */
-static bool zynq_get_ecc_state(void __iomem *base)
-{
-	enum dev_type dt;
-	u32 ecctype;
-
-	dt = zynq_get_dtype(base);
-	if (dt == DEV_UNKNOWN)
-		return false;
-
-	ecctype = readl(base + SCRUB_OFST) & SCRUB_MODE_MASK;
-	if ((ecctype == SCRUB_MODE_SECDED) && (dt == DEV_X2))
-		return true;
-
-	return false;
-}
-
 /**
  * zynqmp_get_ecc_state - Return the controller ECC enable/disable status.
  * @base:	DDR memory controller base address.
@@ -721,9 +559,9 @@ static bool zynqmp_get_ecc_state(void __iomem *base)
 {
 	u32 regval;
 
-	regval = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK;
+	regval = readl(base + ECC_CFG0_OFST) & ECC_CFG0_MODE_MASK;
 
-	return (regval == SCRUB_MODE_SECDED);
+	return (regval == ECC_CFG0_MODE_SECDED);
 }
 
 /**
@@ -740,30 +578,6 @@ static u32 get_memsize(void)
 	return inf.totalram * inf.mem_unit;
 }
 
-/**
- * zynq_get_mtype - Return the controller memory type.
- * @base:	Synopsys ECC status structure.
- *
- * Get the EDAC memory type appropriate for the current controller
- * configuration.
- *
- * Return: a memory type enumeration.
- */
-static enum mem_type zynq_get_mtype(const void __iomem *base)
-{
-	enum mem_type mt;
-	u32 memtype;
-
-	memtype = readl(base + T_ZQ_OFST);
-
-	if (memtype & T_ZQ_DDRMODE_MASK)
-		mt = MEM_DDR3;
-	else
-		mt = MEM_DDR2;
-
-	return mt;
-}
-
 /**
  * zynqmp_get_mtype - Returns controller memory type.
  * @base:	Synopsys ECC status structure.
@@ -778,7 +592,7 @@ static enum mem_type zynqmp_get_mtype(const void __iomem *base)
 	enum mem_type mt;
 	u32 memtype;
 
-	memtype = readl(base + CTRL_OFST);
+	memtype = readl(base + DDR_MSTR_OFST);
 
 	if ((memtype & MEM_TYPE_DDR3) || (memtype & MEM_TYPE_LPDDR3))
 		mt = MEM_DDR3;
@@ -890,14 +704,6 @@ static int setup_irq(struct mem_ctl_info *mci,
 	return 0;
 }
 
-static const struct synps_platform_data zynq_edac_def = {
-	.get_error_info	= zynq_get_error_info,
-	.get_mtype	= zynq_get_mtype,
-	.get_dtype	= zynq_get_dtype,
-	.get_ecc_state	= zynq_get_ecc_state,
-	.quirks		= 0,
-};
-
 static const struct synps_platform_data zynqmp_edac_def = {
 	.get_error_info	= zynqmp_get_error_info,
 	.get_mtype	= zynqmp_get_mtype,
@@ -924,10 +730,6 @@ static const struct synps_platform_data synopsys_edac_def = {
 
 
 static const struct of_device_id synps_edac_match[] = {
-	{
-		.compatible = "xlnx,zynq-ddrc-a05",
-		.data = (void *)&zynq_edac_def
-	},
 	{
 		.compatible = "xlnx,zynqmp-ddrc-2.40a",
 		.data = (void *)&zynqmp_edac_def
@@ -1053,12 +855,12 @@ static ssize_t inject_data_poison_store(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct synps_edac_priv *priv = mci->pvt_info;
 
-	writel(0, priv->baseaddr + DDRC_SWCTL);
+	writel(0, priv->baseaddr + DDR_SWCTL);
 	if (strncmp(data, "CE", 2) == 0)
 		writel(ECC_CEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
 	else
 		writel(ECC_UEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
-	writel(1, priv->baseaddr + DDRC_SWCTL);
+	writel(1, priv->baseaddr + DDR_SWCTL);
 
 	return count;
 }
@@ -1149,8 +951,8 @@ static void setup_column_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 	u32 width, memtype;
 	int index;
 
-	memtype = readl(priv->baseaddr + CTRL_OFST);
-	width = (memtype & ECC_CTRL_BUSWIDTH_MASK) >> ECC_CTRL_BUSWIDTH_SHIFT;
+	memtype = readl(priv->baseaddr + DDR_MSTR_OFST);
+	width = (memtype & DDR_MSTR_BUSWIDTH_MASK) >> DDR_MSTR_BUSWIDTH_SHIFT;
 
 	priv->col_shift[0] = 0;
 	priv->col_shift[1] = 1;
@@ -1291,7 +1093,7 @@ static void setup_address_map(struct synps_edac_priv *priv)
 	for (index = 0; index < 12; index++) {
 		u32 addrmap_offset;
 
-		addrmap_offset = ECC_ADDRMAP0_OFFSET + (index * 4);
+		addrmap_offset = ECC_ADDRMAP0_OFST + (index * 4);
 		addrmap[index] = readl(priv->baseaddr + addrmap_offset);
 	}
 
@@ -1345,7 +1147,7 @@ static int mc_probe(struct platform_device *pdev)
 	layers[1].size = SYNPS_EDAC_NR_CHANS;
 	layers[1].is_virt_csrow = false;
 
-	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers,
+	mci = edac_mc_alloc(EDAC_AUTO_MC_NUM, ARRAY_SIZE(layers), layers,
 			    sizeof(struct synps_edac_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC,
@@ -1387,13 +1189,6 @@ static int mc_probe(struct platform_device *pdev)
 		setup_address_map(priv);
 #endif
 
-	/*
-	 * Start capturing the correctable and uncorrectable errors. A write of
-	 * 0 starts the counters.
-	 */
-	if (!(priv->p_data->quirks & DDR_ECC_INTR_SUPPORT))
-		writel(0x0, baseaddr + ECC_CTRL_OFST);
-
 	return rc;
 
 free_edac_mc:
diff --git a/drivers/edac/zynq_edac.c b/drivers/edac/zynq_edac.c
new file mode 100644
index 000000000000..66a2dedddfef
--- /dev/null
+++ b/drivers/edac/zynq_edac.c
@@ -0,0 +1,504 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Zynq DDR ECC Driver
+ * This driver is based on ppc4xx_edac.c drivers
+ *
+ * Copyright (C) 2012 - 2014 Xilinx, Inc.
+ */
+
+#include <linux/edac.h>
+#include <linux/interrupt.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include "edac_module.h"
+
+/* Number of cs_rows needed per memory controller */
+#define ZYNQ_EDAC_NR_CSROWS		1
+
+/* Number of channels per memory controller */
+#define ZYNQ_EDAC_NR_CHANS		1
+
+/* Granularity of reported error in bytes */
+#define ZYNQ_EDAC_ERR_GRAIN		1
+
+#define ZYNQ_EDAC_MSG_SIZE		256
+
+#define ZYNQ_EDAC_MOD_STRING		"zynq_edac"
+#define ZYNQ_EDAC_MOD_VER		"1"
+
+/* Zynq DDR memory controller ECC registers */
+#define ZYNQ_CTRL_OFST			0x0
+#define ZYNQ_T_ZQ_OFST			0xA4
+
+/* ECC control register */
+#define ZYNQ_ECC_CTRL_OFST		0xC4
+/* ECC log register */
+#define ZYNQ_CE_LOG_OFST		0xC8
+/* ECC address register */
+#define ZYNQ_CE_ADDR_OFST		0xCC
+/* ECC data[31:0] register */
+#define ZYNQ_CE_DATA_31_0_OFST		0xD0
+
+/* Uncorrectable error info registers */
+#define ZYNQ_UE_LOG_OFST		0xDC
+#define ZYNQ_UE_ADDR_OFST		0xE0
+#define ZYNQ_UE_DATA_31_0_OFST		0xE4
+
+#define ZYNQ_STAT_OFST			0xF0
+#define ZYNQ_SCRUB_OFST			0xF4
+
+/* Control register bit field definitions */
+#define ZYNQ_CTRL_BW_MASK		0xC
+#define ZYNQ_CTRL_BW_SHIFT		2
+
+#define ZYNQ_DDRCTL_WDTH_16		1
+#define ZYNQ_DDRCTL_WDTH_32		0
+
+/* ZQ register bit field definitions */
+#define ZYNQ_T_ZQ_DDRMODE_MASK		0x2
+
+/* ECC control register bit field definitions */
+#define ZYNQ_ECC_CTRL_CLR_CE_ERR	0x2
+#define ZYNQ_ECC_CTRL_CLR_UE_ERR	0x1
+
+/* ECC correctable/uncorrectable error log register definitions */
+#define ZYNQ_LOG_VALID			0x1
+#define ZYNQ_CE_LOG_BITPOS_MASK		0xFE
+#define ZYNQ_CE_LOG_BITPOS_SHIFT	1
+
+/* ECC correctable/uncorrectable error address register definitions */
+#define ZYNQ_ADDR_COL_MASK		0xFFF
+#define ZYNQ_ADDR_ROW_MASK		0xFFFF000
+#define ZYNQ_ADDR_ROW_SHIFT		12
+#define ZYNQ_ADDR_BANK_MASK		0x70000000
+#define ZYNQ_ADDR_BANK_SHIFT		28
+
+/* ECC statistic register definitions */
+#define ZYNQ_STAT_UECNT_MASK		0xFF
+#define ZYNQ_STAT_CECNT_MASK		0xFF00
+#define ZYNQ_STAT_CECNT_SHIFT		8
+
+/* ECC scrub register definitions */
+#define ZYNQ_SCRUB_MODE_MASK		0x7
+#define ZYNQ_SCRUB_MODE_SECDED		0x4
+
+/**
+ * struct zynq_ecc_error_info - ECC error log information.
+ * @row:	Row number.
+ * @col:	Column number.
+ * @bank:	Bank number.
+ * @bitpos:	Bit position.
+ * @data:	Data causing the error.
+ */
+struct zynq_ecc_error_info {
+	u32 row;
+	u32 col;
+	u32 bank;
+	u32 bitpos;
+	u32 data;
+};
+
+/**
+ * struct zynq_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 zynq_ecc_status {
+	u32 ce_cnt;
+	u32 ue_cnt;
+	struct zynq_ecc_error_info ceinfo;
+	struct zynq_ecc_error_info ueinfo;
+};
+
+/**
+ * struct zynq_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.
+ */
+struct zynq_edac_priv {
+	void __iomem *baseaddr;
+	char message[ZYNQ_EDAC_MSG_SIZE];
+	struct zynq_ecc_status stat;
+};
+
+/**
+ * zynq_get_error_info - Get the current ECC error info.
+ * @priv:	DDR memory controller private instance data.
+ *
+ * Return: one if there is no error, otherwise zero.
+ */
+static int zynq_get_error_info(struct zynq_edac_priv *priv)
+{
+	struct zynq_ecc_status *p;
+	u32 regval, clearval = 0;
+	void __iomem *base;
+
+	base = priv->baseaddr;
+	p = &priv->stat;
+
+	regval = readl(base + ZYNQ_STAT_OFST);
+	if (!regval)
+		return 1;
+
+	p->ce_cnt = (regval & ZYNQ_STAT_CECNT_MASK) >> ZYNQ_STAT_CECNT_SHIFT;
+	p->ue_cnt = regval & ZYNQ_STAT_UECNT_MASK;
+
+	regval = readl(base + ZYNQ_CE_LOG_OFST);
+	if (!(p->ce_cnt && (regval & ZYNQ_LOG_VALID)))
+		goto ue_err;
+
+	p->ceinfo.bitpos = (regval & ZYNQ_CE_LOG_BITPOS_MASK) >> ZYNQ_CE_LOG_BITPOS_SHIFT;
+	regval = readl(base + ZYNQ_CE_ADDR_OFST);
+	p->ceinfo.row = (regval & ZYNQ_ADDR_ROW_MASK) >> ZYNQ_ADDR_ROW_SHIFT;
+	p->ceinfo.col = regval & ZYNQ_ADDR_COL_MASK;
+	p->ceinfo.bank = (regval & ZYNQ_ADDR_BANK_MASK) >> ZYNQ_ADDR_BANK_SHIFT;
+	p->ceinfo.data = readl(base + ZYNQ_CE_DATA_31_0_OFST);
+	edac_dbg(3, "CE bit position: %d data: %d\n", p->ceinfo.bitpos,
+		 p->ceinfo.data);
+	clearval = ZYNQ_ECC_CTRL_CLR_CE_ERR;
+
+ue_err:
+	regval = readl(base + ZYNQ_UE_LOG_OFST);
+	if (!(p->ue_cnt && (regval & ZYNQ_LOG_VALID)))
+		goto out;
+
+	regval = readl(base + ZYNQ_UE_ADDR_OFST);
+	p->ueinfo.row = (regval & ZYNQ_ADDR_ROW_MASK) >> ZYNQ_ADDR_ROW_SHIFT;
+	p->ueinfo.col = regval & ZYNQ_ADDR_COL_MASK;
+	p->ueinfo.bank = (regval & ZYNQ_ADDR_BANK_MASK) >> ZYNQ_ADDR_BANK_SHIFT;
+	p->ueinfo.data = readl(base + ZYNQ_UE_DATA_31_0_OFST);
+	clearval |= ZYNQ_ECC_CTRL_CLR_UE_ERR;
+
+out:
+	writel(clearval, base + ZYNQ_ECC_CTRL_OFST);
+	writel(0x0, base + ZYNQ_ECC_CTRL_OFST);
+
+	return 0;
+}
+
+/**
+ * handle_error - Handle Correctable and Uncorrectable errors.
+ * @mci:	EDAC memory controller instance.
+ * @p:		Zynq ECC status structure.
+ *
+ * Handles ECC correctable and uncorrectable errors.
+ */
+static void zynq_handle_error(struct mem_ctl_info *mci, struct zynq_ecc_status *p)
+{
+	struct zynq_edac_priv *priv = mci->pvt_info;
+	struct zynq_ecc_error_info *pinf;
+
+	if (p->ce_cnt) {
+		pinf = &p->ceinfo;
+
+		snprintf(priv->message, ZYNQ_EDAC_MSG_SIZE,
+			 "Row %d Bank %d Col %d Bit %d Data 0x%08x",
+			 pinf->row, pinf->bank, pinf->col,
+			 pinf->bitpos, pinf->data);
+
+		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+				     p->ce_cnt, 0, 0, 0, 0, 0, -1,
+				     priv->message, "");
+	}
+
+	if (p->ue_cnt) {
+		pinf = &p->ueinfo;
+
+		snprintf(priv->message, ZYNQ_EDAC_MSG_SIZE,
+			 "Row %d Bank %d Col %d",
+			 pinf->row, pinf->bank, pinf->col);
+
+		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+				     p->ue_cnt, 0, 0, 0, 0, 0, -1,
+				     priv->message, "");
+	}
+
+	memset(p, 0, sizeof(*p));
+}
+
+/**
+ * check_errors - Check controller for ECC errors.
+ * @mci:	EDAC memory controller instance.
+ *
+ * Check and post ECC errors. Called by the polling thread.
+ */
+static void zynq_check_errors(struct mem_ctl_info *mci)
+{
+	struct zynq_edac_priv *priv = mci->pvt_info;
+	int status;
+
+	status = zynq_get_error_info(priv);
+	if (status)
+		return;
+
+	zynq_handle_error(mci, &priv->stat);
+}
+
+/**
+ * zynq_get_dtype - Return the controller memory width.
+ * @base:	DDR memory controller base address.
+ *
+ * Get the EDAC device type width appropriate for the current controller
+ * configuration.
+ *
+ * Return: a device type width enumeration.
+ */
+static enum dev_type zynq_get_dtype(const void __iomem *base)
+{
+	enum dev_type dt;
+	u32 width;
+
+	width = readl(base + ZYNQ_CTRL_OFST);
+	width = (width & ZYNQ_CTRL_BW_MASK) >> ZYNQ_CTRL_BW_SHIFT;
+
+	switch (width) {
+	case ZYNQ_DDRCTL_WDTH_16:
+		dt = DEV_X2;
+		break;
+	case ZYNQ_DDRCTL_WDTH_32:
+		dt = DEV_X4;
+		break;
+	default:
+		dt = DEV_UNKNOWN;
+	}
+
+	return dt;
+}
+
+/**
+ * zynq_get_ecc_state - Return the controller ECC enable/disable status.
+ * @base:	DDR memory controller base address.
+ *
+ * Get the ECC enable/disable status of the controller.
+ *
+ * Return: true if enabled, otherwise false.
+ */
+static bool zynq_get_ecc_state(void __iomem *base)
+{
+	enum dev_type dt;
+	u32 ecctype;
+
+	dt = zynq_get_dtype(base);
+	if (dt == DEV_UNKNOWN)
+		return false;
+
+	ecctype = readl(base + ZYNQ_SCRUB_OFST) & ZYNQ_SCRUB_MODE_MASK;
+	if ((ecctype == ZYNQ_SCRUB_MODE_SECDED) && (dt == DEV_X2))
+		return true;
+
+	return false;
+}
+
+/**
+ * zynq_get_memsize - Read the size of the attached memory device.
+ *
+ * Return: the memory size in bytes.
+ */
+static u32 zynq_get_memsize(void)
+{
+	struct sysinfo inf;
+
+	si_meminfo(&inf);
+
+	return inf.totalram * inf.mem_unit;
+}
+
+/**
+ * zynq_get_mtype - Return the controller memory type.
+ * @base:	Zynq ECC status structure.
+ *
+ * Get the EDAC memory type appropriate for the current controller
+ * configuration.
+ *
+ * Return: a memory type enumeration.
+ */
+static enum mem_type zynq_get_mtype(const void __iomem *base)
+{
+	enum mem_type mt;
+	u32 memtype;
+
+	memtype = readl(base + ZYNQ_T_ZQ_OFST);
+
+	if (memtype & ZYNQ_T_ZQ_DDRMODE_MASK)
+		mt = MEM_DDR3;
+	else
+		mt = MEM_DDR2;
+
+	return mt;
+}
+
+/**
+ * zynq_init_csrows - Initialize the csrow data.
+ * @mci:	EDAC memory controller instance.
+ *
+ * Initialize the chip select rows associated with the EDAC memory
+ * controller instance.
+ */
+static void zynq_init_csrows(struct mem_ctl_info *mci)
+{
+	struct zynq_edac_priv *priv = mci->pvt_info;
+	struct csrow_info *csi;
+	struct dimm_info *dimm;
+	u32 size, row;
+	int j;
+
+	for (row = 0; row < mci->nr_csrows; row++) {
+		csi = mci->csrows[row];
+		size = zynq_get_memsize();
+
+		for (j = 0; j < csi->nr_channels; j++) {
+			dimm		= csi->channels[j]->dimm;
+			dimm->edac_mode	= EDAC_SECDED;
+			dimm->mtype	= zynq_get_mtype(priv->baseaddr);
+			dimm->nr_pages	= (size >> PAGE_SHIFT) / csi->nr_channels;
+			dimm->grain	= ZYNQ_EDAC_ERR_GRAIN;
+			dimm->dtype	= zynq_get_dtype(priv->baseaddr);
+		}
+	}
+}
+
+/**
+ * zynq_mc_init - Initialize one driver instance.
+ * @mci:	EDAC memory controller instance.
+ * @pdev:	platform device.
+ *
+ * Perform initialization of the EDAC memory controller instance and
+ * related driver-private data associated with the memory controller the
+ * instance is bound to.
+ */
+static void zynq_mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
+{
+	struct zynq_edac_priv *priv;
+
+	mci->pdev = &pdev->dev;
+	priv = mci->pvt_info;
+	platform_set_drvdata(pdev, mci);
+
+	/* Initialize controller capabilities and configuration */
+	mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR2;
+	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
+	mci->scrub_cap = SCRUB_FLAG_HW_SRC;
+	mci->scrub_mode = SCRUB_NONE;
+
+	mci->edac_cap = EDAC_FLAG_SECDED;
+	mci->ctl_name = "zynq_ddr_controller";
+	mci->dev_name = ZYNQ_EDAC_MOD_STRING;
+	mci->mod_name = ZYNQ_EDAC_MOD_VER;
+
+	edac_op_state = EDAC_OPSTATE_POLL;
+	mci->edac_check = zynq_check_errors;
+
+	mci->ctl_page_to_phys = NULL;
+
+	zynq_init_csrows(mci);
+}
+
+/**
+ * zynq_mc_probe - Check controller and bind driver.
+ * @pdev:	platform device.
+ *
+ * Probe a specific controller instance for binding with the driver.
+ *
+ * Return: 0 if the controller instance was successfully bound to the
+ * driver; otherwise, < 0 on error.
+ */
+static int zynq_mc_probe(struct platform_device *pdev)
+{
+	struct edac_mc_layer layers[2];
+	struct zynq_edac_priv *priv;
+	struct mem_ctl_info *mci;
+	void __iomem *baseaddr;
+	int rc;
+
+	baseaddr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(baseaddr))
+		return PTR_ERR(baseaddr);
+
+	if (!zynq_get_ecc_state(baseaddr)) {
+		edac_printk(KERN_INFO, EDAC_MC, "ECC not enabled\n");
+		return -ENXIO;
+	}
+
+	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+	layers[0].size = ZYNQ_EDAC_NR_CSROWS;
+	layers[0].is_virt_csrow = true;
+	layers[1].type = EDAC_MC_LAYER_CHANNEL;
+	layers[1].size = ZYNQ_EDAC_NR_CHANS;
+	layers[1].is_virt_csrow = false;
+
+	mci = edac_mc_alloc(EDAC_AUTO_MC_NUM, ARRAY_SIZE(layers), layers,
+			    sizeof(struct zynq_edac_priv));
+	if (!mci) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Failed memory allocation for mc instance\n");
+		return -ENOMEM;
+	}
+
+	priv = mci->pvt_info;
+	priv->baseaddr = baseaddr;
+
+	zynq_mc_init(mci, pdev);
+
+	rc = edac_mc_add_mc(mci);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_MC,
+			    "Failed to register with EDAC core\n");
+		goto free_edac_mc;
+	}
+
+	/*
+	 * Start capturing the correctable and uncorrectable errors. A write of
+	 * 0 starts the counters.
+	 */
+	writel(0x0, baseaddr + ZYNQ_ECC_CTRL_OFST);
+
+	return 0;
+
+free_edac_mc:
+	edac_mc_free(mci);
+
+	return rc;
+}
+
+/**
+ * zynq_mc_remove - Unbind driver from controller.
+ * @pdev:	Platform device.
+ *
+ * Return: Unconditionally 0
+ */
+static int zynq_mc_remove(struct platform_device *pdev)
+{
+	struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+
+	edac_mc_del_mc(&pdev->dev);
+	edac_mc_free(mci);
+
+	return 0;
+}
+
+static const struct of_device_id zynq_edac_match[] = {
+	{ .compatible = "xlnx,zynq-ddrc-a05" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, zynq_edac_match);
+
+static struct platform_driver zynq_edac_mc_driver = {
+	.driver = {
+		   .name = "zynq-edac",
+		   .of_match_table = zynq_edac_match,
+		   },
+	.probe = zynq_mc_probe,
+	.remove = zynq_mc_remove,
+};
+module_platform_driver(zynq_edac_mc_driver);
+
+MODULE_AUTHOR("Xilinx Inc");
+MODULE_DESCRIPTION("Zynq DDR ECC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.35.1


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

* [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (16 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 17/20] EDAC/synopsys: " Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-23 23:42   ` kernel test robot
  2022-08-22 19:07 ` [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming Serge Semin
  2022-08-22 19:07 ` [PATCH 20/20] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros Serge Semin
  19 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

The driver now works with the Synopys DW uMCTL2 DDR IP-core only (Xilinx
Zynq A05 DDRc support has been moved to the dedicated driver). Pretty much
all the currently available IP-core revisions have got the same ECC and
main DDR-config CSRs map. Thus there is no point in supporting the no
longer used platform-specific API like the callbacks responsible for
getting the ECC errors info, memory and device types, ECC state. All of
that data can be retrieved in the same way on all the Synopys DW uMCTL2
DDR controller versions. Similarly there is no longer need in the
DDR_ECC_INTR_SUPPORT and DDR_ECC_DATA_POISON_SUPPORT quirk flags since DW
uMCTL2 always supports IRQs and data poisoning. Let's drop that
infrastructure for good then.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 194 +++++++++--------------------------
 1 file changed, 51 insertions(+), 143 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 0cb84f183aed..e171c8d96274 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -31,9 +31,7 @@
 #define SYNPS_EDAC_MOD_VER		"1"
 
 /* DDR ECC Quirks */
-#define DDR_ECC_INTR_SUPPORT		BIT(0)
-#define DDR_ECC_DATA_POISON_SUPPORT	BIT(1)
-#define SYNPS_ZYNQMP_IRQ_REGS		BIT(2)
+#define SYNPS_ZYNQMP_IRQ_REGS		BIT(0)
 
 /* Synopsys DDR memory controller registers that are relevant to ECC */
 
@@ -278,28 +276,20 @@ struct synps_edac_priv {
 };
 
 /**
- * struct synps_platform_data -  synps platform data structure.
- * @get_error_info:	Get EDAC error info.
- * @get_mtype:		Get mtype.
- * @get_dtype:		Get dtype.
- * @get_ecc_state:	Get ECC state.
- * @quirks:		To differentiate IPs.
+ * struct synps_platform_data - Synopsys uMCTL2 DDRC platform data.
+ * @quirks:	IP-core specific quirks.
  */
 struct synps_platform_data {
-	int (*get_error_info)(struct synps_edac_priv *priv);
-	enum mem_type (*get_mtype)(const void __iomem *base);
-	enum dev_type (*get_dtype)(const void __iomem *base);
-	bool (*get_ecc_state)(void __iomem *base);
-	int quirks;
+	u32 quirks;
 };
 
 /**
- * zynqmp_get_error_info - Get the current ECC error info.
+ * synps_get_error_info - Get the current ECC error info.
  * @priv:	DDR memory controller private instance data.
  *
  * Return: one if there is no error otherwise returns zero.
  */
-static int zynqmp_get_error_info(struct synps_edac_priv *priv)
+static int synps_get_error_info(struct synps_edac_priv *priv)
 {
 	struct synps_ecc_status *p;
 	u32 regval, clearval;
@@ -374,17 +364,11 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 
 	if (p->ce_cnt) {
 		pinf = &p->ceinfo;
-		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Row %d Col %d Bank %d Bank Group %d Bit %d Data 0x%08x",
-				 pinf->row, pinf->col, pinf->bank, pinf->bankgrp,
-				 pinf->bitpos, pinf->data);
-		} else {
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Row %d Bank %d Col %d Bit: %d Data: 0x%08x",
-				 pinf->row, pinf->bank, pinf->col,
-				 pinf->bitpos, pinf->data);
-		}
+
+		snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+			 "Row %d Col %d Bank %d Bank Group %d Bit %d Data 0x%08x",
+			 pinf->row, pinf->col, pinf->bank, pinf->bankgrp,
+			 pinf->bitpos, pinf->data);
 
 		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
 				     p->ce_cnt, 0, 0, 0, 0, 0, -1,
@@ -393,15 +377,10 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 
 	if (p->ue_cnt) {
 		pinf = &p->ueinfo;
-		if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Row %d Col %d Bank %d Bank Group %d",
-				 pinf->row, pinf->col, pinf->bank, pinf->bankgrp);
-		} else {
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Row %d Bank %d Col %d",
-				 pinf->row, pinf->bank, pinf->col);
-		}
+
+		snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+			 "Row %d Col %d Bank %d Bank Group %d",
+			 pinf->row, pinf->col, pinf->bank, pinf->bankgrp);
 
 		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
 				     p->ue_cnt, 0, 0, 0, 0, 0, -1,
@@ -460,13 +439,11 @@ static void disable_intr(struct synps_edac_priv *priv)
  */
 static irqreturn_t intr_handler(int irq, void *dev_id)
 {
-	const struct synps_platform_data *p_data;
 	struct mem_ctl_info *mci = dev_id;
 	struct synps_edac_priv *priv;
 	int status, regval;
 
 	priv = mci->pvt_info;
-	p_data = priv->p_data;
 
 	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
@@ -475,7 +452,7 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
 			return IRQ_NONE;
 	}
 
-	status = p_data->get_error_info(priv);
+	status = synps_get_error_info(priv);
 	if (status)
 		return IRQ_NONE;
 
@@ -489,34 +466,7 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
 }
 
 /**
- * check_errors - Check controller for ECC errors.
- * @mci:	EDAC memory controller instance.
- *
- * Check and post ECC errors. Called by the polling thread.
- */
-static void check_errors(struct mem_ctl_info *mci)
-{
-	const struct synps_platform_data *p_data;
-	struct synps_edac_priv *priv;
-	int status;
-
-	priv = mci->pvt_info;
-	p_data = priv->p_data;
-
-	status = p_data->get_error_info(priv);
-	if (status)
-		return;
-
-	priv->ce_cnt += priv->stat.ce_cnt;
-	priv->ue_cnt += priv->stat.ue_cnt;
-	handle_error(mci, &priv->stat);
-
-	edac_dbg(3, "Total error count CE %d UE %d\n",
-		 priv->ce_cnt, priv->ue_cnt);
-}
-
-/**
- * zynqmp_get_dtype - Return the controller memory width.
+ * synps_get_dtype - Return the controller memory width.
  * @base:	DDR memory controller base address.
  *
  * Get the EDAC device type width appropriate for the current controller
@@ -524,7 +474,7 @@ static void check_errors(struct mem_ctl_info *mci)
  *
  * Return: a device type width enumeration.
  */
-static enum dev_type zynqmp_get_dtype(const void __iomem *base)
+static enum dev_type synps_get_dtype(const void __iomem *base)
 {
 	u32 regval;
 
@@ -548,14 +498,14 @@ static enum dev_type zynqmp_get_dtype(const void __iomem *base)
 }
 
 /**
- * zynqmp_get_ecc_state - Return the controller ECC enable/disable status.
+ * synps_get_ecc_state - Return the controller ECC enable/disable status.
  * @base:	DDR memory controller base address.
  *
  * Get the ECC enable/disable status for the controller.
  *
  * Return: a ECC status boolean i.e true/false - enabled/disabled.
  */
-static bool zynqmp_get_ecc_state(void __iomem *base)
+static bool synps_get_ecc_state(void __iomem *base)
 {
 	u32 regval;
 
@@ -579,7 +529,7 @@ static u32 get_memsize(void)
 }
 
 /**
- * zynqmp_get_mtype - Returns controller memory type.
+ * synps_get_mtype - Returns controller memory type.
  * @base:	Synopsys ECC status structure.
  *
  * Get the EDAC memory type appropriate for the current controller
@@ -587,7 +537,7 @@ static u32 get_memsize(void)
  *
  * Return: a memory type enumeration.
  */
-static enum mem_type zynqmp_get_mtype(const void __iomem *base)
+static enum mem_type synps_get_mtype(const void __iomem *base)
 {
 	enum mem_type mt;
 	u32 memtype;
@@ -616,14 +566,11 @@ static enum mem_type zynqmp_get_mtype(const void __iomem *base)
 static void init_csrows(struct mem_ctl_info *mci)
 {
 	struct synps_edac_priv *priv = mci->pvt_info;
-	const struct synps_platform_data *p_data;
 	struct csrow_info *csi;
 	struct dimm_info *dimm;
 	u32 size, row;
 	int j;
 
-	p_data = priv->p_data;
-
 	for (row = 0; row < mci->nr_csrows; row++) {
 		csi = mci->csrows[row];
 		size = get_memsize();
@@ -631,10 +578,10 @@ static void init_csrows(struct mem_ctl_info *mci)
 		for (j = 0; j < csi->nr_channels; j++) {
 			dimm		= csi->channels[j]->dimm;
 			dimm->edac_mode	= EDAC_SECDED;
-			dimm->mtype	= p_data->get_mtype(priv->baseaddr);
+			dimm->mtype	= synps_get_mtype(priv->baseaddr);
 			dimm->nr_pages	= (size >> PAGE_SHIFT) / csi->nr_channels;
 			dimm->grain	= SYNPS_EDAC_ERR_GRAIN;
-			dimm->dtype	= p_data->get_dtype(priv->baseaddr);
+			dimm->dtype	= synps_get_dtype(priv->baseaddr);
 		}
 	}
 }
@@ -667,12 +614,7 @@ static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 	mci->dev_name = SYNPS_EDAC_MOD_STRING;
 	mci->mod_name = SYNPS_EDAC_MOD_VER;
 
-	if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
-		edac_op_state = EDAC_OPSTATE_INT;
-	} else {
-		edac_op_state = EDAC_OPSTATE_POLL;
-		mci->edac_check = check_errors;
-	}
+	edac_op_state = EDAC_OPSTATE_INT;
 
 	mci->ctl_page_to_phys = NULL;
 
@@ -704,47 +646,6 @@ static int setup_irq(struct mem_ctl_info *mci,
 	return 0;
 }
 
-static const struct synps_platform_data zynqmp_edac_def = {
-	.get_error_info	= zynqmp_get_error_info,
-	.get_mtype	= zynqmp_get_mtype,
-	.get_dtype	= zynqmp_get_dtype,
-	.get_ecc_state	= zynqmp_get_ecc_state,
-	.quirks         = (DDR_ECC_INTR_SUPPORT | SYNPS_ZYNQMP_IRQ_REGS
-#ifdef CONFIG_EDAC_DEBUG
-			  | DDR_ECC_DATA_POISON_SUPPORT
-#endif
-			  ),
-};
-
-static const struct synps_platform_data synopsys_edac_def = {
-	.get_error_info	= zynqmp_get_error_info,
-	.get_mtype	= zynqmp_get_mtype,
-	.get_dtype	= zynqmp_get_dtype,
-	.get_ecc_state	= zynqmp_get_ecc_state,
-	.quirks         = (DDR_ECC_INTR_SUPPORT
-#ifdef CONFIG_EDAC_DEBUG
-			  | DDR_ECC_DATA_POISON_SUPPORT
-#endif
-			  ),
-};
-
-
-static const struct of_device_id synps_edac_match[] = {
-	{
-		.compatible = "xlnx,zynqmp-ddrc-2.40a",
-		.data = (void *)&zynqmp_edac_def
-	},
-	{
-		.compatible = "snps,ddrc-3.80a",
-		.data = (void *)&synopsys_edac_def
-	},
-	{
-		/* end of table */
-	}
-};
-
-MODULE_DEVICE_TABLE(of, synps_edac_match);
-
 #ifdef CONFIG_EDAC_DEBUG
 
 /**
@@ -1135,7 +1036,7 @@ static int mc_probe(struct platform_device *pdev)
 	if (!p_data)
 		return -ENODEV;
 
-	if (!p_data->get_ecc_state(baseaddr)) {
+	if (!synps_get_ecc_state(baseaddr)) {
 		edac_printk(KERN_INFO, EDAC_MC, "ECC not enabled\n");
 		return -ENXIO;
 	}
@@ -1162,11 +1063,9 @@ static int mc_probe(struct platform_device *pdev)
 
 	mc_init(mci, pdev);
 
-	if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
-		rc = setup_irq(mci, pdev);
-		if (rc)
-			goto free_edac_mc;
-	}
+	rc = setup_irq(mci, pdev);
+	if (rc)
+		goto free_edac_mc;
 
 	rc = edac_mc_add_mc(mci);
 	if (rc) {
@@ -1176,17 +1075,13 @@ static int mc_probe(struct platform_device *pdev)
 	}
 
 #ifdef CONFIG_EDAC_DEBUG
-	if (priv->p_data->quirks & DDR_ECC_DATA_POISON_SUPPORT) {
-		rc = edac_create_sysfs_attributes(mci);
-		if (rc) {
-			edac_printk(KERN_ERR, EDAC_MC,
-					"Failed to create sysfs entries\n");
-			goto free_edac_mc;
-		}
+	rc = edac_create_sysfs_attributes(mci);
+	if (rc) {
+		edac_printk(KERN_ERR, EDAC_MC, "Failed to create sysfs entries\n");
+		goto free_edac_mc;
 	}
 
-	if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT)
-		setup_address_map(priv);
+	setup_address_map(priv);
 #endif
 
 	return rc;
@@ -1208,12 +1103,10 @@ static int mc_remove(struct platform_device *pdev)
 	struct mem_ctl_info *mci = platform_get_drvdata(pdev);
 	struct synps_edac_priv *priv = mci->pvt_info;
 
-	if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT)
-		disable_intr(priv);
+	disable_intr(priv);
 
 #ifdef CONFIG_EDAC_DEBUG
-	if (priv->p_data->quirks & DDR_ECC_DATA_POISON_SUPPORT)
-		edac_remove_sysfs_attributes(mci);
+	edac_remove_sysfs_attributes(mci);
 #endif
 
 	edac_mc_del_mc(&pdev->dev);
@@ -1222,6 +1115,21 @@ static int mc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct synps_platform_data zynqmp_edac_def = {
+	.quirks = SYNPS_ZYNQMP_IRQ_REGS,
+};
+
+static const struct synps_platform_data synopsys_edac_def = {
+	.quirks = 0,
+};
+
+static const struct of_device_id synps_edac_match[] = {
+	{ .compatible = "xlnx,zynqmp-ddrc-2.40a", .data = &zynqmp_edac_def },
+	{ .compatible = "snps,ddrc-3.80a", .data = &synopsys_edac_def },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, synps_edac_match);
+
 static struct platform_driver synps_edac_mc_driver = {
 	.driver = {
 		   .name = "synopsys-edac",
-- 
2.35.1


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

* [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (17 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  2022-08-24  1:15   ` kernel test robot
  2022-08-22 19:07 ` [PATCH 20/20] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros Serge Semin
  19 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Currently the driver naming schema is kind of random. There are structures
and methods with the "synps" prefix, there are structures and methods
with no driver-specific prefix, there are methods with the "edac" prefix,
there are structure instances with "zynqmp" and "synopsys" prefixes, there
are macros with "SYNPS", "ECC" and "DDR" prefixes. Moreover some time ago
some of function names were shortened out by completely removing the
vendor-specific prefixes thus leaving the driver with no strict entities
naming convention (see commit bb894bc46ed0 ("EDAC, synopsys: Shorten
static function names")).

All of that makes the code much harder to read for no much reason (except
shorter names utilization) since there is no easy way to distinguish now
the local, EDAC core and global name spaces right from the code context.
Similarly the kernel code index services (like elixir) gets to find the
different functions with the same name, which harden the kernel hacking.

Let's fix all of that by unifying the driver local entity names like
functions, structures and non-CSR-related macros especially seeing the
same approach has been used in the most of the EDAC LLDD. We suggest to
use the "snps" prefix here as the shortest version of the controller
vendor name. While at it add a more detailed controller name (DW uMCTL2
DDRC) to the driver comments and string literals where it's appropriate.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Note "dw" prefix would be even shorter alternative. But we decided to
stick with "snps" since "synopsys" has already been used in the module
name.
---
 drivers/edac/synopsys_edac.c | 238 +++++++++++++++++------------------
 1 file changed, 119 insertions(+), 119 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index e171c8d96274..9ac0c8c4e3b8 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Synopsys DDR ECC Driver
+ * Synopsys DW uMCTL2 DDR ECC Driver
  * This driver is based on ppc4xx_edac.c drivers
  *
  * Copyright (C) 2012 - 2014 Xilinx, Inc.
@@ -17,23 +17,23 @@
 #include "edac_module.h"
 
 /* Number of cs_rows needed per memory controller */
-#define SYNPS_EDAC_NR_CSROWS		1
+#define SNPS_EDAC_NR_CSROWS		1
 
 /* Number of channels per memory controller */
-#define SYNPS_EDAC_NR_CHANS		1
+#define SNPS_EDAC_NR_CHANS		1
 
 /* Granularity of reported error in bytes */
-#define SYNPS_EDAC_ERR_GRAIN		1
+#define SNPS_EDAC_ERR_GRAIN		1
 
-#define SYNPS_EDAC_MSG_SIZE		256
+#define SNPS_EDAC_MSG_SIZE		256
 
-#define SYNPS_EDAC_MOD_STRING		"synps_edac"
-#define SYNPS_EDAC_MOD_VER		"1"
+#define SNPS_EDAC_MOD_STRING		"snps_edac"
+#define SNPS_EDAC_MOD_VER		"1"
 
 /* DDR ECC Quirks */
-#define SYNPS_ZYNQMP_IRQ_REGS		BIT(0)
+#define SNPS_ZYNQMP_IRQ_REGS		BIT(0)
 
-/* Synopsys DDR memory controller registers that are relevant to ECC */
+/* Synopsys uMCTL2 DDR controller registers that are relevant to ECC */
 
 /* DDRC master0 Register */
 #define DDR_MSTR_OFST			0x0
@@ -214,7 +214,7 @@
 #define RANK_B0_BASE			6
 
 /**
- * struct ecc_error_info - ECC error log information.
+ * struct snps_ecc_error_info - ECC error log information.
  * @row:	Row number.
  * @col:	Column number.
  * @bank:	Bank number.
@@ -222,7 +222,7 @@
  * @bitpos:	Bit position.
  * @data:	Data causing the error.
  */
-struct ecc_error_info {
+struct snps_ecc_error_info {
 	u32 row;
 	u32 col;
 	u32 bank;
@@ -232,21 +232,21 @@ struct ecc_error_info {
 };
 
 /**
- * struct synps_ecc_status - ECC status information to report.
+ * struct snps_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 synps_ecc_status {
+struct snps_ecc_status {
 	u32 ce_cnt;
 	u32 ue_cnt;
-	struct ecc_error_info ceinfo;
-	struct ecc_error_info ueinfo;
+	struct snps_ecc_error_info ceinfo;
+	struct snps_ecc_error_info ueinfo;
 };
 
 /**
- * struct synps_edac_priv - DDR memory controller private instance data.
+ * struct snps_edac_priv - DDR memory controller private data.
  * @baseaddr:		Base address of the DDR controller.
  * @lock:		Concurrent CSRs access lock.
  * @message:		Buffer for framing the event specific info.
@@ -259,12 +259,12 @@ struct synps_ecc_status {
  * @bankgrp_shift:	Bit shifts for bank group bit.
  * @rank_shift:		Bit shifts for rank bit.
  */
-struct synps_edac_priv {
+struct snps_edac_priv {
 	void __iomem *baseaddr;
 	spinlock_t lock;
-	char message[SYNPS_EDAC_MSG_SIZE];
-	struct synps_ecc_status stat;
-	const struct synps_platform_data *p_data;
+	char message[SNPS_EDAC_MSG_SIZE];
+	struct snps_ecc_status stat;
+	const struct snps_platform_data *p_data;
 #ifdef CONFIG_EDAC_DEBUG
 	ulong poison_addr;
 	u32 row_shift[18];
@@ -276,22 +276,22 @@ struct synps_edac_priv {
 };
 
 /**
- * struct synps_platform_data - Synopsys uMCTL2 DDRC platform data.
+ * struct snps_platform_data - Synopsys uMCTL2 DDRC platform data.
  * @quirks:	IP-core specific quirks.
  */
-struct synps_platform_data {
+struct snps_platform_data {
 	u32 quirks;
 };
 
 /**
- * synps_get_error_info - Get the current ECC error info.
+ * snps_get_error_info - Get the current ECC error info.
  * @priv:	DDR memory controller private instance data.
  *
  * Return: one if there is no error otherwise returns zero.
  */
-static int synps_get_error_info(struct synps_edac_priv *priv)
+static int snps_get_error_info(struct snps_edac_priv *priv)
 {
-	struct synps_ecc_status *p;
+	struct snps_ecc_status *p;
 	u32 regval, clearval;
 	unsigned long flags;
 	void __iomem *base;
@@ -351,21 +351,21 @@ static int synps_get_error_info(struct synps_edac_priv *priv)
 }
 
 /**
- * handle_error - Handle Correctable and Uncorrectable errors.
+ * snps_handle_error - Handle Correctable and Uncorrectable errors.
  * @mci:	EDAC memory controller instance.
  * @p:		Synopsys ECC status structure.
  *
  * Handles ECC correctable and uncorrectable errors.
  */
-static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
+static void snps_handle_error(struct mem_ctl_info *mci, struct snps_ecc_status *p)
 {
-	struct synps_edac_priv *priv = mci->pvt_info;
-	struct ecc_error_info *pinf;
+	struct snps_edac_priv *priv = mci->pvt_info;
+	struct snps_ecc_error_info *pinf;
 
 	if (p->ce_cnt) {
 		pinf = &p->ceinfo;
 
-		snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+		snprintf(priv->message, SNPS_EDAC_MSG_SIZE,
 			 "Row %d Col %d Bank %d Bank Group %d Bit %d Data 0x%08x",
 			 pinf->row, pinf->col, pinf->bank, pinf->bankgrp,
 			 pinf->bitpos, pinf->data);
@@ -378,7 +378,7 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 	if (p->ue_cnt) {
 		pinf = &p->ueinfo;
 
-		snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+		snprintf(priv->message, SNPS_EDAC_MSG_SIZE,
 			 "Row %d Col %d Bank %d Bank Group %d",
 			 pinf->row, pinf->col, pinf->bank, pinf->bankgrp);
 
@@ -390,7 +390,7 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 	memset(p, 0, sizeof(*p));
 }
 
-static void enable_intr(struct synps_edac_priv *priv)
+static void snps_enable_irq(struct snps_edac_priv *priv)
 {
 	unsigned long flags;
 
@@ -411,7 +411,7 @@ static void enable_intr(struct synps_edac_priv *priv)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
-static void disable_intr(struct synps_edac_priv *priv)
+static void snps_disable_irq(struct snps_edac_priv *priv)
 {
 	unsigned long flags;
 
@@ -431,42 +431,42 @@ static void disable_intr(struct synps_edac_priv *priv)
 }
 
 /**
- * intr_handler - Interrupt Handler for ECC interrupts.
+ * snps_irq_handler - Interrupt Handler for ECC interrupts.
  * @irq:        IRQ number.
  * @dev_id:     Device ID.
  *
  * Return: IRQ_NONE, if interrupt not set or IRQ_HANDLED otherwise.
  */
-static irqreturn_t intr_handler(int irq, void *dev_id)
+static irqreturn_t snps_irq_handler(int irq, void *dev_id)
 {
 	struct mem_ctl_info *mci = dev_id;
-	struct synps_edac_priv *priv;
+	struct snps_edac_priv *priv;
 	int status, regval;
 
 	priv = mci->pvt_info;
 
-	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
+	if (priv->p_data->quirks & SNPS_ZYNQMP_IRQ_REGS) {
 		regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
 		regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
 		if (!(regval & ECC_CE_UE_INTR_MASK))
 			return IRQ_NONE;
 	}
 
-	status = synps_get_error_info(priv);
+	status = snps_get_error_info(priv);
 	if (status)
 		return IRQ_NONE;
 
-	handle_error(mci, &priv->stat);
+	snps_handle_error(mci, &priv->stat);
 
 
-	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS)
+	if (priv->p_data->quirks & SNPS_ZYNQMP_IRQ_REGS)
 		writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
 
 	return IRQ_HANDLED;
 }
 
 /**
- * synps_get_dtype - Return the controller memory width.
+ * snps_get_dtype - Return the controller memory width.
  * @base:	DDR memory controller base address.
  *
  * Get the EDAC device type width appropriate for the current controller
@@ -474,7 +474,7 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
  *
  * Return: a device type width enumeration.
  */
-static enum dev_type synps_get_dtype(const void __iomem *base)
+static enum dev_type snps_get_dtype(const void __iomem *base)
 {
 	u32 regval;
 
@@ -498,14 +498,14 @@ static enum dev_type synps_get_dtype(const void __iomem *base)
 }
 
 /**
- * synps_get_ecc_state - Return the controller ECC enable/disable status.
+ * snps_get_ecc_state - Return the controller ECC enable/disable status.
  * @base:	DDR memory controller base address.
  *
  * Get the ECC enable/disable status for the controller.
  *
  * Return: a ECC status boolean i.e true/false - enabled/disabled.
  */
-static bool synps_get_ecc_state(void __iomem *base)
+static bool snps_get_ecc_state(void __iomem *base)
 {
 	u32 regval;
 
@@ -515,11 +515,11 @@ static bool synps_get_ecc_state(void __iomem *base)
 }
 
 /**
- * get_memsize - Read the size of the attached memory device.
+ * snps_get_memsize - Read the size of the attached memory device.
  *
  * Return: the memory size in bytes.
  */
-static u32 get_memsize(void)
+static u32 snps_get_memsize(void)
 {
 	struct sysinfo inf;
 
@@ -529,7 +529,7 @@ static u32 get_memsize(void)
 }
 
 /**
- * synps_get_mtype - Returns controller memory type.
+ * snps_get_mtype - Returns controller memory type.
  * @base:	Synopsys ECC status structure.
  *
  * Get the EDAC memory type appropriate for the current controller
@@ -537,7 +537,7 @@ static u32 get_memsize(void)
  *
  * Return: a memory type enumeration.
  */
-static enum mem_type synps_get_mtype(const void __iomem *base)
+static enum mem_type snps_get_mtype(const void __iomem *base)
 {
 	enum mem_type mt;
 	u32 memtype;
@@ -557,15 +557,15 @@ static enum mem_type synps_get_mtype(const void __iomem *base)
 }
 
 /**
- * init_csrows - Initialize the csrow data.
+ * snps_init_csrows - Initialize the csrow data.
  * @mci:	EDAC memory controller instance.
  *
  * Initialize the chip select rows associated with the EDAC memory
  * controller instance.
  */
-static void init_csrows(struct mem_ctl_info *mci)
+static void snps_init_csrows(struct mem_ctl_info *mci)
 {
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 	struct csrow_info *csi;
 	struct dimm_info *dimm;
 	u32 size, row;
@@ -573,21 +573,21 @@ static void init_csrows(struct mem_ctl_info *mci)
 
 	for (row = 0; row < mci->nr_csrows; row++) {
 		csi = mci->csrows[row];
-		size = get_memsize();
+		size = snps_get_memsize();
 
 		for (j = 0; j < csi->nr_channels; j++) {
 			dimm		= csi->channels[j]->dimm;
 			dimm->edac_mode	= EDAC_SECDED;
-			dimm->mtype	= synps_get_mtype(priv->baseaddr);
+			dimm->mtype	= snps_get_mtype(priv->baseaddr);
 			dimm->nr_pages	= (size >> PAGE_SHIFT) / csi->nr_channels;
-			dimm->grain	= SYNPS_EDAC_ERR_GRAIN;
-			dimm->dtype	= synps_get_dtype(priv->baseaddr);
+			dimm->grain	= SNPS_EDAC_ERR_GRAIN;
+			dimm->dtype	= snps_get_dtype(priv->baseaddr);
 		}
 	}
 }
 
 /**
- * mc_init - Initialize one driver instance.
+ * snps_mc_init - Initialize one driver instance.
  * @mci:	EDAC memory controller instance.
  * @pdev:	platform device.
  *
@@ -595,9 +595,9 @@ static void init_csrows(struct mem_ctl_info *mci)
  * related driver-private data associated with the memory controller the
  * instance is bound to.
  */
-static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
+static void snps_mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 {
-	struct synps_edac_priv *priv;
+	struct snps_edac_priv *priv;
 
 	mci->pdev = &pdev->dev;
 	priv = mci->pvt_info;
@@ -610,21 +610,22 @@ static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 	mci->scrub_mode = SCRUB_NONE;
 
 	mci->edac_cap = EDAC_FLAG_SECDED;
-	mci->ctl_name = "synps_ddr_controller";
-	mci->dev_name = SYNPS_EDAC_MOD_STRING;
-	mci->mod_name = SYNPS_EDAC_MOD_VER;
+	mci->ctl_name = "snps_umctl2_ddrc";
+	mci->dev_name = SNPS_EDAC_MOD_STRING;
+	mci->mod_name = SNPS_EDAC_MOD_VER;
 
 	edac_op_state = EDAC_OPSTATE_INT;
 
 	mci->ctl_page_to_phys = NULL;
 
-	init_csrows(mci);
+	snps_init_csrows(mci);
 }
 
-static int setup_irq(struct mem_ctl_info *mci,
-		     struct platform_device *pdev)
+
+
+static int snps_setup_irq(struct mem_ctl_info *mci, struct platform_device *pdev)
 {
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 	int ret, irq;
 
 	irq = platform_get_irq(pdev, 0);
@@ -634,14 +635,14 @@ static int setup_irq(struct mem_ctl_info *mci,
 		return irq;
 	}
 
-	ret = devm_request_irq(&pdev->dev, irq, intr_handler,
+	ret = devm_request_irq(&pdev->dev, irq, snps_irq_handler,
 			       0, dev_name(&pdev->dev), mci);
 	if (ret < 0) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed to request IRQ\n");
 		return ret;
 	}
 
-	enable_intr(priv);
+	snps_enable_irq(priv);
 
 	return 0;
 }
@@ -649,13 +650,13 @@ static int setup_irq(struct mem_ctl_info *mci,
 #ifdef CONFIG_EDAC_DEBUG
 
 /**
- * ddr_poison_setup -	Update poison registers.
+ * snps_data_poison_setup - Update poison registers.
  * @priv:		DDR memory controller private instance data.
  *
  * Update poison registers as per DDR mapping.
  * Return: none.
  */
-static void ddr_poison_setup(struct synps_edac_priv *priv)
+static void snps_data_poison_setup(struct snps_edac_priv *priv)
 {
 	int col = 0, row = 0, bank = 0, bankgrp = 0, rank = 0, regval;
 	int index;
@@ -713,7 +714,7 @@ static ssize_t inject_data_error_show(struct device *dev,
 				      char *data)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 
 	return sprintf(data, "Poison0 Addr: 0x%08x\n\rPoison1 Addr: 0x%08x\n\r"
 			"Error injection Address: 0x%lx\n\r",
@@ -727,12 +728,12 @@ static ssize_t inject_data_error_store(struct device *dev,
 				       const char *data, size_t count)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 
 	if (kstrtoul(data, 0, &priv->poison_addr))
 		return -EINVAL;
 
-	ddr_poison_setup(priv);
+	snps_data_poison_setup(priv);
 
 	return count;
 }
@@ -742,7 +743,7 @@ static ssize_t inject_data_poison_show(struct device *dev,
 				       char *data)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 
 	return sprintf(data, "Data Poisoning: %s\n\r",
 			(((readl(priv->baseaddr + ECC_CFG1_OFST)) & 0x3) == 0x3)
@@ -754,7 +755,7 @@ static ssize_t inject_data_poison_store(struct device *dev,
 					const char *data, size_t count)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 
 	writel(0, priv->baseaddr + DDR_SWCTL);
 	if (strncmp(data, "CE", 2) == 0)
@@ -769,7 +770,7 @@ static ssize_t inject_data_poison_store(struct device *dev,
 static DEVICE_ATTR_RW(inject_data_error);
 static DEVICE_ATTR_RW(inject_data_poison);
 
-static int edac_create_sysfs_attributes(struct mem_ctl_info *mci)
+static int snps_create_sysfs_attributes(struct mem_ctl_info *mci)
 {
 	int rc;
 
@@ -782,13 +783,13 @@ static int edac_create_sysfs_attributes(struct mem_ctl_info *mci)
 	return 0;
 }
 
-static void edac_remove_sysfs_attributes(struct mem_ctl_info *mci)
+static void snps_remove_sysfs_attributes(struct mem_ctl_info *mci)
 {
 	device_remove_file(&mci->dev, &dev_attr_inject_data_error);
 	device_remove_file(&mci->dev, &dev_attr_inject_data_poison);
 }
 
-static void setup_row_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+static void snps_setup_row_address_map(struct snps_edac_priv *priv, u32 *addrmap)
 {
 	u32 addrmap_row_b2_10;
 	int index;
@@ -847,7 +848,7 @@ static void setup_row_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 				ROW_MAX_VAL_MASK) + ROW_B17_BASE);
 }
 
-static void setup_column_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+static void snps_setup_column_address_map(struct snps_edac_priv *priv, u32 *addrmap)
 {
 	u32 width, memtype;
 	int index;
@@ -949,7 +950,7 @@ static void setup_column_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 
 }
 
-static void setup_bank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+static void snps_setup_bank_address_map(struct snps_edac_priv *priv, u32 *addrmap)
 {
 	priv->bank_shift[0] = (addrmap[1] & BANK_MAX_VAL_MASK) + BANK_B0_BASE;
 	priv->bank_shift[1] = ((addrmap[1] >> 8) &
@@ -961,7 +962,7 @@ static void setup_bank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 
 }
 
-static void setup_bg_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+static void snps_setup_bg_address_map(struct snps_edac_priv *priv, u32 *addrmap)
 {
 	priv->bankgrp_shift[0] = (addrmap[8] &
 				BANKGRP_MAX_VAL_MASK) + BANKGRP_B0_BASE;
@@ -971,7 +972,7 @@ static void setup_bg_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 
 }
 
-static void setup_rank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+static void snps_setup_rank_address_map(struct snps_edac_priv *priv, u32 *addrmap)
 {
 	priv->rank_shift[0] = ((addrmap[0] & RANK_MAX_VAL_MASK) ==
 				RANK_MAX_VAL_MASK) ? 0 : ((addrmap[0] &
@@ -979,14 +980,14 @@ static void setup_rank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
 }
 
 /**
- * setup_address_map -	Set Address Map by querying ADDRMAP registers.
+ * snps_setup_address_map -	Set Address Map by querying ADDRMAP registers.
  * @priv:		DDR memory controller private instance data.
  *
  * Set Address Map by querying ADDRMAP registers.
  *
  * Return: none.
  */
-static void setup_address_map(struct synps_edac_priv *priv)
+static void snps_setup_address_map(struct snps_edac_priv *priv)
 {
 	u32 addrmap[12];
 	int index;
@@ -998,20 +999,20 @@ static void setup_address_map(struct synps_edac_priv *priv)
 		addrmap[index] = readl(priv->baseaddr + addrmap_offset);
 	}
 
-	setup_row_address_map(priv, addrmap);
+	snps_setup_row_address_map(priv, addrmap);
 
-	setup_column_address_map(priv, addrmap);
+	snps_setup_column_address_map(priv, addrmap);
 
-	setup_bank_address_map(priv, addrmap);
+	snps_setup_bank_address_map(priv, addrmap);
 
-	setup_bg_address_map(priv, addrmap);
+	snps_setup_bg_address_map(priv, addrmap);
 
-	setup_rank_address_map(priv, addrmap);
+	snps_setup_rank_address_map(priv, addrmap);
 }
 #endif /* CONFIG_EDAC_DEBUG */
 
 /**
- * mc_probe - Check controller and bind driver.
+ * snps_mc_probe - Check controller and bind driver.
  * @pdev:	platform device.
  *
  * Probe a specific controller instance for binding with the driver.
@@ -1019,11 +1020,11 @@ static void setup_address_map(struct synps_edac_priv *priv)
  * Return: 0 if the controller instance was successfully bound to the
  * driver; otherwise, < 0 on error.
  */
-static int mc_probe(struct platform_device *pdev)
+static int snps_mc_probe(struct platform_device *pdev)
 {
-	const struct synps_platform_data *p_data;
+	const struct snps_platform_data *p_data;
 	struct edac_mc_layer layers[2];
-	struct synps_edac_priv *priv;
+	struct snps_edac_priv *priv;
 	struct mem_ctl_info *mci;
 	void __iomem *baseaddr;
 	int rc;
@@ -1036,20 +1037,20 @@ static int mc_probe(struct platform_device *pdev)
 	if (!p_data)
 		return -ENODEV;
 
-	if (!synps_get_ecc_state(baseaddr)) {
+	if (!snps_get_ecc_state(baseaddr)) {
 		edac_printk(KERN_INFO, EDAC_MC, "ECC not enabled\n");
 		return -ENXIO;
 	}
 
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
-	layers[0].size = SYNPS_EDAC_NR_CSROWS;
+	layers[0].size = SNPS_EDAC_NR_CSROWS;
 	layers[0].is_virt_csrow = true;
 	layers[1].type = EDAC_MC_LAYER_CHANNEL;
-	layers[1].size = SYNPS_EDAC_NR_CHANS;
+	layers[1].size = SNPS_EDAC_NR_CHANS;
 	layers[1].is_virt_csrow = false;
 
 	mci = edac_mc_alloc(EDAC_AUTO_MC_NUM, ARRAY_SIZE(layers), layers,
-			    sizeof(struct synps_edac_priv));
+			    sizeof(struct snps_edac_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC,
 			    "Failed memory allocation for mc instance\n");
@@ -1061,9 +1062,9 @@ static int mc_probe(struct platform_device *pdev)
 	priv->p_data = p_data;
 	spin_lock_init(&priv->lock);
 
-	mc_init(mci, pdev);
+	snps_mc_init(mci, pdev);
 
-	rc = setup_irq(mci, pdev);
+	rc = snps_setup_irq(mci, pdev);
 	if (rc)
 		goto free_edac_mc;
 
@@ -1075,13 +1076,13 @@ static int mc_probe(struct platform_device *pdev)
 	}
 
 #ifdef CONFIG_EDAC_DEBUG
-	rc = edac_create_sysfs_attributes(mci);
+	rc = snps_create_sysfs_attributes(mci);
 	if (rc) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed to create sysfs entries\n");
 		goto free_edac_mc;
 	}
 
-	setup_address_map(priv);
+	snps_setup_address_map(priv);
 #endif
 
 	return rc;
@@ -1093,20 +1094,20 @@ static int mc_probe(struct platform_device *pdev)
 }
 
 /**
- * mc_remove - Unbind driver from controller.
+ * snps_mc_remove - Unbind driver from device.
  * @pdev:	Platform device.
  *
  * Return: Unconditionally 0
  */
-static int mc_remove(struct platform_device *pdev)
+static int snps_mc_remove(struct platform_device *pdev)
 {
 	struct mem_ctl_info *mci = platform_get_drvdata(pdev);
-	struct synps_edac_priv *priv = mci->pvt_info;
+	struct snps_edac_priv *priv = mci->pvt_info;
 
-	disable_intr(priv);
+	snps_disable_irq(priv);
 
 #ifdef CONFIG_EDAC_DEBUG
-	edac_remove_sysfs_attributes(mci);
+	snps_remove_sysfs_attributes(mci);
 #endif
 
 	edac_mc_del_mc(&pdev->dev);
@@ -1115,32 +1116,31 @@ static int mc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct synps_platform_data zynqmp_edac_def = {
-	.quirks = SYNPS_ZYNQMP_IRQ_REGS,
+static const struct snps_platform_data zynqmp_edac_def = {
+	.quirks = SNPS_ZYNQMP_IRQ_REGS,
 };
 
-static const struct synps_platform_data synopsys_edac_def = {
+static const struct snps_platform_data snps_edac_def = {
 	.quirks = 0,
 };
 
-static const struct of_device_id synps_edac_match[] = {
+static const struct of_device_id snps_edac_match[] = {
 	{ .compatible = "xlnx,zynqmp-ddrc-2.40a", .data = &zynqmp_edac_def },
-	{ .compatible = "snps,ddrc-3.80a", .data = &synopsys_edac_def },
+	{ .compatible = "snps,ddrc-3.80a", .data = &snps_edac_def },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, synps_edac_match);
+MODULE_DEVICE_TABLE(of, snps_edac_match);
 
-static struct platform_driver synps_edac_mc_driver = {
+static struct platform_driver snps_edac_mc_driver = {
 	.driver = {
-		   .name = "synopsys-edac",
-		   .of_match_table = synps_edac_match,
+		   .name = "snps-edac",
+		   .of_match_table = snps_edac_match,
 		   },
-	.probe = mc_probe,
-	.remove = mc_remove,
+	.probe = snps_mc_probe,
+	.remove = snps_mc_remove,
 };
-
-module_platform_driver(synps_edac_mc_driver);
+module_platform_driver(snps_edac_mc_driver);
 
 MODULE_AUTHOR("Xilinx Inc");
-MODULE_DESCRIPTION("Synopsys DDR ECC driver");
+MODULE_DESCRIPTION("Synopsys uMCTL2 DDR ECC driver");
 MODULE_LICENSE("GPL v2");
-- 
2.35.1


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

* [PATCH 20/20] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros
  2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
                   ` (18 preceding siblings ...)
  2022-08-22 19:07 ` [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming Serge Semin
@ 2022-08-22 19:07 ` Serge Semin
  19 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-22 19:07 UTC (permalink / raw)
  To: Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Michail Ivanov,
	Pavel Parkhomenko, Punnaiah Choudary Kalluri, Manish Narani,
	Dinh Nguyen, Rob Herring, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Instead of using the very handy helpers denoted in the subject the driver
has been created with the open-coded {mask,shift} statements. It makes the
code bulky, prone to mistakes and much harder to read. Seeing there are
many places in the driver implementing the CSR fields get/set pattern
let's use the FIELD_GET()/FIELD_PREP() macros introduced in the kernel
specifically for that case. In addition we suggest to use the BIT() and
GENMASK() macros to generate the CSR flags/masks. While at it unify the
row, column, rank, bank and bank group macros names to be looking in the
same way as the fields of the snps_ecc_error_info structure.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 124 +++++++++++++++++------------------
 1 file changed, 61 insertions(+), 63 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 9ac0c8c4e3b8..bcef9672f700 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -6,6 +6,8 @@
  * Copyright (C) 2012 - 2014 Xilinx, Inc.
  */
 
+#include <linux/bits.h>
+#include <linux/bitfield.h>
 #include <linux/edac.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -90,22 +92,19 @@
 #define DDR_MSTR_DEV_X8			0x1
 #define DDR_MSTR_DEV_X16		0x2
 #define DDR_MSTR_DEV_X32		0x3
-#define DDR_MSTR_BUSWIDTH_MASK		0x3000
-#define DDR_MSTR_BUSWIDTH_SHIFT		12
+#define DDR_MSTR_BUSWIDTH_MASK		GENMASK(13, 12)
 #define DDRCTL_EWDTH_16			2
 #define DDRCTL_EWDTH_32			1
 #define DDRCTL_EWDTH_64			0
 
 /* ECC CFG0 register definitions */
-#define ECC_CFG0_MODE_MASK		0x7
+#define ECC_CFG0_MODE_MASK		GENMASK(2, 0)
 #define ECC_CFG0_MODE_SECDED		0x4
 
 /* ECC status register definitions */
-#define ECC_STAT_UECNT_MASK		0xF0000
-#define ECC_STAT_UECNT_SHIFT		16
-#define ECC_STAT_CECNT_MASK		0xF00
-#define ECC_STAT_CECNT_SHIFT		8
-#define ECC_STAT_BITNUM_MASK		0x7F
+#define ECC_STAT_UE_MASK		GENMASK(23, 16)
+#define ECC_STAT_CE_MASK		GENMASK(15, 8)
+#define ECC_STAT_BITNUM_MASK		GENMASK(6, 0)
 
 /* ECC control/clear register definitions */
 #define ECC_CTRL_CLR_CE_ERR		BIT(0)
@@ -116,49 +115,41 @@
 #define ECC_CTRL_EN_UE_IRQ		BIT(9)
 
 /* ECC error count register definitions */
-#define ECC_ERRCNT_UECNT_MASK		0xFFFF0000
-#define ECC_ERRCNT_UECNT_SHIFT		16
-#define ECC_ERRCNT_CECNT_MASK		0xFFFF
+#define ECC_ERRCNT_UECNT_MASK		GENMASK(31, 16)
+#define ECC_ERRCNT_CECNT_MASK		GENMASK(15, 0)
 
 /* DDR QOS Interrupt register definitions */
 #define DDR_QOS_IRQ_STAT_OFST		0x20200
-#define DDR_QOSUE_MASK			0x4
-#define	DDR_QOSCE_MASK			0x2
-#define	ECC_CE_UE_INTR_MASK		0x6
+#define DDR_QOSUE_MASK			BIT(2)
+#define DDR_QOSCE_MASK			BIT(1)
+#define ECC_CE_UE_INTR_MASK		(DDR_QOSUE_MASK | DDR_QOSCE_MASK)
 #define DDR_QOS_IRQ_EN_OFST		0x20208
 #define DDR_QOS_IRQ_DB_OFST		0x2020C
 
 /* ECC Corrected Error Register Mask and Shifts*/
-#define ECC_CEADDR0_RW_MASK		0x3FFFF
-#define ECC_CEADDR0_RNK_MASK		BIT(24)
-#define ECC_CEADDR1_BNKGRP_MASK		0x3000000
-#define ECC_CEADDR1_BNKNR_MASK		0x70000
-#define ECC_CEADDR1_COL_MASK		0xFFF
-#define ECC_CEADDR1_BNKGRP_SHIFT	24
-#define ECC_CEADDR1_BNKNR_SHIFT		16
+#define ECC_CEADDR0_RANK_MASK		GENMASK(27, 24)
+#define ECC_CEADDR0_ROW_MASK		GENMASK(17, 0)
+#define ECC_CEADDR1_BANKGRP_MASK	GENMASK(25, 24)
+#define ECC_CEADDR1_BANK_MASK		GENMASK(23, 16)
+#define ECC_CEADDR1_COL_MASK		GENMASK(11, 0)
 
 /* ECC Poison register shifts */
-#define ECC_POISON0_RANK_SHIFT		24
-#define ECC_POISON0_RANK_MASK		BIT(24)
-#define ECC_POISON0_COLUMN_SHIFT	0
-#define ECC_POISON0_COLUMN_MASK		0xFFF
-#define ECC_POISON1_BG_SHIFT		28
-#define ECC_POISON1_BG_MASK		0x30000000
-#define ECC_POISON1_BANKNR_SHIFT	24
-#define ECC_POISON1_BANKNR_MASK		0x7000000
-#define ECC_POISON1_ROW_SHIFT		0
-#define ECC_POISON1_ROW_MASK		0x3FFFF
+#define ECC_POISON0_RANK_MASK		GENMASK(27, 24)
+#define ECC_POISON0_COL_MASK		GENMASK(11, 0)
+#define ECC_POISON1_BANKGRP_MASK	GENMASK(29, 28)
+#define ECC_POISON1_BANK_MASK		GENMASK(26, 24)
+#define ECC_POISON1_ROW_MASK		GENMASK(17, 0)
 
 /* DDR Memory type defines */
-#define MEM_TYPE_DDR3			0x1
-#define MEM_TYPE_LPDDR3			0x8
-#define MEM_TYPE_DDR2			0x4
-#define MEM_TYPE_DDR4			0x10
-#define MEM_TYPE_LPDDR4			0x20
+#define MEM_TYPE_DDR3			BIT(0)
+#define MEM_TYPE_DDR2			BIT(2)
+#define MEM_TYPE_LPDDR3			BIT(3)
+#define MEM_TYPE_DDR4			BIT(4)
+#define MEM_TYPE_LPDDR4			BIT(5)
 
 /* DDRC ECC CE & UE poison mask */
-#define ECC_CEPOISON_MASK		0x3
-#define ECC_UEPOISON_MASK		0x1
+#define ECC_CEPOISON_MASK		GENMASK(1, 0)
+#define ECC_UEPOISON_MASK		BIT(0)
 
 /* DDRC Device config shifts/masks */
 #define DDR_MAX_ROW_SHIFT		18
@@ -303,38 +294,40 @@ static int snps_get_error_info(struct snps_edac_priv *priv)
 	if (!regval)
 		return 1;
 
-	p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
+	p->ceinfo.bitpos = FIELD_GET(ECC_STAT_BITNUM_MASK, regval);
 
 	regval = readl(base + ECC_ERRCNT_OFST);
-	p->ce_cnt = regval & ECC_ERRCNT_CECNT_MASK;
-	p->ue_cnt = (regval & ECC_ERRCNT_UECNT_MASK) >> ECC_ERRCNT_UECNT_SHIFT;
+	p->ce_cnt = FIELD_GET(ECC_ERRCNT_CECNT_MASK, regval);
+	p->ue_cnt = FIELD_GET(ECC_ERRCNT_UECNT_MASK, regval);
 	if (!p->ce_cnt)
 		goto ue_err;
 
 	regval = readl(base + ECC_CEADDR0_OFST);
-	p->ceinfo.row = (regval & ECC_CEADDR0_RW_MASK);
+	p->ceinfo.row = FIELD_GET(ECC_CEADDR0_ROW_MASK, regval);
+
 	regval = readl(base + ECC_CEADDR1_OFST);
-	p->ceinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
-					ECC_CEADDR1_BNKNR_SHIFT;
-	p->ceinfo.bankgrp = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
-					ECC_CEADDR1_BNKGRP_SHIFT;
-	p->ceinfo.col = (regval & ECC_CEADDR1_COL_MASK);
+	p->ceinfo.bank = FIELD_GET(ECC_CEADDR1_BANK_MASK, regval);
+	p->ceinfo.bankgrp = FIELD_GET(ECC_CEADDR1_BANKGRP_MASK, regval);
+	p->ceinfo.col = FIELD_GET(ECC_CEADDR1_COL_MASK, regval);
+
 	p->ceinfo.data = readl(base + ECC_CSYND0_OFST);
+
 	edac_dbg(2, "ECCCSYN0: 0x%08X ECCCSYN1: 0x%08X ECCCSYN2: 0x%08X\n",
 		 readl(base + ECC_CSYND0_OFST), readl(base + ECC_CSYND1_OFST),
 		 readl(base + ECC_CSYND2_OFST));
+
 ue_err:
 	if (!p->ue_cnt)
 		goto out;
 
 	regval = readl(base + ECC_UEADDR0_OFST);
-	p->ueinfo.row = (regval & ECC_CEADDR0_RW_MASK);
+	p->ueinfo.row = FIELD_GET(ECC_CEADDR0_ROW_MASK, regval);
+
 	regval = readl(base + ECC_UEADDR1_OFST);
-	p->ueinfo.bankgrp = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
-					ECC_CEADDR1_BNKGRP_SHIFT;
-	p->ueinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
-					ECC_CEADDR1_BNKNR_SHIFT;
-	p->ueinfo.col = (regval & ECC_CEADDR1_COL_MASK);
+	p->ueinfo.bankgrp = FIELD_GET(ECC_CEADDR1_BANKGRP_MASK, regval);
+	p->ueinfo.bank = FIELD_GET(ECC_CEADDR1_BANK_MASK, regval);
+	p->ueinfo.col = FIELD_GET(ECC_CEADDR1_COL_MASK, regval);
+
 	p->ueinfo.data = readl(base + ECC_UESYND0_OFST);
 
 out:
@@ -509,7 +502,8 @@ static bool snps_get_ecc_state(void __iomem *base)
 {
 	u32 regval;
 
-	regval = readl(base + ECC_CFG0_OFST) & ECC_CFG0_MODE_MASK;
+	regval = readl(base + ECC_CFG0_OFST);
+	regval = FIELD_GET(ECC_CFG0_MODE_MASK, regval);
 
 	return (regval == ECC_CFG0_MODE_SECDED);
 }
@@ -699,13 +693,13 @@ static void snps_data_poison_setup(struct snps_edac_priv *priv)
 	if (priv->rank_shift[0])
 		rank = (hif_addr >> priv->rank_shift[0]) & BIT(0);
 
-	regval = (rank << ECC_POISON0_RANK_SHIFT) & ECC_POISON0_RANK_MASK;
-	regval |= (col << ECC_POISON0_COLUMN_SHIFT) & ECC_POISON0_COLUMN_MASK;
+	regval = FIELD_PREP(ECC_POISON0_RANK_MASK, rank) |
+		 FIELD_PREP(ECC_POISON0_COL_MASK, col);
 	writel(regval, priv->baseaddr + ECC_POISON0_OFST);
 
-	regval = (bankgrp << ECC_POISON1_BG_SHIFT) & ECC_POISON1_BG_MASK;
-	regval |= (bank << ECC_POISON1_BANKNR_SHIFT) & ECC_POISON1_BANKNR_MASK;
-	regval |= (row << ECC_POISON1_ROW_SHIFT) & ECC_POISON1_ROW_MASK;
+	regval = FIELD_PREP(ECC_POISON1_BANKGRP_MASK, bankgrp) |
+		 FIELD_PREP(ECC_POISON1_BANK_MASK, bank) |
+		 FIELD_PREP(ECC_POISON1_ROW_MASK, row);
 	writel(regval, priv->baseaddr + ECC_POISON1_OFST);
 }
 
@@ -744,10 +738,14 @@ static ssize_t inject_data_poison_show(struct device *dev,
 {
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct snps_edac_priv *priv = mci->pvt_info;
+	const char *errstr;
+	u32 regval;
+
+	regval = readl(priv->baseaddr + ECC_CFG1_OFST);
+	errstr = FIELD_GET(ECC_CEPOISON_MASK, regval) == ECC_CEPOISON_MASK ?
+		 "Correctable Error" : "UnCorrectable Error";
 
-	return sprintf(data, "Data Poisoning: %s\n\r",
-			(((readl(priv->baseaddr + ECC_CFG1_OFST)) & 0x3) == 0x3)
-			? ("Correctable Error") : ("UnCorrectable Error"));
+	return sprintf(data, "Data Poisoning: %s\n\r", errstr);
 }
 
 static ssize_t inject_data_poison_store(struct device *dev,
@@ -854,7 +852,7 @@ static void snps_setup_column_address_map(struct snps_edac_priv *priv, u32 *addr
 	int index;
 
 	memtype = readl(priv->baseaddr + DDR_MSTR_OFST);
-	width = (memtype & DDR_MSTR_BUSWIDTH_MASK) >> DDR_MSTR_BUSWIDTH_SHIFT;
+	width = FIELD_GET(DDR_MSTR_BUSWIDTH_MASK, memtype);
 
 	priv->col_shift[0] = 0;
 	priv->col_shift[1] = 1;
-- 
2.35.1


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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-22 19:07 ` [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support Serge Semin
@ 2022-08-23  8:17   ` Krzysztof Kozlowski
  2022-08-23  8:22     ` Krzysztof Kozlowski
                       ` (2 more replies)
  2022-08-23  8:26   ` Krzysztof Kozlowski
  1 sibling, 3 replies; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23  8:17 UTC (permalink / raw)
  To: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Krzysztof Kozlowski,
	Rob Herring, Manish Narani
  Cc: Serge Semin, Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 22/08/2022 22:07, Serge Semin wrote:
> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> no any reason to have these controllers described by the same bindings.
> Thus let's split them up.
> 
> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> description of the device bindings.

Filename should be based on compatible, so if renaming then
snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
filename anyway. Therefore nack for rename.

BTW, if you perform renames, generate patches with proper -M/-C/-B
arguments so this is detected.


> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++

This is a mess. I did not get any cover letters, any other patches any
description of relation between this and your other one.

It seems you make independent and conflicting changes to the same file,
so this has to be properly organized.

Send entire patchset with cover letter with description of all
dependencies to all maintainers.

This is unreviewable now, so a no.

Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:17   ` Krzysztof Kozlowski
@ 2022-08-23  8:22     ` Krzysztof Kozlowski
  2022-08-23  8:27       ` Serge Semin
  2022-08-23  8:25     ` Serge Semin
  2022-08-23  8:32     ` Serge Semin
  2 siblings, 1 reply; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23  8:22 UTC (permalink / raw)
  To: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani
  Cc: Serge Semin, Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 23/08/2022 11:17, Krzysztof Kozlowski wrote:
> On 22/08/2022 22:07, Serge Semin wrote:
>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
>> no any reason to have these controllers described by the same bindings.
>> Thus let's split them up.
>>
>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
>> description of the device bindings.
> 
> Filename should be based on compatible, so if renaming then
> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> filename anyway. Therefore nack for rename.
> 
> BTW, if you perform renames, generate patches with proper -M/-C/-B
> arguments so this is detected.
> 
> 
>>
>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>> --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
> 
> This is a mess. I did not get any cover letters, any other patches any
> description of relation between this and your other one.
> 
> It seems you make independent and conflicting changes to the same file,
> so this has to be properly organized.
> 
> Send entire patchset with cover letter with description of all
> dependencies to all maintainers.
> 
> This is unreviewable now, so a no.

And also untestable by Rob's bot, so will have to wait.

I found the cover letters, somehow they got buried in inbox.

Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:17   ` Krzysztof Kozlowski
  2022-08-23  8:22     ` Krzysztof Kozlowski
@ 2022-08-23  8:25     ` Serge Semin
  2022-08-23  8:32     ` Serge Semin
  2 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-23  8:25 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
> On 22/08/2022 22:07, Serge Semin wrote:
> > The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> > the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> > uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> > no any reason to have these controllers described by the same bindings.
> > Thus let's split them up.
> > 
> > While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> > descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> > description of the device bindings.
> 
> Filename should be based on compatible, so if renaming then
> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> filename anyway. Therefore nack for rename.
> 
> BTW, if you perform renames, generate patches with proper -M/-C/-B
> arguments so this is detected.
> 
> 
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
> 

> This is a mess. I did not get any cover letters, any other patches any
> description of relation between this and your other one.
> 
> It seems you make independent and conflicting changes to the same file,
> so this has to be properly organized.

Don't hurry with the judgement. Have a better look at your DT-related
inbox.
Link: https://lore.kernel.org/linux-devicetree/20220822190730.27277-1-Sergey.Semin@baikalelectronics.ru/

-Sergey

> 
> Send entire patchset with cover letter with description of all
> dependencies to all maintainers.
> 
> This is unreviewable now, so a no.
> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-22 19:07 ` [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support Serge Semin
  2022-08-23  8:17   ` Krzysztof Kozlowski
@ 2022-08-23  8:26   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23  8:26 UTC (permalink / raw)
  To: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani
  Cc: Serge Semin, Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel, Krzysztof Kozlowski

On 22/08/2022 22:07, Serge Semin wrote:
> -
> -allOf:
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - snps,ddrc-3.80a
> -              - xlnx,zynqmp-ddrc-2.40a
> -    then:
> -      required:
> -        - interrupts
> -    else:
> -      properties:
> -        interrupts: false

This is not a pure "rename"... Organize your patches so only one thing
is done at a time. Rename does not change the functional parts of the
binding...

Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:22     ` Krzysztof Kozlowski
@ 2022-08-23  8:27       ` Serge Semin
  2022-08-23  8:30         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-23  8:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 11:22:08AM +0300, Krzysztof Kozlowski wrote:
> On 23/08/2022 11:17, Krzysztof Kozlowski wrote:
> > On 22/08/2022 22:07, Serge Semin wrote:
> >> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> >> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> >> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> >> no any reason to have these controllers described by the same bindings.
> >> Thus let's split them up.
> >>
> >> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> >> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> >> description of the device bindings.
> > 
> > Filename should be based on compatible, so if renaming then
> > snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> > filename anyway. Therefore nack for rename.
> > 
> > BTW, if you perform renames, generate patches with proper -M/-C/-B
> > arguments so this is detected.
> > 
> > 
> >>
> >> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >> --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
> > 
> > This is a mess. I did not get any cover letters, any other patches any
> > description of relation between this and your other one.
> > 
> > It seems you make independent and conflicting changes to the same file,
> > so this has to be properly organized.
> > 
> > Send entire patchset with cover letter with description of all
> > dependencies to all maintainers.
> > 
> > This is unreviewable now, so a no.
> 

> And also untestable by Rob's bot, so will have to wait.

For what reason it's untestable? The patch has no dependencies from
any other patchset.

-Sergey

> 
> I found the cover letters, somehow they got buried in inbox.
> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:27       ` Serge Semin
@ 2022-08-23  8:30         ` Krzysztof Kozlowski
  2022-08-23  8:36           ` Serge Semin
  0 siblings, 1 reply; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23  8:30 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 23/08/2022 11:27, Serge Semin wrote:
> On Tue, Aug 23, 2022 at 11:22:08AM +0300, Krzysztof Kozlowski wrote:
>> On 23/08/2022 11:17, Krzysztof Kozlowski wrote:
>>> On 22/08/2022 22:07, Serge Semin wrote:
>>>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
>>>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
>>>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
>>>> no any reason to have these controllers described by the same bindings.
>>>> Thus let's split them up.
>>>>
>>>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
>>>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
>>>> description of the device bindings.
>>>
>>> Filename should be based on compatible, so if renaming then
>>> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
>>> filename anyway. Therefore nack for rename.
>>>
>>> BTW, if you perform renames, generate patches with proper -M/-C/-B
>>> arguments so this is detected.
>>>
>>>
>>>>
>>>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>>>> --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
>>>
>>> This is a mess. I did not get any cover letters, any other patches any
>>> description of relation between this and your other one.
>>>
>>> It seems you make independent and conflicting changes to the same file,
>>> so this has to be properly organized.
>>>
>>> Send entire patchset with cover letter with description of all
>>> dependencies to all maintainers.
>>>
>>> This is unreviewable now, so a no.
>>
> 
>> And also untestable by Rob's bot, so will have to wait.
> 
> For what reason it's untestable? The patch has no dependencies from
> any other patchset.

This one is testable, but the next one is not, because it depends on
something. I don't see the reason to split the bindings between
different patchsets.

Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:17   ` Krzysztof Kozlowski
  2022-08-23  8:22     ` Krzysztof Kozlowski
  2022-08-23  8:25     ` Serge Semin
@ 2022-08-23  8:32     ` Serge Semin
  2022-08-23  8:44       ` Krzysztof Kozlowski
  2 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-23  8:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
> On 22/08/2022 22:07, Serge Semin wrote:
> > The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> > the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> > uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> > no any reason to have these controllers described by the same bindings.
> > Thus let's split them up.
> > 
> > While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> > descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> > description of the device bindings.
> 

> Filename should be based on compatible, so if renaming then
> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> filename anyway. Therefore nack for rename.

New requirement? I've submitted not a single patch to the DT-bindings
sources and didn't get any comment from Rob about that. In addition
There are DT bindings with names different from what is defined in the
compatible name. Moreover there are tons of bindings with various
compatible names. What name to choose then? Finally the current name
is too generic to use for actual DW uMCTL2 DDRC controller.

-Sergey

> 
> BTW, if you perform renames, generate patches with proper -M/-C/-B
> arguments so this is detected.
> 
> 
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
> 
> This is a mess. I did not get any cover letters, any other patches any
> description of relation between this and your other one.
> 
> It seems you make independent and conflicting changes to the same file,
> so this has to be properly organized.
> 
> Send entire patchset with cover letter with description of all
> dependencies to all maintainers.
> 
> This is unreviewable now, so a no.
> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:30         ` Krzysztof Kozlowski
@ 2022-08-23  8:36           ` Serge Semin
  0 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-23  8:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 11:30:22AM +0300, Krzysztof Kozlowski wrote:
> On 23/08/2022 11:27, Serge Semin wrote:
> > On Tue, Aug 23, 2022 at 11:22:08AM +0300, Krzysztof Kozlowski wrote:
> >> On 23/08/2022 11:17, Krzysztof Kozlowski wrote:
> >>> On 22/08/2022 22:07, Serge Semin wrote:
> >>>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> >>>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> >>>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> >>>> no any reason to have these controllers described by the same bindings.
> >>>> Thus let's split them up.
> >>>>
> >>>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> >>>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> >>>> description of the device bindings.
> >>>
> >>> Filename should be based on compatible, so if renaming then
> >>> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> >>> filename anyway. Therefore nack for rename.
> >>>
> >>> BTW, if you perform renames, generate patches with proper -M/-C/-B
> >>> arguments so this is detected.
> >>>
> >>>
> >>>>
> >>>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >>>> --->  .../snps,dw-umctl2-ddrc.yaml                  | 51 +++++++++++++
> >>>
> >>> This is a mess. I did not get any cover letters, any other patches any
> >>> description of relation between this and your other one.
> >>>
> >>> It seems you make independent and conflicting changes to the same file,
> >>> so this has to be properly organized.
> >>>
> >>> Send entire patchset with cover letter with description of all
> >>> dependencies to all maintainers.
> >>>
> >>> This is unreviewable now, so a no.
> >>
> > 
> >> And also untestable by Rob's bot, so will have to wait.
> > 
> > For what reason it's untestable? The patch has no dependencies from
> > any other patchset.
> 

> This one is testable, but the next one is not, because it depends on
> something. I don't see the reason to split the bindings between
> different patchsets.

Really, do you want me to collect all 55 patches in a single patchset?

Please read the cover letter more carefully. And please don't hurry
with your judgement before nacking here and there.

-Sergey

> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:32     ` Serge Semin
@ 2022-08-23  8:44       ` Krzysztof Kozlowski
  2022-08-23 11:45         ` Serge Semin
  0 siblings, 1 reply; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23  8:44 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 23/08/2022 11:32, Serge Semin wrote:
> On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
>> On 22/08/2022 22:07, Serge Semin wrote:
>>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
>>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
>>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
>>> no any reason to have these controllers described by the same bindings.
>>> Thus let's split them up.
>>>
>>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
>>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
>>> description of the device bindings.
>>
> 
>> Filename should be based on compatible, so if renaming then
>> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
>> filename anyway. Therefore nack for rename.
> 
> New requirement? I've submitted not a single patch to the DT-bindings
> sources and didn't get any comment from Rob about that. 

This is not a new requirement. It has been since some time and Rob gave
such reviews.

https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@robh.at.kernel.org/

For devices with multiple compatibles that's a bit tricky, but assuming
the bindings describe both original design from Synopsys and it's
implementations, then something closer to Synopsys makes sense.


> In addition
> There are DT bindings with names different from what is defined in the
> compatible name. Moreover there are tons of bindings with various
> compatible names. What name to choose then? Finally the current name
> is too generic to use for actual DW uMCTL2 DDRC controller.

There are thousands of bugs, inconsistencies, naming differences in
kernel. I don't find these as arguments to repeat the practice...so the
bindings file name should be based on the compatible.

Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23  8:44       ` Krzysztof Kozlowski
@ 2022-08-23 11:45         ` Serge Semin
  2022-08-23 12:03           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-23 11:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 11:44:16AM +0300, Krzysztof Kozlowski wrote:
> On 23/08/2022 11:32, Serge Semin wrote:
> > On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
> >> On 22/08/2022 22:07, Serge Semin wrote:
> >>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> >>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> >>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> >>> no any reason to have these controllers described by the same bindings.
> >>> Thus let's split them up.
> >>>
> >>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> >>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> >>> description of the device bindings.
> >>
> > 

> >> Filename should be based on compatible, so if renaming then
> >> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> >> filename anyway. Therefore nack for rename.

Original name was synopsys,ddrc-ecc.yaml which doesn't match any of
the compatible strings. 

> > 
> > New requirement? I've submitted not a single patch to the DT-bindings
> > sources and didn't get any comment from Rob about that. 
> 

> This is not a new requirement. It has been since some time and Rob gave
> such reviews.
> 
> https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@robh.at.kernel.org/

April 2022. So it's new. It would be nice to have it defined somewhere
in docs (writing-bindings.rst?). So does the compatibles order (this
was surprising to me too).

> 
> For devices with multiple compatibles that's a bit tricky, but assuming
> the bindings describe both original design from Synopsys and it's
> implementations, then something closer to Synopsys makes sense.

The closest name would be snps,dw-umctl2-ddrc.yaml. snps,ddrc is too
generic especially for the IP-cores vendor. It doesn't have a
reference to the actual IP-core the device in subject is based on.

> 
> 
> > In addition
> > There are DT bindings with names different from what is defined in the
> > compatible name. Moreover there are tons of bindings with various
> > compatible names. What name to choose then? Finally the current name
> > is too generic to use for actual DW uMCTL2 DDRC controller.
> 

> There are thousands of bugs, inconsistencies, naming differences in
> kernel. I don't find these as arguments to repeat the practice...so the
> bindings file name should be based on the compatible.

Did I ask for an exception? I justified why the renaming was necessary. You
said it goes against the practice of having the DT-schema named as the
device compatible strings and just nacked. But above in this message you said

> "assuming the bindings describe both original design from Synopsys
> and it's implementations, then something closer to Synopsys makes sense"

What I suggest makes more sense than some abstract Synopsys DDRC,
which may refer to a Synopsys DDR controller other than the subject
one. So I see two solutions here:
1. Adding a new generic compatible string like "snps,dw-umctl2-ddrc"
and deprecate the "snps,ddrc-3.80a". It gets to be even more justified
seeing the Synopsys IP-core version has been exported in the device
CSRs since IP-core v3.20a. So having the version attached to the
compatible string was absolutely redundant.
2. Just deprecate the generic compatible string, the new compatible
devices will be supposed to use a vendor-specific compatible strings,
but still rename the DT-bindings file. This makes sense since the
current generic name isn't quiet well structured. It' prefix-part is
too generic and at the same time it refers to a device reversion for
no much reason.

What do you think?

* Note I've got it you'd prefer the renaming being performed in a
separate patch.

-Sergey

> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23 11:45         ` Serge Semin
@ 2022-08-23 12:03           ` Krzysztof Kozlowski
  2022-08-24 17:27             ` Serge Semin
  0 siblings, 1 reply; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-23 12:03 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 23/08/2022 14:45, Serge Semin wrote:
> On Tue, Aug 23, 2022 at 11:44:16AM +0300, Krzysztof Kozlowski wrote:
>> On 23/08/2022 11:32, Serge Semin wrote:
>>> On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
>>>> On 22/08/2022 22:07, Serge Semin wrote:
>>>>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
>>>>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
>>>>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
>>>>> no any reason to have these controllers described by the same bindings.
>>>>> Thus let's split them up.
>>>>>
>>>>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
>>>>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
>>>>> description of the device bindings.
>>>>
>>>
> 
>>>> Filename should be based on compatible, so if renaming then
>>>> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
>>>> filename anyway. Therefore nack for rename.
> 
> Original name was synopsys,ddrc-ecc.yaml which doesn't match any of
> the compatible strings. 
> 
>>>
>>> New requirement? I've submitted not a single patch to the DT-bindings
>>> sources and didn't get any comment from Rob about that. 
>>
> 
>> This is not a new requirement. It has been since some time and Rob gave
>> such reviews.
>>
>> https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@robh.at.kernel.org/
> 
> April 2022. So it's new. It would be nice to have it defined somewhere
> in docs (writing-bindings.rst?). So does the compatibles order (this
> was surprising to me too).
> 
>>
>> For devices with multiple compatibles that's a bit tricky, but assuming
>> the bindings describe both original design from Synopsys and it's
>> implementations, then something closer to Synopsys makes sense.
> 
> The closest name would be snps,dw-umctl2-ddrc.yaml. snps,ddrc is too
> generic especially for the IP-cores vendor. It doesn't have a
> reference to the actual IP-core the device in subject is based on.

You are aware that in such case mistake is not in the file name but in
the compatible?

> 
>>
>>
>>> In addition
>>> There are DT bindings with names different from what is defined in the
>>> compatible name. Moreover there are tons of bindings with various
>>> compatible names. What name to choose then? Finally the current name
>>> is too generic to use for actual DW uMCTL2 DDRC controller.
>>
> 
>> There are thousands of bugs, inconsistencies, naming differences in
>> kernel. I don't find these as arguments to repeat the practice...so the
>> bindings file name should be based on the compatible.
> 
> Did I ask for an exception? I justified why the renaming was necessary. You
> said it goes against the practice of having the DT-schema named as the
> device compatible strings and just nacked. But above in this message you said
> 
>> "assuming the bindings describe both original design from Synopsys
>> and it's implementations, then something closer to Synopsys makes sense"
> 
> What I suggest makes more sense than some abstract Synopsys DDRC,
> which may refer to a Synopsys DDR controller other than the subject
> one. So I see two solutions here:
> 1. Adding a new generic compatible string like "snps,dw-umctl2-ddrc"
> and deprecate the "snps,ddrc-3.80a". 

This might be good idea, although unfortunately replacing compatibles
takes quite a lot of time if you do not want to break any out-of-tree
users (read: other users of bindings).


> It gets to be even more justified
> seeing the Synopsys IP-core version has been exported in the device
> CSRs since IP-core v3.20a. So having the version attached to the
> compatible string was absolutely redundant.

The version might have sense in a way to differentiate from some older
versions, pre 3.20a. The binding was probably incomplete anyway.


> 2. Just deprecate the generic compatible string, the new compatible
> devices will be supposed to use a vendor-specific compatible strings,
> but still rename the DT-bindings file. This makes sense since the
> current generic name isn't quiet well structured. It' prefix-part is
> too generic and at the same time it refers to a device reversion for
> no much reason.

You mean disallow entirely "snps,ddrc-3.80a" and expect everyone to use
device/implementation specific compatible? I guess this depends whether
this custom block can be used without vendor specific addons. For
several other DW blocks we expect to have the generic snsp fallback and
this generic fallback can be used alone in Linux (pcie-designware-plat.c
binds to it).

Here the Linux driver also binds to generic synopsys compatible, so I
would assume it has a meaning and use case on its own.

> 
> What do you think?
> 
> * Note I've got it you'd prefer the renaming being performed in a
> separate patch.

The rename could be in the split patch as here, but then I assume the
rename part to be detected by git and be a pure rename. However:
1. The git did not mark it as rename (you might need to use custom
arguments to -M/-B/-C),
2. There were also changes in the process (allOf:if:then).


Best regards,
Krzysztof

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

* Re: [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API
  2022-08-22 19:07 ` [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
@ 2022-08-23 23:42   ` kernel test robot
  0 siblings, 0 replies; 39+ messages in thread
From: kernel test robot @ 2022-08-23 23:42 UTC (permalink / raw)
  To: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: kbuild-all, linux-media, Serge Semin, Alexey Malahov,
	Michail Ivanov, Pavel Parkhomenko, Punnaiah Choudary Kalluri,
	Manish Narani, Dinh Nguyen, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Hi Serge,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ras/edac-for-next]
[also build test WARNING on robh/for-next linus/master v6.0-rc2 next-20220823]
[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/Serge-Semin/EDAC-mc-synopsys-Various-fixes-and-cleanups/20220823-031020
base:   https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git edac-for-next
config: arm-buildonly-randconfig-r006-20220823 (https://download.01.org/0day-ci/archive/20220824/202208240709.CwTZS8Qo-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ec4a1d5280959bf5c6ff8d63006c62d54e01f9cc
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Serge-Semin/EDAC-mc-synopsys-Various-fixes-and-cleanups/20220823-031020
        git checkout ec4a1d5280959bf5c6ff8d63006c62d54e01f9cc
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/edac/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/edac/synopsys_edac.c: In function 'synps_get_dtype':
   drivers/edac/synopsys_edac.c:485:18: error: implicit declaration of function 'FIELD_GET'; did you mean 'FOLL_GET'? [-Werror=implicit-function-declaration]
     485 |         regval = FIELD_GET(DDR_MSTR_DEV_CFG_MASK, regval);
         |                  ^~~~~~~~~
         |                  FOLL_GET
   drivers/edac/synopsys_edac.c: In function 'mc_init':
>> drivers/edac/synopsys_edac.c:600:33: warning: variable 'priv' set but not used [-Wunused-but-set-variable]
     600 |         struct synps_edac_priv *priv;
         |                                 ^~~~
   cc1: some warnings being treated as errors


vim +/priv +600 drivers/edac/synopsys_edac.c

ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  588  
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  589  /**
225af74d6312b2a Manish Narani             2018-10-04  590   * mc_init - Initialize one driver instance.
225af74d6312b2a Manish Narani             2018-10-04  591   * @mci:	EDAC memory controller instance.
225af74d6312b2a Manish Narani             2018-10-04  592   * @pdev:	platform device.
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  593   *
225af74d6312b2a Manish Narani             2018-10-04  594   * Perform initialization of the EDAC memory controller instance and
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  595   * related driver-private data associated with the memory controller the
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  596   * instance is bound to.
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  597   */
fa9f6b9e1cf9620 Manish Narani             2018-10-04  598  static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  599  {
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06 @600  	struct synps_edac_priv *priv;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  601  
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  602  	mci->pdev = &pdev->dev;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  603  	priv = mci->pvt_info;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  604  	platform_set_drvdata(pdev, mci);
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  605  
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  606  	/* Initialize controller capabilities and configuration */
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  607  	mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR2;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  608  	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
5cfa57cc8481722 Serge Semin               2022-08-22  609  	mci->scrub_cap = SCRUB_FLAG_HW_SRC;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  610  	mci->scrub_mode = SCRUB_NONE;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  611  
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  612  	mci->edac_cap = EDAC_FLAG_SECDED;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  613  	mci->ctl_name = "synps_ddr_controller";
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  614  	mci->dev_name = SYNPS_EDAC_MOD_STRING;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  615  	mci->mod_name = SYNPS_EDAC_MOD_VER;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  616  
b500b4a029d577c Manish Narani             2018-10-25  617  	edac_op_state = EDAC_OPSTATE_INT;
b500b4a029d577c Manish Narani             2018-10-25  618  
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  619  	mci->ctl_page_to_phys = NULL;
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  620  
fa9f6b9e1cf9620 Manish Narani             2018-10-04  621  	init_csrows(mci);
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  622  }
ae9b56e3996dadb Punnaiah Choudary Kalluri 2015-01-06  623  

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

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

* Re: [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming
  2022-08-22 19:07 ` [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming Serge Semin
@ 2022-08-24  1:15   ` kernel test robot
  0 siblings, 0 replies; 39+ messages in thread
From: kernel test robot @ 2022-08-24  1:15 UTC (permalink / raw)
  To: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, James Morse, Robert Richter
  Cc: kbuild-all, linux-media, Serge Semin, Alexey Malahov,
	Michail Ivanov, Pavel Parkhomenko, Punnaiah Choudary Kalluri,
	Manish Narani, Dinh Nguyen, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

Hi Serge,

I love your patch! Yet something to improve:

[auto build test ERROR on ras/edac-for-next]
[also build test ERROR on robh/for-next linus/master v6.0-rc2 next-20220823]
[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/Serge-Semin/EDAC-mc-synopsys-Various-fixes-and-cleanups/20220823-031020
base:   https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git edac-for-next
config: arm-buildonly-randconfig-r006-20220823 (https://download.01.org/0day-ci/archive/20220824/202208240904.liSrmM2A-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/9c500fa8dbd08021742d8f5a48403c8f6292741c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Serge-Semin/EDAC-mc-synopsys-Various-fixes-and-cleanups/20220823-031020
        git checkout 9c500fa8dbd08021742d8f5a48403c8f6292741c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/edac/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/edac/synopsys_edac.c: In function 'snps_enable_irq':
>> drivers/edac/synopsys_edac.c:398:36: error: 'SYNPS_ZYNQMP_IRQ_REGS' undeclared (first use in this function); did you mean 'SNPS_ZYNQMP_IRQ_REGS'?
     398 |         if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
         |                                    ^~~~~~~~~~~~~~~~~~~~~
         |                                    SNPS_ZYNQMP_IRQ_REGS
   drivers/edac/synopsys_edac.c:398:36: note: each undeclared identifier is reported only once for each function it appears in
   drivers/edac/synopsys_edac.c: In function 'snps_disable_irq':
   drivers/edac/synopsys_edac.c:419:36: error: 'SYNPS_ZYNQMP_IRQ_REGS' undeclared (first use in this function); did you mean 'SNPS_ZYNQMP_IRQ_REGS'?
     419 |         if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
         |                                    ^~~~~~~~~~~~~~~~~~~~~
         |                                    SNPS_ZYNQMP_IRQ_REGS
   drivers/edac/synopsys_edac.c: In function 'snps_get_dtype':
   drivers/edac/synopsys_edac.c:485:18: error: implicit declaration of function 'FIELD_GET'; did you mean 'FOLL_GET'? [-Werror=implicit-function-declaration]
     485 |         regval = FIELD_GET(DDR_MSTR_DEV_CFG_MASK, regval);
         |                  ^~~~~~~~~
         |                  FOLL_GET
   drivers/edac/synopsys_edac.c: In function 'snps_mc_init':
   drivers/edac/synopsys_edac.c:600:32: warning: variable 'priv' set but not used [-Wunused-but-set-variable]
     600 |         struct snps_edac_priv *priv;
         |                                ^~~~
   cc1: some warnings being treated as errors


vim +398 drivers/edac/synopsys_edac.c

ae9b56e3996dad Punnaiah Choudary Kalluri 2015-01-06  392  
9c500fa8dbd080 Serge Semin               2022-08-22  393  static void snps_enable_irq(struct snps_edac_priv *priv)
4bcffe941758ee Sherry Sun                2022-04-27  394  {
b8ee875dfc73be Serge Semin               2022-08-22  395  	unsigned long flags;
b8ee875dfc73be Serge Semin               2022-08-22  396  
4bcffe941758ee Sherry Sun                2022-04-27  397  	/* Enable UE/CE Interrupts */
b8ee875dfc73be Serge Semin               2022-08-22 @398  	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
4bcffe941758ee Sherry Sun                2022-04-27  399  		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
4bcffe941758ee Sherry Sun                2022-04-27  400  		       priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
4bcffe941758ee Sherry Sun                2022-04-27  401  
b8ee875dfc73be Serge Semin               2022-08-22  402  		return;
b8ee875dfc73be Serge Semin               2022-08-22  403  	}
b8ee875dfc73be Serge Semin               2022-08-22  404  
b8ee875dfc73be Serge Semin               2022-08-22  405  	/* IRQs Enable/Disable feature has been available since v3.10a */
b8ee875dfc73be Serge Semin               2022-08-22  406  	spin_lock_irqsave(&priv->lock, flags);
b8ee875dfc73be Serge Semin               2022-08-22  407  
b8ee875dfc73be Serge Semin               2022-08-22  408  	writel(ECC_CTRL_EN_CE_IRQ | ECC_CTRL_EN_UE_IRQ,
b8ee875dfc73be Serge Semin               2022-08-22  409  	       priv->baseaddr + ECC_CLR_OFST);
b8ee875dfc73be Serge Semin               2022-08-22  410  
b8ee875dfc73be Serge Semin               2022-08-22  411  	spin_unlock_irqrestore(&priv->lock, flags);
4bcffe941758ee Sherry Sun                2022-04-27  412  }
4bcffe941758ee Sherry Sun                2022-04-27  413  

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

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-23 12:03           ` Krzysztof Kozlowski
@ 2022-08-24 17:27             ` Serge Semin
  2022-08-25  6:06               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 39+ messages in thread
From: Serge Semin @ 2022-08-24 17:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Tue, Aug 23, 2022 at 03:03:51PM +0300, Krzysztof Kozlowski wrote:
> On 23/08/2022 14:45, Serge Semin wrote:
> > On Tue, Aug 23, 2022 at 11:44:16AM +0300, Krzysztof Kozlowski wrote:
> >> On 23/08/2022 11:32, Serge Semin wrote:
> >>> On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:
> >>>> On 22/08/2022 22:07, Serge Semin wrote:
> >>>>> The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
> >>>>> the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
> >>>>> uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
> >>>>> no any reason to have these controllers described by the same bindings.
> >>>>> Thus let's split them up.
> >>>>>
> >>>>> While at it rename the original Synopsys uMCTL2 DT-schema file to a more
> >>>>> descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
> >>>>> description of the device bindings.
> >>>>
> >>>
> > 
> >>>> Filename should be based on compatible, so if renaming then
> >>>> snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
> >>>> filename anyway. Therefore nack for rename.
> > 
> > Original name was synopsys,ddrc-ecc.yaml which doesn't match any of
> > the compatible strings. 
> > 
> >>>
> >>> New requirement? I've submitted not a single patch to the DT-bindings
> >>> sources and didn't get any comment from Rob about that. 
> >>
> > 
> >> This is not a new requirement. It has been since some time and Rob gave
> >> such reviews.
> >>
> >> https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@robh.at.kernel.org/
> > 
> > April 2022. So it's new. It would be nice to have it defined somewhere
> > in docs (writing-bindings.rst?). So does the compatibles order (this
> > was surprising to me too).
> > 
> >>
> >> For devices with multiple compatibles that's a bit tricky, but assuming
> >> the bindings describe both original design from Synopsys and it's
> >> implementations, then something closer to Synopsys makes sense.
> > 
> > The closest name would be snps,dw-umctl2-ddrc.yaml. snps,ddrc is too
> > generic especially for the IP-cores vendor. It doesn't have a
> > reference to the actual IP-core the device in subject is based on.
> 

> You are aware that in such case mistake is not in the file name but in
> the compatible?

Let's compare, what is defined for Synopsys DW uMCTL2 DDRC in the
DT-bindings now:
compatible string: snps,ddrc-3.80a
file name: synopsys,ddrc-ecc.yaml

First of all they don't match at all. Secondly none of the names refer
to the actual IP-core the DT-bindings file is defined for. So in this
case the problem is in both:
1. file name has undefined vendor-prefix. It's supposed to be "snps".
2. file name has "ecc" suffix, which is a device property, but has
nothing to do with the device actual name.
2. actual device name is different. It's DW uMCTL2 DDRC. Just DDRC
doesn't identify the IP-core in subject.

> 
> > 
> >>
> >>
> >>> In addition
> >>> There are DT bindings with names different from what is defined in the
> >>> compatible name. Moreover there are tons of bindings with various
> >>> compatible names. What name to choose then? Finally the current name
> >>> is too generic to use for actual DW uMCTL2 DDRC controller.
> >>
> > 
> >> There are thousands of bugs, inconsistencies, naming differences in
> >> kernel. I don't find these as arguments to repeat the practice...so the
> >> bindings file name should be based on the compatible.
> > 
> > Did I ask for an exception? I justified why the renaming was necessary. You
> > said it goes against the practice of having the DT-schema named as the
> > device compatible strings and just nacked. But above in this message you said
> > 
> >> "assuming the bindings describe both original design from Synopsys
> >> and it's implementations, then something closer to Synopsys makes sense"
> > 
> > What I suggest makes more sense than some abstract Synopsys DDRC,
> > which may refer to a Synopsys DDR controller other than the subject
> > one. So I see two solutions here:
> > 1. Adding a new generic compatible string like "snps,dw-umctl2-ddrc"
> > and deprecate the "snps,ddrc-3.80a". 
> 

> This might be good idea, although unfortunately replacing compatibles
> takes quite a lot of time if you do not want to break any out-of-tree
> users (read: other users of bindings).

Well, I didn't imply the replacement, but "deprecation". It means to
just mark the "snps,ddrc-3.80a" compatible string as deprecated in the
bindings file and define a new one "snps,dw-umctl2-ddrc" which wouldn't have
the "deprecated: true" property set. It shall at least implicitly warn
people not to add new DTS-files with the deprecated device name. As I
see it eventually the dtbs-check tool will be updated to auto-detect
such attempts and, for instance, print a warning.

> 
> 
> > It gets to be even more justified
> > seeing the Synopsys IP-core version has been exported in the device
> > CSRs since IP-core v3.20a. So having the version attached to the
> > compatible string was absolutely redundant.
> 

> The version might have sense in a way to differentiate from some older
> versions, pre 3.20a.

I've almost fully refactored the device driver, and can say for sure
that the driver doesn't implement any Synopsys IP-core versions
specifics:
1. "xlnx,zynq-ddrc-a05" - has absolutely nothing in common with the
Synopsys DW uMCTL2 DDR controller (see the patch log for details).
That's why I've split the driver and DT-bindings up.
2. "xlnx,zynqmp-ddrc-2.40a" - ZynqMP DDR controller based on the
Synopsys uMCTL2 DDRC v2.40a. The device name is vendor-specific
anyway. So seeing there is no any other ZynqMP DDR controller added,
having IP-core version attached to the compatible string has been
redundant in the first place. Anyway the driver implements only the
ZynqMP-platform specifics irrespective to the IP-core version.
3. "snps,ddrc-3.80a" - without my patchsets this represents a
Synopsys DW uMCTL2 DDR controller synthesized with 64-bit DQ-bus and
8xSDRAM words burst config. The later settings have nothing to do with
the IP-core version, meanwhile what is really v3.x-specific isn't
implemented in the driver.

> The binding was probably incomplete anyway.

At the current state it's incomplete indeed. After applying all my
DT-related patches it will get to be almost complete, at least in the
CSRs, IRQs, clocks and resets resources part. There is always room for
the device-specific properties though, but judging by my experience caught
from Rob' reviews such properties aren't welcome if we have the IP-core
version detectable and if we can add the platform-specific compatible
string. The later one could be used to define the platform-specific
parameters right in the driver so the DT-bindings could be kept
relatively simple.

> 
> 
> > 2. Just deprecate the generic compatible string, the new compatible
> > devices will be supposed to use a vendor-specific compatible strings,
> > but still rename the DT-bindings file. This makes sense since the
> > current generic name isn't quiet well structured. It' prefix-part is
> > too generic and at the same time it refers to a device reversion for
> > no much reason.
> 

> You mean disallow entirely "snps,ddrc-3.80a" and expect everyone to use
> device/implementation specific compatible?

Yes as an alternative to the solution 1. described above.

> I guess this depends whether
> this custom block can be used without vendor specific addons. For
> several other DW blocks we expect to have the generic snsp fallback and
> this generic fallback can be used alone in Linux (pcie-designware-plat.c
> binds to it).

Just recently I've got a very long discussion with Rob regarding the
generic fallback compatible string. What he said "drop the generic
compatible fallback string. It proved to be useless." So I had to drop
the generic fallback compatible string from the nodes of my local DTS
files where it was appropriate and update all my DT-related patches to
disallow having vendor-specific and generic compatible strings specified
together.

Note what Rob said concerned the generic compatible "fallback" case,
not the generic compatible string in general. It's ok to have a
generic device name defined irrespective to the platform vendor.
Moreover it's applicable in case of the DW uMCTL2 DDRC IP-core since
first IP-core version is auto-detectable starting from v3.20a and
second I managed to implement auto-detection solutions for almost
all the DDR/ECC-specific parameters. So I am more inclined to the
solution 1) suggested by me in the previous email message:
- deprecate "snps,ddrc-3.80a" string.
- add new generic "snps,dw-umctl2-ddrc" compatible string.
- rename the DT-bindings file.

> 
> Here the Linux driver also binds to generic synopsys compatible, so I
> would assume it has a meaning and use case on its own.

Please see my messages above regarding the current Synopsys DW uMCTL2
EDAC driver implementation.

> 
> > 
> > What do you think?
> > 
> > * Note I've got it you'd prefer the renaming being performed in a
> > separate patch.
> 
> The rename could be in the split patch as here, but then I assume the
> rename part to be detected by git and be a pure rename. However:
> 1. The git did not mark it as rename (you might need to use custom
> arguments to -M/-B/-C),

Of course git hasn't detected it as rename, because aside with renaming
I've split the bindings up. Splitting these two updates up into two
patches will give us what you said. So to speak I suggest the next
updates for v2:
PATCH X. Detach the Zynq A05 DDRC DT-bindings to a separate schema.
PATCH X + 1. Rename the Synopsys DW uMCTL2 DDRC bindings file and add a more
descriptive generic compatible string name.

What do you think?

> 2. There were also changes in the process (allOf:if:then).

Right. But this is in another patchset. I'll address your notes in there.

-Sergey

> 
> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-24 17:27             ` Serge Semin
@ 2022-08-25  6:06               ` Krzysztof Kozlowski
  2022-08-25 13:23                 ` Serge Semin
  0 siblings, 1 reply; 39+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-25  6:06 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On 24/08/2022 20:27, Serge Semin wrote:

> 
> Note what Rob said concerned the generic compatible "fallback" case,
> not the generic compatible string in general. It's ok to have a
> generic device name defined irrespective to the platform vendor.
> Moreover it's applicable in case of the DW uMCTL2 DDRC IP-core since
> first IP-core version is auto-detectable starting from v3.20a and
> second I managed to implement auto-detection solutions for almost
> all the DDR/ECC-specific parameters. So I am more inclined to the
> solution 1) suggested by me in the previous email message:
> - deprecate "snps,ddrc-3.80a" string.
> - add new generic "snps,dw-umctl2-ddrc" compatible string.
> - rename the DT-bindings file.

Sounds ok.

> 
>>
>> Here the Linux driver also binds to generic synopsys compatible, so I
>> would assume it has a meaning and use case on its own.
> 
> Please see my messages above regarding the current Synopsys DW uMCTL2
> EDAC driver implementation.
> 
>>
>>>
>>> What do you think?
>>>
>>> * Note I've got it you'd prefer the renaming being performed in a
>>> separate patch.
>>
>> The rename could be in the split patch as here, but then I assume the
>> rename part to be detected by git and be a pure rename. However:
>> 1. The git did not mark it as rename (you might need to use custom
>> arguments to -M/-B/-C),
> 
> Of course git hasn't detected it as rename, because aside with renaming
> I've split the bindings up. Splitting these two updates up into two
> patches will give us what you said. So to speak I suggest the next
> updates for v2:
> PATCH X. Detach the Zynq A05 DDRC DT-bindings to a separate schema.
> PATCH X + 1. Rename the Synopsys DW uMCTL2 DDRC bindings file and add a more
> descriptive generic compatible string name.
> 
> What do you think?

Regardless of the split the rename can be and should be detected by Git.
That's why we have these options. If it is not detected, you changed too
much during rename, so it is not a rename anymore. Relatively small
amount of changes would still be detected.

> 
>> 2. There were also changes in the process (allOf:if:then).
> 
> Right. But this is in another patchset. I'll address your notes in there.


Best regards,
Krzysztof

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

* Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
  2022-08-25  6:06               ` Krzysztof Kozlowski
@ 2022-08-25 13:23                 ` Serge Semin
  0 siblings, 0 replies; 39+ messages in thread
From: Serge Semin @ 2022-08-25 13:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Serge Semin, Rob Herring, Michal Simek, Borislav Petkov,
	Mauro Carvalho Chehab, Tony Luck, Rob Herring, Manish Narani,
	Alexey Malahov, Michail Ivanov, Pavel Parkhomenko,
	Punnaiah Choudary Kalluri, Dinh Nguyen, James Morse,
	Robert Richter, Krzysztof Kozlowski, devicetree,
	linux-arm-kernel, linux-edac, linux-kernel

On Thu, Aug 25, 2022 at 09:06:42AM +0300, Krzysztof Kozlowski wrote:
> On 24/08/2022 20:27, Serge Semin wrote:
> 
> > 
> > Note what Rob said concerned the generic compatible "fallback" case,
> > not the generic compatible string in general. It's ok to have a
> > generic device name defined irrespective to the platform vendor.
> > Moreover it's applicable in case of the DW uMCTL2 DDRC IP-core since
> > first IP-core version is auto-detectable starting from v3.20a and
> > second I managed to implement auto-detection solutions for almost
> > all the DDR/ECC-specific parameters. So I am more inclined to the
> > solution 1) suggested by me in the previous email message:
> > - deprecate "snps,ddrc-3.80a" string.
> > - add new generic "snps,dw-umctl2-ddrc" compatible string.
> > - rename the DT-bindings file.
> 
> Sounds ok.

Agreed then.

> 
> > 
> >>
> >> Here the Linux driver also binds to generic synopsys compatible, so I
> >> would assume it has a meaning and use case on its own.
> > 
> > Please see my messages above regarding the current Synopsys DW uMCTL2
> > EDAC driver implementation.
> > 
> >>
> >>>
> >>> What do you think?
> >>>
> >>> * Note I've got it you'd prefer the renaming being performed in a
> >>> separate patch.
> >>
> >> The rename could be in the split patch as here, but then I assume the
> >> rename part to be detected by git and be a pure rename. However:
> >> 1. The git did not mark it as rename (you might need to use custom
> >> arguments to -M/-B/-C),
> > 
> > Of course git hasn't detected it as rename, because aside with renaming
> > I've split the bindings up. Splitting these two updates up into two
> > patches will give us what you said. So to speak I suggest the next
> > updates for v2:
> > PATCH X. Detach the Zynq A05 DDRC DT-bindings to a separate schema.
> > PATCH X + 1. Rename the Synopsys DW uMCTL2 DDRC bindings file and add a more
> > descriptive generic compatible string name.
> > 
> > What do you think?
> 

> Regardless of the split the rename can be and should be detected by Git.
> That's why we have these options. If it is not detected, you changed too
> much during rename, so it is not a rename anymore. Relatively small
> amount of changes would still be detected.

Right. I'll make sure the renaming is detected.

-Sergey

> 
> > 
> >> 2. There were also changes in the process (allOf:if:then).
> > 
> > Right. But this is in another patchset. I'll address your notes in there.
> 
> 
> Best regards,
> Krzysztof

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

* Re: [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition
  2022-08-22 19:07 ` [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition Serge Semin
@ 2022-09-01  6:59   ` Borislav Petkov
  0 siblings, 0 replies; 39+ messages in thread
From: Borislav Petkov @ 2022-09-01  6:59 UTC (permalink / raw)
  To: Serge Semin
  Cc: Rob Herring, Michal Simek, Mauro Carvalho Chehab, Tony Luck,
	James Morse, Robert Richter, Serge Semin, Alexey Malahov,
	Michail Ivanov, Pavel Parkhomenko, Punnaiah Choudary Kalluri,
	Manish Narani, Dinh Nguyen, Rob Herring, Krzysztof Kozlowski,
	devicetree, linux-arm-kernel, linux-edac, linux-kernel

On Mon, Aug 22, 2022 at 10:07:22PM +0300, Serge Semin wrote:
> Currently the memory type macros are partly defined with the multiple
> spaces between the macro name and its definition. Let's replace the spaces
> with tabs as the kernel coding style requires.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  include/linux/edac.h | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

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

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

* Re: [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout
  2022-08-22 19:07 ` [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout Serge Semin
@ 2022-09-01  7:00   ` Borislav Petkov
  0 siblings, 0 replies; 39+ messages in thread
From: Borislav Petkov @ 2022-09-01  7:00 UTC (permalink / raw)
  To: Serge Semin
  Cc: Rob Herring, Michal Simek, Mauro Carvalho Chehab, Tony Luck,
	James Morse, Robert Richter, Serge Semin, Alexey Malahov,
	Michail Ivanov, Pavel Parkhomenko, Punnaiah Choudary Kalluri,
	Manish Narani, Dinh Nguyen, Rob Herring, Krzysztof Kozlowski,
	devicetree, linux-arm-kernel, linux-edac, linux-kernel

On Mon, Aug 22, 2022 at 10:07:23PM +0300, Serge Semin wrote:
> The duplicated edac_dbg()-based dimm->nr_pages field was introduced in
> commit 6e84d359b2be ("edac_mc: Cleanup per-dimm_info debug messages"). You
> can find the duplicated line even in the commit message text:
> > [ 1011.380101] EDAC DEBUG: edac_mc_dump_dimm:   dimm->nr_pages = 0x40000
> > [ 1011.380103] EDAC DEBUG: edac_mc_dump_dimm:   dimm->grain = 8
> > [ 1011.380104] EDAC DEBUG: edac_mc_dump_dimm:   dimm->nr_pages = 0x40000
> Let's drop the corresponding edac_dbg() method call then.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  drivers/edac/edac_mc.c | 1 -
>  1 file changed, 1 deletion(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

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

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

end of thread, other threads:[~2022-09-01  7:01 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-22 19:07 [PATCH 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
2022-08-22 19:07 ` [PATCH 01/20] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure Serge Semin
2022-08-22 19:07 ` [PATCH 02/20] EDAC/synopsys: Fix generic device type detection procedure Serge Semin
2022-08-22 19:07 ` [PATCH 03/20] EDAC/synopsys: Fix mci->scrub_cap field setting Serge Semin
2022-08-22 19:07 ` [PATCH 04/20] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse Serge Semin
2022-08-22 19:07 ` [PATCH 05/20] EDAC/synopsys: Fix reading errors count before ECC status Serge Semin
2022-08-22 19:07 ` [PATCH 06/20] EDAC/synopsys: Use platform device devm ioremap method Serge Semin
2022-08-22 19:07 ` [PATCH 07/20] EDAC/synopsys: Drop internal CE and UE counters Serge Semin
2022-08-22 19:07 ` [PATCH 08/20] EDAC/synopsys: Drop local to_mci macro implementation Serge Semin
2022-08-22 19:07 ` [PATCH 09/20] EDAC/synopsys: Drop struct ecc_error_info.blknr field Serge Semin
2022-08-22 19:07 ` [PATCH 10/20] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name Serge Semin
2022-08-22 19:07 ` [PATCH 11/20] EDAC/synopsys: Drop redundant info from error message Serge Semin
2022-08-22 19:07 ` [PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition Serge Semin
2022-09-01  6:59   ` Borislav Petkov
2022-08-22 19:07 ` [PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout Serge Semin
2022-09-01  7:00   ` Borislav Petkov
2022-08-22 19:07 ` [PATCH 14/20] EDAC/mc: Init DIMM labels in MC registration method Serge Semin
2022-08-22 19:07 ` [PATCH 15/20] EDAC/mc: Add MC unique index allocation procedure Serge Semin
2022-08-22 19:07 ` [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support Serge Semin
2022-08-23  8:17   ` Krzysztof Kozlowski
2022-08-23  8:22     ` Krzysztof Kozlowski
2022-08-23  8:27       ` Serge Semin
2022-08-23  8:30         ` Krzysztof Kozlowski
2022-08-23  8:36           ` Serge Semin
2022-08-23  8:25     ` Serge Semin
2022-08-23  8:32     ` Serge Semin
2022-08-23  8:44       ` Krzysztof Kozlowski
2022-08-23 11:45         ` Serge Semin
2022-08-23 12:03           ` Krzysztof Kozlowski
2022-08-24 17:27             ` Serge Semin
2022-08-25  6:06               ` Krzysztof Kozlowski
2022-08-25 13:23                 ` Serge Semin
2022-08-23  8:26   ` Krzysztof Kozlowski
2022-08-22 19:07 ` [PATCH 17/20] EDAC/synopsys: " Serge Semin
2022-08-22 19:07 ` [PATCH 18/20] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
2022-08-23 23:42   ` kernel test robot
2022-08-22 19:07 ` [PATCH 19/20] EDAC/synopsys: Unify the driver entities naming Serge Semin
2022-08-24  1:15   ` kernel test robot
2022-08-22 19:07 ` [PATCH 20/20] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros Serge Semin

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