* [PATCH 0/2] sb_edac: i2c_imc staging submission prep @ 2014-08-14 21:45 Andy Lutomirski 2014-08-14 21:45 ` [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h Andy Lutomirski 2014-08-14 21:45 ` [PATCH 2/2] sb_edac: Claim a different PCI device Andy Lutomirski 0 siblings, 2 replies; 10+ messages in thread From: Andy Lutomirski @ 2014-08-14 21:45 UTC (permalink / raw) To: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman Cc: Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Andy Lutomirski I'd like to submit my i2c_imc driver to -staging, but the sb_edac driver is currently squatting on my pci id :) sb_edac is a strange beast: it uses registers from several PCI devcies, but the one that it registers with the driver core is the SMBUS controller. This trivial series moves sb_edac's PCI ids to pci_ids.h (they're not exclusive to the EDAC hardware) and changes the PCI ID that is used to detect the EDAC hardware. I think that i2c_imc is a good staging candidate: the driver is IMO quite clean, the hardware is very common, and I know of some users (unrelated to me!) that use it for development, but it's not yet acceptable as a real driver. In particular, it needs confirmation from Intel as to whether it handshakes correctly with BIOS. In the mean time, it's perfectly safe to use *if you know that your system isn't doing something special with its DIMM SMBUS registers*. I have reason to believe that I may be able to get a information or a review from the right people at Intel in a couple of months, and I suspect that some people in the NV-DIMM community would be interested in this stuff. I realize that the timing is a bit awkward here. These patches have been floating around for almost a year. I'd be okay with them going in for 3.17 or 3.18. If I understand correctly, the deadline for staging drivers is much later than the merge window, but I don't want to submit the i2c_imc driver itself to staging until these prep patches are in. Andy Lutomirski (2): Move Intel SNB device ids from sb_edac to pci_ids.h sb_edac: Claim a different PCI device drivers/edac/sb_edac.c | 32 +------------------------------- include/linux/pci_ids.h | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 31 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h 2014-08-14 21:45 [PATCH 0/2] sb_edac: i2c_imc staging submission prep Andy Lutomirski @ 2014-08-14 21:45 ` Andy Lutomirski 2014-09-05 23:41 ` Bjorn Helgaas 2014-09-08 14:08 ` Aristeu Rozanski 2014-08-14 21:45 ` [PATCH 2/2] sb_edac: Claim a different PCI device Andy Lutomirski 1 sibling, 2 replies; 10+ messages in thread From: Andy Lutomirski @ 2014-08-14 21:45 UTC (permalink / raw) To: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman Cc: Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Andy Lutomirski, Rui Wang The i2c_imc driver will use two of them, and moving only part of the list seems messier. Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> Cc: Rui Wang <ruiv.wang@gmail.com> Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- drivers/edac/sb_edac.c | 30 ------------------------------ include/linux/pci_ids.h | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index deea0dc9999b..a2597e9313c6 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -52,36 +52,6 @@ static int probed; #define GET_BITFIELD(v, lo, hi) \ (((v) & GENMASK_ULL(hi, lo)) >> (lo)) -/* - * sbridge Memory Controller Registers - */ - -/* - * FIXME: For now, let's order by device function, as it makes - * easier for driver's development process. This table should be - * moved to pci_id.h when submitted upstream - */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ - - /* - * Currently, unused, but will be needed in the future - * implementations, as they hold the error counters - */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ - /* Devices 12 Function 6, Offsets 0x80 to 0xcc */ static const u32 sbridge_dram_rule[] = { 0x80, 0x88, 0x90, 0x98, 0xa0, diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 7fa31731c854..e0e6801c3d80 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2816,7 +2816,22 @@ #define PCI_DEVICE_ID_INTEL_UNC_R2PCIE 0x3c43 #define PCI_DEVICE_ID_INTEL_UNC_R3QPI0 0x3c44 #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 -- 1.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h 2014-08-14 21:45 ` [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h Andy Lutomirski @ 2014-09-05 23:41 ` Bjorn Helgaas 2014-09-05 23:42 ` Andy Lutomirski 2014-09-08 14:08 ` Aristeu Rozanski 1 sibling, 1 reply; 10+ messages in thread From: Bjorn Helgaas @ 2014-09-05 23:41 UTC (permalink / raw) To: Andy Lutomirski Cc: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman, Borislav Petkov, linux-edac, linux-kernel, linux-pci, Rui Wang On Thu, Aug 14, 2014 at 02:45:40PM -0700, Andy Lutomirski wrote: > The i2c_imc driver will use two of them, and moving only part of > the list seems messier. > > Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > Cc: Rui Wang <ruiv.wang@gmail.com> > Signed-off-by: Andy Lutomirski <luto@amacapital.net> Will these IDs actually be used by two drivers by time you're done? It looks like maybe you're removing them from sb_edac.c and adding them to i2c_imc.c, which would still only be one place. But if they will end up being used in more than one place, Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/edac/sb_edac.c | 30 ------------------------------ > include/linux/pci_ids.h | 15 +++++++++++++++ > 2 files changed, 15 insertions(+), 30 deletions(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index deea0dc9999b..a2597e9313c6 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -52,36 +52,6 @@ static int probed; > #define GET_BITFIELD(v, lo, hi) \ > (((v) & GENMASK_ULL(hi, lo)) >> (lo)) > > -/* > - * sbridge Memory Controller Registers > - */ > - > -/* > - * FIXME: For now, let's order by device function, as it makes > - * easier for driver's development process. This table should be > - * moved to pci_id.h when submitted upstream > - */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ > - > - /* > - * Currently, unused, but will be needed in the future > - * implementations, as they hold the error counters > - */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ > - > /* Devices 12 Function 6, Offsets 0x80 to 0xcc */ > static const u32 sbridge_dram_rule[] = { > 0x80, 0x88, 0x90, 0x98, 0xa0, > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 7fa31731c854..e0e6801c3d80 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -2816,7 +2816,22 @@ > #define PCI_DEVICE_ID_INTEL_UNC_R2PCIE 0x3c43 > #define PCI_DEVICE_ID_INTEL_UNC_R3QPI0 0x3c44 > #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ > #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ > #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f > #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 > #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 > -- > 1.9.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h 2014-09-05 23:41 ` Bjorn Helgaas @ 2014-09-05 23:42 ` Andy Lutomirski 0 siblings, 0 replies; 10+ messages in thread From: Andy Lutomirski @ 2014-09-05 23:42 UTC (permalink / raw) To: Bjorn Helgaas Cc: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman, Borislav Petkov, linux-edac, linux-kernel, linux-pci, Rui Wang On Fri, Sep 5, 2014 at 4:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Thu, Aug 14, 2014 at 02:45:40PM -0700, Andy Lutomirski wrote: >> The i2c_imc driver will use two of them, and moving only part of >> the list seems messier. >> >> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> >> Cc: Rui Wang <ruiv.wang@gmail.com> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> > > Will these IDs actually be used by two drivers by time you're done? It > looks like maybe you're removing them from sb_edac.c and adding them to > i2c_imc.c, which would still only be one place. > > But if they will end up being used in more than one place, > They will. Both drivers will need to poke at (different) configuration space registers on the same devices. There's one shared register, but that one is read-only. --Andy > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> --- >> drivers/edac/sb_edac.c | 30 ------------------------------ >> include/linux/pci_ids.h | 15 +++++++++++++++ >> 2 files changed, 15 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c >> index deea0dc9999b..a2597e9313c6 100644 >> --- a/drivers/edac/sb_edac.c >> +++ b/drivers/edac/sb_edac.c >> @@ -52,36 +52,6 @@ static int probed; >> #define GET_BITFIELD(v, lo, hi) \ >> (((v) & GENMASK_ULL(hi, lo)) >> (lo)) >> >> -/* >> - * sbridge Memory Controller Registers >> - */ >> - >> -/* >> - * FIXME: For now, let's order by device function, as it makes >> - * easier for driver's development process. This table should be >> - * moved to pci_id.h when submitted upstream >> - */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ >> - >> - /* >> - * Currently, unused, but will be needed in the future >> - * implementations, as they hold the error counters >> - */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ >> -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ >> - >> /* Devices 12 Function 6, Offsets 0x80 to 0xcc */ >> static const u32 sbridge_dram_rule[] = { >> 0x80, 0x88, 0x90, 0x98, 0xa0, >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index 7fa31731c854..e0e6801c3d80 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -2816,7 +2816,22 @@ >> #define PCI_DEVICE_ID_INTEL_UNC_R2PCIE 0x3c43 >> #define PCI_DEVICE_ID_INTEL_UNC_R3QPI0 0x3c44 >> #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ >> #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ >> +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ >> #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f >> #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 >> #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 >> -- >> 1.9.3 >> -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h 2014-08-14 21:45 ` [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h Andy Lutomirski 2014-09-05 23:41 ` Bjorn Helgaas @ 2014-09-08 14:08 ` Aristeu Rozanski 1 sibling, 0 replies; 10+ messages in thread From: Aristeu Rozanski @ 2014-09-08 14:08 UTC (permalink / raw) To: Andy Lutomirski Cc: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman, Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Rui Wang On Thu, Aug 14, 2014 at 02:45:40PM -0700, Andy Lutomirski wrote: > The i2c_imc driver will use two of them, and moving only part of > the list seems messier. > > Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > Cc: Rui Wang <ruiv.wang@gmail.com> > Signed-off-by: Andy Lutomirski <luto@amacapital.net> > --- > drivers/edac/sb_edac.c | 30 ------------------------------ > include/linux/pci_ids.h | 15 +++++++++++++++ > 2 files changed, 15 insertions(+), 30 deletions(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index deea0dc9999b..a2597e9313c6 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -52,36 +52,6 @@ static int probed; > #define GET_BITFIELD(v, lo, hi) \ > (((v) & GENMASK_ULL(hi, lo)) >> (lo)) > > -/* > - * sbridge Memory Controller Registers > - */ > - > -/* > - * FIXME: For now, let's order by device function, as it makes > - * easier for driver's development process. This table should be > - * moved to pci_id.h when submitted upstream > - */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ > - > - /* > - * Currently, unused, but will be needed in the future > - * implementations, as they hold the error counters > - */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ > -#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ > - > /* Devices 12 Function 6, Offsets 0x80 to 0xcc */ > static const u32 sbridge_dram_rule[] = { > 0x80, 0x88, 0x90, 0x98, 0xa0, > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 7fa31731c854..e0e6801c3d80 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -2816,7 +2816,22 @@ > #define PCI_DEVICE_ID_INTEL_UNC_R2PCIE 0x3c43 > #define PCI_DEVICE_ID_INTEL_UNC_R3QPI0 0x3c44 > #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS 0x3c71 /* 15.1 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0 0x3c72 /* 16.2 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1 0x3c73 /* 16.3 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2 0x3c76 /* 16.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3 0x3c77 /* 16.7 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 /* 14.0 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 /* 15.0 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa /* 15.2 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab /* 15.3 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac /* 15.4 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad /* 15.5 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 /* 17.0 */ > #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 /* 12.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 /* 13.6 */ > +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 /* 12.7 */ > #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f > #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 > #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 Acked-by: Aristeu Rozanski <aris@redhat.com> -- Aristeu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] sb_edac: Claim a different PCI device 2014-08-14 21:45 [PATCH 0/2] sb_edac: i2c_imc staging submission prep Andy Lutomirski 2014-08-14 21:45 ` [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h Andy Lutomirski @ 2014-08-14 21:45 ` Andy Lutomirski 2014-09-08 14:10 ` Aristeu Rozanski 1 sibling, 1 reply; 10+ messages in thread From: Andy Lutomirski @ 2014-08-14 21:45 UTC (permalink / raw) To: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman Cc: Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Andy Lutomirski, Rui Wang sb_edac controls a large number of different PCI functions. Rather than registering as a normal PCI driver for all of them, it registers for just one so that it gets probed and, at probe time, it looks for all the others. Coincidentally, the device it registers for also contains the SMBUS registers, so the PCI core will refuse to probe both sb_edac and a future iMC SMBUS driver. The drivers don't actually conflict, so just change sb_edac's device table to probe a different device. An alternative fix would be to merge the two drivers, but sb_edac will also refuse to load on non-ECC systems, whereas i2c_imc would still be useful without ECC. The only user-visible change should be that sb_edac appears to bind a different device. Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> Cc: Rui Wang <ruiv.wang@gmail.com> Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- drivers/edac/sb_edac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index a2597e9313c6..e3bc2cced580 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -432,7 +432,7 @@ static const struct pci_id_table pci_dev_descr_ibridge_table[] = { * pci_device_id table for which devices we are looking for */ 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)}, {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)}, {0,} /* 0 terminated list. */ }; -- 1.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sb_edac: Claim a different PCI device 2014-08-14 21:45 ` [PATCH 2/2] sb_edac: Claim a different PCI device Andy Lutomirski @ 2014-09-08 14:10 ` Aristeu Rozanski 2014-09-08 21:49 ` Andy Lutomirski 0 siblings, 1 reply; 10+ messages in thread From: Aristeu Rozanski @ 2014-09-08 14:10 UTC (permalink / raw) To: Andy Lutomirski Cc: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman, Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Rui Wang On Thu, Aug 14, 2014 at 02:45:41PM -0700, Andy Lutomirski wrote: > sb_edac controls a large number of different PCI functions. Rather > than registering as a normal PCI driver for all of them, it > registers for just one so that it gets probed and, at probe time, it > looks for all the others. > > Coincidentally, the device it registers for also contains the SMBUS > registers, so the PCI core will refuse to probe both sb_edac and a > future iMC SMBUS driver. The drivers don't actually conflict, so > just change sb_edac's device table to probe a different device. > > An alternative fix would be to merge the two drivers, but sb_edac > will also refuse to load on non-ECC systems, whereas i2c_imc would > still be useful without ECC. > > The only user-visible change should be that sb_edac appears to bind > a different device. > > Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > Cc: Rui Wang <ruiv.wang@gmail.com> > Signed-off-by: Andy Lutomirski <luto@amacapital.net> > --- > drivers/edac/sb_edac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index a2597e9313c6..e3bc2cced580 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -432,7 +432,7 @@ static const struct pci_id_table pci_dev_descr_ibridge_table[] = { > * pci_device_id table for which devices we are looking for > */ > 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)}, > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)}, > {0,} /* 0 terminated list. */ > }; Acked-by: Aristeu Rozanski <aris@redhat.com> -- Aristeu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sb_edac: Claim a different PCI device 2014-09-08 14:10 ` Aristeu Rozanski @ 2014-09-08 21:49 ` Andy Lutomirski 2014-09-08 23:41 ` Greg Kroah-Hartman 0 siblings, 1 reply; 10+ messages in thread From: Andy Lutomirski @ 2014-09-08 21:49 UTC (permalink / raw) To: Aristeu Rozanski Cc: Mauro Carvalho Chehab, Doug Thompson, Greg Kroah-Hartman, Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Rui Wang On Mon, Sep 8, 2014 at 7:10 AM, Aristeu Rozanski <aris@redhat.com> wrote: > On Thu, Aug 14, 2014 at 02:45:41PM -0700, Andy Lutomirski wrote: >> sb_edac controls a large number of different PCI functions. Rather >> than registering as a normal PCI driver for all of them, it >> registers for just one so that it gets probed and, at probe time, it >> looks for all the others. >> >> Coincidentally, the device it registers for also contains the SMBUS >> registers, so the PCI core will refuse to probe both sb_edac and a >> future iMC SMBUS driver. The drivers don't actually conflict, so >> just change sb_edac's device table to probe a different device. >> >> An alternative fix would be to merge the two drivers, but sb_edac >> will also refuse to load on non-ECC systems, whereas i2c_imc would >> still be useful without ECC. >> >> The only user-visible change should be that sb_edac appears to bind >> a different device. >> >> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> >> Cc: Rui Wang <ruiv.wang@gmail.com> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> >> --- >> drivers/edac/sb_edac.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c >> index a2597e9313c6..e3bc2cced580 100644 >> --- a/drivers/edac/sb_edac.c >> +++ b/drivers/edac/sb_edac.c >> @@ -432,7 +432,7 @@ static const struct pci_id_table pci_dev_descr_ibridge_table[] = { >> * pci_device_id table for which devices we are looking for >> */ >> 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)}, >> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)}, >> {0,} /* 0 terminated list. */ >> }; > > Acked-by: Aristeu Rozanski <aris@redhat.com> > Whose tree will this go through? FWIW, this git am can't apply this to 3.17-rc4, but git am --3way can, and it's a one-liner in any case. Should I send a trivially rebased version? --Andy > -- > Aristeu > -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sb_edac: Claim a different PCI device 2014-09-08 21:49 ` Andy Lutomirski @ 2014-09-08 23:41 ` Greg Kroah-Hartman 2014-09-09 9:25 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2014-09-08 23:41 UTC (permalink / raw) To: Andy Lutomirski Cc: Aristeu Rozanski, Mauro Carvalho Chehab, Doug Thompson, Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Rui Wang On Mon, Sep 08, 2014 at 02:49:59PM -0700, Andy Lutomirski wrote: > On Mon, Sep 8, 2014 at 7:10 AM, Aristeu Rozanski <aris@redhat.com> wrote: > > On Thu, Aug 14, 2014 at 02:45:41PM -0700, Andy Lutomirski wrote: > >> sb_edac controls a large number of different PCI functions. Rather > >> than registering as a normal PCI driver for all of them, it > >> registers for just one so that it gets probed and, at probe time, it > >> looks for all the others. > >> > >> Coincidentally, the device it registers for also contains the SMBUS > >> registers, so the PCI core will refuse to probe both sb_edac and a > >> future iMC SMBUS driver. The drivers don't actually conflict, so > >> just change sb_edac's device table to probe a different device. > >> > >> An alternative fix would be to merge the two drivers, but sb_edac > >> will also refuse to load on non-ECC systems, whereas i2c_imc would > >> still be useful without ECC. > >> > >> The only user-visible change should be that sb_edac appears to bind > >> a different device. > >> > >> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > >> Cc: Rui Wang <ruiv.wang@gmail.com> > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> > >> --- > >> drivers/edac/sb_edac.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > >> index a2597e9313c6..e3bc2cced580 100644 > >> --- a/drivers/edac/sb_edac.c > >> +++ b/drivers/edac/sb_edac.c > >> @@ -432,7 +432,7 @@ static const struct pci_id_table pci_dev_descr_ibridge_table[] = { > >> * pci_device_id table for which devices we are looking for > >> */ > >> 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)}, > >> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)}, > >> {0,} /* 0 terminated list. */ > >> }; > > > > Acked-by: Aristeu Rozanski <aris@redhat.com> > > > > Whose tree will this go through? Mauro should have a tree for this as he's the subsystem maintainer. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] sb_edac: Claim a different PCI device 2014-09-08 23:41 ` Greg Kroah-Hartman @ 2014-09-09 9:25 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 10+ messages in thread From: Mauro Carvalho Chehab @ 2014-09-09 9:25 UTC (permalink / raw) To: Andy Lutomirski Cc: Greg Kroah-Hartman, Aristeu Rozanski, Doug Thompson, Borislav Petkov, Bjorn Helgaas, linux-edac, linux-kernel, linux-pci, Rui Wang Em Mon, 08 Sep 2014 16:41:31 -0700 Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu: > On Mon, Sep 08, 2014 at 02:49:59PM -0700, Andy Lutomirski wrote: > > On Mon, Sep 8, 2014 at 7:10 AM, Aristeu Rozanski <aris@redhat.com> wrote: > > > On Thu, Aug 14, 2014 at 02:45:41PM -0700, Andy Lutomirski wrote: > > >> sb_edac controls a large number of different PCI functions. Rather > > >> than registering as a normal PCI driver for all of them, it > > >> registers for just one so that it gets probed and, at probe time, it > > >> looks for all the others. > > >> > > >> Coincidentally, the device it registers for also contains the SMBUS > > >> registers, so the PCI core will refuse to probe both sb_edac and a > > >> future iMC SMBUS driver. The drivers don't actually conflict, so > > >> just change sb_edac's device table to probe a different device. > > >> > > >> An alternative fix would be to merge the two drivers, but sb_edac > > >> will also refuse to load on non-ECC systems, whereas i2c_imc would > > >> still be useful without ECC. > > >> > > >> The only user-visible change should be that sb_edac appears to bind > > >> a different device. > > >> > > >> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > > >> Cc: Rui Wang <ruiv.wang@gmail.com> > > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> > > >> --- > > >> drivers/edac/sb_edac.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > > >> index a2597e9313c6..e3bc2cced580 100644 > > >> --- a/drivers/edac/sb_edac.c > > >> +++ b/drivers/edac/sb_edac.c > > >> @@ -432,7 +432,7 @@ static const struct pci_id_table pci_dev_descr_ibridge_table[] = { > > >> * pci_device_id table for which devices we are looking for > > >> */ > > >> 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)}, > > >> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)}, > > >> {0,} /* 0 terminated list. */ > > >> }; > > > > > > Acked-by: Aristeu Rozanski <aris@redhat.com> > > > > > > > Whose tree will this go through? > > Mauro should have a tree for this as he's the subsystem maintainer. Yes, I'll handle it. > > FWIW, this git am can't apply this to 3.17-rc4, but git am --3way can, > > and it's a one-liner in any case. Should I send a trivially rebased > > version? No need. I'll fix any merge conflicts that might arise. Regards. Mauro ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-09 9:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-14 21:45 [PATCH 0/2] sb_edac: i2c_imc staging submission prep Andy Lutomirski 2014-08-14 21:45 ` [PATCH 1/2] Move Intel SNB device ids from sb_edac to pci_ids.h Andy Lutomirski 2014-09-05 23:41 ` Bjorn Helgaas 2014-09-05 23:42 ` Andy Lutomirski 2014-09-08 14:08 ` Aristeu Rozanski 2014-08-14 21:45 ` [PATCH 2/2] sb_edac: Claim a different PCI device Andy Lutomirski 2014-09-08 14:10 ` Aristeu Rozanski 2014-09-08 21:49 ` Andy Lutomirski 2014-09-08 23:41 ` Greg Kroah-Hartman 2014-09-09 9:25 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).