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

2007-06-25 Thread Arjan van de Ven
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

2007-06-25 Thread Kok, Auke

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

2007-06-25 Thread Alan Cox
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

2007-06-25 Thread Greg KH
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

2007-06-25 Thread Greg KH
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

2007-06-25 Thread Alan Cox
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

2007-06-25 Thread Kok, Auke

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

2007-06-25 Thread Arjan van de Ven
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

2007-06-24 Thread Auke Kok
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

2007-06-24 Thread Auke Kok
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/