| Message ID | 1383150426-24730-2-git-send-email-arozansk@redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Oct 30, 2013 at 12:26:55PM -0400, Aristeu Rozanski wrote: > According to the comment, it should be done before submitting upstream. > > Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> > --- > drivers/edac/sb_edac.c | 21 ++------------------- > include/linux/pci_ids.h | 11 +++++++++++ > 2 files changed, 13 insertions(+), 19 deletions(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index e04462b..4cdd948 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -57,26 +57,9 @@ static int probed; > */ > > /* > - * 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 Why, is anything else besides sb_edac.c using those?
Em Wed, 30 Oct 2013 17:40:20 +0100 Borislav Petkov <bp@alien8.de> escreveu: > On Wed, Oct 30, 2013 at 12:26:55PM -0400, Aristeu Rozanski wrote: > > According to the comment, it should be done before submitting upstream. > > > > Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> > > --- > > drivers/edac/sb_edac.c | 21 ++------------------- > > include/linux/pci_ids.h | 11 +++++++++++ > > 2 files changed, 13 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > > index e04462b..4cdd948 100644 > > --- a/drivers/edac/sb_edac.c > > +++ b/drivers/edac/sb_edac.c > > @@ -57,26 +57,9 @@ static int probed; > > */ > > > > /* > > - * 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 > > Why, is anything else besides sb_edac.c using those? > I think that this patch makes sense, as the other Sandy Bridge registers are at pci_ids.h. So, it is a matter of consistency. Also, I won't doubt that other drivers could need to access those, as there are other things there on a few of those PCI IDs. Yet, on patch 12/12, you're adding the Ivy Bridge specific PCI IDs inside the driver. So, IMHO, you should either move all to pci_ids.h or to keep them inside the driver. My personal taste would be to move them all to pci_ids.h. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 30, 2013 at 11:19 AM, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > Em Wed, 30 Oct 2013 17:40:20 +0100 > Borislav Petkov <bp@alien8.de> escreveu: > >> On Wed, Oct 30, 2013 at 12:26:55PM -0400, Aristeu Rozanski wrote: >> > According to the comment, it should be done before submitting upstream. >> > >> > Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> >> > --- >> > drivers/edac/sb_edac.c | 21 ++------------------- >> > include/linux/pci_ids.h | 11 +++++++++++ >> > 2 files changed, 13 insertions(+), 19 deletions(-) >> > >> > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c >> > index e04462b..4cdd948 100644 >> > --- a/drivers/edac/sb_edac.c >> > +++ b/drivers/edac/sb_edac.c >> > @@ -57,26 +57,9 @@ static int probed; >> > */ >> > >> > /* >> > - * 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 >> >> Why, is anything else besides sb_edac.c using those? >> > I think that this patch makes sense, as the other Sandy Bridge registers > are at pci_ids.h. So, it is a matter of consistency. > > Also, I won't doubt that other drivers could need to access those, as > there are other things there on a few of those PCI IDs. > > Yet, on patch 12/12, you're adding the Ivy Bridge specific PCI IDs > inside the driver. > > So, IMHO, you should either move all to pci_ids.h or to keep them inside > the driver. > > My personal taste would be to move them all to pci_ids.h. As long as it follows the guideline at the top of pci_ids.h (add them there only if they are shared between multiple drivers), I'm fine with putting them in pci_ids.h. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 30, 2013 at 11:21:49AM -0600, Bjorn Helgaas wrote: > On Wed, Oct 30, 2013 at 11:19 AM, Mauro Carvalho Chehab > <m.chehab@samsung.com> wrote: > > Em Wed, 30 Oct 2013 17:40:20 +0100 > > Borislav Petkov <bp@alien8.de> escreveu: > > > >> On Wed, Oct 30, 2013 at 12:26:55PM -0400, Aristeu Rozanski wrote: > >> > According to the comment, it should be done before submitting upstream. > >> > > >> > Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> > >> > --- > >> > drivers/edac/sb_edac.c | 21 ++------------------- > >> > include/linux/pci_ids.h | 11 +++++++++++ > >> > 2 files changed, 13 insertions(+), 19 deletions(-) > >> > > >> > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > >> > index e04462b..4cdd948 100644 > >> > --- a/drivers/edac/sb_edac.c > >> > +++ b/drivers/edac/sb_edac.c > >> > @@ -57,26 +57,9 @@ static int probed; > >> > */ > >> > > >> > /* > >> > - * 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 > >> > >> Why, is anything else besides sb_edac.c using those? > >> > > I think that this patch makes sense, as the other Sandy Bridge registers > > are at pci_ids.h. So, it is a matter of consistency. > > > > Also, I won't doubt that other drivers could need to access those, as > > there are other things there on a few of those PCI IDs. > > > > Yet, on patch 12/12, you're adding the Ivy Bridge specific PCI IDs > > inside the driver. > > > > So, IMHO, you should either move all to pci_ids.h or to keep them inside > > the driver. > > > > My personal taste would be to move them all to pci_ids.h. > > As long as it follows the guideline at the top of pci_ids.h (add them > there only if they are shared between multiple drivers), I'm fine with > putting them in pci_ids.h. Fair enough, I think we can drop the first patch. I checked and all the patches apply without the first one and it builds fine. Mauro, want me to resubmit the patchset anyway?
Em Wed, 30 Oct 2013 16:35:17 -0400 Aristeu Rozanski <arozansk@redhat.com> escreveu: > On Wed, Oct 30, 2013 at 11:21:49AM -0600, Bjorn Helgaas wrote: > > On Wed, Oct 30, 2013 at 11:19 AM, Mauro Carvalho Chehab > > <m.chehab@samsung.com> wrote: > > > Em Wed, 30 Oct 2013 17:40:20 +0100 > > > Borislav Petkov <bp@alien8.de> escreveu: > > > > > >> On Wed, Oct 30, 2013 at 12:26:55PM -0400, Aristeu Rozanski wrote: > > >> > According to the comment, it should be done before submitting upstream. > > >> > > > >> > Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> > > >> > --- > > >> > drivers/edac/sb_edac.c | 21 ++------------------- > > >> > include/linux/pci_ids.h | 11 +++++++++++ > > >> > 2 files changed, 13 insertions(+), 19 deletions(-) > > >> > > > >> > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > > >> > index e04462b..4cdd948 100644 > > >> > --- a/drivers/edac/sb_edac.c > > >> > +++ b/drivers/edac/sb_edac.c > > >> > @@ -57,26 +57,9 @@ static int probed; > > >> > */ > > >> > > > >> > /* > > >> > - * 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 > > >> > > >> Why, is anything else besides sb_edac.c using those? > > >> > > > I think that this patch makes sense, as the other Sandy Bridge registers > > > are at pci_ids.h. So, it is a matter of consistency. > > > > > > Also, I won't doubt that other drivers could need to access those, as > > > there are other things there on a few of those PCI IDs. > > > > > > Yet, on patch 12/12, you're adding the Ivy Bridge specific PCI IDs > > > inside the driver. > > > > > > So, IMHO, you should either move all to pci_ids.h or to keep them inside > > > the driver. > > > > > > My personal taste would be to move them all to pci_ids.h. > > > > As long as it follows the guideline at the top of pci_ids.h (add them > > there only if they are shared between multiple drivers), I'm fine with > > putting them in pci_ids.h. > > Fair enough, I think we can drop the first patch. I checked and all the > patches apply without the first one and it builds fine. Mauro, want me > to resubmit the patchset anyway? No need. Thanks! Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index e04462b..4cdd948 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -57,26 +57,9 @@ static int probed; */ /* - * 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 + * Currently, unused, but will be needed in the future + * implementations, as they hold the error counters */ -#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 */ diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 97fbecd..d7d4757 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2812,7 +2812,18 @@ #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 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0 0x3ca0 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA 0x3ca8 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0 0x3caa +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1 0x3cab +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2 0x3cac +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3 0x3cad +#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO 0x3cb8 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0 0x3cf4 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR 0x3cf5 +#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1 0x3cf6 #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3
According to the comment, it should be done before submitting upstream. Signed-off-by: Aristeu Rozanski <arozansk@redhat.com> --- drivers/edac/sb_edac.c | 21 ++------------------- include/linux/pci_ids.h | 11 +++++++++++ 2 files changed, 13 insertions(+), 19 deletions(-)