2015-02-05 11:51:08

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH] sb_edac: Fix detection on SNB machines

From: Borislav Petkov <[email protected]>

Commit 50d1bb93672f ("sb_edac: add support for Haswell based systems")
broke the driver on my SNB box with PCI ID 0x3ca0:

3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
...

because its probe routine gets handed in pdev->device: 0x3ca0 but we're
matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA and the
probing fails.

Adding 0x3ca0 fixes the issue and driver loads successfully again:

[ 2449.013120] EDAC DEBUG: sbridge_init:
[ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
[ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
[ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
[ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
[ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
[ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
...

Add a debug printk while at it to be able to catch the failure in the
future and dump driver version on successful load.

Cc: Tony Luck <[email protected]>
Cc: Aristeu Rozanski <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/edac/sb_edac.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 63aa6730e89e..2e1c03a64751 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -2448,6 +2448,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
type = IVY_BRIDGE;
break;
case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
+ case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
type = SANDY_BRIDGE;
break;
@@ -2460,8 +2461,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
type = BROADWELL;
break;
}
- if (unlikely(rc < 0))
+ if (unlikely(rc < 0)) {
+ edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
goto fail0;
+ }
+
mc = 0;

list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
@@ -2474,7 +2478,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto fail1;
}

- sbridge_printk(KERN_INFO, "Driver loaded.\n");
+ sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);

mutex_unlock(&sbridge_edac_lock);
return 0;
--
2.2.0.33.gc18b867


2015-02-05 12:37:46

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Thu, Feb 05, 2015 at 12:50:35PM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Commit 50d1bb93672f ("sb_edac: add support for Haswell based systems")
> broke the driver on my SNB box with PCI ID 0x3ca0:
>
> 3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
> 00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
> ...
>
> because its probe routine gets handed in pdev->device: 0x3ca0 but we're
> matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA and the
> probing fails.

Or was it Andy who broke it?!

>From looking at

d0585cd815fa ("sb_edac: Claim a different PCI device")

static const struct pci_device_id sbridge_pci_tbl[] = {
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA)},
+ {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0)},

Yeah, it looks like it:

We're working with the IMC_TA device for matching but we get handed in
the IMC_HA0 device after this patch which confirms with my observations.

tztztz, people, can we decide on one PCI device and stick with it?

:-)

[ Leaving in the rest for Andy. ]

