| Message ID | 4E2E0788.3010507@iki.fi (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Em 25-07-2011 21:17, Antti Palosaari escreveu: > Signed-off-by: Antti Palosaari <crope@iki.fi> > --- > drivers/media/dvb/dvb-usb/dvb-usb-dvb.c | 85 +++++++++++++++++++++++------- > drivers/media/dvb/dvb-usb/dvb-usb-init.c | 4 ++ > drivers/media/dvb/dvb-usb/dvb-usb.h | 11 +++- > 3 files changed, 78 insertions(+), 22 deletions(-) > > diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > index d8c0bd9..5e34df7 100644 > --- a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > +++ b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > @@ -162,8 +162,11 @@ static int dvb_usb_fe_wakeup(struct dvb_frontend *fe) > > dvb_usb_device_power_ctrl(adap->dev, 1); > > - if (adap->fe_init) > - adap->fe_init(fe); > + if (adap->props.frontend_ctrl) > + adap->props.frontend_ctrl(fe, 1); > + > + if (adap->fe_init[fe->id]) > + adap->fe_init[fe->id](fe); > > return 0; > } > @@ -172,45 +175,89 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe) > { > struct dvb_usb_adapter *adap = fe->dvb->priv; > > - if (adap->fe_sleep) > - adap->fe_sleep(fe); > + if (adap->fe_sleep[fe->id]) > + adap->fe_sleep[fe->id](fe); > + > + if (adap->props.frontend_ctrl) > + adap->props.frontend_ctrl(fe, 0); > > return dvb_usb_device_power_ctrl(adap->dev, 0); > } > > int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap) > { > + int ret, i, x; > + > + memset(adap->fe, 0, sizeof(adap->fe)); > + > if (adap->props.frontend_attach == NULL) { > - err("strange: '%s' #%d doesn't want to attach a frontend.",adap->dev->desc->name, adap->id); > + err("strange: '%s' #%d doesn't want to attach a frontend.", > + adap->dev->desc->name, adap->id); > + > return 0; > } > > - /* re-assign sleep and wakeup functions */ > - if (adap->props.frontend_attach(adap) == 0 && adap->fe[0] != NULL) { > - adap->fe_init = adap->fe[0]->ops.init; adap->fe[0]->ops.init = dvb_usb_fe_wakeup; > - adap->fe_sleep = adap->fe[0]->ops.sleep; adap->fe[0]->ops.sleep = dvb_usb_fe_sleep; > + /* register all given adapter frontends */ > + if (adap->props.num_frontends) > + x = adap->props.num_frontends - 1; > + else > + x = 0; > + > + 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; } > + > + return 0; > + } > > - if (dvb_register_frontend(&adap->dvb_adap, adap->fe[0])) { > - err("Frontend registration failed."); > - dvb_frontend_detach(adap->fe[0]); > - adap->fe[0] = NULL; > - return -ENODEV; > + adap->fe[i]->id = i; > + > + /* re-assign sleep and wakeup functions */ > + adap->fe_init[i] = adap->fe[i]->ops.init; > + adap->fe[i]->ops.init = dvb_usb_fe_wakeup; > + adap->fe_sleep[i] = adap->fe[i]->ops.sleep; > + adap->fe[i]->ops.sleep = dvb_usb_fe_sleep; > + > + 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. > + 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. > + if (i == 0) > + return -ENODEV; > + else > + return 0; > } > > /* only attach the tuner if the demod is there */ > if (adap->props.tuner_attach != NULL) > adap->props.tuner_attach(adap); > - } else > - err("no frontend was attached by '%s'",adap->dev->desc->name); > + } > > return 0; > } > > int dvb_usb_adapter_frontend_exit(struct dvb_usb_adapter *adap) > { > - if (adap->fe[0] != NULL) { > - dvb_unregister_frontend(adap->fe[0]); > - dvb_frontend_detach(adap->fe[0]); > + int i; > + > + /* unregister all given adapter frontends */ > + if (adap->props.num_frontends) > + i = adap->props.num_frontends - 1; > + else > + i = 0; > + > + for (; i >= 0; i--) { > + if (adap->fe[i] != NULL) { > + dvb_unregister_frontend(adap->fe[i]); > + dvb_frontend_detach(adap->fe[i]); > + } > } > + > return 0; > } > diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-init.c b/drivers/media/dvb/dvb-usb/dvb-usb-init.c > index 2e3ea0f..f9af348 100644 > --- a/drivers/media/dvb/dvb-usb/dvb-usb-init.c > +++ b/drivers/media/dvb/dvb-usb/dvb-usb-init.c > @@ -77,6 +77,10 @@ static int dvb_usb_adapter_init(struct dvb_usb_device *d, short *adapter_nrs) > return ret; > } > > + /* 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. > + > d->num_adapters_initialized++; > d->state |= DVB_USB_STATE_DVB; > } > diff --git a/drivers/media/dvb/dvb-usb/dvb-usb.h b/drivers/media/dvb/dvb-usb/dvb-usb.h > index 2e57bff..a3e77b2 100644 > --- a/drivers/media/dvb/dvb-usb/dvb-usb.h > +++ b/drivers/media/dvb/dvb-usb/dvb-usb.h > @@ -124,6 +124,8 @@ struct usb_data_stream_properties { > * @caps: capabilities of the DVB USB device. > * @pid_filter_count: number of PID filter position in the optional hardware > * PID-filter. > + * @num_frontends: number of frontends of the DVB USB adapter. > + * @frontend_ctrl: called to power on/off active frontend. > * @streaming_ctrl: called to start and stop the MPEG2-TS streaming of the > * device (not URB submitting/killing). > * @pid_filter_ctrl: called to en/disable the PID filter, if any. > @@ -141,7 +143,9 @@ struct dvb_usb_adapter_properties { > #define DVB_USB_ADAP_RECEIVES_204_BYTE_TS 0x08 > int caps; > int pid_filter_count; > + int num_frontends; > > + int (*frontend_ctrl) (struct dvb_frontend *, int); > int (*streaming_ctrl) (struct dvb_usb_adapter *, int); > int (*pid_filter_ctrl) (struct dvb_usb_adapter *, int); > int (*pid_filter) (struct dvb_usb_adapter *, int, u16, int); > @@ -345,6 +349,7 @@ struct usb_data_stream { > * > * @stream: the usb data stream. > */ > +#define MAX_NO_OF_FE_PER_ADAP 2 > struct dvb_usb_adapter { > struct dvb_usb_device *dev; > struct dvb_usb_adapter_properties props; > @@ -363,11 +368,11 @@ struct dvb_usb_adapter { > struct dmxdev dmxdev; > struct dvb_demux demux; > struct dvb_net dvb_net; > - struct dvb_frontend *fe[1]; > + struct dvb_frontend *fe[MAX_NO_OF_FE_PER_ADAP]; > int max_feed_count; > > - int (*fe_init) (struct dvb_frontend *); > - int (*fe_sleep) (struct dvb_frontend *); > + int (*fe_init[MAX_NO_OF_FE_PER_ADAP]) (struct dvb_frontend *); > + int (*fe_sleep[MAX_NO_OF_FE_PER_ADAP]) (struct dvb_frontend *); > > struct usb_data_stream stream; > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kewl.org> 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
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@kewl.org> > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/27/2011 11:06 PM, Mauro Carvalho Chehab wrote: > 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. Let the device driver made decision! I can add check that if ret < 0 then fail as fatal, but if there is ret=0 and fe=NULL is not fatal. But since it was discarded earlier I didnt added such check. There is many devices have like 2 demod places on board. Then it is housed as demod#0 + demod#1, #demod0 or #demod1. You will not know that before trying to attach demod. Thus it *must* be possible to discard missing demod silently. Surely there is soon someone end user making bug reports if you print error on that case or even deregister whole device. >>>> + 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. Lets add it later, like next merge window. >>>> + /* 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@kewl.org> >> 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. Peace of cake. Anyhow, we still have 2 adapters for DVB USB for that and have been long than I can remember. So I cannot real see point of doing that. regards Antti
On Wed, 2011-07-27 at 16:06 -0300, Mauro Carvalho Chehab wrote: > Em 25-07-2011 21:17, Antti Palosaari escreveu: > > Signed-off-by: Antti Palosaari <crope@iki.fi> > > --- > > drivers/media/dvb/dvb-usb/dvb-usb-dvb.c | 85 +++++++++++++++++++++++------- > > drivers/media/dvb/dvb-usb/dvb-usb-init.c | 4 ++ > > drivers/media/dvb/dvb-usb/dvb-usb.h | 11 +++- > > 3 files changed, 78 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > > index d8c0bd9..5e34df7 100644 > > --- a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > > +++ b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c > > @@ -162,8 +162,11 @@ static int dvb_usb_fe_wakeup(struct dvb_frontend *fe) > > > > dvb_usb_device_power_ctrl(adap->dev, 1); > > > > - if (adap->fe_init) > > - adap->fe_init(fe); > > + if (adap->props.frontend_ctrl) > > + adap->props.frontend_ctrl(fe, 1); > > + > > + if (adap->fe_init[fe->id]) > > + adap->fe_init[fe->id](fe); > > > > return 0; > > } > > @@ -172,45 +175,89 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe) > > { > > struct dvb_usb_adapter *adap = fe->dvb->priv; > > > > - if (adap->fe_sleep) > > - adap->fe_sleep(fe); > > + if (adap->fe_sleep[fe->id]) > > + adap->fe_sleep[fe->id](fe); > > + > > + if (adap->props.frontend_ctrl) > > + adap->props.frontend_ctrl(fe, 0); > > > > return dvb_usb_device_power_ctrl(adap->dev, 0); > > } > > > > int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap) > > { > > + int ret, i, x; > > + > > + memset(adap->fe, 0, sizeof(adap->fe)); > > + > > if (adap->props.frontend_attach == NULL) { > > - err("strange: '%s' #%d doesn't want to attach a frontend.",adap->dev->desc->name, adap->id); > > + err("strange: '%s' #%d doesn't want to attach a frontend.", > > + adap->dev->desc->name, adap->id); > > + > > return 0; > > } > > > > - /* re-assign sleep and wakeup functions */ > > - if (adap->props.frontend_attach(adap) == 0 && adap->fe[0] != NULL) { > > - adap->fe_init = adap->fe[0]->ops.init; adap->fe[0]->ops.init = dvb_usb_fe_wakeup; > > - adap->fe_sleep = adap->fe[0]->ops.sleep; adap->fe[0]->ops.sleep = dvb_usb_fe_sleep; > > + /* register all given adapter frontends */ > > + if (adap->props.num_frontends) > > + x = adap->props.num_frontends - 1; > > + else > > + x = 0; > > + > > + 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; > } > > > + > > + return 0; > > + } > > > > - if (dvb_register_frontend(&adap->dvb_adap, adap->fe[0])) { > > - err("Frontend registration failed."); > > - dvb_frontend_detach(adap->fe[0]); > > - adap->fe[0] = NULL; > > - return -ENODEV; > > + adap->fe[i]->id = i; > > + > > + /* re-assign sleep and wakeup functions */ > > + adap->fe_init[i] = adap->fe[i]->ops.init; > > + adap->fe[i]->ops.init = dvb_usb_fe_wakeup; > > + adap->fe_sleep[i] = adap->fe[i]->ops.sleep; > > + adap->fe[i]->ops.sleep = dvb_usb_fe_sleep; > > + > > + 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. > > > + 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. > From memory, I recall the existing code doesn't detach the frontend even if the device driver forces an error. So, the device driver must detach the frontend first. The trouble is that dvb-usb is becoming dated as new drivers tend to work around it. No one likes to touch it, out of fear of breaking existing drivers. I think perhaps some kind of legacy wrapper is needed here, with the moving of dvb-usb to its own core, so more development work can be done. Regards Malcolm -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/28/2011 01:07 AM, Malcolm Priestley wrote: > On Wed, 2011-07-27 at 16:06 -0300, Mauro Carvalho Chehab wrote: >> Em 25-07-2011 21:17, Antti Palosaari escreveu: >>> Signed-off-by: Antti Palosaari<crope@iki.fi> >>> + 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. >> >> From memory, I recall the existing code doesn't detach the frontend even > if the device driver forces an error. So, the device driver must detach > the frontend first. Yes, if you return for example error (or fe == NULL) for .tuner_attach() it does not detach or deregister it - just silently discard all. I thought very many times those when implementing this and keep very careful not to change old functionality. > The trouble is that dvb-usb is becoming dated as new drivers tend to > work around it. No one likes to touch it, out of fear of breaking > existing drivers. Yes, so true. I have thought that too. There is a lot of things I want to change but those are very hard without massive work. * We should have priv at the very first. No priv for FW DL example. * Remote keytable should be property of certain device model not adapter * There should be way to set count of adapter (and fe) in runtime (this is why I allowed to fail 2nd FE attach silently) * no probe (read eeprom etc) callback (I think read MAC could be renamed for probe) * no FE power control (GPIOs etc) that MFE patch adds this too * maybe probe1 and probe2 callbacks needed. sequence something like plug device => probe1 => download FW => probe2 => attach demod > I think perhaps some kind of legacy wrapper is needed here, with the > moving of dvb-usb to its own core, so more development work can be done. Wish like to thank you these comments Malcolm! regards Antti
On Thursday 28 July 2011 00:23:06 Antti Palosaari wrote: > On 07/28/2011 01:07 AM, Malcolm Priestley wrote: > > On Wed, 2011-07-27 at 16:06 -0300, Mauro Carvalho Chehab wrote: > >> Em 25-07-2011 21:17, Antti Palosaari escreveu: > >>> Signed-off-by: Antti Palosaari<crope@iki.fi> > >>> > >>> + 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. > >> > >> From memory, I recall the existing code doesn't detach the frontend > >> even > > > > if the device driver forces an error. So, the device driver must detach > > the frontend first. > > Yes, if you return for example error (or fe == NULL) for .tuner_attach() > it does not detach or deregister it - just silently discard all. I > thought very many times those when implementing this and keep very > careful not to change old functionality. > > > The trouble is that dvb-usb is becoming dated as new drivers tend to > > work around it. No one likes to touch it, out of fear of breaking > > existing drivers. > > Yes, so true. I have thought that too. There is a lot of things I want > to change but those are very hard without massive work. > > * We should have priv at the very first. No priv for FW DL example. > * Remote keytable should be property of certain device model not adapter > * There should be way to set count of adapter (and fe) in runtime (this > is why I allowed to fail 2nd FE attach silently) > * no probe (read eeprom etc) callback (I think read MAC could be renamed > for probe) > * no FE power control (GPIOs etc) that MFE patch adds this too > * maybe probe1 and probe2 callbacks needed. sequence something like plug > device => probe1 => download FW => probe2 => attach demod If I had more time I'd add * handle suspend/resume calls properly for buggy USB firmwares (iow: all devices I saw) -- Patrick Boettcher - KernelLabs http://www.kernellabs.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: > 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. For more you add configuration parameters more it goes complex. Now it calls attach as many times as .num_frontends is set in adapter configuration. It is currently only DRX-K frontend which does not behave like other FEs. You have added similar hacks to em28xx and now DVB USB. Maybe it could be easier to change DRX-K driver to attach and register as others. Also I see it very easy at least in theory to register as one DRX-K FE normally and then hack 2nd FE in device driver (which is I think done other drivers using that chip too). regards Antti
On 07/31/2011 09:28 PM, Patrick Boettcher wrote: > On Thursday 28 July 2011 00:23:06 Antti Palosaari wrote: >> * We should have priv at the very first. No priv for FW DL example. >> * Remote keytable should be property of certain device model not adapter >> * There should be way to set count of adapter (and fe) in runtime (this >> is why I allowed to fail 2nd FE attach silently) >> * no probe (read eeprom etc) callback (I think read MAC could be renamed >> for probe) >> * no FE power control (GPIOs etc) that MFE patch adds this too >> * maybe probe1 and probe2 callbacks needed. sequence something like plug >> device => probe1 => download FW => probe2 => attach demod > > If I had more time I'd add > > * handle suspend/resume calls properly for buggy USB firmwares (iow: all > devices I saw) I think I will try to change next that priv is accessible at the very first. That's rather big problem since there is multiple drivers needing priv for communication (buffers, msg seq numbers, etc) for example fw loading. After that I see important to move remote keytable mapping to level where is USB IDs, device name and such info. regards Antti
Em 31-07-2011 22:22, Antti Palosaari escreveu: > On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >> 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. > > For more you add configuration parameters more it goes complex. Now it > calls attach as many times as .num_frontends is set in adapter > configuration. True, but even on a device with separate frontends, it needs to have some way to track what's the fe number that is free, and even this won't work. The logic there expects that, for the first call, it will fill fe[0], for the second call, it will fill fe[1], and so on. If, for whatever reason, a call to for fe[0] fails, the driver may do the wrong thing, e. g. it may be filling fe[0] while fe[1] is expected, fe[1] while fe[2] is expected, and so on. As I said before: or it should be called once, or it needs to pass a parameter to the driver to indicate what fe is expected to be filled. > It is currently only DRX-K frontend which does not behave > like other FEs. You have added similar hacks to em28xx and now DVB USB. > Maybe it could be easier to change DRX-K driver to attach and register > as others. Also I see it very easy at least in theory to register as one > DRX-K FE normally and then hack 2nd FE in device driver (which is I > think done other drivers using that chip too). The current way of handling DRX-K is a temporary solution, while we don't come to a conclusion about how should we address a single frontend, like DRX-K, that supports multiple delivery systems. I agree that this is ugly, but mapping the same frontend as two separate FE's is also ugly. When MFE were first added, it were to be used by two different frontends. When the same FE implements more than one delivery system, there are currently two ways of implementing it: using MFE, or using DVB S2API. Both ways are currently used, depending on the delivery systems. This is messy. Regards, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/01/2011 05:24 AM, Mauro Carvalho Chehab wrote: > Em 31-07-2011 22:22, Antti Palosaari escreveu: >> On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >>> 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. I am planning to change DVB USB MFE call .frontend_attach() only once. Is there any comments about that? Currently there is anysee, ttusb2 and cx88 drivers which uses MFE and change is needed ASAP before more MFE devices are coming. Also .num_frontends can be removed after that, since DVB USB will just loop through 0 to MAX FEs and register all FEs found (fe pointer !NULL). CURRENTLY: ========== .frontend_attach() if (adap->fe_adap[0].fe == NULL) adap->fe_adap[0].fe = dvb_attach(DVB-T) else if (adap->fe_adap[1].fe == NULL) adap->fe_adap[1].fe = dvb_attach(DVB-C) else if (adap->fe_adap[2].fe == NULL) adap->fe_adap[2].fe = dvb_attach(DVB-S) PLANNED: ======== .frontend_attach() adap->fe_adap[0].fe = dvb_attach(DVB-T) adap->fe_adap[1].fe = dvb_attach(DVB-C) adap->fe_adap[2].fe = dvb_attach(DVB-S) regards Antti
On Wed, Sep 7, 2011 at 12:51 PM, Antti Palosaari <crope@iki.fi> wrote: > On 08/01/2011 05:24 AM, Mauro Carvalho Chehab wrote: >> >> Em 31-07-2011 22:22, Antti Palosaari escreveu: >>> >>> On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >>>> >>>> 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. > > I am planning to change DVB USB MFE call .frontend_attach() only once. Is > there any comments about that? > > Currently there is anysee, ttusb2 and cx88 drivers which uses MFE and change > is needed ASAP before more MFE devices are coming. > > Also .num_frontends can be removed after that, since DVB USB will just loop > through 0 to MAX FEs and register all FEs found (fe pointer !NULL). > > CURRENTLY: > ========== > .frontend_attach() > if (adap->fe_adap[0].fe == NULL) > adap->fe_adap[0].fe = dvb_attach(DVB-T) > else if (adap->fe_adap[1].fe == NULL) > adap->fe_adap[1].fe = dvb_attach(DVB-C) > else if (adap->fe_adap[2].fe == NULL) > adap->fe_adap[2].fe = dvb_attach(DVB-S) > > PLANNED: > ======== > .frontend_attach() > adap->fe_adap[0].fe = dvb_attach(DVB-T) > adap->fe_adap[1].fe = dvb_attach(DVB-C) > adap->fe_adap[2].fe = dvb_attach(DVB-S) Antti, I don't understand exactly what you are proposing -- Is this a change for the anysee driver? ...or is it a change for the dvb-usb framework? ...or is it a change to dvb-core, and every driver in the subsystem? In the anysee driver, I see that you are using this: .frontend_attach() if (adap->fe_adap[0].fe == NULL) adap->fe_adap[0].fe = dvb_attach(DVB-T) else if (adap->fe_adap[1].fe == NULL) adap->fe_adap[1].fe = dvb_attach(DVB-C) else if (adap->fe_adap[2].fe == NULL) adap->fe_adap[2].fe = dvb_attach(DVB-S) I have no problem if you want to change the anysee driver to remove the second dvb_usb_adap_fe_props context, and replace with the following: .frontend_attach() adap->fe_adap[0].fe = dvb_attach(DVB-T) adap->fe_adap[1].fe = dvb_attach(DVB-C) adap->fe_adap[2].fe = dvb_attach(DVB-S) I believe this will work in the anysee driver for you, even with my changes that got merged yesterday... However, I do *not* believe that such change should propogate to the dvb-usb framework or dvb-core itself, because it can have a large negative impact on the drivers using it. For example, my mxl111sf driver was merged yesterday. Since it is the initial driver merge, it currently only supports one frontend (ATSC). The device also supports two other delivery systems, and has two additional dtv demodulators, each attached via a separate input bus to the USB device, each streaming on a separate USB endpoint. Many demod drivers do an ID test or some other kind of initialization during the _attach() function. A device like the mxl111sf would have to manipulate the USB device state and alter the bus operations before and after each frontend attachment in order for the _attach() calls to succeed, not to mention the separate calls needed for bus negotiation to power on the correct demodulator and initialize its streaming data path. I repeat, if this is a change that is specific to your anysee driver, then it seems like a good idea to me. However, if your plan is to change dvb-usb itself, and / or dvb-core, then I'd really like to have a better idea of the implications that this change will bring forth. So, to help reduce the confusion, can you clarify exactly what code you plan to change, and what impact it will have on the drivers that exist today? Best Regards, Michael Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 7, 2011 at 1:41 PM, Michael Krufky <mkrufky@kernellabs.com> wrote: > On Wed, Sep 7, 2011 at 12:51 PM, Antti Palosaari <crope@iki.fi> wrote: >> On 08/01/2011 05:24 AM, Mauro Carvalho Chehab wrote: >>> >>> Em 31-07-2011 22:22, Antti Palosaari escreveu: >>>> >>>> On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >>>>> >>>>> 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. >> >> I am planning to change DVB USB MFE call .frontend_attach() only once. Is >> there any comments about that? >> >> Currently there is anysee, ttusb2 and cx88 drivers which uses MFE and change >> is needed ASAP before more MFE devices are coming. >> >> Also .num_frontends can be removed after that, since DVB USB will just loop >> through 0 to MAX FEs and register all FEs found (fe pointer !NULL). >> >> CURRENTLY: >> ========== >> .frontend_attach() >> if (adap->fe_adap[0].fe == NULL) >> adap->fe_adap[0].fe = dvb_attach(DVB-T) >> else if (adap->fe_adap[1].fe == NULL) >> adap->fe_adap[1].fe = dvb_attach(DVB-C) >> else if (adap->fe_adap[2].fe == NULL) >> adap->fe_adap[2].fe = dvb_attach(DVB-S) >> >> PLANNED: >> ======== >> .frontend_attach() >> adap->fe_adap[0].fe = dvb_attach(DVB-T) >> adap->fe_adap[1].fe = dvb_attach(DVB-C) >> adap->fe_adap[2].fe = dvb_attach(DVB-S) > > Antti, > > I don't understand exactly what you are proposing -- Is this a change > for the anysee driver? ...or is it a change for the dvb-usb > framework? ...or is it a change to dvb-core, and every driver in the > subsystem? > > In the anysee driver, I see that you are using this: > > .frontend_attach() > if (adap->fe_adap[0].fe == NULL) > adap->fe_adap[0].fe = dvb_attach(DVB-T) > else if (adap->fe_adap[1].fe == NULL) > adap->fe_adap[1].fe = dvb_attach(DVB-C) > else if (adap->fe_adap[2].fe == NULL) > adap->fe_adap[2].fe = dvb_attach(DVB-S) > > I have no problem if you want to change the anysee driver to remove > the second dvb_usb_adap_fe_props context, and replace with the > following: > > > .frontend_attach() > adap->fe_adap[0].fe = dvb_attach(DVB-T) > adap->fe_adap[1].fe = dvb_attach(DVB-C) > adap->fe_adap[2].fe = dvb_attach(DVB-S) > > I believe this will work in the anysee driver for you, even with my > changes that got merged yesterday... However, I do *not* believe that > such change should propogate to the dvb-usb framework or dvb-core > itself, because it can have a large negative impact on the drivers > using it. > > For example, my mxl111sf driver was merged yesterday. Since it is the > initial driver merge, it currently only supports one frontend (ATSC). > The device also supports two other delivery systems, and has two > additional dtv demodulators, each attached via a separate input bus to > the USB device, each streaming on a separate USB endpoint. > > Many demod drivers do an ID test or some other kind of initialization > during the _attach() function. A device like the mxl111sf would have > to manipulate the USB device state and alter the bus operations before > and after each frontend attachment in order for the _attach() calls to > succeed, not to mention the separate calls needed for bus negotiation > to power on the correct demodulator and initialize its streaming data > path. > > I repeat, if this is a change that is specific to your anysee driver, > then it seems like a good idea to me. However, if your plan is to > change dvb-usb itself, and / or dvb-core, then I'd really like to have > a better idea of the implications that this change will bring forth. > > So, to help reduce the confusion, can you clarify exactly what code > you plan to change, and what impact it will have on the drivers that > exist today? > > Best Regards, > > Michael Krufky > ADDENDUM: For the anysee driver, for your single .frontend_attach() adap->fe_adap[0].fe = dvb_attach(DVB-T) adap->fe_adap[1].fe = dvb_attach(DVB-C) adap->fe_adap[2].fe = dvb_attach(DVB-S) ...for this to work in today's dvb-usb code without modification to the dvb-usb framework, i believe that you should do a test for (adap->fe_adap[0].fe && adap->fe_adap[1].fe && adap->fe_adap[2].fe ) ... if null, then attach, if not null, then exit -- this will prevent the second context's initialization from occurring twice. Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> On Wed, Sep 7, 2011 at 12:51 PM, Antti Palosaari <crope@iki.fi> wrote: >>> Also .num_frontends can be removed after that, since DVB USB will just loop >>> through 0 to MAX FEs and register all FEs found (fe pointer !NULL). We need to keep .num_frontends and .num_adapters both, because my next change to dvb-usb is to convert the hard-sized array of struct dvb_usb_adapter[MAX_NO_OF_ADAPTER_PER_DEVICE] and struct dvb_usb_fe_adapter[MAX_NO_OF_FE_PER_ADAP] to a dynamic-allocated array of pointers, to reduce the size of *all* dvb-usb drivers. I didn't push all changes yet because I thought we should test each change for a few days before merging too much all at once. I wanted to prevent the introduction of instability by making this a gradual change so we can test things one by one. Should I wait a bit before making the conversion of the hardcoded sized-arrays to dynamic sized property pointers? Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2011 08:45 PM, Michael Krufky wrote: > On Wed, Sep 7, 2011 at 1:41 PM, Michael Krufky<mkrufky@kernellabs.com> wrote: >> On Wed, Sep 7, 2011 at 12:51 PM, Antti Palosaari<crope@iki.fi> wrote: >>> On 08/01/2011 05:24 AM, Mauro Carvalho Chehab wrote: >>>> >>>> Em 31-07-2011 22:22, Antti Palosaari escreveu: >>>>> >>>>> On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >>>>>> >>>>>> 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. >>> >>> I am planning to change DVB USB MFE call .frontend_attach() only once. Is >>> there any comments about that? >>> >>> Currently there is anysee, ttusb2 and cx88 drivers which uses MFE and change >>> is needed ASAP before more MFE devices are coming. >>> >>> Also .num_frontends can be removed after that, since DVB USB will just loop >>> through 0 to MAX FEs and register all FEs found (fe pointer !NULL). >>> >>> CURRENTLY: >>> ========== >>> .frontend_attach() >>> if (adap->fe_adap[0].fe == NULL) >>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>> else if (adap->fe_adap[1].fe == NULL) >>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>> else if (adap->fe_adap[2].fe == NULL) >>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >>> >>> PLANNED: >>> ======== >>> .frontend_attach() >>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >> >> Antti, >> >> I don't understand exactly what you are proposing -- Is this a change >> for the anysee driver? ...or is it a change for the dvb-usb >> framework? ...or is it a change to dvb-core, and every driver in the >> subsystem? >> >> In the anysee driver, I see that you are using this: >> >> .frontend_attach() >> if (adap->fe_adap[0].fe == NULL) >> adap->fe_adap[0].fe = dvb_attach(DVB-T) >> else if (adap->fe_adap[1].fe == NULL) >> adap->fe_adap[1].fe = dvb_attach(DVB-C) >> else if (adap->fe_adap[2].fe == NULL) >> adap->fe_adap[2].fe = dvb_attach(DVB-S) >> >> I have no problem if you want to change the anysee driver to remove >> the second dvb_usb_adap_fe_props context, and replace with the >> following: >> >> >> .frontend_attach() >> adap->fe_adap[0].fe = dvb_attach(DVB-T) >> adap->fe_adap[1].fe = dvb_attach(DVB-C) >> adap->fe_adap[2].fe = dvb_attach(DVB-S) >> >> I believe this will work in the anysee driver for you, even with my >> changes that got merged yesterday... However, I do *not* believe that >> such change should propogate to the dvb-usb framework or dvb-core >> itself, because it can have a large negative impact on the drivers >> using it. >> >> For example, my mxl111sf driver was merged yesterday. Since it is the >> initial driver merge, it currently only supports one frontend (ATSC). >> The device also supports two other delivery systems, and has two >> additional dtv demodulators, each attached via a separate input bus to >> the USB device, each streaming on a separate USB endpoint. >> >> Many demod drivers do an ID test or some other kind of initialization >> during the _attach() function. A device like the mxl111sf would have >> to manipulate the USB device state and alter the bus operations before >> and after each frontend attachment in order for the _attach() calls to >> succeed, not to mention the separate calls needed for bus negotiation >> to power on the correct demodulator and initialize its streaming data >> path. >> >> I repeat, if this is a change that is specific to your anysee driver, >> then it seems like a good idea to me. However, if your plan is to >> change dvb-usb itself, and / or dvb-core, then I'd really like to have >> a better idea of the implications that this change will bring forth. >> >> So, to help reduce the confusion, can you clarify exactly what code >> you plan to change, and what impact it will have on the drivers that >> exist today? >> >> Best Regards, >> >> Michael Krufky >> > > ADDENDUM: > > For the anysee driver, for your single .frontend_attach() > adap->fe_adap[0].fe = dvb_attach(DVB-T) > adap->fe_adap[1].fe = dvb_attach(DVB-C) > adap->fe_adap[2].fe = dvb_attach(DVB-S) > > ...for this to work in today's dvb-usb code without modification to > the dvb-usb framework, i believe that you should do a test for > (adap->fe_adap[0].fe&& adap->fe_adap[1].fe&& adap->fe_adap[2].fe ) > ... if null, then attach, if not null, then exit -- this will prevent > the second context's initialization from occurring twice. Yes, I now saw when looked latest anysee driver that you moved .streaming_ctrl(), .frontend_attach() and .tuner_attach() to frontend property. OK, it is not then relevant anymore to change register all as once. What is size_of_priv used? regards Antti
On Wed, Sep 7, 2011 at 2:20 PM, Antti Palosaari <crope@iki.fi> wrote: > On 09/07/2011 08:45 PM, Michael Krufky wrote: >> >> On Wed, Sep 7, 2011 at 1:41 PM, Michael Krufky<mkrufky@kernellabs.com> >> wrote: >>> >>> On Wed, Sep 7, 2011 at 12:51 PM, Antti Palosaari<crope@iki.fi> wrote: >>>> >>>> On 08/01/2011 05:24 AM, Mauro Carvalho Chehab wrote: >>>>> >>>>> Em 31-07-2011 22:22, Antti Palosaari escreveu: >>>>>> >>>>>> On 08/01/2011 03:46 AM, Mauro Carvalho Chehab wrote: >>>>>>> >>>>>>> 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. >>>> >>>> I am planning to change DVB USB MFE call .frontend_attach() only once. >>>> Is >>>> there any comments about that? >>>> >>>> Currently there is anysee, ttusb2 and cx88 drivers which uses MFE and >>>> change >>>> is needed ASAP before more MFE devices are coming. >>>> >>>> Also .num_frontends can be removed after that, since DVB USB will just >>>> loop >>>> through 0 to MAX FEs and register all FEs found (fe pointer !NULL). >>>> >>>> CURRENTLY: >>>> ========== >>>> .frontend_attach() >>>> if (adap->fe_adap[0].fe == NULL) >>>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>>> else if (adap->fe_adap[1].fe == NULL) >>>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>>> else if (adap->fe_adap[2].fe == NULL) >>>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >>>> >>>> PLANNED: >>>> ======== >>>> .frontend_attach() >>>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >>> >>> Antti, >>> >>> I don't understand exactly what you are proposing -- Is this a change >>> for the anysee driver? ...or is it a change for the dvb-usb >>> framework? ...or is it a change to dvb-core, and every driver in the >>> subsystem? >>> >>> In the anysee driver, I see that you are using this: >>> >>> .frontend_attach() >>> if (adap->fe_adap[0].fe == NULL) >>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>> else if (adap->fe_adap[1].fe == NULL) >>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>> else if (adap->fe_adap[2].fe == NULL) >>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >>> >>> I have no problem if you want to change the anysee driver to remove >>> the second dvb_usb_adap_fe_props context, and replace with the >>> following: >>> >>> >>> .frontend_attach() >>> adap->fe_adap[0].fe = dvb_attach(DVB-T) >>> adap->fe_adap[1].fe = dvb_attach(DVB-C) >>> adap->fe_adap[2].fe = dvb_attach(DVB-S) >>> >>> I believe this will work in the anysee driver for you, even with my >>> changes that got merged yesterday... However, I do *not* believe that >>> such change should propogate to the dvb-usb framework or dvb-core >>> itself, because it can have a large negative impact on the drivers >>> using it. >>> >>> For example, my mxl111sf driver was merged yesterday. Since it is the >>> initial driver merge, it currently only supports one frontend (ATSC). >>> The device also supports two other delivery systems, and has two >>> additional dtv demodulators, each attached via a separate input bus to >>> the USB device, each streaming on a separate USB endpoint. >>> >>> Many demod drivers do an ID test or some other kind of initialization >>> during the _attach() function. A device like the mxl111sf would have >>> to manipulate the USB device state and alter the bus operations before >>> and after each frontend attachment in order for the _attach() calls to >>> succeed, not to mention the separate calls needed for bus negotiation >>> to power on the correct demodulator and initialize its streaming data >>> path. >>> >>> I repeat, if this is a change that is specific to your anysee driver, >>> then it seems like a good idea to me. However, if your plan is to >>> change dvb-usb itself, and / or dvb-core, then I'd really like to have >>> a better idea of the implications that this change will bring forth. >>> >>> So, to help reduce the confusion, can you clarify exactly what code >>> you plan to change, and what impact it will have on the drivers that >>> exist today? >>> >>> Best Regards, >>> >>> Michael Krufky >>> >> >> ADDENDUM: >> >> For the anysee driver, for your single .frontend_attach() >> adap->fe_adap[0].fe = dvb_attach(DVB-T) >> adap->fe_adap[1].fe = dvb_attach(DVB-C) >> adap->fe_adap[2].fe = dvb_attach(DVB-S) >> >> ...for this to work in today's dvb-usb code without modification to >> the dvb-usb framework, i believe that you should do a test for >> (adap->fe_adap[0].fe&& adap->fe_adap[1].fe&& adap->fe_adap[2].fe ) >> ... if null, then attach, if not null, then exit -- this will prevent >> the second context's initialization from occurring twice. > > Yes, I now saw when looked latest anysee driver that you moved > .streaming_ctrl(), .frontend_attach() and .tuner_attach() to frontend > property. OK, it is not then relevant anymore to change register all as > once. > > What is size_of_priv used? size_of_priv is a signal to the dvb-usb framework to allocate memory of size, size_of_priv to track device state at a given context. If you look in dvb-usb.h, there was always a size_of_priv / void *priv at the dvb_usb_device context level, and there was always a size_of_priv / void *priv at the dvb_usb_adapter context level. After my MFE patch, there is now a size_of_priv / void *priv at the dvb_usb_fe_adapter context level. This private state structure is used to track state at the context of each dvb_usb_fe_adap, to manage the environment needed to switch between the various attached frontends. You may take a look in mxl111sf.c to see how this is used (ie, struct mxl111sf_adap_state *adap_state = adap->fe_adap[fe->id].priv;) If size_of_priv is left undefined, it is initialized to 0, and the void *priv pointer remains null. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2011 09:36 PM, Michael Krufky wrote: > On Wed, Sep 7, 2011 at 2:20 PM, Antti Palosaari<crope@iki.fi> wrote: >> Yes, I now saw when looked latest anysee driver that you moved >> .streaming_ctrl(), .frontend_attach() and .tuner_attach() to frontend >> property. OK, it is not then relevant anymore to change register all as >> once. >> >> What is size_of_priv used? > > size_of_priv is a signal to the dvb-usb framework to allocate memory > of size, size_of_priv to track device state at a given context. If > you look in dvb-usb.h, there was always a size_of_priv / void *priv at > the dvb_usb_device context level, and there was always a size_of_priv > / void *priv at the dvb_usb_adapter context level. After my MFE > patch, there is now a size_of_priv / void *priv at the > dvb_usb_fe_adapter context level. This private state structure is > used to track state at the context of each dvb_usb_fe_adap, to manage > the environment needed to switch between the various attached > frontends. You may take a look in mxl111sf.c to see how this is used > (ie, struct mxl111sf_adap_state *adap_state = > adap->fe_adap[fe->id].priv;) > > If size_of_priv is left undefined, it is initialized to 0, and the > void *priv pointer remains null. I marvel at there was 3 states, one for device, one for each adapter and now even one for each frontend. Surely enough, generally only device state is used. And your new driver seems to even use that new FE priv added. regards Antti
On Wed, Sep 7, 2011 at 5:10 PM, Antti Palosaari <crope@iki.fi> wrote: > On 09/07/2011 09:36 PM, Michael Krufky wrote: >> >> On Wed, Sep 7, 2011 at 2:20 PM, Antti Palosaari<crope@iki.fi> wrote: > >>> Yes, I now saw when looked latest anysee driver that you moved >>> .streaming_ctrl(), .frontend_attach() and .tuner_attach() to frontend >>> property. OK, it is not then relevant anymore to change register all as >>> once. >>> >>> What is size_of_priv used? >> >> size_of_priv is a signal to the dvb-usb framework to allocate memory >> of size, size_of_priv to track device state at a given context. If >> you look in dvb-usb.h, there was always a size_of_priv / void *priv at >> the dvb_usb_device context level, and there was always a size_of_priv >> / void *priv at the dvb_usb_adapter context level. After my MFE >> patch, there is now a size_of_priv / void *priv at the >> dvb_usb_fe_adapter context level. This private state structure is >> used to track state at the context of each dvb_usb_fe_adap, to manage >> the environment needed to switch between the various attached >> frontends. You may take a look in mxl111sf.c to see how this is used >> (ie, struct mxl111sf_adap_state *adap_state = >> adap->fe_adap[fe->id].priv;) >> >> If size_of_priv is left undefined, it is initialized to 0, and the >> void *priv pointer remains null. > > I marvel at there was 3 states, one for device, one for each adapter and now > even one for each frontend. Surely enough, generally only device state is > used. And your new driver seems to even use that new FE priv added. My new driver requires state tracking at the dvb_usb_fe_adapter context level, but it does *not* require state tracking at the dvb_usb_adapter level. The driver also has state tracking at the dvb_usb_device context level. It needs this tracking at both levels, but not at the middle adapter level. dib0700, however, requires state tracking at the dvb_usb_adapter context level and it also requires state tracking at the dvb_usb_device context level. Most devices that have multiple adapters follow this same schema for tracking state within various context levels. The private state tracking structures are allocated dynamically as needed, and only if size_of_priv is defined. Device property contexts that do not define size_of_priv simply do not allocate any additional memory for state tracking. Having the ability to track private state within each context level gives the dvb-usb framework the maximum flexibility to work with various styles of both simple and complex digital media receiver devices. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c index d8c0bd9..5e34df7 100644 --- a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c +++ b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c @@ -162,8 +162,11 @@ static int dvb_usb_fe_wakeup(struct dvb_frontend *fe) dvb_usb_device_power_ctrl(adap->dev, 1); - if (adap->fe_init) - adap->fe_init(fe); + if (adap->props.frontend_ctrl) + adap->props.frontend_ctrl(fe, 1); + + if (adap->fe_init[fe->id]) + adap->fe_init[fe->id](fe); return 0; } @@ -172,45 +175,89 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe) { struct dvb_usb_adapter *adap = fe->dvb->priv; - if (adap->fe_sleep) - adap->fe_sleep(fe); + if (adap->fe_sleep[fe->id]) + adap->fe_sleep[fe->id](fe); + + if (adap->props.frontend_ctrl) + adap->props.frontend_ctrl(fe, 0); return dvb_usb_device_power_ctrl(adap->dev, 0); } int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap) { + int ret, i, x; + + memset(adap->fe, 0, sizeof(adap->fe)); + if (adap->props.frontend_attach == NULL) { - err("strange: '%s' #%d doesn't want to attach a frontend.",adap->dev->desc->name, adap->id); + err("strange: '%s' #%d doesn't want to attach a frontend.", + adap->dev->desc->name, adap->id); + return 0; } - /* re-assign sleep and wakeup functions */ - if (adap->props.frontend_attach(adap) == 0 && adap->fe[0] != NULL) { - adap->fe_init = adap->fe[0]->ops.init; adap->fe[0]->ops.init = dvb_usb_fe_wakeup; - adap->fe_sleep = adap->fe[0]->ops.sleep; adap->fe[0]->ops.sleep = dvb_usb_fe_sleep; + /* register all given adapter frontends */ + if (adap->props.num_frontends) + x = adap->props.num_frontends - 1; + else + x = 0; + + 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); + + return 0; + } - if (dvb_register_frontend(&adap->dvb_adap, adap->fe[0])) { - err("Frontend registration failed."); - dvb_frontend_detach(adap->fe[0]); - adap->fe[0] = NULL; - return -ENODEV; + adap->fe[i]->id = i; + + /* re-assign sleep and wakeup functions */ + adap->fe_init[i] = adap->fe[i]->ops.init; + adap->fe[i]->ops.init = dvb_usb_fe_wakeup; + adap->fe_sleep[i] = adap->fe[i]->ops.sleep; + adap->fe[i]->ops.sleep = dvb_usb_fe_sleep; + + if (dvb_register_frontend(&adap->dvb_adap, adap->fe[i])) { + err("Frontend %d registration failed.", i); + dvb_frontend_detach(adap->fe[i]); + adap->fe[i] = NULL; + /* In error case, do not try register more FEs, + * still leaving already registered FEs alive. */ + if (i == 0) + return -ENODEV; + else + return 0; } /* only attach the tuner if the demod is there */ if (adap->props.tuner_attach != NULL) adap->props.tuner_attach(adap); - } else - err("no frontend was attached by '%s'",adap->dev->desc->name); + } return 0; } int dvb_usb_adapter_frontend_exit(struct dvb_usb_adapter *adap) { - if (adap->fe[0] != NULL) { - dvb_unregister_frontend(adap->fe[0]); - dvb_frontend_detach(adap->fe[0]); + int i; + + /* unregister all given adapter frontends */ + if (adap->props.num_frontends) + i = adap->props.num_frontends - 1; + else + i = 0; + + for (; i >= 0; i--) { + if (adap->fe[i] != NULL) { + dvb_unregister_frontend(adap->fe[i]); + dvb_frontend_detach(adap->fe[i]); + } } + return 0; } diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-init.c b/drivers/media/dvb/dvb-usb/dvb-usb-init.c index 2e3ea0f..f9af348 100644 --- a/drivers/media/dvb/dvb-usb/dvb-usb-init.c +++ b/drivers/media/dvb/dvb-usb/dvb-usb-init.c @@ -77,6 +77,10 @@ static int dvb_usb_adapter_init(struct dvb_usb_device *d, short *adapter_nrs) return ret; } + /* use exclusive FE lock if there is multiple shared FEs */ + if (adap->fe[1]) + adap->dvb_adap.mfe_shared = 1; + d->num_adapters_initialized++; d->state |= DVB_USB_STATE_DVB; } diff --git a/drivers/media/dvb/dvb-usb/dvb-usb.h b/drivers/media/dvb/dvb-usb/dvb-usb.h index 2e57bff..a3e77b2 100644 --- a/drivers/media/dvb/dvb-usb/dvb-usb.h +++ b/drivers/media/dvb/dvb-usb/dvb-usb.h @@ -124,6 +124,8 @@ struct usb_data_stream_properties { * @caps: capabilities of the DVB USB device. * @pid_filter_count: number of PID filter position in the optional hardware * PID-filter. + * @num_frontends: number of frontends of the DVB USB adapter. + * @frontend_ctrl: called to power on/off active frontend. * @streaming_ctrl: called to start and stop the MPEG2-TS streaming of the * device (not URB submitting/killing).
Signed-off-by: Antti Palosaari <crope@iki.fi> --- drivers/media/dvb/dvb-usb/dvb-usb-dvb.c | 85 +++++++++++++++++++++++------- drivers/media/dvb/dvb-usb/dvb-usb-init.c | 4 ++ drivers/media/dvb/dvb-usb/dvb-usb.h | 11 +++- 3 files changed, 78 insertions(+), 22 deletions(-) * @pid_filter_ctrl: called to en/disable the PID filter, if any. @@ -141,7 +143,9 @@ struct dvb_usb_adapter_properties { #define DVB_USB_ADAP_RECEIVES_204_BYTE_TS 0x08 int caps; int pid_filter_count; + int num_frontends; + int (*frontend_ctrl) (struct dvb_frontend *, int); int (*streaming_ctrl) (struct dvb_usb_adapter *, int); int (*pid_filter_ctrl) (struct dvb_usb_adapter *, int); int (*pid_filter) (struct dvb_usb_adapter *, int, u16, int); @@ -345,6 +349,7 @@ struct usb_data_stream { * * @stream: the usb data stream. */ +#define MAX_NO_OF_FE_PER_ADAP 2 struct dvb_usb_adapter { struct dvb_usb_device *dev; struct dvb_usb_adapter_properties props; @@ -363,11 +368,11 @@ struct dvb_usb_adapter { struct dmxdev dmxdev; struct dvb_demux demux; struct dvb_net dvb_net; - struct dvb_frontend *fe[1]; + struct dvb_frontend *fe[MAX_NO_OF_FE_PER_ADAP]; int max_feed_count; - int (*fe_init) (struct dvb_frontend *); - int (*fe_sleep) (struct dvb_frontend *); + int (*fe_init[MAX_NO_OF_FE_PER_ADAP]) (struct dvb_frontend *); + int (*fe_sleep[MAX_NO_OF_FE_PER_ADAP]) (struct dvb_frontend *); struct usb_data_stream stream;