* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
2006-06-05 20:53 ` [PATCH 2/3] pci: bcm43xx " Greg KH
@ 2006-06-05 21:09 ` Jiri Slaby
2006-06-06 1:18 ` Jeff Garzik
2006-06-06 12:58 ` Michael Buesch
2 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2006-06-05 21:09 UTC (permalink / raw)
To: Greg KH
Cc: Greg KH, Linux Kernel Mailing List, linux-pci, jgarzik, netdev,
mb, st3, linville
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 <jirislaby@gmail.com>
>>
>> ---
>> commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
>> tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
>> parent d59f9ea8489749f59cd0c7333a4784cab964daa8
>> author Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
>> committer Jiri Slaby <ku@bellona.localdomain> 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
\_.-^-._ jirislaby@gmail.com _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
2006-06-05 20:53 ` [PATCH 2/3] pci: bcm43xx " Greg KH
2006-06-05 21:09 ` Jiri Slaby
@ 2006-06-06 1:18 ` Jeff Garzik
2006-06-06 6:50 ` Greg KH
2006-06-06 12:58 ` Michael Buesch
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Garzik @ 2006-06-06 1:18 UTC (permalink / raw)
To: Greg KH
Cc: Jiri Slaby, Greg KH, Linux Kernel Mailing List, linux-pci,
jgarzik, netdev, mb, st3, linville
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
2006-06-06 1:18 ` Jeff Garzik
@ 2006-06-06 6:50 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2006-06-06 6:50 UTC (permalink / raw)
To: Jeff Garzik
Cc: Greg KH, Jiri Slaby, Linux Kernel Mailing List, linux-pci,
jgarzik, netdev, mb, st3, linville
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
2006-06-05 20:53 ` [PATCH 2/3] pci: bcm43xx " Greg KH
2006-06-05 21:09 ` Jiri Slaby
2006-06-06 1:18 ` Jeff Garzik
@ 2006-06-06 12:58 ` Michael Buesch
2 siblings, 0 replies; 6+ messages in thread
From: Michael Buesch @ 2006-06-06 12:58 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg KH, Linux Kernel Mailing List, linux-pci, jgarzik, netdev,
mb, st3, linville
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 <jirislaby@gmail.com>
> >
> > ---
> > commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
> > tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
> > parent d59f9ea8489749f59cd0c7333a4784cab964daa8
> > author Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
> > committer Jiri Slaby <ku@bellona.localdomain> 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 <mb@bu3sch.de>
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.
^ permalink raw reply [flat|nested] 6+ messages in thread