Mailing List Archive: [PATCH 1/2] [RESEND] PCI: read revision ID by default

[PATCH 1/2] [RESEND] PCI: read revision ID by default

Jun 24, 2007, 8:19 PM

Post #1 of 5 (272 views)

Currently there are 97 occurrences where drivers need the pci
revision ID. We can do this once for all devices. Even the pci
subsystem needs the revision several times for quirks. The extra
u8 member pads out nicely in the pci_dev struct.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

arch/powerpc/kernel/pci_64.c | 2 ++
arch/sparc64/kernel/pci.c | 1 +
drivers/pci/probe.c | 1 +
include/linux/pci.h | 1 +
4 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 249cca2..b2c55ca 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -367,8 +367,10 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
sprintf(pci_name(dev), "%04x:%02x:%02x.%d", pci_domain_nr(bus),
dev->bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn));
dev->class = get_int_prop(node, "class-code", 0);
+ dev->revision = get_int_prop(node, "revision-id", 0);

DBG(" class: 0x%x\n", dev->class);
+ DBG(" revision: 0x%x\n", dev->revision);

dev->current_state = 4; /* unknown power state */
dev->error_state = pci_channel_io_normal;
diff --git a/arch/sparc64/kernel/pci.c b/arch/sparc64/kernel/pci.c
index 81f4a5e..55ad1b8 100644
--- a/arch/sparc64/kernel/pci.c
+++ b/arch/sparc64/kernel/pci.c
@@ -448,6 +448,7 @@ struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
*/
pci_read_config_dword(dev, PCI_CLASS_REVISION, &class);
dev->class = class >> 8;
+ dev->revision = class & 0xff;

sprintf(pci_name(dev), "%04x:%02x:%02x.%d", pci_domain_nr(bus),
dev->bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn));
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e48fcf0..a574b7f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -702,6 +702,7 @@ static int pci_setup_device(struct pci_dev * dev)
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));

pci_read_config_dword(dev, PCI_CLASS_REVISION, &class);
+ dev->revision = class & 0xff;
class >>= 8; /* upper 3 bytes */
dev->class = class;
class >>= 8;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fbf3766..9847936 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -138,6 +138,7 @@ struct pci_dev {
unsigned short subsystem_vendor;
unsigned short subsystem_device;
unsigned int class; /* 3 bytes: (base,sub,prog-if) */
+ u8 revision; /* PCI revision, low byte of class word */
u8 hdr_type; /* PCI header type (`multi' flag masked out) */
u8 rom_base_reg; /* which config register controls the ROM */
u8 pin; /* which interrupt pin this device uses */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH 1/2] [RESEND] PCI: read revision ID by default [ In reply to ]

greg at kroah

Jun 25, 2007, 12:20 AM

Post #2 of 5 (261 views)

Permalink

On Sun, Jun 24, 2007 at 08:19:18PM -0700, Auke Kok wrote:
> Currently there are 97 occurrences where drivers need the pci
> revision ID. We can do this once for all devices. Even the pci
> subsystem needs the revision several times for quirks. The extra
> u8 member pads out nicely in the pci_dev struct.
>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>

Thanks, I've updated both of these in my tree.

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH 1/2] [RESEND] PCI: read revision ID by default [ In reply to ]

alan at lxorguk

Jun 25, 2007, 2:01 AM

Post #3 of 5 (260 views)

Permalink

On Sun, 24 Jun 2007 20:19:18 -0700
Auke Kok <auke-jan.h.kok@intel.com> wrote:

> Currently there are 97 occurrences where drivers need the pci
> revision ID. We can do this once for all devices. Even the pci
> subsystem needs the revision several times for quirks. The extra
> u8 member pads out nicely in the pci_dev struct.
>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>

Seems worth caching yes. Care needed changing the other uses however
that the revision isn't changeable on the hardware in some magic fashion.

Acked-by: Alan Cox <alan@redhat.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH 1/2] [RESEND] PCI: read revision ID by default [ In reply to ]

auke-jan.h.kok at intel

Jun 25, 2007, 11:49 AM

Post #4 of 5 (258 views)

Permalink

Alan Cox wrote:
> On Sun, 24 Jun 2007 20:19:18 -0700
> Auke Kok <auke-jan.h.kok@intel.com> wrote:
>
>> Currently there are 97 occurrences where drivers need the pci
>> revision ID. We can do this once for all devices. Even the pci
>> subsystem needs the revision several times for quirks. The extra
>> u8 member pads out nicely in the pci_dev struct.
>>
>> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
>
> Seems worth caching yes. Care needed changing the other uses however
> that the revision isn't changeable on the hardware in some magic fashion.

if that happens the device could also change pci ID... I don't think we can
guard against that kind of abuse anyway. While this is perfectly possible (e.g.
network cards carrying this info in their eeproms), we should still be OK with
the core patch, but the device driver itself needs to pull the information
manually from the device using the pci_get_config_byte() method.

Auke
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH 1/2] [RESEND] PCI: read revision ID by default [ In reply to ]

arjan at infradead

Jun 25, 2007, 12:48 PM

Post #5 of 5 (253 views)

Permalink

On Mon, 2007-06-25 at 11:49 -0700, Kok, Auke wrote:
> Alan Cox wrote:
> > On Sun, 24 Jun 2007 20:19:18 -0700
> > Auke Kok <auke-jan.h.kok@intel.com> wrote:
> >
> >> Currently there are 97 occurrences where drivers need the pci
> >> revision ID. We can do this once for all devices. Even the pci
> >> subsystem needs the revision several times for quirks. The extra
> >> u8 member pads out nicely in the pci_dev struct.
> >>
> >> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> >
> > Seems worth caching yes. Care needed changing the other uses however
> > that the revision isn't changeable on the hardware in some magic fashion.
>
> if that happens the device could also change pci ID... I don't think we can
> guard against that kind of abuse anyway. While this is perfectly possible (e.g.
> network cards carrying this info in their eeproms), we should still be OK with
> the core patch, but the device driver itself needs to pull the information
> manually from the device using the pci_get_config_byte() method.

the other thing btw is that this approach allows us to use quirks and
the like to fix up "crap".

Maybe a good rule-of-thumb (and as with any other rule of thumb there
are exceptions): Drivers should NEVER touch the standard part of PCI
config space without going through a specific PCI layer API for the
value they want.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/