linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
To: Michal Simek <michal.simek@xilinx.com>,
	Borislav Petkov <bp@alien8.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Robert Richter <rric@kernel.org>,
	Manish Narani <manish.narani@xilinx.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Serge Semin <fancer.lancer@gmail.com>,
	Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
	Michail Ivanov <Michail.Ivanov@baikalelectronics.ru>,
	Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
	Punnaiah Choudary Kalluri  <punnaiah.choudary.kalluri@xilinx.com>,
	Dinh Nguyen <dinguyen@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	<devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Borislav Petkov <bp@suse.de>
Subject: [PATCH RESEND v3 02/17] EDAC/synopsys: Fix generic device type detection procedure
Date: Fri, 30 Sep 2022 02:26:57 +0300	[thread overview]
Message-ID: <20220929232712.12202-3-Sergey.Semin@baikalelectronics.ru> (raw)
In-Reply-To: <20220929232712.12202-1-Sergey.Semin@baikalelectronics.ru>

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>

---

Changelog v2:
- Include "linux/bitfield.h" header file to get the FIELD_GET macro
  definition. (@tbot)
---
 drivers/edac/synopsys_edac.c | 57 +++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index c78fb5781ff9..17960f7ca29b 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2012 - 2014 Xilinx, Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/edac.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -142,7 +143,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 +677,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 +732,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.37.3



  parent reply	other threads:[~2022-10-01  1:14 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 23:26 [PATCH RESEND v3 00/17] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
2022-09-29 23:26 ` [PATCH RESEND v3 01/17] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure Serge Semin
2022-10-31 17:24   ` Borislav Petkov
2023-09-01 11:24     ` Serge Semin
2022-09-29 23:26 ` Serge Semin [this message]
2022-09-29 23:26 ` [PATCH RESEND v3 03/17] EDAC/synopsys: Fix mci->scrub_cap field setting Serge Semin
2022-09-29 23:26 ` [PATCH RESEND v3 04/17] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 05/17] EDAC/synopsys: Fix reading errors count before ECC status Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 06/17] EDAC/synopsys: Use platform device devm ioremap method Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 07/17] EDAC/synopsys: Drop internal CE and UE counters Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 08/17] EDAC/synopsys: Drop local to_mci macro implementation Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 09/17] EDAC/synopsys: Drop struct ecc_error_info.blknr field Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 10/17] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 11/17] EDAC/synopsys: Drop redundant info from error message Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 12/17] EDAC/mc: Init DIMM labels in MC registration method Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 13/17] EDAC/mc: Add MC unique index allocation procedure Serge Semin
2022-10-12 17:29   ` Borislav Petkov
2022-10-12 20:01     ` Serge Semin
2022-10-12 20:33       ` Borislav Petkov
2022-10-12 20:44         ` Tony Luck
2022-10-12 21:31           ` Serge Semin
2022-10-12 22:30         ` Serge Semin
2022-10-13  9:38           ` Borislav Petkov
2022-09-29 23:27 ` [PATCH RESEND v3 14/17] EDAC/synopsys: Detach Zynq DDRC controller support Serge Semin
2022-09-30 14:42   ` Borislav Petkov
2022-10-06 12:17     ` Serge Semin
2022-10-06 13:25       ` Borislav Petkov
2022-10-08  0:42         ` Serge Semin
2022-10-12 17:28           ` Borislav Petkov
2022-10-12 19:27             ` Serge Semin
2022-10-12 19:44               ` Borislav Petkov
2022-10-12 20:55                 ` Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 15/17] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 16/17] EDAC/synopsys: Unify the driver entities naming Serge Semin
2022-09-29 23:27 ` [PATCH RESEND v3 17/17] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros Serge Semin
2022-09-30 14:29 ` [PATCH RESEND v3 00/17] EDAC/mc/synopsys: Various fixes and cleanups Borislav Petkov
2022-10-06  7:13   ` Michal Simek
2023-08-18 23:06 ` Serge Semin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220929232712.12202-3-Sergey.Semin@baikalelectronics.ru \
    --to=sergey.semin@baikalelectronics.ru \
    --cc=Alexey.Malahov@baikalelectronics.ru \
    --cc=Michail.Ivanov@baikalelectronics.ru \
    --cc=Pavel.Parkhomenko@baikalelectronics.ru \
    --cc=bp@alien8.de \
    --cc=bp@suse.de \
    --cc=devicetree@vger.kernel.org \
    --cc=dinguyen@kernel.org \
    --cc=fancer.lancer@gmail.com \
    --cc=james.morse@arm.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manish.narani@xilinx.com \
    --cc=mchehab@kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=punnaiah.choudary.kalluri@xilinx.com \
    --cc=robh+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=rric@kernel.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).