> Adding 0x3ca0 fixes the issue and driver loads successfully again:
>
> [ 2449.013120] EDAC DEBUG: sbridge_init:
> [ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> [ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
> [ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> [ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> [ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
> [ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> ...
>
> Add a debug printk while at it to be able to catch the failure in the
> future and dump driver version on successful load.
>
> Cc: Tony Luck <[email protected]>
> Cc: Aristeu Rozanski <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> ---
> drivers/edac/sb_edac.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> index 63aa6730e89e..2e1c03a64751 100644
> --- a/drivers/edac/sb_edac.c
> +++ b/drivers/edac/sb_edac.c
> @@ -2448,6 +2448,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> type = IVY_BRIDGE;
> break;
> case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
> + case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
> rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
> type = SANDY_BRIDGE;
> break;
> @@ -2460,8 +2461,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> type = BROADWELL;
> break;
> }
> - if (unlikely(rc < 0))
> + if (unlikely(rc < 0)) {
> + edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
> goto fail0;
> + }
> +
> mc = 0;
>
> list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
> @@ -2474,7 +2478,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> goto fail1;
> }
>
> - sbridge_printk(KERN_INFO, "Driver loaded.\n");
> + sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);
>
> mutex_unlock(&sbridge_edac_lock);
> return 0;
> --
> 2.2.0.33.gc18b867
>
>

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-02-08 16:54:30

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Feb 5, 2015 4:37 AM, "Borislav Petkov" <[email protected]> wrote:
>
> On Thu, Feb 05, 2015 at 12:50:35PM +0100, Borislav Petkov wrote:
> > From: Borislav Petkov <[email protected]>
> >
> > Commit 50d1bb93672f ("sb_edac: add support for Haswell based systems")
> > broke the driver on my SNB box with PCI ID 0x3ca0:
> >
> > 3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
> > 00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
> > ...
> >
> > because its probe routine gets handed in pdev->device: 0x3ca0 but we're
> > matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA and the
> > probing fails.
>
> Or was it Andy who broke it?!
>
> From looking at
>
> d0585cd815fa ("sb_edac: Claim a different PCI device")
>
> static const struct pci_device_id sbridge_pci_tbl[] = {
> - {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA)},
> + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0)},
>
> Yeah, it looks like it:
>
> We're working with the IMC_TA device for matching but we get handed in
> the IMC_HA0 device after this patch which confirms with my observations.
>
> tztztz, people, can we decide on one PCI device and stick with it?

Can you send your CPU model and the full output of lspci -nn.

If there isn't an obvious fix, I'd be okay with reverting, too. The
i2c stuff is delayed because the outcome of my investigation is that
it has real problems, and it'll probably have to wait until later this
year, or a scary module param, to deal with it.

--Andy

>
> :-)
>
> [ Leaving in the rest for Andy. ]
>
> > Adding 0x3ca0 fixes the issue and driver loads successfully again:
> >
> > [ 2449.013120] EDAC DEBUG: sbridge_init:
> > [ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> > [ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
> > [ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> > [ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> > [ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
> > [ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> > ...
> >
> > Add a debug printk while at it to be able to catch the failure in the
> > future and dump driver version on successful load.
> >
> > Cc: Tony Luck <[email protected]>
> > Cc: Aristeu Rozanski <[email protected]>
> > Cc: Mauro Carvalho Chehab <[email protected]>
> > Signed-off-by: Borislav Petkov <[email protected]>
> > ---
> > drivers/edac/sb_edac.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> > index 63aa6730e89e..2e1c03a64751 100644
> > --- a/drivers/edac/sb_edac.c
> > +++ b/drivers/edac/sb_edac.c
> > @@ -2448,6 +2448,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > type = IVY_BRIDGE;
> > break;
> > case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
> > + case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
> > rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
> > type = SANDY_BRIDGE;
> > break;
> > @@ -2460,8 +2461,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > type = BROADWELL;
> > break;
> > }
> > - if (unlikely(rc < 0))
> > + if (unlikely(rc < 0)) {
> > + edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
> > goto fail0;
> > + }
> > +
> > mc = 0;
> >
> > list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
> > @@ -2474,7 +2478,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > goto fail1;
> > }
> >
> > - sbridge_printk(KERN_INFO, "Driver loaded.\n");
> > + sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);
> >
> > mutex_unlock(&sbridge_edac_lock);
> > return 0;
> > --
> > 2.2.0.33.gc18b867
> >
> >
>
> --
> Regards/Gruss,
> Boris.
>
> ECO tip #101: Trim your mails when you reply.
> --

2015-02-08 18:19:53

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Sun, Feb 08, 2015 at 08:54:06AM -0800, Andy Lutomirski wrote:
> If there isn't an obvious fix, I'd be okay with reverting, too. The

I think the obvious fix is this:

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 63aa6730e89e..1acf57ba4c86 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -2447,7 +2447,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_ibridge_table);
type = IVY_BRIDGE;
break;
- case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
+ case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
type = SANDY_BRIDGE;
break;
---

because you changed it to match the

PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0

device in the id_table but sbridge_probe() still matches against

PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA

when it is about to call sbridge_get_all_devices() to get the rest of
the SNB PCI devices. Currently, it simply doesn't match any of the case
options in the switch-case statement there.

And this is what I'm seeing here: we match the 0x3ca8 device in
sbridge_probe() but it gets handed in 0x3ca0 due to your change.

I'll run this tomorrow to confirm but this looks like the correct fix to
me. And it makes sense for a change :)

Thanks.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-02-09 12:17:55

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Sun, Feb 08, 2015 at 07:19:16PM +0100, Borislav Petkov wrote:
> On Sun, Feb 08, 2015 at 08:54:06AM -0800, Andy Lutomirski wrote:
> > If there isn't an obvious fix, I'd be okay with reverting, too. The
>
> I think the obvious fix is this:

Yes, it is, of course. If no one objects, I'll send it to Linus after
3.20-rc1 is out.

---
From: Borislav Petkov <[email protected]>
Subject: [PATCH] sb_edac: Fix detection on SNB machines

d0585cd815fa ("sb_edac: Claim a different PCI device") changed the
probing of sb_edac to look for PCI device 0x3ca0:

3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
...

but we're matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA
in sbridge_probe() therefore the probing fails.

Changing it to probe for 0x3ca0 (PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0),
.i.e., the 14.0 device, fixes the issue and driver loads successfully
again:

[ 2449.013120] EDAC DEBUG: sbridge_init:
[ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
[ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
[ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
[ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
[ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
[ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
...

Add a debug printk while at it to be able to catch the failure in the
future and dump driver version on successful load.

Fixes: d0585cd815fa ("sb_edac: Claim a different PCI device")
Cc: [email protected] # 3.18
Cc: Tony Luck <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/edac/sb_edac.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 63aa6730e89e..1acf57ba4c86 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -2447,7 +2447,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_ibridge_table);
type = IVY_BRIDGE;
break;
- case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
+ case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
type = SANDY_BRIDGE;
break;
@@ -2460,8 +2460,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
type = BROADWELL;
break;
}
- if (unlikely(rc < 0))
+ if (unlikely(rc < 0)) {
+ edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
goto fail0;
+ }
+
mc = 0;

list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
@@ -2474,7 +2477,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto fail1;
}

- sbridge_printk(KERN_INFO, "Driver loaded.\n");
+ sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);

mutex_unlock(&sbridge_edac_lock);
return 0;
--
2.2.0.33.gc18b867


--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-02-09 14:21:26

by Aristeu Rozanski

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Mon, Feb 09, 2015 at 01:17:17PM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
> Subject: [PATCH] sb_edac: Fix detection on SNB machines
>
> d0585cd815fa ("sb_edac: Claim a different PCI device") changed the
> probing of sb_edac to look for PCI device 0x3ca0:
>
> 3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
> 00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
> ...
>
> but we're matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA
> in sbridge_probe() therefore the probing fails.
>
> Changing it to probe for 0x3ca0 (PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0),
> .i.e., the 14.0 device, fixes the issue and driver loads successfully
> again:
>
> [ 2449.013120] EDAC DEBUG: sbridge_init:
> [ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> [ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
> [ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> [ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> [ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
> [ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> ...
>
> Add a debug printk while at it to be able to catch the failure in the
> future and dump driver version on successful load.
>
> Fixes: d0585cd815fa ("sb_edac: Claim a different PCI device")
> Cc: [email protected] # 3.18
> Cc: Tony Luck <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> ---
> drivers/edac/sb_edac.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> index 63aa6730e89e..1acf57ba4c86 100644
> --- a/drivers/edac/sb_edac.c
> +++ b/drivers/edac/sb_edac.c
> @@ -2447,7 +2447,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_ibridge_table);
> type = IVY_BRIDGE;
> break;
> - case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
> + case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
> rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
> type = SANDY_BRIDGE;
> break;
> @@ -2460,8 +2460,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> type = BROADWELL;
> break;
> }
> - if (unlikely(rc < 0))
> + if (unlikely(rc < 0)) {
> + edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
> goto fail0;
> + }
> +
> mc = 0;
>
> list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
> @@ -2474,7 +2477,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> goto fail1;
> }
>
> - sbridge_printk(KERN_INFO, "Driver loaded.\n");
> + sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);
>
> mutex_unlock(&sbridge_edac_lock);
> return 0;

looks good to me

Acked-by: Aristeu Rozanski <[email protected]>


--
Aristeu

2015-02-09 15:23:26

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

Em Mon, 09 Feb 2015 09:20:58 -0500
Aristeu Rozanski <[email protected]> escreveu:

> On Mon, Feb 09, 2015 at 01:17:17PM +0100, Borislav Petkov wrote:
> > From: Borislav Petkov <[email protected]>
> > Subject: [PATCH] sb_edac: Fix detection on SNB machines
> >
> > d0585cd815fa ("sb_edac: Claim a different PCI device") changed the
> > probing of sb_edac to look for PCI device 0x3ca0:
> >
> > 3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
> > 00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
> > ...
> >
> > but we're matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA
> > in sbridge_probe() therefore the probing fails.
> >
> > Changing it to probe for 0x3ca0 (PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0),
> > .i.e., the 14.0 device, fixes the issue and driver loads successfully
> > again:
> >
> > [ 2449.013120] EDAC DEBUG: sbridge_init:
> > [ 2449.017029] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> > [ 2449.022368] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca0
> > [ 2449.028498] EDAC sbridge: Seeking for: PCI ID 8086:3ca0
> > [ 2449.033768] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> > [ 2449.039028] EDAC DEBUG: sbridge_get_onedevice: Detected 8086:3ca8
> > [ 2449.045155] EDAC sbridge: Seeking for: PCI ID 8086:3ca8
> > ...
> >
> > Add a debug printk while at it to be able to catch the failure in the
> > future and dump driver version on successful load.
> >
> > Fixes: d0585cd815fa ("sb_edac: Claim a different PCI device")
> > Cc: [email protected] # 3.18
> > Cc: Tony Luck <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Mauro Carvalho Chehab <[email protected]>
> > Signed-off-by: Borislav Petkov <[email protected]>
> > ---
> > drivers/edac/sb_edac.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> > index 63aa6730e89e..1acf57ba4c86 100644
> > --- a/drivers/edac/sb_edac.c
> > +++ b/drivers/edac/sb_edac.c
> > @@ -2447,7 +2447,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_ibridge_table);
> > type = IVY_BRIDGE;
> > break;
> > - case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA:
> > + case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
> > rc = sbridge_get_all_devices(&num_mc, pci_dev_descr_sbridge_table);
> > type = SANDY_BRIDGE;
> > break;
> > @@ -2460,8 +2460,11 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > type = BROADWELL;
> > break;
> > }
> > - if (unlikely(rc < 0))
> > + if (unlikely(rc < 0)) {
> > + edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
> > goto fail0;
> > + }
> > +
> > mc = 0;
> >
> > list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
> > @@ -2474,7 +2477,7 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > goto fail1;
> > }
> >
> > - sbridge_printk(KERN_INFO, "Driver loaded.\n");
> > + sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);
> >
> > mutex_unlock(&sbridge_edac_lock);
> > return 0;
>
> looks good to me
>
> Acked-by: Aristeu Rozanski <[email protected]>
>
>

Looks good to me.

Acked-by: Mauro Carvalho Chehab <[email protected]>

2015-02-09 15:33:41

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] sb_edac: Fix detection on SNB machines

On Mon, Feb 9, 2015 at 7:23 AM, Mauro Carvalho Chehab
<[email protected]> wrote:
> Em Mon, 09 Feb 2015 09:20:58 -0500
> Aristeu Rozanski <[email protected]> escreveu:
>
>> On Mon, Feb 09, 2015 at 01:17:17PM +0100, Borislav Petkov wrote:
>> > From: Borislav Petkov <[email protected]>
>> > Subject: [PATCH] sb_edac: Fix detection on SNB machines
>> >
>> > d0585cd815fa ("sb_edac: Claim a different PCI device") changed the
>> > probing of sb_edac to look for PCI device 0x3ca0:
>> >
>> > 3f:0e.0 System peripheral: Intel Corporation Xeon E5/Core i7 Processor Home Agent (rev 07)
>> > 00: 86 80 a0 3c 00 00 00 00 07 00 80 08 00 00 80 00
>> > ...
>> >
>> > but we're matching for 0x3ca8, i.e. PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA
>> > in sbridge_probe() therefore the probing fails.
>> >
>> > Changing it to probe for 0x3ca0 (PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0),
>> > .i.e., the 14.0 device, fixes the issue and driver loads successfully
>> > again:

Looks good to me, too.

--Andy