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

Trent Piepho xyzzy at speakeasy.org
Sat Jan 19 14:28:49 CET 2008


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.

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.



More information about the linux-dvb mailing list