Trent Piepho wrote:
> 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.
True.
> 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 doesn't have anything to do with accessing the wrong PCI function
as what
you have been trying to explain, as PCI function is based on Device ID
and not
SubDevice ID .. (pointless)
> 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.
A noise out for nothing, that shouldn't have mattered ? But anyhow, the
patch a
while back changed that behaviour, the driver being loaded for all the
devices.
So how come this patch needs to have that description ?
>>>> +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.
what's the harm in it ?
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
