| Message ID | 201107142200.52970.jareguero@telefonica.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hello I did some review for this since I was interested of adding MFE for Anysee driver which is rather similar (dvb-usb-framework). I found this patch have rather major issue(s) which should be fixed properly. * it does not compile drivers/media/dvb/dvb-usb/dvb-usb.h:24:21: fatal error: dvb-pll.h: No such file or directory * it puts USB-bridge functionality to the frontend driver * 1st FE, TDA10023, is passed as pointer inside config to 2nd FE TDA10048. Much of glue sleep, i2c etc, those calls are wrapped back to to 1st FE... * no exclusive locking between MFEs. What happens if both are accessed same time? Almost all those are somehow chained to same issue, few calls to 2nd FE are wrapped to 1st FE. Maybe IOCTL override callback could help if those are really needed. regards Antti On 07/14/2011 11:00 PM, Jose Alberto Reguero wrote: > The attached patch try to fix the problems mentioned. > > Jose Alberto > > Signed-off-by: Jose Alberto Reguero<jareguero@telefonica.net> > > > > ttusb2-3.diff > > > diff -ur linux/drivers/media/dvb/dvb-usb/ttusb2.c linux.new/drivers/media/dvb/dvb-usb/ttusb2.c > --- linux/drivers/media/dvb/dvb-usb/ttusb2.c 2011-01-10 16:24:45.000000000 +0100 > +++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-07-14 12:55:36.645944109 +0200 > @@ -30,6 +30,7 @@ > #include "tda826x.h" > #include "tda10086.h" > #include "tda1002x.h" > +#include "tda10048.h" > #include "tda827x.h" > #include "lnbp21.h" > > @@ -44,6 +45,7 @@ > struct ttusb2_state { > u8 id; > u16 last_rc_key; > + struct dvb_frontend *fe; > }; > > static int ttusb2_msg(struct dvb_usb_device *d, u8 cmd, > @@ -82,7 +84,7 @@ > { > struct dvb_usb_device *d = i2c_get_adapdata(adap); > static u8 obuf[60], ibuf[60]; > - int i,read; > + int i, read1, read2; > > if (mutex_lock_interruptible(&d->i2c_mutex)< 0) > return -EAGAIN; > @@ -91,27 +93,33 @@ > warn("more than 2 i2c messages at a time is not handled yet. TODO."); > > for (i = 0; i< num; i++) { > - read = i+1< num&& (msg[i+1].flags& I2C_M_RD); > + read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); > + read2 = msg[i].flags& I2C_M_RD; > > - obuf[0] = (msg[i].addr<< 1) | read; > + obuf[0] = (msg[i].addr<< 1) | (read1 | read2); > obuf[1] = msg[i].len; > > /* read request */ > - if (read) > + if (read1) > obuf[2] = msg[i+1].len; > + else if (read2) > + obuf[2] = msg[i].len; > else > obuf[2] = 0; > > - memcpy(&obuf[3],msg[i].buf,msg[i].len); > + memcpy(&obuf[3], msg[i].buf, msg[i].len); > > if (ttusb2_msg(d, CMD_I2C_XFER, obuf, msg[i].len+3, ibuf, obuf[2] + 3)< 0) { > err("i2c transfer failed."); > break; > } > > - if (read) { > - memcpy(msg[i+1].buf,&ibuf[3],msg[i+1].len); > - i++; > + if (read1 || read2) { > + if (read1) { > + memcpy(msg[i+1].buf,&ibuf[3], msg[i+1].len); > + i++; > + } else if (read2) > + memcpy(msg[i].buf,&ibuf[3], msg[i].len); > } > } > > @@ -190,6 +198,21 @@ > .deltaf = 0xa511, > }; > > +static struct tda10048_config tda10048_config = { > + .demod_address = 0x10>> 1, > + .output_mode = TDA10048_PARALLEL_OUTPUT, > + .inversion = TDA10048_INVERSION_ON, > + .dtv6_if_freq_khz = TDA10048_IF_4000, > + .dtv7_if_freq_khz = TDA10048_IF_4500, > + .dtv8_if_freq_khz = TDA10048_IF_5000, > + .clk_freq_khz = TDA10048_CLK_16000, > + .no_firmware = 1, > +}; > + > +static struct tda827x_config tda827x_config = { > + .config = 0, > +}; > + > static int ttusb2_frontend_tda10086_attach(struct dvb_usb_adapter *adap) > { > if (usb_set_interface(adap->dev->udev,0,3)< 0) > @@ -205,18 +228,32 @@ > > static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap) > { > + > + struct ttusb2_state *state; > if (usb_set_interface(adap->dev->udev, 0, 3)< 0) > err("set interface to alts=3 failed"); > + state = (struct ttusb2_state *)adap->dev->priv; > if ((adap->fe = dvb_attach(tda10023_attach,&tda10023_config,&adap->dev->i2c_adap, 0x48)) == NULL) { > deb_info("TDA10023 attach failed\n"); > return -ENODEV; > } > + adap->fe->id = 1; > + tda10048_config.fe = adap->fe; > + if ((state->fe = dvb_attach(tda10048_attach,&tda10048_config,&adap->dev->i2c_adap)) == NULL) { > + deb_info("TDA10048 attach failed\n"); > + return -ENODEV; > + } > + dvb_register_frontend(&adap->dvb_adap, state->fe); > + if (dvb_attach(tda827x_attach, state->fe, 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { > + printk(KERN_ERR "%s: No tda827x found!\n", __func__); > + return -ENODEV; > + } > return 0; > } > > static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap) > { > - if (dvb_attach(tda827x_attach, adap->fe, 0x61,&adap->dev->i2c_adap, NULL) == NULL) { > + if (dvb_attach(tda827x_attach, adap->fe, 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { > printk(KERN_ERR "%s: No tda827x found!\n", __func__); > return -ENODEV; > } > @@ -242,6 +279,19 @@ > static struct dvb_usb_device_properties ttusb2_properties_s2400; > static struct dvb_usb_device_properties ttusb2_properties_ct3650; > > +static void ttusb2_usb_disconnect(struct usb_interface *intf) > +{ > + struct dvb_usb_device *d = usb_get_intfdata(intf); > + struct ttusb2_state *state; > + > + state = (struct ttusb2_state *)d->priv; > + if (state->fe) { > + dvb_unregister_frontend(state->fe); > + dvb_frontend_detach(state->fe); > + } > + dvb_usb_device_exit(intf); > +} > + > static int ttusb2_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > @@ -422,7 +472,7 @@ > static struct usb_driver ttusb2_driver = { > .name = "dvb_usb_ttusb2", > .probe = ttusb2_probe, > - .disconnect = dvb_usb_device_exit, > + .disconnect = ttusb2_usb_disconnect, > .id_table = ttusb2_table, > }; > > diff -ur linux/drivers/media/dvb/frontends/Makefile linux.new/drivers/media/dvb/frontends/Makefile > --- linux/drivers/media/dvb/frontends/Makefile 2011-05-06 05:45:29.000000000 +0200 > +++ linux.new/drivers/media/dvb/frontends/Makefile 2011-07-05 01:36:24.621564185 +0200 > @@ -4,6 +4,7 @@ > > EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ > EXTRA_CFLAGS += -Idrivers/media/common/tuners/ > +EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-usb/ > > stb0899-objs = stb0899_drv.o stb0899_algo.o > stv0900-objs = stv0900_core.o stv0900_sw.o > diff -ur linux/drivers/media/dvb/frontends/tda10048.c linux.new/drivers/media/dvb/frontends/tda10048.c > --- linux/drivers/media/dvb/frontends/tda10048.c 2010-10-25 01:34:58.000000000 +0200 > +++ linux.new/drivers/media/dvb/frontends/tda10048.c 2011-07-05 01:57:47.758466025 +0200 > @@ -30,6 +30,7 @@ > #include "dvb_frontend.h" > #include "dvb_math.h" > #include "tda10048.h" > +#include "dvb-usb.h" > > #define TDA10048_DEFAULT_FIRMWARE "dvb-fe-tda10048-1.0.fw" > #define TDA10048_DEFAULT_FIRMWARE_SIZE 24878 > @@ -214,6 +215,8 @@ > { TDA10048_CLK_16000, TDA10048_IF_3800, 10, 3, 0 }, > { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 }, > { TDA10048_CLK_16000, TDA10048_IF_4300, 10, 3, 0 }, > + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 }, > + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 }, > { TDA10048_CLK_16000, TDA10048_IF_36130, 10, 3, 0 }, > }; > > @@ -429,6 +432,19 @@ > return 0; > } > > +static int tda10048_set_pll(struct dvb_frontend *fe) > +{ > + struct tda10048_state *state = fe->demodulator_priv; > + > + dprintk(1, "%s()\n", __func__); > + > + tda10048_writereg(state, TDA10048_CONF_PLL1, 0x0f); > + tda10048_writereg(state, TDA10048_CONF_PLL2, (u8)(state->pll_mfactor)); > + tda10048_writereg(state, TDA10048_CONF_PLL3, tda10048_readreg(state, TDA10048_CONF_PLL3) | ((u8)(state->pll_nfactor) | 0x40)); > + > + return 0; > +} > + > static int tda10048_set_if(struct dvb_frontend *fe, enum fe_bandwidth bw) > { > struct tda10048_state *state = fe->demodulator_priv; > @@ -478,6 +494,9 @@ > dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor); > dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor); > > + /* Set the pll registers */ > + tda10048_set_pll(fe); > + > /* Calculate the sample frequency */ > state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45); > state->sample_freq /= (state->pll_nfactor + 1); > @@ -710,12 +729,16 @@ > if (config->disable_gate_access) > return 0; > > - if (enable) > - return tda10048_writereg(state, TDA10048_CONF_C4_1, > - tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); > - else > - return tda10048_writereg(state, TDA10048_CONF_C4_1, > - tda10048_readreg(state, TDA10048_CONF_C4_1)& 0xfd); > + if (config->fe&& config->fe->ops.i2c_gate_ctrl) { > + return config->fe->ops.i2c_gate_ctrl(config->fe, enable); > + } else { > + if (enable) > + return tda10048_writereg(state, TDA10048_CONF_C4_1, > + tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); > + else > + return tda10048_writereg(state, TDA10048_CONF_C4_1, > + tda10048_readreg(state, TDA10048_CONF_C4_1)& 0xfd); > + } > } > > static int tda10048_output_mode(struct dvb_frontend *fe, int serial) > @@ -772,20 +795,45 @@ > return 0; > } > > +static int tda10048_sleep(struct dvb_frontend *fe) > +{ > + struct tda10048_state *state = fe->demodulator_priv; > + struct tda10048_config *config =&state->config; > + struct dvb_usb_adapter *adap; > + > + dprintk(1, "%s()\n", __func__); > + > + if (config->fe) { > + adap = fe->dvb->priv; > + if (adap->dev->props.power_ctrl) > + adap->dev->props.power_ctrl(adap->dev, 0); > + } > + > + return 0; > +} > + > /* Establish sane defaults and load firmware. */ > static int tda10048_init(struct dvb_frontend *fe) > { > struct tda10048_state *state = fe->demodulator_priv; > struct tda10048_config *config =&state->config; > + struct dvb_usb_adapter *adap; > int ret = 0, i; > > dprintk(1, "%s()\n", __func__); > > + if (config->fe) { > + adap = fe->dvb->priv; > + if (adap->dev->props.power_ctrl) > + adap->dev->props.power_ctrl(adap->dev, 1); > + } > + > + > /* Apply register defaults */ > for (i = 0; i< ARRAY_SIZE(init_tab); i++) > tda10048_writereg(state, init_tab[i].reg, init_tab[i].data); > > - if (state->fwloaded == 0) > + if ((state->fwloaded == 0)&& (!config->no_firmware)) > ret = tda10048_firmware_upload(fe); > > /* Set either serial or parallel */ > @@ -1174,6 +1222,7 @@ > > .release = tda10048_release, > .init = tda10048_init, > + .sleep = tda10048_sleep, > .i2c_gate_ctrl = tda10048_i2c_gate_ctrl, > .set_frontend = tda10048_set_frontend, > .get_frontend = tda10048_get_frontend, > diff -ur linux/drivers/media/dvb/frontends/tda10048.h linux.new/drivers/media/dvb/frontends/tda10048.h > --- linux/drivers/media/dvb/frontends/tda10048.h 2010-07-03 23:22:08.000000000 +0200 > +++ linux.new/drivers/media/dvb/frontends/tda10048.h 2011-07-05 02:02:42.775466043 +0200 > @@ -51,6 +51,7 @@ > #define TDA10048_IF_4300 4300 > #define TDA10048_IF_4500 4500 > #define TDA10048_IF_4750 4750 > +#define TDA10048_IF_5000 5000 > #define TDA10048_IF_36130 36130 > u16 dtv6_if_freq_khz; > u16 dtv7_if_freq_khz; > @@ -62,6 +63,10 @@ > > /* Disable I2C gate access */ > u8 disable_gate_access; > + > + u8 no_firmware; > + > + struct dvb_frontend *fe; > }; > > #if defined(CONFIG_DVB_TDA10048) || \
MFE really needs to be implemented in the dvb-usb framework itself. I had started to work on this some time ago but had to stop working on it due to lack of time, and the fact that there was no public driver that I could use as a poster-child for the new functionality. In the end, rather than implement MFE correctly, I implemented each frontend as a separate adapter (which is *wrong* , but it sufficed for now) and added a locking mechanism in the bridge driver to prevent the frontends from being used simultaneously in an external driver that I was working on. If interested, I can post skeleton code that would illustrate how to get that done, but the better solution would be to truly implement MFE in dvb-usb. As of now, there is no public release driver that can use MFE in the dvb-usb framework. That is part of why I had to stop working on this. Do you plan to implement it in the dvb-usb framework? (...and regression test the other dvb-usb drivers) ?? Regards, Mike Krufky On Mon, Jul 18, 2011 at 4:28 PM, Antti Palosaari <crope@iki.fi> wrote: > Hello > I did some review for this since I was interested of adding MFE for Anysee > driver which is rather similar (dvb-usb-framework). > > I found this patch have rather major issue(s) which should be fixed > properly. > > * it does not compile > drivers/media/dvb/dvb-usb/dvb-usb.h:24:21: fatal error: dvb-pll.h: No such > file or directory > > * it puts USB-bridge functionality to the frontend driver > > * 1st FE, TDA10023, is passed as pointer inside config to 2nd FE TDA10048. > Much of glue sleep, i2c etc, those calls are wrapped back to to 1st FE... > > * no exclusive locking between MFEs. What happens if both are accessed same > time? > > > Almost all those are somehow chained to same issue, few calls to 2nd FE are > wrapped to 1st FE. Maybe IOCTL override callback could help if those are > really needed. > > > regards > Antti > > On 07/14/2011 11:00 PM, Jose Alberto Reguero wrote: >> >> The attached patch try to fix the problems mentioned. >> >> Jose Alberto >> >> Signed-off-by: Jose Alberto Reguero<jareguero@telefonica.net> >> >> >> >> ttusb2-3.diff >> >> >> diff -ur linux/drivers/media/dvb/dvb-usb/ttusb2.c >> linux.new/drivers/media/dvb/dvb-usb/ttusb2.c >> --- linux/drivers/media/dvb/dvb-usb/ttusb2.c 2011-01-10 >> 16:24:45.000000000 +0100 >> +++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-07-14 >> 12:55:36.645944109 +0200 >> @@ -30,6 +30,7 @@ >> #include "tda826x.h" >> #include "tda10086.h" >> #include "tda1002x.h" >> +#include "tda10048.h" >> #include "tda827x.h" >> #include "lnbp21.h" >> >> @@ -44,6 +45,7 @@ >> struct ttusb2_state { >> u8 id; >> u16 last_rc_key; >> + struct dvb_frontend *fe; >> }; >> >> static int ttusb2_msg(struct dvb_usb_device *d, u8 cmd, >> @@ -82,7 +84,7 @@ >> { >> struct dvb_usb_device *d = i2c_get_adapdata(adap); >> static u8 obuf[60], ibuf[60]; >> - int i,read; >> + int i, read1, read2; >> >> if (mutex_lock_interruptible(&d->i2c_mutex)< 0) >> return -EAGAIN; >> @@ -91,27 +93,33 @@ >> warn("more than 2 i2c messages at a time is not handled >> yet. TODO."); >> >> for (i = 0; i< num; i++) { >> - read = i+1< num&& (msg[i+1].flags& I2C_M_RD); >> + read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); >> + read2 = msg[i].flags& I2C_M_RD; >> >> - obuf[0] = (msg[i].addr<< 1) | read; >> + obuf[0] = (msg[i].addr<< 1) | (read1 | read2); >> obuf[1] = msg[i].len; >> >> /* read request */ >> - if (read) >> + if (read1) >> obuf[2] = msg[i+1].len; >> + else if (read2) >> + obuf[2] = msg[i].len; >> else >> obuf[2] = 0; >> >> - memcpy(&obuf[3],msg[i].buf,msg[i].len); >> + memcpy(&obuf[3], msg[i].buf, msg[i].len); >> >> if (ttusb2_msg(d, CMD_I2C_XFER, obuf, msg[i].len+3, ibuf, >> obuf[2] + 3)< 0) { >> err("i2c transfer failed."); >> break; >> } >> >> - if (read) { >> - memcpy(msg[i+1].buf,&ibuf[3],msg[i+1].len); >> - i++; >> + if (read1 || read2) { >> + if (read1) { >> + memcpy(msg[i+1].buf,&ibuf[3], >> msg[i+1].len); >> + i++; >> + } else if (read2) >> + memcpy(msg[i].buf,&ibuf[3], msg[i].len); >> } >> } >> >> @@ -190,6 +198,21 @@ >> .deltaf = 0xa511, >> }; >> >> +static struct tda10048_config tda10048_config = { >> + .demod_address = 0x10>> 1, >> + .output_mode = TDA10048_PARALLEL_OUTPUT, >> + .inversion = TDA10048_INVERSION_ON, >> + .dtv6_if_freq_khz = TDA10048_IF_4000, >> + .dtv7_if_freq_khz = TDA10048_IF_4500, >> + .dtv8_if_freq_khz = TDA10048_IF_5000, >> + .clk_freq_khz = TDA10048_CLK_16000, >> + .no_firmware = 1, >> +}; >> + >> +static struct tda827x_config tda827x_config = { >> + .config = 0, >> +}; >> + >> static int ttusb2_frontend_tda10086_attach(struct dvb_usb_adapter *adap) >> { >> if (usb_set_interface(adap->dev->udev,0,3)< 0) >> @@ -205,18 +228,32 @@ >> >> static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap) >> { >> + >> + struct ttusb2_state *state; >> if (usb_set_interface(adap->dev->udev, 0, 3)< 0) >> err("set interface to alts=3 failed"); >> + state = (struct ttusb2_state *)adap->dev->priv; >> if ((adap->fe = >> dvb_attach(tda10023_attach,&tda10023_config,&adap->dev->i2c_adap, 0x48)) == >> NULL) { >> deb_info("TDA10023 attach failed\n"); >> return -ENODEV; >> } >> + adap->fe->id = 1; >> + tda10048_config.fe = adap->fe; >> + if ((state->fe = >> dvb_attach(tda10048_attach,&tda10048_config,&adap->dev->i2c_adap)) == NULL) >> { >> + deb_info("TDA10048 attach failed\n"); >> + return -ENODEV; >> + } >> + dvb_register_frontend(&adap->dvb_adap, state->fe); >> + if (dvb_attach(tda827x_attach, state->fe, >> 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { >> + printk(KERN_ERR "%s: No tda827x found!\n", __func__); >> + return -ENODEV; >> + } >> return 0; >> } >> >> static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap) >> { >> - if (dvb_attach(tda827x_attach, adap->fe, >> 0x61,&adap->dev->i2c_adap, NULL) == NULL) { >> + if (dvb_attach(tda827x_attach, adap->fe, >> 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { >> printk(KERN_ERR "%s: No tda827x found!\n", __func__); >> return -ENODEV; >> } >> @@ -242,6 +279,19 @@ >> static struct dvb_usb_device_properties ttusb2_properties_s2400; >> static struct dvb_usb_device_properties ttusb2_properties_ct3650; >> >> +static void ttusb2_usb_disconnect(struct usb_interface *intf) >> +{ >> + struct dvb_usb_device *d = usb_get_intfdata(intf); >> + struct ttusb2_state *state; >> + >> + state = (struct ttusb2_state *)d->priv; >> + if (state->fe) { >> + dvb_unregister_frontend(state->fe); >> + dvb_frontend_detach(state->fe); >> + } >> + dvb_usb_device_exit(intf); >> +} >> + >> static int ttusb2_probe(struct usb_interface *intf, >> const struct usb_device_id *id) >> { >> @@ -422,7 +472,7 @@ >> static struct usb_driver ttusb2_driver = { >> .name = "dvb_usb_ttusb2", >> .probe = ttusb2_probe, >> - .disconnect = dvb_usb_device_exit, >> + .disconnect = ttusb2_usb_disconnect, >> .id_table = ttusb2_table, >> }; >> >> diff -ur linux/drivers/media/dvb/frontends/Makefile >> linux.new/drivers/media/dvb/frontends/Makefile >> --- linux/drivers/media/dvb/frontends/Makefile 2011-05-06 >> 05:45:29.000000000 +0200 >> +++ linux.new/drivers/media/dvb/frontends/Makefile 2011-07-05 >> 01:36:24.621564185 +0200 >> @@ -4,6 +4,7 @@ >> >> EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ >> EXTRA_CFLAGS += -Idrivers/media/common/tuners/ >> +EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-usb/ >> >> stb0899-objs = stb0899_drv.o stb0899_algo.o >> stv0900-objs = stv0900_core.o stv0900_sw.o >> diff -ur linux/drivers/media/dvb/frontends/tda10048.c >> linux.new/drivers/media/dvb/frontends/tda10048.c >> --- linux/drivers/media/dvb/frontends/tda10048.c 2010-10-25 >> 01:34:58.000000000 +0200 >> +++ linux.new/drivers/media/dvb/frontends/tda10048.c 2011-07-05 >> 01:57:47.758466025 +0200 >> @@ -30,6 +30,7 @@ >> #include "dvb_frontend.h" >> #include "dvb_math.h" >> #include "tda10048.h" >> +#include "dvb-usb.h" >> >> #define TDA10048_DEFAULT_FIRMWARE "dvb-fe-tda10048-1.0.fw" >> #define TDA10048_DEFAULT_FIRMWARE_SIZE 24878 >> @@ -214,6 +215,8 @@ >> { TDA10048_CLK_16000, TDA10048_IF_3800, 10, 3, 0 }, >> { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 }, >> { TDA10048_CLK_16000, TDA10048_IF_4300, 10, 3, 0 }, >> + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 }, >> + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 }, >> { TDA10048_CLK_16000, TDA10048_IF_36130, 10, 3, 0 }, >> }; >> >> @@ -429,6 +432,19 @@ >> return 0; >> } >> >> +static int tda10048_set_pll(struct dvb_frontend *fe) >> +{ >> + struct tda10048_state *state = fe->demodulator_priv; >> + >> + dprintk(1, "%s()\n", __func__); >> + >> + tda10048_writereg(state, TDA10048_CONF_PLL1, 0x0f); >> + tda10048_writereg(state, TDA10048_CONF_PLL2, >> (u8)(state->pll_mfactor)); >> + tda10048_writereg(state, TDA10048_CONF_PLL3, >> tda10048_readreg(state, TDA10048_CONF_PLL3) | ((u8)(state->pll_nfactor) | >> 0x40)); >> + >> + return 0; >> +} >> + >> static int tda10048_set_if(struct dvb_frontend *fe, enum fe_bandwidth bw) >> { >> struct tda10048_state *state = fe->demodulator_priv; >> @@ -478,6 +494,9 @@ >> dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor); >> dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor); >> >> + /* Set the pll registers */ >> + tda10048_set_pll(fe); >> + >> /* Calculate the sample frequency */ >> state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45); >> state->sample_freq /= (state->pll_nfactor + 1); >> @@ -710,12 +729,16 @@ >> if (config->disable_gate_access) >> return 0; >> >> - if (enable) >> - return tda10048_writereg(state, TDA10048_CONF_C4_1, >> - tda10048_readreg(state, TDA10048_CONF_C4_1) | >> 0x02); >> - else >> - return tda10048_writereg(state, TDA10048_CONF_C4_1, >> - tda10048_readreg(state, TDA10048_CONF_C4_1)& >> 0xfd); >> + if (config->fe&& config->fe->ops.i2c_gate_ctrl) { >> + return config->fe->ops.i2c_gate_ctrl(config->fe, enable); >> + } else { >> + if (enable) >> + return tda10048_writereg(state, >> TDA10048_CONF_C4_1, >> + tda10048_readreg(state, >> TDA10048_CONF_C4_1) | 0x02); >> + else >> + return tda10048_writereg(state, >> TDA10048_CONF_C4_1, >> + tda10048_readreg(state, >> TDA10048_CONF_C4_1)& 0xfd); >> + } >> } >> >> static int tda10048_output_mode(struct dvb_frontend *fe, int serial) >> @@ -772,20 +795,45 @@ >> return 0; >> } >> >> +static int tda10048_sleep(struct dvb_frontend *fe) >> +{ >> + struct tda10048_state *state = fe->demodulator_priv; >> + struct tda10048_config *config =&state->config; >> + struct dvb_usb_adapter *adap; >> + >> + dprintk(1, "%s()\n", __func__); >> + >> + if (config->fe) { >> + adap = fe->dvb->priv; >> + if (adap->dev->props.power_ctrl) >> + adap->dev->props.power_ctrl(adap->dev, 0); >> + } >> + >> + return 0; >> +} >> + >> /* Establish sane defaults and load firmware. */ >> static int tda10048_init(struct dvb_frontend *fe) >> { >> struct tda10048_state *state = fe->demodulator_priv; >> struct tda10048_config *config =&state->config; >> + struct dvb_usb_adapter *adap; >> int ret = 0, i; >> >> dprintk(1, "%s()\n", __func__); >> >> + if (config->fe) { >> + adap = fe->dvb->priv; >> + if (adap->dev->props.power_ctrl) >> + adap->dev->props.power_ctrl(adap->dev, 1); >> + } >> + >> + >> /* Apply register defaults */ >> for (i = 0; i< ARRAY_SIZE(init_tab); i++) >> tda10048_writereg(state, init_tab[i].reg, >> init_tab[i].data); >> >> - if (state->fwloaded == 0) >> + if ((state->fwloaded == 0)&& (!config->no_firmware)) >> ret = tda10048_firmware_upload(fe); >> >> /* Set either serial or parallel */ >> @@ -1174,6 +1222,7 @@ >> >> .release = tda10048_release, >> .init = tda10048_init, >> + .sleep = tda10048_sleep, >> .i2c_gate_ctrl = tda10048_i2c_gate_ctrl, >> .set_frontend = tda10048_set_frontend, >> .get_frontend = tda10048_get_frontend, >> diff -ur linux/drivers/media/dvb/frontends/tda10048.h >> linux.new/drivers/media/dvb/frontends/tda10048.h >> --- linux/drivers/media/dvb/frontends/tda10048.h 2010-07-03 >> 23:22:08.000000000 +0200 >> +++ linux.new/drivers/media/dvb/frontends/tda10048.h 2011-07-05 >> 02:02:42.775466043 +0200 >> @@ -51,6 +51,7 @@ >> #define TDA10048_IF_4300 4300 >> #define TDA10048_IF_4500 4500 >> #define TDA10048_IF_4750 4750 >> +#define TDA10048_IF_5000 5000 >> #define TDA10048_IF_36130 36130 >> u16 dtv6_if_freq_khz; >> u16 dtv7_if_freq_khz; >> @@ -62,6 +63,10 @@ >> >> /* Disable I2C gate access */ >> u8 disable_gate_access; >> + >> + u8 no_firmware; >> + >> + struct dvb_frontend *fe; >> }; >> >> #if defined(CONFIG_DVB_TDA10048) || \ > > > -- > http://palosaari.fi/ > -- > 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 > -- 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 Lunes, 18 de Julio de 2011 22:28:41 Antti Palosaari escribió: > Hello > I did some review for this since I was interested of adding MFE for > Anysee driver which is rather similar (dvb-usb-framework). > > I found this patch have rather major issue(s) which should be fixed > properly. > > * it does not compile > drivers/media/dvb/dvb-usb/dvb-usb.h:24:21: fatal error: dvb-pll.h: No > such file or directory Perhaps you need to add: -Idrivers/media/dvb/frontends/ in: drivers/media/dvb/frontends/Makefile I compile the driver with: git://linuxtv.org/mchehab/new_build.git and I not have this problem. > > * it puts USB-bridge functionality to the frontend driver > > * 1st FE, TDA10023, is passed as pointer inside config to 2nd FE > TDA10048. Much of glue sleep, i2c etc, those calls are wrapped back to > to 1st FE... > > * no exclusive locking between MFEs. What happens if both are accessed > same time? > > > Almost all those are somehow chained to same issue, few calls to 2nd FE > are wrapped to 1st FE. Maybe IOCTL override callback could help if those > are really needed. There are two problems: First, the two frontends (tda10048 and tda10023) use tda10023 i2c gate to talk with the tuner. The second is that with dvb-usb, there is only one frontend, and if you wake up the second frontend, the adapter is not wake up. That can be avoided the way I do in the patch, or mantaining the adapter alwais on. Jose Alberto > > > regards > Antti > > On 07/14/2011 11:00 PM, Jose Alberto Reguero wrote: > > The attached patch try to fix the problems mentioned. > > > > Jose Alberto > > > > Signed-off-by: Jose Alberto Reguero<jareguero@telefonica.net> > > > > > > > > ttusb2-3.diff > > > > > > diff -ur linux/drivers/media/dvb/dvb-usb/ttusb2.c > > linux.new/drivers/media/dvb/dvb-usb/ttusb2.c --- > > linux/drivers/media/dvb/dvb-usb/ttusb2.c 2011-01-10 16:24:45.000000000 > > +0100 +++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-07-14 > > 12:55:36.645944109 +0200 @@ -30,6 +30,7 @@ > > > > #include "tda826x.h" > > #include "tda10086.h" > > #include "tda1002x.h" > > > > +#include "tda10048.h" > > > > #include "tda827x.h" > > #include "lnbp21.h" > > > > @@ -44,6 +45,7 @@ > > > > struct ttusb2_state { > > > > u8 id; > > u16 last_rc_key; > > > > + struct dvb_frontend *fe; > > > > }; > > > > static int ttusb2_msg(struct dvb_usb_device *d, u8 cmd, > > > > @@ -82,7 +84,7 @@ > > > > { > > > > struct dvb_usb_device *d = i2c_get_adapdata(adap); > > static u8 obuf[60], ibuf[60]; > > > > - int i,read; > > + int i, read1, read2; > > > > if (mutex_lock_interruptible(&d->i2c_mutex)< 0) > > > > return -EAGAIN; > > > > @@ -91,27 +93,33 @@ > > > > warn("more than 2 i2c messages at a time is not handled yet. TODO."); > > > > for (i = 0; i< num; i++) { > > > > - read = i+1< num&& (msg[i+1].flags& I2C_M_RD); > > + read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); > > + read2 = msg[i].flags& I2C_M_RD; > > > > - obuf[0] = (msg[i].addr<< 1) | read; > > + obuf[0] = (msg[i].addr<< 1) | (read1 | read2); > > > > obuf[1] = msg[i].len; > > > > /* read request */ > > > > - if (read) > > + if (read1) > > > > obuf[2] = msg[i+1].len; > > > > + else if (read2) > > + obuf[2] = msg[i].len; > > > > else > > > > obuf[2] = 0; > > > > - memcpy(&obuf[3],msg[i].buf,msg[i].len); > > + memcpy(&obuf[3], msg[i].buf, msg[i].len); > > > > if (ttusb2_msg(d, CMD_I2C_XFER, obuf, msg[i].len+3, ibuf, obuf[2] + > > 3)< 0) { > > > > err("i2c transfer failed."); > > break; > > > > } > > > > - if (read) { > > - memcpy(msg[i+1].buf,&ibuf[3],msg[i+1].len); > > - i++; > > + if (read1 || read2) { > > + if (read1) { > > + memcpy(msg[i+1].buf,&ibuf[3], msg[i+1].len); > > + i++; > > + } else if (read2) > > + memcpy(msg[i].buf,&ibuf[3], msg[i].len); > > > > } > > > > } > > > > @@ -190,6 +198,21 @@ > > > > .deltaf = 0xa511, > > > > }; > > > > +static struct tda10048_config tda10048_config = { > > + .demod_address = 0x10>> 1, > > + .output_mode = TDA10048_PARALLEL_OUTPUT, > > + .inversion = TDA10048_INVERSION_ON, > > + .dtv6_if_freq_khz = TDA10048_IF_4000, > > + .dtv7_if_freq_khz = TDA10048_IF_4500, > > + .dtv8_if_freq_khz = TDA10048_IF_5000, > > + .clk_freq_khz = TDA10048_CLK_16000, > > + .no_firmware = 1, > > +}; > > + > > +static struct tda827x_config tda827x_config = { > > + .config = 0, > > +}; > > + > > > > static int ttusb2_frontend_tda10086_attach(struct dvb_usb_adapter > > *adap) { > > > > if (usb_set_interface(adap->dev->udev,0,3)< 0) > > > > @@ -205,18 +228,32 @@ > > > > static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter > > *adap) { > > > > + > > + struct ttusb2_state *state; > > > > if (usb_set_interface(adap->dev->udev, 0, 3)< 0) > > > > err("set interface to alts=3 failed"); > > > > + state = (struct ttusb2_state *)adap->dev->priv; > > > > if ((adap->fe = > > dvb_attach(tda10023_attach,&tda10023_config,&adap->dev->i2c_adap, > > 0x48)) == NULL) { > > > > deb_info("TDA10023 attach failed\n"); > > return -ENODEV; > > > > } > > > > + adap->fe->id = 1; > > + tda10048_config.fe = adap->fe; > > + if ((state->fe = > > dvb_attach(tda10048_attach,&tda10048_config,&adap->dev->i2c_adap)) == > > NULL) { + deb_info("TDA10048 attach failed\n"); > > + return -ENODEV; > > + } > > + dvb_register_frontend(&adap->dvb_adap, state->fe); > > + if (dvb_attach(tda827x_attach, state->fe, > > 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { + printk(KERN_ERR > > "%s: No tda827x found!\n", __func__); > > + return -ENODEV; > > + } > > > > return 0; > > > > } > > > > static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap) > > { > > > > - if (dvb_attach(tda827x_attach, adap->fe, 0x61,&adap->dev->i2c_adap, > > NULL) == NULL) { + if (dvb_attach(tda827x_attach, adap->fe, > > 0x61,&adap->dev->i2c_adap,&tda827x_config) == NULL) { > > > > printk(KERN_ERR "%s: No tda827x found!\n", __func__); > > return -ENODEV; > > > > } > > > > @@ -242,6 +279,19 @@ > > > > static struct dvb_usb_device_properties ttusb2_properties_s2400; > > static struct dvb_usb_device_properties ttusb2_properties_ct3650; > > > > +static void ttusb2_usb_disconnect(struct usb_interface *intf) > > +{ > > + struct dvb_usb_device *d = usb_get_intfdata(intf); > > + struct ttusb2_state *state; > > + > > + state = (struct ttusb2_state *)d->priv; > > + if (state->fe) { > > + dvb_unregister_frontend(state->fe); > > + dvb_frontend_detach(state->fe); > > + } > > + dvb_usb_device_exit(intf); > > +} > > + > > > > static int ttusb2_probe(struct usb_interface *intf, > > > > const struct usb_device_id *id) > > > > { > > > > @@ -422,7 +472,7 @@ > > > > static struct usb_driver ttusb2_driver = { > > > > .name = "dvb_usb_ttusb2", > > .probe = ttusb2_probe, > > > > - .disconnect = dvb_usb_device_exit, > > + .disconnect = ttusb2_usb_disconnect, > > > > .id_table = ttusb2_table, > > > > }; > > > > diff -ur linux/drivers/media/dvb/frontends/Makefile > > linux.new/drivers/media/dvb/frontends/Makefile --- > > linux/drivers/media/dvb/frontends/Makefile 2011-05-06 05:45:29.000000000 > > +0200 +++ linux.new/drivers/media/dvb/frontends/Makefile 2011-07-05 > > 01:36:24.621564185 +0200 @@ -4,6 +4,7 @@ > > > > EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ > > EXTRA_CFLAGS += -Idrivers/media/common/tuners/ > > > > +EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-usb/ > > > > stb0899-objs = stb0899_drv.o stb0899_algo.o > > stv0900-objs = stv0900_core.o stv0900_sw.o > > > > diff -ur linux/drivers/media/dvb/frontends/tda10048.c > > linux.new/drivers/media/dvb/frontends/tda10048.c --- > > linux/drivers/media/dvb/frontends/tda10048.c 2010-10-25 > > 01:34:58.000000000 +0200 +++ > > linux.new/drivers/media/dvb/frontends/tda10048.c 2011-07-05 > > 01:57:47.758466025 +0200 @@ -30,6 +30,7 @@ > > > > #include "dvb_frontend.h" > > #include "dvb_math.h" > > #include "tda10048.h" > > > > +#include "dvb-usb.h" > > > > #define TDA10048_DEFAULT_FIRMWARE "dvb-fe-tda10048-1.0.fw" > > #define TDA10048_DEFAULT_FIRMWARE_SIZE 24878 > > > > @@ -214,6 +215,8 @@ > > > > { TDA10048_CLK_16000, TDA10048_IF_3800, 10, 3, 0 }, > > { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 }, > > { TDA10048_CLK_16000, TDA10048_IF_4300, 10, 3, 0 }, > > > > + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 }, > > + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 }, > > > > { TDA10048_CLK_16000, TDA10048_IF_36130, 10, 3, 0 }, > > > > }; > > > > @@ -429,6 +432,19 @@ > > > > return 0; > > > > } > > > > +static int tda10048_set_pll(struct dvb_frontend *fe) > > +{ > > + struct tda10048_state *state = fe->demodulator_priv; > > + > > + dprintk(1, "%s()\n", __func__); > > + > > + tda10048_writereg(state, TDA10048_CONF_PLL1, 0x0f); > > + tda10048_writereg(state, TDA10048_CONF_PLL2, (u8)(state- >pll_mfactor)); > > + tda10048_writereg(state, TDA10048_CONF_PLL3, tda10048_readreg(state, > > TDA10048_CONF_PLL3) | ((u8)(state->pll_nfactor) | 0x40)); + > > + return 0; > > +} > > + > > > > static int tda10048_set_if(struct dvb_frontend *fe, enum fe_bandwidth > > bw) { > > > > struct tda10048_state *state = fe->demodulator_priv; > > > > @@ -478,6 +494,9 @@ > > > > dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor); > > dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor); > > > > + /* Set the pll registers */ > > + tda10048_set_pll(fe); > > + > > > > /* Calculate the sample frequency */ > > state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45); > > state->sample_freq /= (state->pll_nfactor + 1); > > > > @@ -710,12 +729,16 @@ > > > > if (config->disable_gate_access) > > > > return 0; > > > > - if (enable) > > - return tda10048_writereg(state, TDA10048_CONF_C4_1, > > - tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); > > - else > > - return tda10048_writereg(state, TDA10048_CONF_C4_1, > > - tda10048_readreg(state, TDA10048_CONF_C4_1)& 0xfd); > > + if (config->fe&& config->fe->ops.i2c_gate_ctrl) { > > + return config->fe->ops.i2c_gate_ctrl(config->fe, enable); > > + } else { > > + if (enable) > > + return tda10048_writereg(state, TDA10048_CONF_C4_1, > > + tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); > > + else > > + return tda10048_writereg(state, TDA10048_CONF_C4_1, > > + tda10048_readreg(state, TDA10048_CONF_C4_1)& 0xfd); > > + } > > > > } > > > > static int tda10048_output_mode(struct dvb_frontend *fe, int serial) > > > > @@ -772,20 +795,45 @@ > > > > return 0; > > > > } > > > > +static int tda10048_sleep(struct dvb_frontend *fe) > > +{ > > + struct tda10048_state *state = fe->demodulator_priv; > > + struct tda10048_config *config =&state->config; > > + struct dvb_usb_adapter *adap; > > + > > + dprintk(1, "%s()\n", __func__); > > + > > + if (config->fe) { > > + adap = fe->dvb->priv; > > + if (adap->dev->props.power_ctrl) > > + adap->dev->props.power_ctrl(adap->dev, 0); > > + } > > + > > + return 0; > > +} > > + > > > > /* Establish sane defaults and load firmware. */ > > static int tda10048_init(struct dvb_frontend *fe) > > { > > > > struct tda10048_state *state = fe->demodulator_priv; > > struct tda10048_config *config =&state->config; > > > > + struct dvb_usb_adapter *adap; > > > > int ret = 0, i; > > > > dprintk(1, "%s()\n", __func__); > > > > + if (config->fe) { > > + adap = fe->dvb->priv; > > + if (adap->dev->props.power_ctrl) > > + adap->dev->props.power_ctrl(adap->dev, 1); > > + } > > + > > + > > > > /* Apply register defaults */ > > for (i = 0; i< ARRAY_SIZE(init_tab); i++) > > > > tda10048_writereg(state, init_tab[i].reg, init_tab[i].data); > > > > - if (state->fwloaded == 0) > > + if ((state->fwloaded == 0)&& (!config->no_firmware)) > > > > ret = tda10048_firmware_upload(fe); > > > > /* Set either serial or parallel */ > > > > @@ -1174,6 +1222,7 @@ > > > > .release = tda10048_release, > > .init = tda10048_init, > > > > + .sleep = tda10048_sleep, > > > > .i2c_gate_ctrl = tda10048_i2c_gate_ctrl, > > .set_frontend = tda10048_set_frontend, > > .get_frontend = tda10048_get_frontend, > > > > diff -ur linux/drivers/media/dvb/frontends/tda10048.h > > linux.new/drivers/media/dvb/frontends/tda10048.h --- > > linux/drivers/media/dvb/frontends/tda10048.h 2010-07-03 > > 23:22:08.000000000 +0200 +++ > > linux.new/drivers/media/dvb/frontends/tda10048.h 2011-07-05 > > 02:02:42.775466043 +0200 @@ -51,6 +51,7 @@ > > > > #define TDA10048_IF_4300 4300 > > #define TDA10048_IF_4500 4500 > > #define TDA10048_IF_4750 4750 > > > > +#define TDA10048_IF_5000 5000 > > > > #define TDA10048_IF_36130 36130 > > > > u16 dtv6_if_freq_khz; > > u16 dtv7_if_freq_khz; > > > > @@ -62,6 +63,10 @@ > > > > /* Disable I2C gate access */ > > u8 disable_gate_access; > > > > + > > + u8 no_firmware; > > + > > + struct dvb_frontend *fe; > > > > }; > > > > #if defined(CONFIG_DVB_TDA10048) || \ -- 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/19/2011 02:00 AM, Jose Alberto Reguero wrote: > On Lunes, 18 de Julio de 2011 22:28:41 Antti Palosaari escribió: >> Hello >> I did some review for this since I was interested of adding MFE for >> Anysee driver which is rather similar (dvb-usb-framework). >> >> I found this patch have rather major issue(s) which should be fixed >> properly. >> >> * it does not compile >> drivers/media/dvb/dvb-usb/dvb-usb.h:24:21: fatal error: dvb-pll.h: No >> such file or directory > > Perhaps you need to add: > -Idrivers/media/dvb/frontends/ > in: > drivers/media/dvb/frontends/Makefile > I compile the driver with: > git://linuxtv.org/mchehab/new_build.git > and I not have this problem. Maybe, I was running latest Git. Not big issue. >> * it puts USB-bridge functionality to the frontend driver >> >> * 1st FE, TDA10023, is passed as pointer inside config to 2nd FE >> TDA10048. Much of glue sleep, i2c etc, those calls are wrapped back to >> to 1st FE... >> >> * no exclusive locking between MFEs. What happens if both are accessed >> same time? >> >> >> Almost all those are somehow chained to same issue, few calls to 2nd FE >> are wrapped to 1st FE. Maybe IOCTL override callback could help if those >> are really needed. > > There are two problems: > > First, the two frontends (tda10048 and tda10023) use tda10023 i2c gate to > talk with the tuner. Very easy to implement correctly. Attach tda10023 first and after that tda10048. Override tda10048 .i2c_gate_ctrl() with tda10023 .i2c_gate_ctrl() immediately after tda10048 attach inside ttusb2.c. Now you have both demods (FEs) .i2c_gate_ctrl() which will control physically tda10023 I2C-gate as tuner is behind it. > The second is that with dvb-usb, there is only one frontend, and if you wake > up the second frontend, the adapter is not wake up. That can be avoided the > way I do in the patch, or mantaining the adapter alwais on. I think that could be also avoided similarly overriding demod callbacks and adding some more logic inside ttusb2.c. Proper fix that later problem is surely correct MFE support for DVB-USB-framework. I am now looking for it, lets see how difficult it will be. regards Antti
On Martes, 19 de Julio de 2011 01:44:54 Antti Palosaari escribió: > On 07/19/2011 02:00 AM, Jose Alberto Reguero wrote: > > On Lunes, 18 de Julio de 2011 22:28:41 Antti Palosaari escribió: > >> Hello > >> I did some review for this since I was interested of adding MFE for > >> Anysee driver which is rather similar (dvb-usb-framework). > >> > >> I found this patch have rather major issue(s) which should be fixed > >> properly. > >> > >> * it does not compile > >> drivers/media/dvb/dvb-usb/dvb-usb.h:24:21: fatal error: dvb-pll.h: No > >> such file or directory > > > > Perhaps you need to add: > > -Idrivers/media/dvb/frontends/ > > in: > > drivers/media/dvb/frontends/Makefile > > I compile the driver with: > > git://linuxtv.org/mchehab/new_build.git > > and I not have this problem. > > Maybe, I was running latest Git. Not big issue. > > >> * it puts USB-bridge functionality to the frontend driver > >> > >> * 1st FE, TDA10023, is passed as pointer inside config to 2nd FE > >> TDA10048. Much of glue sleep, i2c etc, those calls are wrapped back to > >> to 1st FE... > >> > >> * no exclusive locking between MFEs. What happens if both are accessed > >> same time? > >> > >> > >> Almost all those are somehow chained to same issue, few calls to 2nd FE > >> are wrapped to 1st FE. Maybe IOCTL override callback could help if those > >> are really needed. > > > > There are two problems: > > > > First, the two frontends (tda10048 and tda10023) use tda10023 i2c gate > > to talk with the tuner. > > Very easy to implement correctly. Attach tda10023 first and after that > tda10048. Override tda10048 .i2c_gate_ctrl() with tda10023 > .i2c_gate_ctrl() immediately after tda10048 attach inside ttusb2.c. Now > you have both demods (FEs) .i2c_gate_ctrl() which will control > physically tda10023 I2C-gate as tuner is behind it. > I try that, but don't work. I get an oops. Because the i2c gate function of the tda10023 driver use: struct tda10023_state* state = fe->demodulator_priv; to get the i2c adress. When called from tda10048, don't work. Jose Alberto > > The second is that with dvb-usb, there is only one frontend, and if you > > wake up the second frontend, the adapter is not wake up. That can be > > avoided the way I do in the patch, or mantaining the adapter alwais on. > > I think that could be also avoided similarly overriding demod callbacks > and adding some more logic inside ttusb2.c. > > Proper fix that later problem is surely correct MFE support for > DVB-USB-framework. I am now looking for it, lets see how difficult it > will be. > > > regards > Antti -- 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 -ur linux/drivers/media/dvb/dvb-usb/ttusb2.c linux.new/drivers/media/dvb/dvb-usb/ttusb2.c --- linux/drivers/media/dvb/dvb-usb/ttusb2.c 2011-01-10 16:24:45.000000000 +0100 +++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-07-14 12:55:36.645944109 +0200 @@ -30,6 +30,7 @@ #include "tda826x.h" #include "tda10086.h" #include "tda1002x.h" +#include "tda10048.h" #include "tda827x.h" #include "lnbp21.h" @@ -44,6 +45,7 @@ struct ttusb2_state { u8 id; u16 last_rc_key; + struct dvb_frontend *fe; }; static int ttusb2_msg(struct dvb_usb_device *d, u8 cmd, @@ -82,7 +84,7 @@ { struct dvb_usb_device *d = i2c_get_adapdata(adap); static u8 obuf[60], ibuf[60]; - int i,read; + int i, read1, read2; if (mutex_lock_interruptible(&d->i2c_mutex) < 0) return -EAGAIN; @@ -91,27 +93,33 @@ warn("more than 2 i2c messages at a time is not handled yet. TODO."); for (i = 0; i < num; i++) { - read = i+1 < num && (msg[i+1].flags & I2C_M_RD); + read1 = i+1 < num && (msg[i+1].flags & I2C_M_RD); + read2 = msg[i].flags & I2C_M_RD; - obuf[0] = (msg[i].addr << 1) | read; + obuf[0] = (msg[i].addr << 1) | (read1 | read2); obuf[1] = msg[i].len; /* read request */ - if (read) + if (read1) obuf[2] = msg[i+1].len; + else if (read2) + obuf[2] = msg[i].len; else obuf[2] = 0; - memcpy(&obuf[3],msg[i].buf,msg[i].len); + memcpy(&obuf[3], msg[i].buf, msg[i].len); if (ttusb2_msg(d, CMD_I2C_XFER, obuf, msg[i].len+3, ibuf, obuf[2] + 3) < 0) { err("i2c transfer failed."); break; } - if (read) { - memcpy(msg[i+1].buf,&ibuf[3],msg[i+1].len); - i++; + if (read1 || read2) { + if (read1) { + memcpy(msg[i+1].buf, &ibuf[3], msg[i+1].len); + i++; + } else if (read2) + memcpy(msg[i].buf, &ibuf[3], msg[i].len); } } @@ -190,6 +198,21 @@ .deltaf = 0xa511, }; +static struct tda10048_config tda10048_config = { + .demod_address = 0x10 >> 1, + .output_mode = TDA10048_PARALLEL_OUTPUT, + .inversion = TDA10048_INVERSION_ON, + .dtv6_if_freq_khz = TDA10048_IF_4000, + .dtv7_if_freq_khz = TDA10048_IF_4500, + .dtv8_if_freq_khz = TDA10048_IF_5000, + .clk_freq_khz = TDA10048_CLK_16000, + .no_firmware = 1, +}; + +static struct tda827x_config tda827x_config = { + .config = 0, +}; + static int ttusb2_frontend_tda10086_attach(struct dvb_usb_adapter *adap) { if (usb_set_interface(adap->dev->udev,0,3) < 0) @@ -205,18 +228,32 @@ static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap) { + + struct ttusb2_state *state; if (usb_set_interface(adap->dev->udev, 0, 3) < 0) err("set interface to alts=3 failed"); + state = (struct ttusb2_state *)adap->dev->priv; if ((adap->fe = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) { deb_info("TDA10023 attach failed\n"); return -ENODEV; } + adap->fe->id = 1; + tda10048_config.fe = adap->fe; + if ((state->fe = dvb_attach(tda10048_attach, &tda10048_config, &adap->dev->i2c_adap)) == NULL) { + deb_info("TDA10048 attach failed\n"); + return -ENODEV; + } + dvb_register_frontend(&adap->dvb_adap, state->fe); + if (dvb_attach(tda827x_attach, state->fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) { + printk(KERN_ERR "%s: No tda827x found!\n", __func__); + return -ENODEV; + } return 0; } static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap) { - if (dvb_attach(tda827x_attach, adap->fe, 0x61, &adap->dev->i2c_adap, NULL) == NULL) { + if (dvb_attach(tda827x_attach, adap->fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) { printk(KERN_ERR "%s: No tda827x found!\n", __func__); return -ENODEV; } @@ -242,6 +279,19 @@ static struct dvb_usb_device_properties ttusb2_properties_s2400; static struct dvb_usb_device_properties ttusb2_properties_ct3650; +static void ttusb2_usb_disconnect(struct usb_interface *intf) +{ + struct dvb_usb_device *d = usb_get_intfdata(intf); + struct ttusb2_state *state; + + state = (struct ttusb2_state *)d->priv; + if (state->fe) { + dvb_unregister_frontend(state->fe); + dvb_frontend_detach(state->fe); + } + dvb_usb_device_exit(intf); +} + static int ttusb2_probe(struct usb_interface *intf, const struct usb_device_id *id) { @@ -422,7 +472,7 @@ static struct usb_driver ttusb2_driver = { .name = "dvb_usb_ttusb2", .probe = ttusb2_probe, - .disconnect = dvb_usb_device_exit, + .disconnect = ttusb2_usb_disconnect, .id_table = ttusb2_table, }; diff -ur linux/drivers/media/dvb/frontends/Makefile linux.new/drivers/media/dvb/frontends/Makefile --- linux/drivers/media/dvb/frontends/Makefile 2011-05-06 05:45:29.000000000 +0200 +++ linux.new/drivers/media/dvb/frontends/Makefile 2011-07-05 01:36:24.621564185 +0200 @@ -4,6 +4,7 @@ EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ EXTRA_CFLAGS += -Idrivers/media/common/tuners/ +EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-usb/ stb0899-objs = stb0899_drv.o stb0899_algo.o stv0900-objs = stv0900_core.o stv0900_sw.o diff -ur linux/drivers/media/dvb/frontends/tda10048.c linux.new/drivers/media/dvb/frontends/tda10048.c --- linux/drivers/media/dvb/frontends/tda10048.c 2010-10-25 01:34:58.000000000 +0200 +++ linux.new/drivers/media/dvb/frontends/tda10048.c 2011-07-05 01:57:47.758466025 +0200 @@ -30,6 +30,7 @@ #include "dvb_frontend.h" #include "dvb_math.h" #include "tda10048.h" +#include "dvb-usb.h" #define TDA10048_DEFAULT_FIRMWARE "dvb-fe-tda10048-1.0.fw" #define TDA10048_DEFAULT_FIRMWARE_SIZE 24878 @@ -214,6 +215,8 @@ { TDA10048_CLK_16000, TDA10048_IF_3800, 10, 3, 0 }, { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 }, { TDA10048_CLK_16000, TDA10048_IF_4300, 10, 3, 0 }, + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 }, + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 }, { TDA10048_CLK_16000, TDA10048_IF_36130, 10, 3, 0 }, }; @@ -429,6 +432,19 @@ return 0; } +static int tda10048_set_pll(struct dvb_frontend *fe) +{ + struct tda10048_state *state = fe->demodulator_priv; + + dprintk(1, "%s()\n", __func__); + + tda10048_writereg(state, TDA10048_CONF_PLL1, 0x0f); + tda10048_writereg(state, TDA10048_CONF_PLL2, (u8)(state->pll_mfactor)); + tda10048_writereg(state, TDA10048_CONF_PLL3, tda10048_readreg(state, TDA10048_CONF_PLL3) | ((u8)(state->pll_nfactor) | 0x40)); + + return 0; +} + static int tda10048_set_if(struct dvb_frontend *fe, enum fe_bandwidth bw) { struct tda10048_state *state = fe->demodulator_priv; @@ -478,6 +494,9 @@ dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor); dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor); + /* Set the pll registers */ + tda10048_set_pll(fe); + /* Calculate the sample frequency */ state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45); state->sample_freq /= (state->pll_nfactor + 1); @@ -710,12 +729,16 @@ if (config->disable_gate_access) return 0; - if (enable) - return tda10048_writereg(state, TDA10048_CONF_C4_1, - tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); - else - return tda10048_writereg(state, TDA10048_CONF_C4_1, - tda10048_readreg(state, TDA10048_CONF_C4_1) & 0xfd); + if (config->fe && config->fe->ops.i2c_gate_ctrl) { + return config->fe->ops.i2c_gate_ctrl(config->fe, enable); + } else { + if (enable) + return tda10048_writereg(state, TDA10048_CONF_C4_1, + tda10048_readreg(state, TDA10048_CONF_C4_1) | 0x02); + else + return tda10048_writereg(state, TDA10048_CONF_C4_1, + tda10048_readreg(state, TDA10048_CONF_C4_1) & 0xfd); + } } static int tda10048_output_mode(struct dvb_frontend *fe, int serial) @@ -772,20 +795,45 @@ return 0; } +static int tda10048_sleep(struct dvb_frontend *fe) +{ + struct tda10048_state *state = fe->demodulator_priv; + struct tda10048_config *config = &state->config; + struct dvb_usb_adapter *adap; + + dprintk(1, "%s()\n", __func__); + + if (config->fe) { + adap = fe->dvb->priv; + if (adap->dev->props.power_ctrl) + adap->dev->props.power_ctrl(adap->dev, 0); + } + + return 0; +} + /* Establish sane defaults and load firmware. */ static int tda10048_init(struct dvb_frontend *fe) { struct tda10048_state *state = fe->demodulator_priv; struct tda10048_config *config = &state->config; + struct dvb_usb_adapter *adap; int ret = 0, i; dprintk(1, "%s()\n", __func__); + if (config->fe) { + adap = fe->dvb->priv; + if (adap->dev->props.power_ctrl) + adap->dev->props.power_ctrl(adap->dev, 1); + } + + /* Apply register defaults */ for (i = 0; i < ARRAY_SIZE(init_tab); i++) tda10048_writereg(state, init_tab[i].reg, init_tab[i].data); - if (state->fwloaded == 0) + if ((state->fwloaded == 0) && (!config->no_firmware)) ret = tda10048_firmware_upload(fe); /* Set either serial or parallel */ @@ -1174,6 +1222,7 @@ .release = tda10048_release, .init = tda10048_init, + .sleep = tda10048_sleep, .i2c_gate_ctrl = tda10048_i2c_gate_ctrl, .set_frontend = tda10048_set_frontend, .get_frontend = tda10048_get_frontend, diff -ur linux/drivers/media/dvb/frontends/tda10048.h linux.new/drivers/media/dvb/frontends/tda10048.h --- linux/drivers/media/dvb/frontends/tda10048.h 2010-07-03 23:22:08.000000000 +0200 +++ linux.new/drivers/media/dvb/frontends/tda10048.h 2011-07-05 02:02:42.775466043 +0200 @@ -51,6 +51,7 @@ #define TDA10048_IF_4300 4300 #define TDA10048_IF_4500 4500 #define TDA10048_IF_4750 4750 +#define TDA10048_IF_5000 5000 #define TDA10048_IF_36130 36130 u16 dtv6_if_freq_khz; u16 dtv7_if_freq_khz; @@ -62,6 +63,10 @@ /* Disable I2C gate access */ u8 disable_gate_access; + + u8 no_firmware; + + struct dvb_frontend *fe; }; #if defined(CONFIG_DVB_TDA10048) || \