On Sun, 20 Jan 2008, Mauro Carvalho Chehab wrote:
> On Sat, 19 Jan 2008 05:28:49 -0800 (PST)
> Trent Piepho <xyzzy@xxxxxxxxxxxxx> 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@xxxxxxxxx>
> > >
> > > 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.
That doesn't solve the problem. It's inherent in the bt878 design, as it
uses the same pci function for either sound or dvb. So a bt878 chip
function 1 can load one and only one driver from ALSA, OSS, or DVB. For
most chips, there is only one driver that applies to a given pci device and
function.
In some cases it is possible to tell which driver to use via
sub-vendor/device data, but not always. The ALSA driver only auto-loads
for known good subdata, and has a module option to force loading when that
won't work. The DVB driver has the wildcard sub-vendor/device in the alias
table, IMHO a mistake, and will try to load for for all bt878 chips.
The DVB driver used to load for cards with sub-vendor/device 0x0000,0x0000,
but a previous patch changed that. An effect that that _should_ have been
mentioned in the description of the patch responsible, but was not.
This patch changes the DVB driver to not auto-load for all bt878 chips. A
rather significant change that should be described in the patch
description, but is not.
> > > +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.
I don't think you understand. That code checks if the pci_device_id
pointer passed to the probe function points to a spot in the driver's pci
id table. It has nothing to do with the driver_data field being modified.
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
