Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1566537 > unrolled thread

Re: edac/i7300_edac.c:307: strange macro ?

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: edac/i7300_edac.c:307: strange macro ? Borislav Petkov <bp@alien8.de> - 2017-01-25 13:00 +0100
    Re: edac/i7300_edac.c:307: strange macro ? Borislav Petkov <bp@alien8.de> - 2017-01-25 16:40 +0100
      Re: edac/i7300_edac.c:307: strange macro ? Mauro Carvalho Chehab <mchehab@osg.samsung.com> - 2017-01-26 10:00 +0100
        Re: edac/i7300_edac.c:307: strange macro ? Borislav Petkov <bp@alien8.de> - 2017-01-26 11:40 +0100

#1566537 — Re: edac/i7300_edac.c:307: strange macro ?

On Wed, Jan 11, 2017 at 11:04:22PM +0000, David Binderman wrote:
> Thanks for the confirmation that my best guess looks good to you.

So what now, is there a fix in the form of a patch going to appear on
the horizon anytime soon?

:-)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [next] | [standalone]

#1566685

On Wed, Jan 25, 2017 at 12:04:04PM +0000, David Binderman wrote:
> You'll have a very long wait to get a linux patch from me.
> 
> I am happy for someone else to invent a patch.

Ah ok, I thought you wanted to give it a try and would want me to help
you with it. :-)

Anyway, here it is:

---
From: Borislav Petkov <bp@suse.de>
Date: Wed, 25 Jan 2017 16:08:27 +0100
Subject: [PATCH] EDAC, i7300: Test for the second channel properly

REDMEMB[17] is the ECC_Locator bit, which, when set, identifies the
CS[3:2] as the simbols in error. And thus the second channel.

The macro computing it was wrong so get rid of it (it was used at one
place only) and get rid of the conditional too. Generates better code
this way anyway.

Signed-off-by: Borislav Petkov <bp@suse.de>
Reported-by: David Binderman <dcb314@hotmail.com>
---
 drivers/edac/i7300_edac.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
index 0a912bf6de00..e391f5a716be 100644
--- a/drivers/edac/i7300_edac.c
+++ b/drivers/edac/i7300_edac.c
@@ -304,7 +304,6 @@ static const char *ferr_global_lo_name[] = {
 #define REDMEMA		0xdc
 
 #define REDMEMB		0x7c
-  #define IS_SECOND_CH(v)	((v) * (1 << 17))
 
 #define RECMEMA		0xe0
   #define RECMEMA_BANK(v)	(((v) >> 12) & 7)
@@ -483,8 +482,9 @@ static void i7300_process_fbd_error(struct mem_ctl_info *mci)
 		pci_read_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
 				     REDMEMB, &value);
 		channel = (branch << 1);
-		if (IS_SECOND_CH(value))
-			channel++;
+
+		/* Second channel ? */
+		channel += !!(value & BIT(17));
 
 		/* Clear the error bit */
 		pci_write_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
-- 
2.11.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1567188

Em Wed, 25 Jan 2017 16:37:28 +0100
Borislav Petkov <bp@alien8.de> escreveu:

> On Wed, Jan 25, 2017 at 12:04:04PM +0000, David Binderman wrote:
> > You'll have a very long wait to get a linux patch from me.
> > 
> > I am happy for someone else to invent a patch.  
> 
> Ah ok, I thought you wanted to give it a try and would want me to help
> you with it. :-)
> 
> Anyway, here it is:
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Wed, 25 Jan 2017 16:08:27 +0100
> Subject: [PATCH] EDAC, i7300: Test for the second channel properly
> 
> REDMEMB[17] is the ECC_Locator bit, which, when set, identifies the
> CS[3:2] as the simbols in error. And thus the second channel.
> 
> The macro computing it was wrong so get rid of it (it was used at one
> place only) and get rid of the conditional too. Generates better code
> this way anyway.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Reported-by: David Binderman <dcb314@hotmail.com>

Seems OK to me. Do you want to put it on your tree, or do you
prefer if I put it on mine?

If you prefer to send via your tree:

Reviewed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>


> ---
>  drivers/edac/i7300_edac.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
> index 0a912bf6de00..e391f5a716be 100644
> --- a/drivers/edac/i7300_edac.c
> +++ b/drivers/edac/i7300_edac.c
> @@ -304,7 +304,6 @@ static const char *ferr_global_lo_name[] = {
>  #define REDMEMA		0xdc
>  
>  #define REDMEMB		0x7c
> -  #define IS_SECOND_CH(v)	((v) * (1 << 17))
>  
>  #define RECMEMA		0xe0
>    #define RECMEMA_BANK(v)	(((v) >> 12) & 7)
> @@ -483,8 +482,9 @@ static void i7300_process_fbd_error(struct mem_ctl_info *mci)
>  		pci_read_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
>  				     REDMEMB, &value);
>  		channel = (branch << 1);
> -		if (IS_SECOND_CH(value))
> -			channel++;
> +
> +		/* Second channel ? */
> +		channel += !!(value & BIT(17));
>  
>  		/* Clear the error bit */
>  		pci_write_config_dword(pvt->pci_dev_16_1_fsb_addr_map,


-- 
Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1567311

On Thu, Jan 26, 2017 at 06:57:44AM -0200, Mauro Carvalho Chehab wrote:
> Seems OK to me. Do you want to put it on your tree, or do you
> prefer if I put it on mine?

I can carry it.

> If you prefer to send via your tree:
> 
> Reviewed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web