Re: [PATCH 1/2] [RESEND] PCI: read revision ID by default
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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> > > > > 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 [EMAIL PROTECTED] 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
Alan Cox wrote: On Sun, 24 Jun 2007 20:19:18 -0700 Auke Kok <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 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
On Sun, 24 Jun 2007 20:19:18 -0700 Auke Kok <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] 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
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 <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 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
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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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
On Sun, 24 Jun 2007 20:19:18 -0700 Auke Kok [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] 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
Alan Cox wrote: On Sun, 24 Jun 2007 20:19:18 -0700 Auke Kok [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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
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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] [RESEND] PCI: read revision ID by default
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 <[EMAIL PROTECTED]> --- 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, ); 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, ); + 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 intclass; /* 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] [RESEND] PCI: read revision ID by default
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 [EMAIL PROTECTED] --- 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 intclass; /* 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/