[PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Shubhrajyoti Datta]

Currently the error count from status register is being read which
is not correct. Fix the issue by reading the count from the
error count register(ERRCNT).

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v2:
Remove the cumulative count change

 drivers/edac/synopsys_edac.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index f05ff02c0656..1a9a5b886903 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -164,6 +164,11 @@
 #define ECC_STAT_CECNT_SHIFT		8
 #define ECC_STAT_BITNUM_MASK		0x7F
 
+/* ECC error count register definitions */
+#define ECC_ERRCNT_UECNT_MASK		0xFFFF0000
+#define ECC_ERRCNT_UECNT_SHIFT		16
+#define ECC_ERRCNT_CECNT_MASK		0xFFFF
+
 /* DDR QOS Interrupt register definitions */
 #define DDR_QOS_IRQ_STAT_OFST		0x20200
 #define DDR_QOSUE_MASK			0x4
@@ -423,14 +428,16 @@ 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->ce_cnt = (regval & ECC_STAT_CECNT_MASK) >> ECC_STAT_CECNT_SHIFT;
-	p->ue_cnt = (regval & ECC_STAT_UECNT_MASK) >> ECC_STAT_UECNT_SHIFT;
-	if (!p->ce_cnt)
-		goto ue_err;
 
 	p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
 
-- 
2.17.1

Re: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Michal Simek]

On 3/18/22 11:19, Shubhrajyoti Datta wrote:
> Currently the error count from status register is being read which
> is not correct. Fix the issue by reading the count from the
> error count register(ERRCNT).
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2:
> Remove the cumulative count change


CR number?
M

> 
>   drivers/edac/synopsys_edac.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index f05ff02c0656..1a9a5b886903 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -164,6 +164,11 @@
>   #define ECC_STAT_CECNT_SHIFT		8
>   #define ECC_STAT_BITNUM_MASK		0x7F
>   
> +/* ECC error count register definitions */
> +#define ECC_ERRCNT_UECNT_MASK		0xFFFF0000
> +#define ECC_ERRCNT_UECNT_SHIFT		16
> +#define ECC_ERRCNT_CECNT_MASK		0xFFFF
> +
>   /* DDR QOS Interrupt register definitions */
>   #define DDR_QOS_IRQ_STAT_OFST		0x20200
>   #define DDR_QOSUE_MASK			0x4
> @@ -423,14 +428,16 @@ 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->ce_cnt = (regval & ECC_STAT_CECNT_MASK) >> ECC_STAT_CECNT_SHIFT;
> -	p->ue_cnt = (regval & ECC_STAT_UECNT_MASK) >> ECC_STAT_UECNT_SHIFT;
> -	if (!p->ce_cnt)
> -		goto ue_err;
>   
>   	p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
>   

Re: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Michal Simek]

On 3/18/22 11:33, Michal Simek wrote:
> 
> 
> On 3/18/22 11:19, Shubhrajyoti Datta wrote:
>> Currently the error count from status register is being read which
>> is not correct. Fix the issue by reading the count from the
>> error count register(ERRCNT).
>>
>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>> ---
>> v2:
>> Remove the cumulative count change
> 
> 
> CR number?

Please ignore my comment. I thought it is internal patch.

Thanks,
Michal

RE: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Shubhrajyoti Datta]

> -----Original Message-----
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Sent: Friday, March 18, 2022 3:49 PM
> To: linux-edac@vger.kernel.org
> Cc: Michal Simek <michals@xilinx.com>; rric@kernel.org; bp@alien8.de;
> mchehab@kernel.org; tony.luck@intel.com; git <git@xilinx.com>;
> Shubhrajyoti Datta <shubhraj@xilinx.com>
> Subject: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error
> count
> 
> Currently the error count from status register is being read which is not
> correct. Fix the issue by reading the count from the error count
> register(ERRCNT).
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
If there are no further comments can the patch be  merged.

Re: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Borislav Petkov]

On Fri, Mar 18, 2022 at 03:49:00PM +0530, Shubhrajyoti Datta wrote:
> Currently the error count from status register is being read which
> is not correct. Fix the issue by reading the count from the
> error count register(ERRCNT).
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

I'm guessing

Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller")

and also

Cc: <stable@vger.kernel.org>

?

Also, that driver has a maintainer:

$ ./scripts/get_maintainer.pl -f drivers/edac/synopsys_edac.c
Michal Simek <michal.simek@xilinx.com> (supporter:ARM/ZYNQ ARCHITECTURE)

Is he going to ACK this or so?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH v2] edac: synopsys: Fix the issue in reporting of the error count

[Shubhrajyoti Datta]

> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Thursday, April 14, 2022 1:47 PM
> To: Shubhrajyoti Datta <shubhraj@xilinx.com>; Michal Simek
> <michals@xilinx.com>
> Cc: linux-edac@vger.kernel.org; rric@kernel.org; mchehab@kernel.org;
> tony.luck@intel.com; git <git@xilinx.com>
> Subject: Re: [PATCH v2] edac: synopsys: Fix the issue in reporting of the
> error count
> 
> On Fri, Mar 18, 2022 at 03:49:00PM +0530, Shubhrajyoti Datta wrote:
> > Currently the error count from status register is being read which is
> > not correct. Fix the issue by reading the count from the error count
> > register(ERRCNT).
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> I'm guessing
> 
> Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR
> controller")
> 
> and also
> 
> Cc: <stable@vger.kernel.org>
> 
Thanks for the review 
Fixed in the next version

> ?
> 
> Also, that driver has a maintainer:
> 
> $ ./scripts/get_maintainer.pl -f drivers/edac/synopsys_edac.c Michal Simek
> <michal.simek@xilinx.com> (supporter:ARM/ZYNQ ARCHITECTURE)
> 
> Is he going to ACK this or so?
> 
> --
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette