2022-09-10 19:45:16

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 05/19] EDAC/synopsys: Fix reading errors count before ECC status

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 <[email protected]>
---
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 da1d90a87778..558d3b3e6864 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -423,18 +423,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.37.2


2022-09-12 05:30:11

by Datta, Shubhrajyoti

[permalink] [raw]
Subject: RE: [PATCH v2 05/19] EDAC/synopsys: Fix reading errors count before ECC status

[AMD Official Use Only - General]



> -----Original Message-----
> From: Serge Semin <[email protected]>
> Sent: Sunday, September 11, 2022 1:12 AM
> To: Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Michal Simek
> <[email protected]>; Borislav Petkov <[email protected]>; Mauro
> Carvalho Chehab <[email protected]>; Tony Luck
> <[email protected]>; James Morse <[email protected]>; Robert
> Richter <[email protected]>; Shubhrajyoti Datta
> <[email protected]>
> Cc: Serge Semin <[email protected]>; Serge Semin
> <[email protected]>; Alexey Malahov
> <[email protected]>; Michail Ivanov
> <[email protected]>; Pavel Parkhomenko
> <[email protected]>; Punnaiah Choudary Kalluri
> <[email protected]>; Manish Narani
> <[email protected]>; Dinh Nguyen <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; Borislav Petkov <[email protected]>
> Subject: [PATCH v2 05/19] EDAC/synopsys: Fix reading errors count before
> ECC status
>
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> 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 <[email protected]>


Reviewed-by: Shubhrajyoti Datta <[email protected]>

> ---
> 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 da1d90a87778..558d3b3e6864 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -423,18 +423,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.37.2