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.
I'm talking about adding another bad behavior: let MFE to have one of the FE's
failing.
>
>
>>> + if (dvb_register_frontend(&adap->dvb_adap, adap->fe[i])) {
>>> + err("Frontend %d registration failed.", i);
>>> + dvb_frontend_detach(adap->fe[i]);
>>
>> There is a special case here: for DRX-K, we can't call dvb_frontend_detach().
>> as just one drxk_attach() returns the two pointers. While this is not fixed,
>> we need to add some logic here to check if the adapter were attached.
>
> There is currently no DVB USB driver using DRX-K thus it is not problem. If someone will add such support can fix DRX-K driver.
I agree. I was just rising the question. The glue should be bound together with the first
dvb-usb DRX-K driver.
> Also take into account it is possible to register only 1 FE using DVB USB and manually register 2nd FE as it have been those days.
>
>
>>> + adap->fe[i] = NULL;
>>> + /* In error case, do not try register more FEs,
>>> + * still leaving already registered FEs alive. */
>>
>> I think that the proper thing to do is to detach everything, if one of
>> the attach fails. There isn't much sense on keeping the device partially
>> initialized.
> It is not attach which fails here, it is dvb_register_frontend registration.
Yes, sure. I've miss-named.
> And I thought that too since it is rather major problem. But as it seems to be very easy to do it like that I did. Not big issue though.
Core functions should be more strict than drivers. So, I think it should really
enforce an all-or-nothing behavior here.
>>> + /* use exclusive FE lock if there is multiple shared FEs */
>>> + if (adap->fe[1])
>>> + adap->dvb_adap.mfe_shared = 1;
>>
>> Why? multiple FE doesn't mean that they're mutually exclusive.
>
> And again we ran same MFE discussion..... But if you look DVB USB you can see its main idea
> have been to register multiple adapters in case of not exclusive frontends. That have been
> most likely implemented like that from day-0 of DVB USB.
>
> Another point is that it is not that patch which adds support for MFE. It is old issue and
> I am not responsible of it. See patch where this decision is made in year 2008:
>
> commit 59b1842da1c6f33ad2e8da82d3dfb3445751d964
> Author: Darron Broad <darron@xxxxxxxx>
> Date: Sat Oct 11 11:44:05 2008 -0300
>
> V4L/DVB (9227): MFE: Add multi-frontend mutual exclusion
>
>
> Why to change again old behaviour?
>
>
> Could you now read all with care. As I pointed out almost all your findings are are decisions made *earlier*.
>
> Only thing which is not coming from oldies is that you suggest to unregister all frontends if one register fails.
You miss-understood me. I'm not saying that we should drop support for mfe_shared flag.
I'm just saying that dvb-usb core should not assume that a driver with more than one
FE have them into a mutually exclusive configuration.
If you look at dvb-core, the default for it is mfe_shared = 0. We should keep the same
default for dvb-usb.
In other words, it should be up to the drivers to change mfe_shared flag to 1, and not
to the core.
Cheers,
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
