Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-06 Thread Greg KH 
On Mon, Jun 05, 2006 at 09:18:18PM -0400, Jeff Garzik wrote:
 On Mon, Jun 05, 2006 at 01:53:09PM -0700, Greg KH wrote:
  Why not just use the proper pci interface?  Why poke around in another
  pci device to steal an irq, when that irq might not even be valid?
  (irqs are not valid until pci_enable_device() is called on them...)
 
 Answered this question the last time you asked.
 
 Answer:  this is an embedded platform that needs such poking.  The
 wireless device is _another_ device.

Ugh, sorry, too many patches, too many different threads...  You are
right...

greg k-h
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-06 Thread Michael Buesch 
On Monday 05 June 2006 22:53, Greg KH wrote:
 On Mon, Jun 05, 2006 at 10:20:07PM +0200, Jiri Slaby wrote:
  bcm43xx avoid pci_find_device
  
  Change pci_find_device to safer pci_get_device with support for more
  devices.
  
  Signed-off-by: Jiri Slaby [EMAIL PROTECTED]
  
  ---
  commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
  tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
  parent d59f9ea8489749f59cd0c7333a4784cab964daa8
  author Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
  committer Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
  
   drivers/net/wireless/bcm43xx/bcm43xx_main.c |   21 -
   1 files changed, 16 insertions(+), 5 deletions(-)
  
  diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
  b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
  index 22b8fa6..d1a9975 100644
  --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
  +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
  @@ -2133,6 +2133,13 @@ out:
  return err;
   }
   
  +#ifdef CONFIG_BCM947XX
  +static struct pci_device_id bcm43xx_47xx_ids[] = {
  +   { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
  +   { 0 }
  +};
  +#endif
  +
   static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
   {
  int res;
  @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
  bcm-irq = bcm-pci_dev-irq;
   #ifdef CONFIG_BCM947XX
  if (bcm-pci_dev-bus-number == 0) {
  -   struct pci_dev *d = NULL;
  -   /* FIXME: we will probably need more device IDs here... */
  -   d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
  -   if (d != NULL) {
  -   bcm-irq = d-irq;
  +   struct pci_dev *d;
  +   struct pci_device_id *id;
  +   for (id = bcm43xx_47xx_ids; id-vendor; id++) {
  +   d = pci_get_device(id-vendor, id-device, NULL);
  +   if (d != NULL) {
  +   bcm-irq = d-irq;
  +   pci_dev_put(d);
  +   break;
  +   }
 
 This will not work if you have more than one of the same devices in the
 system.
 
 Well, the original code will not either :(
 
 Why not just use the proper pci interface?  Why poke around in another
 pci device to steal an irq, when that irq might not even be valid?
 (irqs are not valid until pci_enable_device() is called on them...)

Ok, if someone really wants to have this patch in mainline.

Signed-off-by: Michael Buesch [EMAIL PROTECTED]

But the whole purpose of this patch is really questionable.
* This code is only compiled for the OpenWRT Router kernel.
  This kernel does not use softmac, but dscape stack.
  So nobody will ever actually use this code.
  One could even argue, if the code should be removed from
  the softmac driver, but I think openwrt people use it for
  some kind of hacking, testing, whatever.
  It's not so much code, so I don't care. It does not add
  maintainance work.
* Do we really need to increment some reference counters?
  I mean, we are asking for a bus here. This bus is not
  hotpluggable or something and will never go away. Either it
  is soldered on the board or not. That is what we test here.

So, Jiri, if you really want to have this patch upstream, you
have my Signed-off-by. If nobody else cares, it will get lost
in the deep black netdev hole. ;)

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Jiri Slaby 
John W. Linville napsal(a):
 On Fri, May 26, 2006 at 01:29:12PM +0159, Jiri Slaby wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Jeff Garzik napsal(a):
 Jiri Slaby wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Jeff Garzik napsal(a):
 The point is that you don't need to loop over the table,
 pci_match_one_device() does that for you.
 The problem is, that there is no such function, I think.
 If you take a look at pci_dev_present:
 The function you want is pci_dev_present().
 Nope, it returns only 0/1.
 
 Did we get a resolution on this?  I don't think Jeff is going to pull
 this patch from me until you satisfy him that it is correct... :-)
Yup: to use pci_get_device in some nice loop.
But unfortunately here is no time now, that's the problem, I'll post corrected
ones next month, sorry.

regards,
-- 
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Michael Buesch 
On Monday 05 June 2006 22:18, Jiri Slaby wrote:
 bcm43xx avoid pci_find_device
 
 Change pci_find_device to safer pci_get_device with support for more
 devices.

I am wondering about the reference count.
From docbook:

256  * pci_get_device - begin or continue searching for a PCI device by 
vendor/device id
257  * @vendor: PCI vendor id to match, or %PCI_ANY_ID to match all vendor ids
258  * @device: PCI device id to match, or %PCI_ANY_ID to match all device ids
259  * @from: Previous PCI device found in search, or %NULL for new search.
260  *
261  * Iterates through the list of known PCI devices.  If a PCI device is
262  * found with a matching @vendor and @device, the reference count to the
  ^^
263  * device is incremented and a pointer to its device structure is returned.
   ^
264  * Otherwise, %NULL is returned.  A new search is initiated by passing %NULL
265  * to the @from argument.  Otherwise if @from is not %NULL, searches 
continue
266  * from next device on the global list.  The reference count for @from is
267  * always decremented if it is not %NULL.

Who is going to decrement it, once the device is not used anymore.
not used anymore is ifconfig down in the case of bcm43xx.
You will call pci_get_device on each ifconfig up.

 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]
 
 ---
 commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
 tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
 parent d59f9ea8489749f59cd0c7333a4784cab964daa8
 author Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 
  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index 22b8fa6..d1a9975 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2133,6 +2133,13 @@ out:
   return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_47xx_ids[] = {
 + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 + { 0 }
 +};
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
   int res;
 @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
   bcm-irq = bcm-pci_dev-irq;
  #ifdef CONFIG_BCM947XX
   if (bcm-pci_dev-bus-number == 0) {
 - struct pci_dev *d = NULL;
 - /* FIXME: we will probably need more device IDs here... */
 - d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 - if (d != NULL) {
 - bcm-irq = d-irq;
 + struct pci_dev *d;
 + struct pci_device_id *id;
 + for (id = bcm43xx_47xx_ids; id-vendor; id++) {
 + d = pci_get_device(id-vendor, id-device, NULL);
 + if (d != NULL) {
 + bcm-irq = d-irq;
 + pci_dev_put(d);
 + break;
 + }
   }
   }
  #endif
 

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Jiri Slaby 
Michael Buesch napsal(a):
 On Monday 05 June 2006 22:18, Jiri Slaby wrote:
 bcm43xx avoid pci_find_device

 Change pci_find_device to safer pci_get_device with support for more
 devices.
 
 I am wondering about the reference count.
 From docbook:
 
 256  * pci_get_device - begin or continue searching for a PCI device by 
 vendor/device id
 257  * @vendor: PCI vendor id to match, or %PCI_ANY_ID to match all vendor ids
 258  * @device: PCI device id to match, or %PCI_ANY_ID to match all device ids
 259  * @from: Previous PCI device found in search, or %NULL for new search.
 260  *
 261  * Iterates through the list of known PCI devices.  If a PCI device is
 262  * found with a matching @vendor and @device, the reference count to the
   ^^
 263  * device is incremented and a pointer to its device structure is 
 returned.
^
 264  * Otherwise, %NULL is returned.  A new search is initiated by passing 
 %NULL
 265  * to the @from argument.  Otherwise if @from is not %NULL, searches 
 continue
 266  * from next device on the global list.  The reference count for @from is
 267  * always decremented if it is not %NULL.
 
 Who is going to decrement it, once the device is not used anymore.
 not used anymore is ifconfig down in the case of bcm43xx.
 You will call pci_get_device on each ifconfig up.
 
 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

 ---
 commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
 tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
 parent d59f9ea8489749f59cd0c7333a4784cab964daa8
 author Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159

  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)

 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index 22b8fa6..d1a9975 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2133,6 +2133,13 @@ out:
  return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_47xx_ids[] = {
 +{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 +{ 0 }
 +};
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
  int res;
 @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
  bcm-irq = bcm-pci_dev-irq;
  #ifdef CONFIG_BCM947XX
  if (bcm-pci_dev-bus-number == 0) {
 -struct pci_dev *d = NULL;
 -/* FIXME: we will probably need more device IDs here... */
 -d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 -if (d != NULL) {
 -bcm-irq = d-irq;
 +struct pci_dev *d;
 +struct pci_device_id *id;
 +for (id = bcm43xx_47xx_ids; id-vendor; id++) {
 +d = pci_get_device(id-vendor, id-device, NULL);
 +if (d != NULL) {
 +bcm-irq = d-irq;
 +pci_dev_put(d);
here?
 +break;
 +}
  }
  }
  #endif

 

regards,
-- 
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Greg KH 
On Mon, Jun 05, 2006 at 10:20:07PM +0200, Jiri Slaby wrote:
 bcm43xx avoid pci_find_device
 
 Change pci_find_device to safer pci_get_device with support for more
 devices.
 
 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]
 
 ---
 commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
 tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
 parent d59f9ea8489749f59cd0c7333a4784cab964daa8
 author Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 
  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index 22b8fa6..d1a9975 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2133,6 +2133,13 @@ out:
   return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_47xx_ids[] = {
 + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 + { 0 }
 +};
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
   int res;
 @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
   bcm-irq = bcm-pci_dev-irq;
  #ifdef CONFIG_BCM947XX
   if (bcm-pci_dev-bus-number == 0) {
 - struct pci_dev *d = NULL;
 - /* FIXME: we will probably need more device IDs here... */
 - d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 - if (d != NULL) {
 - bcm-irq = d-irq;
 + struct pci_dev *d;
 + struct pci_device_id *id;
 + for (id = bcm43xx_47xx_ids; id-vendor; id++) {
 + d = pci_get_device(id-vendor, id-device, NULL);
 + if (d != NULL) {
 + bcm-irq = d-irq;
 + pci_dev_put(d);
 + break;
 + }

This will not work if you have more than one of the same devices in the
system.

Well, the original code will not either :(

Why not just use the proper pci interface?  Why poke around in another
pci device to steal an irq, when that irq might not even be valid?
(irqs are not valid until pci_enable_device() is called on them...)

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Jiri Slaby 
Greg KH napsal(a):
 On Mon, Jun 05, 2006 at 10:20:07PM +0200, Jiri Slaby wrote:
 bcm43xx avoid pci_find_device

 Change pci_find_device to safer pci_get_device with support for more
 devices.

 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

 ---
 commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
 tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
 parent d59f9ea8489749f59cd0c7333a4784cab964daa8
 author Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Mon, 05 Jun 2006 22:01:03 +0159

  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)

 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index 22b8fa6..d1a9975 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2133,6 +2133,13 @@ out:
  return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_47xx_ids[] = {
 +{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 +{ 0 }
 +};
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
  int res;
 @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
  bcm-irq = bcm-pci_dev-irq;
  #ifdef CONFIG_BCM947XX
  if (bcm-pci_dev-bus-number == 0) {
 -struct pci_dev *d = NULL;
 -/* FIXME: we will probably need more device IDs here... */
 -d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 -if (d != NULL) {
 -bcm-irq = d-irq;
 +struct pci_dev *d;
 +struct pci_device_id *id;
 +for (id = bcm43xx_47xx_ids; id-vendor; id++) {
 +d = pci_get_device(id-vendor, id-device, NULL);
 +if (d != NULL) {
 +bcm-irq = d-irq;
 +pci_dev_put(d);
 +break;
 +}
 
 This will not work if you have more than one of the same devices in the
 system.
 
 Well, the original code will not either :(
 
 Why not just use the proper pci interface?  Why poke around in another
 pci device to steal an irq, when that irq might not even be valid?
 (irqs are not valid until pci_enable_device() is called on them...)
Ok, this is some bus or something, not real device, so no pci probing or
anything else could be done. There is only one in the system, if any, since
CONFIG_BCM947XX is set for some sort of embedded systems, but this would be
uttered by somebody more responsible.

Should I call pci_enable_device anyway, I am confused now :/? And then, should I
hold the reference all time until ifdown for this some kind of parent?

thanks,
-- 
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-06-05 Thread Jeff Garzik 
On Mon, Jun 05, 2006 at 01:53:09PM -0700, Greg KH wrote:
 Why not just use the proper pci interface?  Why poke around in another
 pci device to steal an irq, when that irq might not even be valid?
 (irqs are not valid until pci_enable_device() is called on them...)

Answered this question the last time you asked.

Answer:  this is an embedded platform that needs such poking.  The
wireless device is _another_ device.

Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jiri Slaby 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):
 Jiri Slaby wrote:
 bcm43xx avoid pci_find_device

 Change pci_find_device to safer pci_get_device with support for more
 devices.

 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

 ---
 commit 1d3b6caf027fe53351c645523587aeac40bc3e47
 tree ae37c86b633442cdf8a7a19ac287542724081c90
 parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d
 author Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12
 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006
 01:49:12 +0159

  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   20 
  1 files changed, 16 insertions(+), 4 deletions(-)

 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index b488f77..56d2fc6 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2131,6 +2131,13 @@ out:
  return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_ids[] = {
 +{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 +{ 0 }
 +};
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
  int res;
 @@ -2141,10 +2148,15 @@ static int bcm43xx_initialize_irq(struct
  #ifdef CONFIG_BCM947XX
  if (bcm-pci_dev-bus-number == 0) {
  struct pci_dev *d = NULL;
 -/* FIXME: we will probably need more device IDs here... */
 -d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 -if (d != NULL) {
 -bcm-irq = d-irq;
 +struct pci_device_id *id = bcm43xx_ids;
 +while (id-vendor) {
 +d = pci_get_device(id-vendor, id-device, NULL);
 +if (d != NULL) {
 +bcm-irq = d-irq;
 +pci_dev_put(d);
 +break;
 
 You'll want to use pci_match_device() or pci_match_one_device()
 [I forget which one]
Why? Matching is done by pci_get_device() or pci_get_subsys(), respectively.
[pci_match_device() is for matching dev - drv, you meant 
pci_match_one_device()]

thanks,
- --
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEdtY+MsxVwznUen4RAkmFAJ9FFm6nCgnGCdZAcPqv2H99rBNMzwCeK3DA
nPBv8s+ldDrSOpin+mGdDdg=
=MBag
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jeff Garzik 

Jiri Slaby wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):

Jiri Slaby wrote:

bcm43xx avoid pci_find_device

Change pci_find_device to safer pci_get_device with support for more
devices.

Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

---
commit 1d3b6caf027fe53351c645523587aeac40bc3e47
tree ae37c86b633442cdf8a7a19ac287542724081c90
parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d
author Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12
+0159
committer Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006
01:49:12 +0159

 drivers/net/wireless/bcm43xx/bcm43xx_main.c |   20 
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
index b488f77..56d2fc6 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -2131,6 +2131,13 @@ out:
 return err;
 }
 
+#ifdef CONFIG_BCM947XX

+static struct pci_device_id bcm43xx_ids[] = {
+{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
+{ 0 }
+};
+#endif
+
 static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
 {
 int res;
@@ -2141,10 +2148,15 @@ static int bcm43xx_initialize_irq(struct
 #ifdef CONFIG_BCM947XX
 if (bcm-pci_dev-bus-number == 0) {
 struct pci_dev *d = NULL;
-/* FIXME: we will probably need more device IDs here... */
-d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
-if (d != NULL) {
-bcm-irq = d-irq;
+struct pci_device_id *id = bcm43xx_ids;
+while (id-vendor) {
+d = pci_get_device(id-vendor, id-device, NULL);
+if (d != NULL) {
+bcm-irq = d-irq;
+pci_dev_put(d);
+break;

You'll want to use pci_match_device() or pci_match_one_device()
[I forget which one]

Why? Matching is done by pci_get_device() or pci_get_subsys(), respectively.
[pci_match_device() is for matching dev - drv, you meant 
pci_match_one_device()]


The FIXME says we will probably need more device IDs here.

Thus, if you are touching this area, it would make sense to add the 
capability to easily add a second (and third, fourth...) PCI ID.  And 
that means pci_match_one_device() and a pci_device_id table.


Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jiri Slaby 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):
 Jiri Slaby wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Jeff Garzik napsal(a):
 Jiri Slaby wrote:
 bcm43xx avoid pci_find_device

 Change pci_find_device to safer pci_get_device with support for more
 devices.

 Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

 ---
 commit 1d3b6caf027fe53351c645523587aeac40bc3e47
 tree ae37c86b633442cdf8a7a19ac287542724081c90
 parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d
 author Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12
 +0159
 committer Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006
 01:49:12 +0159

  drivers/net/wireless/bcm43xx/bcm43xx_main.c |   20
 
  1 files changed, 16 insertions(+), 4 deletions(-)

 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index b488f77..56d2fc6 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2131,6 +2131,13 @@ out:
  return err;
  }
  
 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_ids[] = {
 +{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
 +{ 0 }
 +};
Table is here ^^^. You just add an entry, and that's it.
 +#endif
 +
  static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
  {
  int res;
 @@ -2141,10 +2148,15 @@ static int bcm43xx_initialize_irq(struct
  #ifdef CONFIG_BCM947XX
  if (bcm-pci_dev-bus-number == 0) {
  struct pci_dev *d = NULL;
 -/* FIXME: we will probably need more device IDs here... */
 -d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
 -if (d != NULL) {
 -bcm-irq = d-irq;
 +struct pci_device_id *id = bcm43xx_ids;
 +while (id-vendor) {
 +d = pci_get_device(id-vendor, id-device, NULL);
 +if (d != NULL) {
 +bcm-irq = d-irq;
 +pci_dev_put(d);
 +break;
 You'll want to use pci_match_device() or pci_match_one_device()
 [I forget which one]
 Why? Matching is done by pci_get_device() or pci_get_subsys(),
 respectively.
 [pci_match_device() is for matching dev - drv, you meant
 pci_match_one_device()]
 
 The FIXME says we will probably need more device IDs here.
Yup.
 
 Thus, if you are touching this area, it would make sense to add the
 capability to easily add a second (and third, fourth...) PCI ID.  And
 that means pci_match_one_device() and a pci_device_id table.
But the while loop do the work: unless id-vendor != NULL, do the matching with
the current raw (id) of the table, then jump to the next raw (id++).

pci_get_device returns NULL if the device with id-vendor, id-device wasn't
found, then we try next raw, otherwise, we break the loop.

Implementations before and now do the same strangeness -- assume there is only
one device (?shouldn't matter?, since it is embedded).

cheers,
- --
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEdtlbMsxVwznUen4RAo/OAJsHy6sED+a9QYmbcaGTMUjwSYm4vACgwfQL
GhmfbtwskPB3Dnvw8HfJzpE=
=+kxv
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jeff Garzik 

Jiri Slaby wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):

Jiri Slaby wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):

Jiri Slaby wrote:

bcm43xx avoid pci_find_device

Change pci_find_device to safer pci_get_device with support for more
devices.

Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

---
commit 1d3b6caf027fe53351c645523587aeac40bc3e47
tree ae37c86b633442cdf8a7a19ac287542724081c90
parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d
author Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12
+0159
committer Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006
01:49:12 +0159

 drivers/net/wireless/bcm43xx/bcm43xx_main.c |   20

 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
index b488f77..56d2fc6 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -2131,6 +2131,13 @@ out:
 return err;
 }
 
+#ifdef CONFIG_BCM947XX

+static struct pci_device_id bcm43xx_ids[] = {
+{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
+{ 0 }
+};

Table is here ^^^. You just add an entry, and that's it.

+#endif
+
 static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
 {
 int res;
@@ -2141,10 +2148,15 @@ static int bcm43xx_initialize_irq(struct
 #ifdef CONFIG_BCM947XX
 if (bcm-pci_dev-bus-number == 0) {
 struct pci_dev *d = NULL;
-/* FIXME: we will probably need more device IDs here... */
-d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
-if (d != NULL) {
-bcm-irq = d-irq;
+struct pci_device_id *id = bcm43xx_ids;
+while (id-vendor) {
+d = pci_get_device(id-vendor, id-device, NULL);
+if (d != NULL) {
+bcm-irq = d-irq;
+pci_dev_put(d);
+break;

You'll want to use pci_match_device() or pci_match_one_device()
[I forget which one]

Why? Matching is done by pci_get_device() or pci_get_subsys(),
respectively.
[pci_match_device() is for matching dev - drv, you meant
pci_match_one_device()]

The FIXME says we will probably need more device IDs here.

Yup.

Thus, if you are touching this area, it would make sense to add the
capability to easily add a second (and third, fourth...) PCI ID.  And
that means pci_match_one_device() and a pci_device_id table.

But the while loop do the work: unless id-vendor != NULL, do the matching with
the current raw (id) of the table, then jump to the next raw (id++).

pci_get_device returns NULL if the device with id-vendor, id-device wasn't
found, then we try next raw, otherwise, we break the loop.

Implementations before and now do the same strangeness -- assume there is only
one device (?shouldn't matter?, since it is embedded).


The point is that you don't need to loop over the table, 
pci_match_one_device() does that for you.


And this code, like the gt96100_eth code, is testing the existence of 
certain platform devices, to be certain that it can proceed with certain 
platform-specific duties.


Thus we don't care about matching multiple devices -- an unlikely case 
-- but we do care about making the code as small as possible by calling 
a standard PCI match function which searches through a list of PCI IDs.


Jeff


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jiri Slaby 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):
 The point is that you don't need to loop over the table,
 pci_match_one_device() does that for you.
The problem is, that there is no such function, I think.
If you take a look at pci_dev_present:
http://sosdg.org/~coywolf/lxr/source/drivers/pci/search.c#L270
They traverse the table in similar way as I do.

pci_match_one_device() just checks (one to one) values without any looping.

regards,
- --
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEdt5HMsxVwznUen4RAqt8AJ9pzaDey2zn399lrahelv17w8IiDgCguUwa
4xOX7pUX2Au/WBsbJbnNwBE=
=P1cu
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jeff Garzik 

Jiri Slaby wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):

The point is that you don't need to loop over the table,
pci_match_one_device() does that for you.

The problem is, that there is no such function, I think.
If you take a look at pci_dev_present:


The function you want is pci_dev_present().

Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jiri Slaby 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jeff Garzik napsal(a):
 Jiri Slaby wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Jeff Garzik napsal(a):
 The point is that you don't need to loop over the table,
 pci_match_one_device() does that for you.
 The problem is, that there is no such function, I think.
 If you take a look at pci_dev_present:
 
 The function you want is pci_dev_present().
Nope, it returns only 0/1.

regards,
- --
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEduafMsxVwznUen4RAjfzAKCaxRAK1nN5qx+akiA59E5Mq/ZPcgCffRwa
vwAz0SPClr6sCYy+DOjtilE=
=DHzA
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Michael Buesch 
On Friday 26 May 2006 12:33, you wrote:
  --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
  +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
  @@ -2131,6 +2131,13 @@ out:
   return err;
   }
 
  +#ifdef CONFIG_BCM947XX
  +static struct pci_device_id bcm43xx_ids[] = {

Call it
static struct pci_device_id bcm43xx_47xx_ids[] = {
please.

And; _important_; if you submit this change, _also_
do a patch against the devicescape version of the driver in
John Linville's wireless-dev tree
drivers/net/wireless/d80211/bcm43xx
in the tree at 
git://kernel.org/pub/scm/linux/kernel/git/linville/wireless-dev.git
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-26 Thread Jiri Slaby 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Michael Buesch napsal(a):
 On Friday 26 May 2006 12:33, you wrote:
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -2131,6 +2131,13 @@ out:
  return err;
  }

 +#ifdef CONFIG_BCM947XX
 +static struct pci_device_id bcm43xx_ids[] = {
 
 Call it
 static struct pci_device_id bcm43xx_47xx_ids[] = {
 please.
 
 And; _important_; if you submit this change, _also_
 do a patch against the devicescape version of the driver in
 John Linville's wireless-dev tree
 drivers/net/wireless/d80211/bcm43xx
 in the tree at 
 git://kernel.org/pub/scm/linux/kernel/git/linville/wireless-dev.git
Ok, thanks.

- --
Jiri Slaby www.fi.muni.cz/~xslaby
\_.-^-._   [EMAIL PROTECTED]   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEduvXMsxVwznUen4RAqQcAJ9j870AGMn5jXW68tEQHZXltAenmQCfX9Ik
oyRfuNnKxGHu8HGVvcDVJHM=
=/NUD
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device

2006-05-25 Thread Jeff Garzik 

Jiri Slaby wrote:

bcm43xx avoid pci_find_device

Change pci_find_device to safer pci_get_device with support for more
devices.

Signed-off-by: Jiri Slaby [EMAIL PROTECTED]

---
commit 1d3b6caf027fe53351c645523587aeac40bc3e47
tree ae37c86b633442cdf8a7a19ac287542724081c90
parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d
author Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12 +0159
committer Jiri Slaby [EMAIL PROTECTED] Fri, 26 May 2006 01:49:12 +0159

 drivers/net/wireless/bcm43xx/bcm43xx_main.c |   20 
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
index b488f77..56d2fc6 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -2131,6 +2131,13 @@ out:
return err;
 }
 
+#ifdef CONFIG_BCM947XX

+static struct pci_device_id bcm43xx_ids[] = {
+   { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
+   { 0 }
+};
+#endif
+
 static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
 {
int res;
@@ -2141,10 +2148,15 @@ static int bcm43xx_initialize_irq(struct
 #ifdef CONFIG_BCM947XX
if (bcm-pci_dev-bus-number == 0) {
struct pci_dev *d = NULL;
-   /* FIXME: we will probably need more device IDs here... */
-   d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
-   if (d != NULL) {
-   bcm-irq = d-irq;
+   struct pci_device_id *id = bcm43xx_ids;
+   while (id-vendor) {
+   d = pci_get_device(id-vendor, id-device, NULL);
+   if (d != NULL) {
+   bcm-irq = d-irq;
+   pci_dev_put(d);
+   break;


You'll want to use pci_match_device() or pci_match_one_device()
[I forget which one]

Jeff



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html