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

Mauro Carvalho Chehab mchehab at infradead.org
Sun Jan 20 12:01:45 CET 2008


On Sat, 19 Jan 2008 05:28:49 -0800 (PST)
Trent Piepho <xyzzy at speakeasy.org> wrote:

> I wish people would use the patch creating system from Hg, since it would
> the format mistakes evident in this patch.
> 
> 1. no patch title
> 2. s-o-b's in wrong order
> 3. diffstat included
> 4. patch against git source and not hg
> 
> On Sat, 19 Jan 2008, Akinobu Mita wrote:
> > From: Akinobu Mita <akinobu.mita at gmail.com>
> >
> > This patch moves the subsystem ID and subsystem vendor ID check from probing
> > function to the PCI generic function by describing subsystem IDs in
> > pci_device_id table. This enables to add new PCI IDs to a device driver pci_ids
> > table at runtime by new_id file in sysfs pci driver tree.
> 
> This patch also changes the driver from having a pci alias to auto-load for
> all bt878 cards to instead only auto-load for specific cards.  Some
> distributions will probably want to adjust their modprobe rules.  I know
> some have delt with both snd-bt87x and bt878 loading for the same devices
> in various ways, e.g. adding one or the other to their modprobe blacklist
> files.

The proper solution here seems to be moving snd-bt87x to our tree, and create
some shared code to be used by snd-bt87x, dvb-bt8xx, to avoid the
driver conflicts.

> I think that new_id will only work if hotplug is turned on.  Without
> hotplug, there will be no way to load the driver for other devices (such as
> those with eprom trouble).
> 
> > +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.
>
> 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.

This data can't be changed by pci core, since driver_data is a priv element. It
is used as a pointer on some drivers, while others use it as an integer value to
an array (like most V4L/DVB and ALSA drivers). The above code should work
properly. If pci touches on this value, it would break the support for the rest
of v4l/dvb drivers, so I think we don't need to worry about this.

Cheers,
Mauro



More information about the linux-dvb mailing list