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.
+ 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.
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. 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.
+ /* 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.
regards
Antti
--
http://palosaari.fi/
--
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
