[PATCH V2] EDAC: synopsys: Fix back to back snprintf() messages for CE/UE

[PATCH V2] EDAC: synopsys: Fix back to back snprintf() messages for CE/UE

[Sherry Sun]

The current way which call snprintf() function two or three times to
output message for CE/UE is incorrect, because it won't output all the
message needed, instead it will only output the last message in
snprintf(). So the simplest and most effective way to fix this problem
is combining all the snprintf() message needed for CE/UE into one
snprintf() function.

Fixes: b500b4a029d57 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller")
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: James Morse <james.morse@arm.com>
---
Changes in V2:
- Separated this patch from the original patchset.
- Change the subject to be more concise and clear.

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

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 2d263382d797..880ffd833718 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -479,20 +479,14 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 		pinf = &p->ceinfo;
 		if (!priv->p_data->quirks) {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Bank %d Col %d ",
-				  "CE", pinf->row, pinf->bank, pinf->col);
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Bit Position: %d Data: 0x%08x\n",
+				 "DDR ECC error type:%s Row %d Bank %d Col %d Bit Position: %d Data: 0x%08x",
+				 "CE", pinf->row, pinf->bank, pinf->col,
 				 pinf->bitpos, pinf->data);
 		} else {
 			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "DDR ECC error type:%s Row %d Bank %d Col %d ",
-				  "CE", pinf->row, pinf->bank, pinf->col);
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "BankGroup Number %d Block Number %d ",
-				 pinf->bankgrpnr, pinf->blknr);
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "Bit Position: %d Data: 0x%08x\n",
+				 "DDR ECC error type:%s Row %d Bank %d Col %d BankGroup Number %d Block Number %d Bit Position: %d Data: 0x%08x",
+				 "CE", pinf->row, pinf->bank, pinf->col,
+				 pinf->bankgrpnr, pinf->blknr,
 				 pinf->bitpos, pinf->data);
 		}
 
@@ -509,10 +503,8 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 				"UE", pinf->row, pinf->bank, pinf->col);
 		} 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);
-			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
-				 "BankGroup Number %d Block Number %d",
+				 "DDR ECC error type :%s Row %d Bank %d Col %d BankGroup Number %d Block Number %d",
+				 "UE", pinf->row, pinf->bank, pinf->col,
 				 pinf->bankgrpnr, pinf->blknr);
 		}
 
-- 
2.17.1

Re: [PATCH V2] EDAC: synopsys: Fix back to back snprintf() messages for CE/UE

[Borislav Petkov]

On Thu, Feb 27, 2020 at 04:34:12PM +0800, Sherry Sun wrote:
> The current way which call snprintf() function two or three times to
> output message for CE/UE is incorrect, because it won't output all the
> message needed, instead it will only output the last message in
> snprintf(). So the simplest and most effective way to fix this problem
> is combining all the snprintf() message needed for CE/UE into one
> snprintf() function.
> 
> Fixes: b500b4a029d57 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller")
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> Reviewed-by: James Morse <james.morse@arm.com>
> ---
> Changes in V2:
> - Separated this patch from the original patchset.
> - Change the subject to be more concise and clear.
> 
> ---
>  drivers/edac/synopsys_edac.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 2d263382d797..880ffd833718 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -479,20 +479,14 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
>  		pinf = &p->ceinfo;
>  		if (!priv->p_data->quirks) {
>  			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "DDR ECC error type:%s Row %d Bank %d Col %d ",
> -				  "CE", pinf->row, pinf->bank, pinf->col);
> -			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "Bit Position: %d Data: 0x%08x\n",
> +				 "DDR ECC error type:%s Row %d Bank %d Col %d Bit Position: %d Data: 0x%08x",
> +				 "CE", pinf->row, pinf->bank, pinf->col,
>  				 pinf->bitpos, pinf->data);
>  		} else {
>  			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "DDR ECC error type:%s Row %d Bank %d Col %d ",
> -				  "CE", pinf->row, pinf->bank, pinf->col);
> -			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "BankGroup Number %d Block Number %d ",
> -				 pinf->bankgrpnr, pinf->blknr);
> -			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "Bit Position: %d Data: 0x%08x\n",
> +				 "DDR ECC error type:%s Row %d Bank %d Col %d BankGroup Number %d Block Number %d Bit Position: %d Data: 0x%08x",
> +				 "CE", pinf->row, pinf->bank, pinf->col,
> +				 pinf->bankgrpnr, pinf->blknr,
>  				 pinf->bitpos, pinf->data);
>  		}
>  
> @@ -509,10 +503,8 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
>  				"UE", pinf->row, pinf->bank, pinf->col);
>  		} 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);
> -			snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
> -				 "BankGroup Number %d Block Number %d",
> +				 "DDR ECC error type :%s Row %d Bank %d Col %d BankGroup Number %d Block Number %d",
> +				 "UE", pinf->row, pinf->bank, pinf->col,
>  				 pinf->bankgrpnr, pinf->blknr);
>  		}
>  
> -- 

I'm wondering how Manish did not see this in testing? CCed.

Anyway, queueing for urgent.

Thx.

-- 
Regards/Gruss,
    Boris.

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