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

[Jiri Slaby <jirislaby@gmail.com>]

-----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).

cheers,
- --
Jiri Slaby         www.fi.muni.cz/~xslaby
\_.-^-._   jirislaby@gmail.com   _.-^-._/
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-----