This patchset is a first one in the series created in the framework of
my Synopsys DW uMCTL2 DDRC-related work:
[1: In-progress v4] EDAC/mc/synopsys: Various fixes and cleanups
Link: ---you are looking at it---
[2: In-progress v4] EDAC/synopsys: Add generic DDRC info and address mapping
Link: ---to be submitted---
[3: In-progress v4] EDAC/synopsys: Add generic resources and Scrub support
Link: ---to be submitted---
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.
The main goal of the entire set of the changes provided in the mentioned
patchsets is to as much as possible specialise the synopsys_edac.c driver
to be working with the Synopsys DW uMCTL2 DDR controllers of various
versions and synthesized parameters, and add useful error-detection
features.
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 the 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/ECCCTL 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
DDRCs) so the provided modifications would be useful in both drivers.
First the platform resource open-coded IO-space remapping is replaced with
the devm_platform_ioremap_resource() method call for the sake of the code
simplification. Secondly the next redundant entities are dropped: 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 detaching the Zynq A05-related code an unique MC index
allocation infrastructure is added to the MCI core. It's required since
after splitting the driver up both supported types of memory devices could
be correctly probed on the same platform. Note even though it's currently
unsupported by the synsopsys_edac.c driver it's claimed to be possible by
the original driver author (it was a reason of having two unrelated
devices supported in a single driver). Finally the Xilinx Zynq A05 part of
the driver is moved out to a dedicated driver. 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 a set of the local coding style
cleanups are provided: unify the DW uMCTL2 DDRC driver entities naming and
replace the open-coded "shift/mask" pattern with the kernel helpers like
BIT/GENMASK/FIELD_x in there. It shall significantly improve the code
readability.
Changelog v2:
- Move Synopsys DW uMCTL2 DDRC bindings file renaming to a separate patch.
(@Krzysztof)
- Introduce a new compatible string "snps,dw-umctl2-ddrc" matching the new
DT-schema name.
- Forgot to fix some of the prefix of the SYNPS_ZYNQMP_IRQ_REGS macro
in several places. (@tbot)
- Drop the no longer used "priv" pointer from the mc_init() function.
(@tbot)
- Include "linux/bitfield.h" header file to get the FIELD_GET macro
definition. (@tbot)
- Drop the already merged in patches:
[PATCH 12/20] EDAC/mc: Replace spaces with tabs in memtype flags definition
[PATCH 13/20] EDAC/mc: Drop duplicated dimm->nr_pages debug printout
Changelog v3:
- Drop the no longer used "priv" pointer from the mc_init() function.
(@tbot)
- Drop the merged in patches:
[PATCH v2 14/19] dt-bindings: memory: snps: Detach Zynq DDRC controller support
[PATCH v2 15/19] dt-bindings: memory: snps: Use more descriptive device name
(@Krzysztof)
Changelog v4:
- Remove Rob, Krzysztof and DT-mailing list from Cc since the respective
patches have already been merged in.
- Add a new patch
[PATCH v4 6/20] EDAC/synopsys: Fix misleading IRQ self-cleared quirk flag
detached from the very first patch of the series.
- Add a new patch
[PATCH v4 15/20] EDAC/mc: Re-use generic unique MC index allocation procedure
- Add a new patch
[PATCH v4 18/20] EDAC/synopsys: Unify CSRs macro declarations
collecting the changes from various patches of the series.
- Drop redundant empty lines left by mistake.
- Drop private counters access from the check_errors() method too.
- Rebase onto the kernel v6.6-rcX.
Signed-off-by: Serge Semin <[email protected]>
Cc: Punnaiah Choudary Kalluri <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Serge Semin (20):
EDAC/synopsys: Fix ECC status data and IRQ disable race condition
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: Fix misleading IRQ self-cleared quirk flag
EDAC/synopsys: Use platform device devm ioremap method
EDAC/synopsys: Drop internal CE and UE counters
EDAC/synopsys: Drop local to_mci() macro definition
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 the error messages
EDAC/mc: Init DIMM labels in MC registration method
EDAC/mc: Add generic unique MC index allocation procedure
EDAC/mc: Re-use generic unique MC index allocation procedure
EDAC/synopsys: Detach Zynq A05 DDRC support to separate driver
EDAC/synopsys: Drop unused platform-specific setup API
EDAC/synopsys: Unify CSRs macro declarations
EDAC/synopsys: Unify struct/macro/function prefixes
EDAC/synopsys: Convert to using BIT/GENMASK/FIELD_x macros
MAINTAINERS | 1 +
drivers/edac/Kconfig | 9 +-
drivers/edac/Makefile | 1 +
drivers/edac/dmc520_edac.c | 4 +-
drivers/edac/edac_mc.c | 135 ++++-
drivers/edac/edac_mc.h | 4 +
drivers/edac/pasemi_edac.c | 5 +-
drivers/edac/ppc4xx_edac.c | 5 +-
drivers/edac/synopsys_edac.c | 966 ++++++++++++-----------------------
drivers/edac/zynq_edac.c | 501 ++++++++++++++++++
10 files changed, 962 insertions(+), 669 deletions(-)
create mode 100644 drivers/edac/zynq_edac.c
--
2.41.0
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 was
added by mistake as a result of the copy-paste from the Half DQ bus width
mode part of the same function. Even though the field is supposed to be
always set to the UNUSED value (0x1F) drop parsing it anyway so to
simplify the setup_column_address_map() method a tiny bit.
Fixes: 1a81361f75d8 ("EDAC, synopsys: Add Error Injection support for ZynqMP DDR controller")
Signed-off-by: Serge Semin <[email protected]>
---
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 65790097beb2..308da6f82d3d 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -1237,10 +1237,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.41.0
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 <[email protected]>
---
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 fff4b07ff6ac..6b81ac9dda8b 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -89,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 */
@@ -527,7 +527,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);
@@ -536,6 +536,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);
@@ -547,7 +551,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);
@@ -578,11 +582,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))
@@ -599,8 +599,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;
@@ -914,7 +914,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
@@ -926,7 +926,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.41.0
Currently the EDAC subsystem relies on the low-level device drivers to
select an unique index for each memory controller available in the system.
Here are the already implemented approaches:
1. Fixed zero id. The vast majority of the drivers expect to have a single
memory controller in the system.
2. Calculate based on a platform-specific way (Pre-defined devices order,
PCIe-bus address, Numa node ID + PCIe-function number, etc).
3. Use platform_device->id.
4. Use custom ACPI/OF property value.
5. Use locally maintained static MC counter.
Create a generic method of the MC index allocation which could be utilized
for the case 5 (it doesn't imply any strict memory controller order) and
which would prevent the new MC EDAC drivers re-implementing the approaches
3 and 4. Moreover it will be useful for the cases when a platform is
equipped with memory-controllers of different types [1] and which are
probed by different drivers [2].
[1] Link: https://lore.kernel.org/all/9dc2a947-d2ab-4f00-8ed3-d2499cb6fdfd@BN1BFFO11FD002.protection.gbl/
[2] Link: https://lore.kernel.org/linux-edac/BY5PR12MB4258CB67B70D71F107EC1E9DDB3E9@BY5PR12MB4258.namprd12.prod.outlook.com
The suggested implementation is based on the IDA kernel API and implies
the next semantics:
1. If a particular MC index is specified it will be registered in the
IDR pool unless the specified ID has already been reserved.
2. If a special MC index is specified (EDAC_AUTO_MC_NUM) the EDAC
core will check whether there is a "mcID" alias is defined in the device
tree and use the ID from there if it's found.
3. Otherwise a next free index will be allocated and assigned to the
registered memory controller.
Signed-off-by: Serge Semin <[email protected]>
---
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.41.0
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 <[email protected]>
---
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 014a2176c2c1..b463bd802961 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -675,26 +675,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;
}
/**
@@ -731,19 +730,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.41.0
The mem_ctl_info.scrub_cap field is supposed to be set with the ECC
scrub-related flags. Instead the driver erroneously initializes it with
the SCRUB_HW_SRC flag ID. It's definitely wrong, though it hasn't caused
any problem so far since the structure field isn't used by the EDAC core.
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 <[email protected]>
---
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 b463bd802961..65790097beb2 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -856,7 +856,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.41.0
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 use
the FIELD_GET()/FIELD_PREP() macros introduced in the kernel specifically
for that case. In addition 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 having a suffix similar to the
snps_ecc_error_info structure fields name.
Signed-off-by: Serge Semin <[email protected]>
---
drivers/edac/synopsys_edac.c | 137 +++++++++++++++++------------------
1 file changed, 67 insertions(+), 70 deletions(-)
diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index bf23ed6e1779..327023e35d42 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/bitfield.h>
+#include <linux/bits.h>
#include <linux/edac.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -90,33 +92,29 @@
#define ZYNQMP_DDR_QOS_IRQ_DB_OFST 0x2020C
/* DDR Master register definitions */
-#define DDR_MSTR_DEV_CFG_MASK 0xC0000000
-#define DDR_MSTR_DEV_CFG_SHIFT 30
+#define DDR_MSTR_DEV_CFG_MASK GENMASK(31, 30)
#define DDR_MSTR_DEV_X4 0
#define DDR_MSTR_DEV_X8 1
#define DDR_MSTR_DEV_X16 2
#define DDR_MSTR_DEV_X32 3
-#define DDR_MSTR_BUSWIDTH_MASK 0x3000
-#define DDR_MSTR_BUSWIDTH_SHIFT 12
+#define DDR_MSTR_BUSWIDTH_MASK GENMASK(13, 12)
#define DDR_MSTR_BUSWIDTH_16 2
#define DDR_MSTR_BUSWIDTH_32 1
#define DDR_MSTR_BUSWIDTH_64 0
-#define DDR_MSTR_MEM_LPDDR4 0x20
-#define DDR_MSTR_MEM_DDR4 0x10
-#define DDR_MSTR_MEM_LPDDR3 0x8
-#define DDR_MSTR_MEM_DDR2 0x4
-#define DDR_MSTR_MEM_DDR3 0x1
+#define DDR_MSTR_MEM_LPDDR4 BIT(5)
+#define DDR_MSTR_MEM_DDR4 BIT(4)
+#define DDR_MSTR_MEM_LPDDR3 BIT(3)
+#define DDR_MSTR_MEM_DDR2 BIT(2)
+#define DDR_MSTR_MEM_DDR3 BIT(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)
@@ -127,34 +125,26 @@
#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
-
-/* 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
-
-/* 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_ERRCNT_UECNT_MASK GENMASK(31, 16)
+#define ECC_ERRCNT_CECNT_MASK GENMASK(15, 0)
+
+/* ECC Corrected Error register definitions */
+#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 definitions */
+#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)
/* 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
@@ -210,9 +200,9 @@
#define RANK_B0_BASE 6
/* ZynqMP DDR QOS Interrupt register definitions */
-#define ZYNQMP_DDR_QOS_UE_MASK 0x4
-#define ZYNQMP_DDR_QOS_CE_MASK 0x2
-#define ZYNQMP_DDR_QOS_IRQ_MASK 0x6
+#define ZYNQMP_DDR_QOS_UE_MASK BIT(2)
+#define ZYNQMP_DDR_QOS_CE_MASK BIT(1)
+#define ZYNQMP_DDR_QOS_IRQ_MASK (ZYNQMP_DDR_QOS_UE_MASK | ZYNQMP_DDR_QOS_CE_MASK)
/**
* struct snps_ecc_error_info - ECC error log information.
@@ -304,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:
spin_lock_irqsave(&priv->reglock, flags);
@@ -484,7 +476,7 @@ static enum dev_type snps_get_dtype(const void __iomem *base)
if (!(regval & DDR_MSTR_MEM_DDR4))
return DEV_UNKNOWN;
- regval = (regval & DDR_MSTR_DEV_CFG_MASK) >> DDR_MSTR_DEV_CFG_SHIFT;
+ regval = FIELD_GET(DDR_MSTR_DEV_CFG_MASK, regval);
switch (regval) {
case DDR_MSTR_DEV_X4:
return DEV_X4;
@@ -511,7 +503,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);
}
@@ -698,13 +691,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);
}
@@ -743,10 +736,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,
@@ -853,7 +850,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.41.0
Currently the custom error messages are needlessly long so the logged text
gets to be printed in several lines in console. There is some
duplicated/redundant information which can be freely removed from it: drop
the message prefix "DDR ECC error type:%s" since the resultant text
printed to the log by the edac_mc_printk() method will contain the error
type and the memory controller id referring to the device detected the
error anyway; with no harm to readability shorten out the phrase "Bit
Position" to being just "Bit".
Signed-off-by: Serge Semin <[email protected]>
---
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 f1ec44cdd87f..62f498b6dfed 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -479,13 +479,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);
}
@@ -498,13 +498,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.41.0
None of the ecc_error_info structure fields have "nr" suffix even though
each of them do represent some number (row number, column number, bank
number). Drop the suffix from the bankgrpnr field name for the sake of
unification then. Similarly 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 <[email protected]>
---
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 f2bcc3f79bf2..f1ec44cdd87f 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -269,17 +269,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;
};
/**
@@ -430,7 +430,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);
@@ -444,7 +444,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;
@@ -479,9 +479,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",
@@ -498,9 +498,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.41.0