linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Serge Semin <fancer.lancer@gmail.com>
To: Michal Simek <michal.simek@amd.com>,
	Alexander Stein <alexander.stein@ew.tq-group.com>,
	Borislav Petkov <bp@alien8.de>, Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Robert Richter <rric@kernel.org>,
	Manish Narani <manish.narani@xilinx.com>
Cc: Serge Semin <fancer.lancer@gmail.com>,
	Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@xilinx.com>,
	Dinh Nguyen <dinguyen@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, Borislav Petkov <bp@suse.de>
Subject: [PATCH v5 02/20] EDAC/synopsys: Fix generic device type detection procedure
Date: Thu, 22 Feb 2024 21:12:47 +0300	[thread overview]
Message-ID: <20240222181324.28242-3-fancer.lancer@gmail.com> (raw)
In-Reply-To: <20240222181324.28242-1-fancer.lancer@gmail.com>

First of all the enum dev_type constants describe the memory DRAM chips
used at the stick, not the entire DQ-bus width (see the enumeration kdoc
for details). So what is returned from the zynqmp_get_dtype() function and
then specified to the dimm_info->dtype field is definitely incorrect.
Secondly the DRAM chips type has nothing to do with the data bus width
specified in the MSTR.data_bus_width CSR field. That CSR field just
determines the part of the whole DQ-bus currently used to access the data
from the all DRAM memory 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 utilized 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.

Fix all of the mistakes described above in the zynqmp_get_dtype() and
zynqmp_get_ecc_state() methods: specify actual DRAM chips data width only
for the DDR4 protocol and return that it's UNKNOWN in the rest of the
cases; determine ECC availability by the ECCCFG0.ecc_mode field state
only (that field can't be modified anyway if the IP-core was synthesized
with no ECC support).

[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 <fancer.lancer@gmail.com>

---

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

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 0168b05e3ca1..455d2fcfd8c1 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -674,26 +674,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 + CTRL_OFST);
+	if (!(regval & MEM_TYPE_DDR4))
+		return DEV_UNKNOWN;
+
+	regval = (regval & DDRC_MSTR_CFG_MASK) >> DDRC_MSTR_CFG_SHIFT;
+	switch (regval) {
+	case DDRC_MSTR_CFG_X4_MASK:
+		return DEV_X4;
+	case DDRC_MSTR_CFG_X8_MASK:
+		return DEV_X8;
+	case DDRC_MSTR_CFG_X16_MASK:
+		return DEV_X16;
+	case DDRC_MSTR_CFG_X32_MASK:
+		return DEV_X32;
 	}
 
-	return dt;
+	return DEV_UNKNOWN;
 }
 
 /**
@@ -730,19 +729,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.43.0


  parent reply	other threads:[~2024-02-22 18:14 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-22 18:12 [PATCH v5 00/20] EDAC/mc/synopsys: Various fixes and cleanups Serge Semin
2024-02-22 18:12 ` [PATCH v5 01/20] EDAC/synopsys: Fix ECC status data and IRQ disable race condition Serge Semin
2024-04-15 18:36   ` Borislav Petkov
2024-04-16 10:06     ` Serge Semin
2024-04-21 10:07       ` Borislav Petkov
2024-04-25 12:52         ` Serge Semin
2024-05-06 10:20           ` Borislav Petkov
2024-05-06 11:27             ` Serge Semin
2024-05-06 12:12               ` Borislav Petkov
2024-02-22 18:12 ` Serge Semin [this message]
2024-02-22 18:12 ` [PATCH v5 03/20] EDAC/synopsys: Fix mci->scrub_cap field setting Serge Semin
2024-02-22 18:12 ` [PATCH v5 04/20] EDAC/synopsys: Drop erroneous ADDRMAP4.addrmap_col_b10 parse Serge Semin
2024-02-22 18:12 ` [PATCH v5 05/20] EDAC/synopsys: Fix reading errors count before ECC status Serge Semin
2024-02-22 18:12 ` [PATCH v5 06/20] EDAC/synopsys: Fix misleading IRQ self-cleared quirk flag Serge Semin
2024-02-22 18:12 ` [PATCH v5 07/20] EDAC/synopsys: Use platform device devm ioremap method Serge Semin
2024-02-22 18:12 ` [PATCH v5 08/20] EDAC/synopsys: Drop internal CE and UE counters Serge Semin
2024-02-22 18:12 ` [PATCH v5 09/20] EDAC/synopsys: Drop local to_mci() macro definition Serge Semin
2024-02-22 18:12 ` [PATCH v5 10/20] EDAC/synopsys: Drop struct ecc_error_info.blknr field Serge Semin
2024-02-22 18:12 ` [PATCH v5 11/20] EDAC/synopsys: Shorten out struct ecc_error_info.bankgrpnr field name Serge Semin
2024-02-22 18:12 ` [PATCH v5 12/20] EDAC/synopsys: Drop redundant info from the error messages Serge Semin
2024-02-22 18:12 ` [PATCH v5 13/20] EDAC/mc: Init DIMM labels in MC registration method Serge Semin
2024-02-22 18:12 ` [PATCH v5 14/20] EDAC/mc: Add generic unique MC index allocation procedure Serge Semin
2024-02-22 18:13 ` [PATCH v5 15/20] EDAC/mc: Re-use " Serge Semin
2024-02-22 18:13 ` [PATCH v5 16/20] EDAC/synopsys: Detach Zynq A05 DDRC support to separate driver Serge Semin
2024-02-22 18:13 ` [PATCH v5 17/20] EDAC/synopsys: Drop unused platform-specific setup API Serge Semin
2024-02-22 18:13 ` [PATCH v5 18/20] EDAC/synopsys: Unify CSRs macro declarations Serge Semin
2024-02-22 18:13 ` [PATCH v5 19/20] EDAC/synopsys: Unify struct/macro/function prefixes Serge Semin
2024-02-22 18:13 ` [PATCH v5 20/20] EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros Serge Semin
2024-03-06  5:27 ` [PATCH v5 00/20] EDAC/mc/synopsys: Various fixes and cleanups Shubhrajyoti Datta

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=20240222181324.28242-3-fancer.lancer@gmail.com \
    --to=fancer.lancer@gmail.com \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=bp@suse.de \
    --cc=dinguyen@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=james.morse@arm.com \
    --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@amd.com \
    --cc=punnaiah.choudary.kalluri@xilinx.com \
    --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).