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>
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
Subject: [PATCH v5 06/20] EDAC/synopsys: Fix misleading IRQ self-cleared quirk flag
Date: Thu, 22 Feb 2024 21:12:51 +0300	[thread overview]
Message-ID: <20240222181324.28242-7-fancer.lancer@gmail.com> (raw)
In-Reply-To: <20240222181324.28242-1-fancer.lancer@gmail.com>

The DDR_ECC_INTR_SELF_CLEAR quirk flag was initially added in the commit
f7824ded4149 ("EDAC/synopsys: Add support for version 3 of the Synopsys
EDAC DDR") in order to distinguish the ZynqMP DDRC (based on DW uMCTL2
DDRC v2.40a) and the announced in that commit Synopsys DDR controller
v3.80a. The selected name is misleading for the next reasons:

1. None of the Synopsys DW uMCTL2 DDR IP-core has the UE/CE IRQs
auto or self cleared. The IRQ signals (ecc_corrected_err_intr and
ecc_uncorrected_err_intr) are cleared together with the rest of the ECC
error data by means of writing 1's to the respective ECCCLR bits. It
worked like that in DW uMCTL2 DDRC v2.x IP-core and it's still true for
the modern DW uMCTL2 DDRC v3.x.

2. The IRQ-related registers accessed unless the denoted quirk is
specified are actually Xilinx Zynq-specific. None of the Synopsys DW uMCTL
DDRC IP-core have any registers at the offsets 0x20200/0x20208/0x2020C.
The most modern DW uMCTL2 DDRC v3.91a IP-core available has CSRs space end
at the 0x43dc offset. The older IP-cores have even smaller registers
space.

3. What was actually introduced in the DW uMCTL2 DDRC v3.10 by Synopsys is
the IRQ enable flags which older DW uMCTL2 DDRC IP-core didn't have. They
were added to the ECCCLR register (the CSR was also renamed to ECCCTL in
the v3.10 IP-core HW databook). So since then there have been no point in
having a vendor-specific IRQs masking solution like described in 2. and
the IRQ signal can be now shared even for the native DW uMCTL2 DDR
controllers.

So let's harmonize the quirked IRQs code based on the statements above:
rename the DDR_ECC_INTR_SELF_CLEAR quirk flag to SYNPS_ZYNQMP_IRQ_REGS
thus indicating the ZynqMP-specific IRQ CSRs; add the new quirk flag to
the ZynqMP platform data; drop the misleading comments about the
auto-cleared ue/ce flags; add a comment about the new IRQ enable flags
added in v3.10 IP-core.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>

---

Changelog v4:
- This is a new patch detached from
  [PATCH v3 01/17] EDAC/synopsys: Fix native uMCTL2 IRQs handling procedure
---
 drivers/edac/synopsys_edac.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index fbaf3d9ad517..9f79f14e57b2 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -88,7 +88,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 */
@@ -526,7 +526,7 @@ 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)) {
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 		       priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
 
@@ -535,6 +535,10 @@ static void enable_intr(struct synps_edac_priv *priv)
 
 	spin_lock_irqsave(&priv->reglock, flags);
 
+	/*
+	 * IRQs Enable/Disable flags have been available since v3.10a.
+	 * This is noop for the older controllers.
+	 */
 	writel(DDR_UE_MASK | DDR_CE_MASK,
 	       priv->baseaddr + ECC_CLR_OFST);
 
@@ -546,7 +550,7 @@ 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)) {
+	if (priv->p_data->quirks & SYNPS_ZYNQMP_IRQ_REGS) {
 		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 		       priv->baseaddr + DDR_QOS_IRQ_DB_OFST);
 
@@ -577,11 +581,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))
@@ -598,8 +598,8 @@ 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);
 
 	return IRQ_HANDLED;
@@ -913,7 +913,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
@@ -925,7 +925,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
-- 
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 ` [PATCH v5 02/20] EDAC/synopsys: Fix generic device type detection procedure Serge Semin
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 ` Serge Semin [this message]
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-7-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=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=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).