Messages in this thread | | | Date | Fri, 26 May 2006 06:37:16 -0400 | From | Jeff Garzik <> | Subject | Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device |
| |
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 <jirislaby@gmail.com> >>>>> >>>>> --- >>>>> commit 1d3b6caf027fe53351c645523587aeac40bc3e47 >>>>> tree ae37c86b633442cdf8a7a19ac287542724081c90 >>>>> parent ab3443d79c94d0ae6a9e020daefa4d29eccff50d >>>>> author Jiri Slaby <ku@bellona.localdomain> Fri, 26 May 2006 01:49:12 >>>>> +0159 >>>>> committer Jiri Slaby <ku@bellona.localdomain> 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 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/
| |