[linux-dvb] [v4l-dvb-maintainer] [PATCH 1/2] bt878: remove handcrafted PCI subsystem ID check

Trent Piepho xyzzy at speakeasy.org
Sat Jan 19 19:03:36 CET 2008


On Sun, 20 Jan 2008, Akinobu Mita wrote:
> > > +static const char * __devinit card_name(const struct pci_device_id *id)
> > > +{
> > > +     if (id >= bt878_pci_tbl &&
> > > +         id < bt878_pci_tbl + ARRAY_SIZE(bt878_pci_tbl) - 1)
> > > +             return (const char *)id->driver_data;
> >
> > Are you sure this is safe?  I don't remember reading that the pci_device_id
> > passed to the pci driver probe method must be from the device id table.
> > Couldn't the device id be a local variable from whatever calls probe, as
> > long as driver_data is set to the correct value?  I looked at the current
> > pci bus code, and it does always pass a pointer into the driver's pci id
> > table.  But that could change.
>
> Yes, that could change. And then it always returns "Unknown".

Which would be wrong.

> The safe way is:
>
> static const char * __devinit card_name(const struct pci_device_id *id)
> {
>         int i;
>
>         for (i = 0; i < ARRAY_SIZE(bt878_pci_tbl) - 1; i++) {
>                 if (bt878_pci_tbl[i].vendor = id->vendor &&
>                     bt878_pci_tbl[i].device = id->device &&
>                     bt878_pci_tbl[i].subvendor = id->subvendor &&
>                     bt878_pci_tbl[i].subdevice = id->subdevice)
>                         return (const char *)bt878_pci_tbl.driver_data;
>         }
>         return "Unknown";
> }

The pci layer has a function to do this for you.  But it's unnecessary.

> > It seems like a better method would just be to check if driver_data is 0,
> > which will be the case if the id was added dynamically.
> >
>
> driver_data can be non-zero value. we can set it through new_id.
> But there are many possibilities to break kernel by writing

Nope, driver_data will always be 0.  Try it.  You must set
dynids.use_driver_data to allow driver data to be passed via sysfs for just
this reason.

> Acutually I don't have a special preference for any of which.
> Because we can remove the function card_name() after submitting
> all the card name information that bt878.c holds to PCI ID repository.
> (http://pci-ids.ucw.cz/iii/?i=109e0878)
>
> Then we can get the card name from lspci instead of seeing dmesg.

It nice to know that the driver identified the card.  It would be nice if
all four bt8x8 drivers could share a card database, but that's not so easy.



More information about the linux-dvb mailing list