Em 27-07-2011 16:49, Antti Palosaari escreveu:
> On 07/27/2011 10:06 PM, Mauro Carvalho Chehab wrote:
>
>>> + for (i = 0; i<= x; i++) {
>>> + ret = adap->props.frontend_attach(adap);
>>> + if (ret || adap->fe[i] == NULL) {
>>> + /* only print error when there is no FE at all */
>>> + if (i == 0)
>>> + err("no frontend was attached by '%s'",
>>> + adap->dev->desc->name);
>>
>> This doesn't seem right. One thing is to accept adap->fe[1] to be
>> NULL. Another thing is to accept an error at the attach. IMO, the
>> logic should be something like:
>>
>> if (ret< 0)
>> return ret;
>>
>> if (!i&& !adap->fe[0]) {
>> err("no adapter!");
>> return -ENODEV;
>> }
>
> Heh, I tried to keep it functioning as earlier not to break anything! Only thing it does now differently is that it keeps silent when 2nd FE attach fails since we don't know always before fe attach if there is fe or not.
>
> So since it *does not change old behaviour* it must be OK. Let fix old problems later. There is millions of DVB USB callbacks failing silently - like tuner_attach etc.
>
> Surely I want also fix many old issues but it is always too risky.
Added support for DRX-K way at dvb-usb:
http://git.linuxtv.org/mchehab/experimental.git/commitdiff/765b3db218f1e9af6432251c2ebe59bc9660cd42
http://git.linuxtv.org/mchehab/experimental.git/commitdiff/37fa5797c58068cc60cca6829bd662cd4f883cfa
One bad thing I noticed with the API is that it calls adap->props.frontend_attach(adap)
several times, instead of just one, without even passing an argument for the driver to
know that it was called twice.
IMO, there are two ways of doing the attach:
1) call it only once, and, inside the driver, it will loop to add the other FE's;
2) add a parameter, at the call, to say what FE needs to be initialized.
I think (1) is preferred, as it is more flexible, allowing the driver to test for
several types of frontends.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
