* Re: [PATCH] add support for the dvb-t part of CT-3650 v2
[not found] ` <201107070057.06317.jareguero@telefonica.net>
@ 2011-07-13 12:41 ` Mauro Carvalho Chehab
2011-07-14 20:00 ` [PATCH] add support for the dvb-t part of CT-3650 v3 Jose Alberto Reguero
2011-07-16 11:38 ` [PATCH] improve recection with limits frecuenies and tda827x Jose Alberto Reguero
0 siblings, 2 replies; 28+ messages in thread
From: Mauro Carvalho Chehab @ 2011-07-13 12:41 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: linux-media, Michael Krufky
Em 06-07-2011 19:57, Jose Alberto Reguero escreveu:
> This patch add suport for the dvb-t part of CT-3650.
>
> Jose Alberto
>
> Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
> patches/lmml_951522_add_support_for_the_dvb_t_part_of_ct_3650_v2.patch
> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Subject: add support for the dvb-t part of CT-3650 v2
> Date: Wed, 06 Jul 2011 22:57:04 -0000
> From: Jose Alberto Reguero <jareguero@telefonica.net>
> X-Patchwork-Id: 951522
> Message-Id: <201107070057.06317.jareguero@telefonica.net>
> To: linux-media@vger.kernel.org
>
> This patch add suport for the dvb-t part of CT-3650.
>
> Jose Alberto
>
> Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
>
>
> diff -ur linux/drivers/media/common/tuners/tda827x.c linux.new/drivers/media/common/tuners/tda827x.c
> --- linux/drivers/media/common/tuners/tda827x.c 2010-07-03 23:22:08.000000000 +0200
> +++ linux.new/drivers/media/common/tuners/tda827x.c 2011-07-04 12:00:29.931561053 +0200
> @@ -135,14 +135,29 @@
>
> static int tuner_transfer(struct dvb_frontend *fe,
> struct i2c_msg *msg,
> - const int size)
> + int size)
> {
> int rc;
> struct tda827x_priv *priv = fe->tuner_priv;
> + struct i2c_msg msgr[2];
>
> if (fe->ops.i2c_gate_ctrl)
> fe->ops.i2c_gate_ctrl(fe, 1);
> - rc = i2c_transfer(priv->i2c_adap, msg, size);
> + if (priv->cfg->i2cr && (msg->flags == I2C_M_RD)) {
> + msgr[0].addr = msg->addr;
> + msgr[0].flags = 0;
> + msgr[0].len = msg->len - 1;
> + msgr[0].buf = msg->buf;
> + msgr[1].addr = msg->addr;
> + msgr[1].flags = I2C_M_RD;
> + msgr[1].len = 1;
> + msgr[1].buf = msg->buf;
> + size = 2;
> + rc = i2c_transfer(priv->i2c_adap, msgr, size);
> + msg->buf[msg->len - 1] = msgr[1].buf[0];
> + } else {
> + rc = i2c_transfer(priv->i2c_adap, msg, size);
> + }
> if (fe->ops.i2c_gate_ctrl)
> fe->ops.i2c_gate_ctrl(fe, 0);
No. You should be applying such fix at the I2C adapter instead. This is the
code at the ttusb2 driver:
static int ttusb2_i2c_xfer(struct i2c_adapter *adap,struct i2c_msg msg[],int num)
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
static u8 obuf[60], ibuf[60];
int i,read;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
if (num > 2)
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);
obuf[0] = (msg[i].addr << 1) | read;
obuf[1] = msg[i].len;
/* read request */
if (read)
obuf[2] = msg[i+1].len;
else
obuf[2] = 0;
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++;
}
}
mutex_unlock(&d->i2c_mutex);
return i;
}
Clearly, this routine has issues, as it assumes that all transfers with reads will
be broken into just two msgs, where the first one is a write, and a second one is a
read, and that no transfers will be bigger than 2 messages.
It shouldn't be hard to adapt it to handle transfers on either way, and to remove
the limit of 2 transfers.
>
> @@ -540,7 +555,7 @@
> if_freq = 5000000;
> break;
> }
> - tuner_freq = params->frequency + if_freq;
> + tuner_freq = params->frequency;
>
> if (fe->ops.info.type == FE_QAM) {
> dprintk("%s select tda827xa_dvbc\n", __func__);
> @@ -554,6 +569,8 @@
> i++;
> }
>
> + tuner_freq += if_freq;
> +
> N = ((tuner_freq + 31250) / 62500) << frequency_map[i].spd;
> buf[0] = 0; // subaddress
> buf[1] = N >> 8;
This seems to be a bug fix, but you're touching only at the DVB-C. If the table lookup
should not consider if_freq, the same fix is needed on the other similar logics at the
driver.
Also, please send such patch in separate.
> diff -ur linux/drivers/media/common/tuners/tda827x.h linux.new/drivers/media/common/tuners/tda827x.h
> --- linux/drivers/media/common/tuners/tda827x.h 2010-07-03 23:22:08.000000000 +0200
> +++ linux.new/drivers/media/common/tuners/tda827x.h 2011-05-21 22:48:31.484340005 +0200
> @@ -38,6 +38,8 @@
> int switch_addr;
>
> void (*agcf)(struct dvb_frontend *fe);
> +
> + u8 i2cr;
> };
Nack. Fix the transfer routine at the ttusb2 side.
> 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-05 12:35:51.842182196 +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,
> @@ -190,6 +190,22 @@
> .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 = {
> + .i2cr = 1,
> + .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 +221,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 +272,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);
CodingStyle: don't put a space on the above. Please, always check your patches
with ./script/checkpatch.pl
> +}
> +
> static int ttusb2_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> @@ -422,7 +465,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) || \
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-13 12:41 ` [PATCH] add support for the dvb-t part of CT-3650 v2 Mauro Carvalho Chehab
@ 2011-07-14 20:00 ` Jose Alberto Reguero
2011-07-18 20:28 ` Antti Palosaari
2011-07-16 11:38 ` [PATCH] improve recection with limits frecuenies and tda827x Jose Alberto Reguero
1 sibling, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-14 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Michael Krufky
[-- Attachment #1: Type: Text/Plain, Size: 15607 bytes --]
On Miércoles, 13 de Julio de 2011 14:41:30 Mauro Carvalho Chehab escribió:
> Em 06-07-2011 19:57, Jose Alberto Reguero escreveu:
> > This patch add suport for the dvb-t part of CT-3650.
> >
> > Jose Alberto
> >
> > Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
> >
> > patches/lmml_951522_add_support_for_the_dvb_t_part_of_ct_3650_v2.patch
> > Content-Type: text/plain; charset="utf-8"
> > MIME-Version: 1.0
> > Content-Transfer-Encoding: 7bit
> > Subject: add support for the dvb-t part of CT-3650 v2
> > Date: Wed, 06 Jul 2011 22:57:04 -0000
> > From: Jose Alberto Reguero <jareguero@telefonica.net>
> > X-Patchwork-Id: 951522
> > Message-Id: <201107070057.06317.jareguero@telefonica.net>
> > To: linux-media@vger.kernel.org
> >
> > This patch add suport for the dvb-t part of CT-3650.
> >
> > Jose Alberto
> >
> > Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
> >
> >
> > diff -ur linux/drivers/media/common/tuners/tda827x.c
> > linux.new/drivers/media/common/tuners/tda827x.c ---
> > linux/drivers/media/common/tuners/tda827x.c 2010-07-03
> > 23:22:08.000000000 +0200 +++
> > linux.new/drivers/media/common/tuners/tda827x.c 2011-07-04
> > 12:00:29.931561053 +0200 @@ -135,14 +135,29 @@
> >
> > static int tuner_transfer(struct dvb_frontend *fe,
> >
> > struct i2c_msg *msg,
> >
> > - const int size)
> > + int size)
> >
> > {
> >
> > int rc;
> > struct tda827x_priv *priv = fe->tuner_priv;
> >
> > + struct i2c_msg msgr[2];
> >
> > if (fe->ops.i2c_gate_ctrl)
> >
> > fe->ops.i2c_gate_ctrl(fe, 1);
> >
> > - rc = i2c_transfer(priv->i2c_adap, msg, size);
> > + if (priv->cfg->i2cr && (msg->flags == I2C_M_RD)) {
> > + msgr[0].addr = msg->addr;
> > + msgr[0].flags = 0;
> > + msgr[0].len = msg->len - 1;
> > + msgr[0].buf = msg->buf;
> > + msgr[1].addr = msg->addr;
> > + msgr[1].flags = I2C_M_RD;
> > + msgr[1].len = 1;
> > + msgr[1].buf = msg->buf;
> > + size = 2;
> > + rc = i2c_transfer(priv->i2c_adap, msgr, size);
> > + msg->buf[msg->len - 1] = msgr[1].buf[0];
> > + } else {
> > + rc = i2c_transfer(priv->i2c_adap, msg, size);
> > + }
> >
> > if (fe->ops.i2c_gate_ctrl)
> >
> > fe->ops.i2c_gate_ctrl(fe, 0);
>
> No. You should be applying such fix at the I2C adapter instead. This is the
> code at the ttusb2 driver:
>
> static int ttusb2_i2c_xfer(struct i2c_adapter *adap,struct i2c_msg
> msg[],int num) {
> struct dvb_usb_device *d = i2c_get_adapdata(adap);
> static u8 obuf[60], ibuf[60];
> int i,read;
>
> if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
> return -EAGAIN;
>
> if (num > 2)
> 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);
>
> obuf[0] = (msg[i].addr << 1) | read;
> obuf[1] = msg[i].len;
>
> /* read request */
> if (read)
> obuf[2] = msg[i+1].len;
> else
> obuf[2] = 0;
>
> 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++;
> }
> }
>
> mutex_unlock(&d->i2c_mutex);
> return i;
> }
>
> Clearly, this routine has issues, as it assumes that all transfers with
> reads will be broken into just two msgs, where the first one is a write,
> and a second one is a read, and that no transfers will be bigger than 2
> messages.
>
> It shouldn't be hard to adapt it to handle transfers on either way, and to
> remove the limit of 2 transfers.
>
> > @@ -540,7 +555,7 @@
> >
> > if_freq = 5000000;
> > break;
> >
> > }
> >
> > - tuner_freq = params->frequency + if_freq;
> > + tuner_freq = params->frequency;
> >
> > if (fe->ops.info.type == FE_QAM) {
> >
> > dprintk("%s select tda827xa_dvbc\n", __func__);
> >
> > @@ -554,6 +569,8 @@
> >
> > i++;
> >
> > }
> >
> > + tuner_freq += if_freq;
> > +
> >
> > N = ((tuner_freq + 31250) / 62500) << frequency_map[i].spd;
> > buf[0] = 0; // subaddress
> > buf[1] = N >> 8;
>
> This seems to be a bug fix, but you're touching only at the DVB-C. If the
> table lookup should not consider if_freq, the same fix is needed on the
> other similar logics at the driver.
>
> Also, please send such patch in separate.
>
> > diff -ur linux/drivers/media/common/tuners/tda827x.h
> > linux.new/drivers/media/common/tuners/tda827x.h ---
> > linux/drivers/media/common/tuners/tda827x.h 2010-07-03
> > 23:22:08.000000000 +0200 +++
> > linux.new/drivers/media/common/tuners/tda827x.h 2011-05-21
> > 22:48:31.484340005 +0200 @@ -38,6 +38,8 @@
> >
> > int switch_addr;
> >
> > void (*agcf)(struct dvb_frontend *fe);
> >
> > +
> > + u8 i2cr;
> >
> > };
>
> Nack. Fix the transfer routine at the ttusb2 side.
>
> > 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-05
> > 12:35:51.842182196 +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,
> >
> > @@ -190,6 +190,22 @@
> >
> > .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 = {
> > + .i2cr = 1,
> > + .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 +221,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 +272,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);
>
> CodingStyle: don't put a space on the above. Please, always check your
> patches with ./script/checkpatch.pl
>
> > +}
> > +
> >
> > static int ttusb2_probe(struct usb_interface *intf,
> >
> > const struct usb_device_id *id)
> >
> > {
> >
> > @@ -422,7 +465,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) || \
The attached patch try to fix the problems mentioned.
Jose Alberto
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
[-- Attachment #2: ttusb2-3.diff --]
[-- Type: text/x-patch, Size: 9629 bytes --]
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) || \
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] improve recection with limits frecuenies and tda827x
2011-07-13 12:41 ` [PATCH] add support for the dvb-t part of CT-3650 v2 Mauro Carvalho Chehab
2011-07-14 20:00 ` [PATCH] add support for the dvb-t part of CT-3650 v3 Jose Alberto Reguero
@ 2011-07-16 11:38 ` Jose Alberto Reguero
1 sibling, 0 replies; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-16 11:38 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Michael Krufky
[-- Attachment #1: Type: Text/Plain, Size: 15826 bytes --]
On Miércoles, 13 de Julio de 2011 14:41:30 Mauro Carvalho Chehab escribió:
> Em 06-07-2011 19:57, Jose Alberto Reguero escreveu:
> > This patch add suport for the dvb-t part of CT-3650.
> >
> > Jose Alberto
> >
> > Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
> >
> > patches/lmml_951522_add_support_for_the_dvb_t_part_of_ct_3650_v2.patch
> > Content-Type: text/plain; charset="utf-8"
> > MIME-Version: 1.0
> > Content-Transfer-Encoding: 7bit
> > Subject: add support for the dvb-t part of CT-3650 v2
> > Date: Wed, 06 Jul 2011 22:57:04 -0000
> > From: Jose Alberto Reguero <jareguero@telefonica.net>
> > X-Patchwork-Id: 951522
> > Message-Id: <201107070057.06317.jareguero@telefonica.net>
> > To: linux-media@vger.kernel.org
> >
> > This patch add suport for the dvb-t part of CT-3650.
> >
> > Jose Alberto
> >
> > Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
> >
> >
> > diff -ur linux/drivers/media/common/tuners/tda827x.c
> > linux.new/drivers/media/common/tuners/tda827x.c ---
> > linux/drivers/media/common/tuners/tda827x.c 2010-07-03
> > 23:22:08.000000000 +0200 +++
> > linux.new/drivers/media/common/tuners/tda827x.c 2011-07-04
> > 12:00:29.931561053 +0200 @@ -135,14 +135,29 @@
> >
> > static int tuner_transfer(struct dvb_frontend *fe,
> >
> > struct i2c_msg *msg,
> >
> > - const int size)
> > + int size)
> >
> > {
> >
> > int rc;
> > struct tda827x_priv *priv = fe->tuner_priv;
> >
> > + struct i2c_msg msgr[2];
> >
> > if (fe->ops.i2c_gate_ctrl)
> >
> > fe->ops.i2c_gate_ctrl(fe, 1);
> >
> > - rc = i2c_transfer(priv->i2c_adap, msg, size);
> > + if (priv->cfg->i2cr && (msg->flags == I2C_M_RD)) {
> > + msgr[0].addr = msg->addr;
> > + msgr[0].flags = 0;
> > + msgr[0].len = msg->len - 1;
> > + msgr[0].buf = msg->buf;
> > + msgr[1].addr = msg->addr;
> > + msgr[1].flags = I2C_M_RD;
> > + msgr[1].len = 1;
> > + msgr[1].buf = msg->buf;
> > + size = 2;
> > + rc = i2c_transfer(priv->i2c_adap, msgr, size);
> > + msg->buf[msg->len - 1] = msgr[1].buf[0];
> > + } else {
> > + rc = i2c_transfer(priv->i2c_adap, msg, size);
> > + }
> >
> > if (fe->ops.i2c_gate_ctrl)
> >
> > fe->ops.i2c_gate_ctrl(fe, 0);
>
> No. You should be applying such fix at the I2C adapter instead. This is the
> code at the ttusb2 driver:
>
> static int ttusb2_i2c_xfer(struct i2c_adapter *adap,struct i2c_msg
> msg[],int num) {
> struct dvb_usb_device *d = i2c_get_adapdata(adap);
> static u8 obuf[60], ibuf[60];
> int i,read;
>
> if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
> return -EAGAIN;
>
> if (num > 2)
> 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);
>
> obuf[0] = (msg[i].addr << 1) | read;
> obuf[1] = msg[i].len;
>
> /* read request */
> if (read)
> obuf[2] = msg[i+1].len;
> else
> obuf[2] = 0;
>
> 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++;
> }
> }
>
> mutex_unlock(&d->i2c_mutex);
> return i;
> }
>
> Clearly, this routine has issues, as it assumes that all transfers with
> reads will be broken into just two msgs, where the first one is a write,
> and a second one is a read, and that no transfers will be bigger than 2
> messages.
>
> It shouldn't be hard to adapt it to handle transfers on either way, and to
> remove the limit of 2 transfers.
>
> > @@ -540,7 +555,7 @@
> >
> > if_freq = 5000000;
> > break;
> >
> > }
> >
> > - tuner_freq = params->frequency + if_freq;
> > + tuner_freq = params->frequency;
> >
> > if (fe->ops.info.type == FE_QAM) {
> >
> > dprintk("%s select tda827xa_dvbc\n", __func__);
> >
> > @@ -554,6 +569,8 @@
> >
> > i++;
> >
> > }
> >
> > + tuner_freq += if_freq;
> > +
> >
> > N = ((tuner_freq + 31250) / 62500) << frequency_map[i].spd;
> > buf[0] = 0; // subaddress
> > buf[1] = N >> 8;
>
> This seems to be a bug fix, but you're touching only at the DVB-C. If the
> table lookup should not consider if_freq, the same fix is needed on the
> other similar logics at the driver.
>
> Also, please send such patch in separate.
>
Only tested with tda827xa and DVB-T and two limit frecuencies.
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
Jose Alberto
> > diff -ur linux/drivers/media/common/tuners/tda827x.h
> > linux.new/drivers/media/common/tuners/tda827x.h ---
> > linux/drivers/media/common/tuners/tda827x.h 2010-07-03
> > 23:22:08.000000000 +0200 +++
> > linux.new/drivers/media/common/tuners/tda827x.h 2011-05-21
> > 22:48:31.484340005 +0200 @@ -38,6 +38,8 @@
> >
> > int switch_addr;
> >
> > void (*agcf)(struct dvb_frontend *fe);
> >
> > +
> > + u8 i2cr;
> >
> > };
>
> Nack. Fix the transfer routine at the ttusb2 side.
>
> > 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-05
> > 12:35:51.842182196 +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,
> >
> > @@ -190,6 +190,22 @@
> >
> > .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 = {
> > + .i2cr = 1,
> > + .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 +221,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 +272,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);
>
> CodingStyle: don't put a space on the above. Please, always check your
> patches with ./script/checkpatch.pl
>
> > +}
> > +
> >
> > static int ttusb2_probe(struct usb_interface *intf,
> >
> > const struct usb_device_id *id)
> >
> > {
> >
> > @@ -422,7 +465,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
[-- Attachment #2: tda727x.diff --]
[-- Type: text/x-patch, Size: 1042 bytes --]
diff -ur linux/drivers/media/common/tuners/tda827x.c linux.new/drivers/media/common/tuners/tda827x.c
--- linux/drivers/media/common/tuners/tda827x.c 2010-07-03 23:22:08.000000000 +0200
+++ linux.new/drivers/media/common/tuners/tda827x.c 2011-07-16 13:20:40.426284643 +0200
@@ -176,7 +176,7 @@
if_freq = 5000000;
break;
}
- tuner_freq = params->frequency + if_freq;
+ tuner_freq = params->frequency;
i = 0;
while (tda827x_table[i].lomax < tuner_freq) {
@@ -185,6 +185,8 @@
i++;
}
+ tuner_freq += if_freq;
+
N = ((tuner_freq + 125000) / 250000) << (tda827x_table[i].spd + 2);
buf[0] = 0;
buf[1] = (N>>8) | 0x40;
@@ -540,7 +542,7 @@
if_freq = 5000000;
break;
}
- tuner_freq = params->frequency + if_freq;
+ tuner_freq = params->frequency;
if (fe->ops.info.type == FE_QAM) {
dprintk("%s select tda827xa_dvbc\n", __func__);
@@ -554,6 +556,8 @@
i++;
}
+ tuner_freq += if_freq;
+
N = ((tuner_freq + 31250) / 62500) << frequency_map[i].spd;
buf[0] = 0; // subaddress
buf[1] = N >> 8;
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-14 20:00 ` [PATCH] add support for the dvb-t part of CT-3650 v3 Jose Alberto Reguero
@ 2011-07-18 20:28 ` Antti Palosaari
2011-07-18 21:31 ` Michael Krufky
[not found] ` <201107190100.16802.jareguero@telefonica.net>
0 siblings, 2 replies; 28+ messages in thread
From: Antti Palosaari @ 2011-07-18 20:28 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
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/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-18 20:28 ` Antti Palosaari
@ 2011-07-18 21:31 ` Michael Krufky
[not found] ` <201107190100.16802.jareguero@telefonica.net>
1 sibling, 0 replies; 28+ messages in thread
From: Michael Krufky @ 2011-07-18 21:31 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Jose Alberto Reguero, Mauro Carvalho Chehab, linux-media
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
[not found] ` <201107190100.16802.jareguero@telefonica.net>
@ 2011-07-18 23:44 ` Antti Palosaari
2011-07-19 8:25 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-18 23:44 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
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
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-18 23:44 ` Antti Palosaari
@ 2011-07-19 8:25 ` Jose Alberto Reguero
2011-07-19 23:07 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-19 8:25 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-19 8:25 ` Jose Alberto Reguero
@ 2011-07-19 23:07 ` Antti Palosaari
2011-07-22 11:32 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-19 23:07 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]
On 07/19/2011 11:25 AM, Jose Alberto Reguero wrote:
> 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ó:
>>> 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.
Signed-off-by: Antti Palosaari <crope@iki.fi>
Test attached patches and try to fix if they are not working. Most
likely not working since I don't have HW to test... I tested MFE parts
using Anysee, so it should be working. I changed rather much your ttusb2
and tda10048 patches, size reduced something like 50% or more. Still
ttusb2 I2C-adapter changes made looks rather complex. Try to double
check if those can be done easier. There is many drivers to look example
from.
DVB USB MFE is something like RFC. I know FE exclusive lock is missing,
no need to mention that :) But other comments are welcome! I left three
old "unneeded" pointers to struct dvb_usb_adapter to reduce changing all
the drivers.
regards
Antti
--
http://palosaari.fi/
[-- Attachment #2: dvb_usb_mfe_v1.patch --]
[-- Type: text/plain, Size: 4433 bytes --]
diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
index b3cb626..51c716f 100644
--- a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
@@ -162,8 +162,8 @@ 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->mfe_init[fe->id])
+ adap->mfe_init[fe->id](fe);
return 0;
}
@@ -172,35 +172,66 @@ 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->mfe_sleep[fe->id])
+ adap->mfe_sleep[fe->id](fe);
return dvb_usb_device_power_ctrl(adap->dev, 0);
}
int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap)
{
+ int ret, i, num_frontends;
+
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 != NULL) {
- adap->fe_init = adap->fe->ops.init; adap->fe->ops.init = dvb_usb_fe_wakeup;
- adap->fe_sleep = adap->fe->ops.sleep; adap->fe->ops.sleep = dvb_usb_fe_sleep;
+ if (adap->props.num_frontends)
+ num_frontends = adap->props.num_frontends;
+ else
+ num_frontends = 1;
+
+ for (i = 0; i < num_frontends; i++) {
+
+ ret = adap->props.frontend_attach(adap);
+ if (ret)
+ break;
+
+ /* glue for backward compatibility */
+ if (i == 0) {
+ if (adap->mfe[i])
+ adap->fe = adap->mfe[i];
+ else
+ adap->mfe[i] = adap->fe;
+ }
+
+ if (adap->mfe[i] == NULL)
+ break;
+
+ /* re-assign sleep and wakeup functions */
+ adap->mfe_init[i] = adap->mfe[i]->ops.init;
+ adap->mfe_sleep[i] = adap->mfe[i]->ops.sleep;
+ adap->mfe[i]->ops.init = dvb_usb_fe_wakeup;
+ adap->mfe[i]->ops.sleep = dvb_usb_fe_sleep;
- if (dvb_register_frontend(&adap->dvb_adap, adap->fe)) {
- err("Frontend registration failed.");
+ adap->mfe[i]->id = i;
+ if (dvb_register_frontend(&adap->dvb_adap, adap->mfe[i])) {
+ err("Frontend %d registration failed.", i);
dvb_frontend_detach(adap->fe);
- adap->fe = NULL;
+ adap->mfe[i] = NULL;
+ if (adap->mfe[0] == NULL)
+ adap->fe = NULL;
return -ENODEV;
}
/* only attach the tuner if the demod is there */
if (adap->props.tuner_attach != NULL)
adap->props.tuner_attach(adap);
- } else
+ }
+
+ if (adap->mfe[0] == NULL)
err("no frontend was attached by '%s'",adap->dev->desc->name);
return 0;
@@ -208,9 +239,22 @@ int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap)
int dvb_usb_adapter_frontend_exit(struct dvb_usb_adapter *adap)
{
- if (adap->fe != NULL) {
- dvb_unregister_frontend(adap->fe);
- dvb_frontend_detach(adap->fe);
+ int i, num_frontends;
+
+ if (adap->props.num_frontends)
+ num_frontends = adap->props.num_frontends;
+ else
+ num_frontends = 1;
+
+ for (i = 0; i < num_frontends; i++) {
+ if (adap->mfe[i] != NULL) {
+ dvb_unregister_frontend(adap->mfe[i]);
+ dvb_frontend_detach(adap->mfe[i]);
+ }
}
+
+ if (adap->mfe[0] == NULL)
+ adap->fe = NULL;
+
return 0;
}
diff --git a/drivers/media/dvb/dvb-usb/dvb-usb.h b/drivers/media/dvb/dvb-usb/dvb-usb.h
index 7d35d07..05d7032 100644
--- a/drivers/media/dvb/dvb-usb/dvb-usb.h
+++ b/drivers/media/dvb/dvb-usb/dvb-usb.h
@@ -146,6 +146,7 @@ struct dvb_usb_adapter_properties {
int (*pid_filter_ctrl) (struct dvb_usb_adapter *, int);
int (*pid_filter) (struct dvb_usb_adapter *, int, u16, int);
+ int num_frontends;
int (*frontend_attach) (struct dvb_usb_adapter *);
int (*tuner_attach) (struct dvb_usb_adapter *);
@@ -359,15 +360,19 @@ struct dvb_usb_adapter {
int pid_filtering;
/* dvb */
+#define DVB_USB_FE_MAX_NUM 2
struct dvb_adapter dvb_adap;
struct dmxdev dmxdev;
struct dvb_demux demux;
struct dvb_net dvb_net;
struct dvb_frontend *fe;
+ struct dvb_frontend *mfe[DVB_USB_FE_MAX_NUM];
int max_feed_count;
int (*fe_init) (struct dvb_frontend *);
int (*fe_sleep) (struct dvb_frontend *);
+ int (*mfe_init[DVB_USB_FE_MAX_NUM]) (struct dvb_frontend *);
+ int (*mfe_sleep[DVB_USB_FE_MAX_NUM]) (struct dvb_frontend *);
struct usb_data_stream stream;
[-- Attachment #3: tda10048_v1.patch --]
[-- Type: text/plain, Size: 2242 bytes --]
diff --git a/drivers/media/dvb/frontends/tda10048.c b/drivers/media/dvb/frontends/tda10048.c
index 93f6a75..c77ad1e 100644
--- a/drivers/media/dvb/frontends/tda10048.c
+++ b/drivers/media/dvb/frontends/tda10048.c
@@ -214,6 +214,8 @@ static struct pll_tab {
{ 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 },
};
@@ -478,6 +480,11 @@ static int tda10048_set_if(struct dvb_frontend *fe, enum fe_bandwidth bw)
dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor);
dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor);
+ /* Set the pll registers */
+ 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));
+
/* Calculate the sample frequency */
state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
state->sample_freq /= (state->pll_nfactor + 1);
@@ -1123,7 +1130,7 @@ struct dvb_frontend *tda10048_attach(const struct tda10048_config *config,
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
diff --git a/drivers/media/dvb/frontends/tda10048.h b/drivers/media/dvb/frontends/tda10048.h
index 8828cea..3da0e0c 100644
--- a/drivers/media/dvb/frontends/tda10048.h
+++ b/drivers/media/dvb/frontends/tda10048.h
@@ -51,6 +51,7 @@ struct tda10048_config {
#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,8 @@ struct tda10048_config {
/* Disable I2C gate access */
u8 disable_gate_access;
+
+ bool no_firmware;
};
#if defined(CONFIG_DVB_TDA10048) || \
[-- Attachment #4: ttusb2_v1.patch --]
[-- Type: text/plain, Size: 4223 bytes --]
diff --git a/drivers/media/dvb/dvb-usb/ttusb2.c b/drivers/media/dvb/dvb-usb/ttusb2.c
index 0d4709f..59f5614 100644
--- a/drivers/media/dvb/dvb-usb/ttusb2.c
+++ b/drivers/media/dvb/dvb-usb/ttusb2.c
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,7 @@ static int ttusb2_i2c_xfer(struct i2c_adapter *adap,struct i2c_msg msg[],int num
{
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 +92,33 @@ static int ttusb2_i2c_xfer(struct i2c_adapter *adap,struct i2c_msg msg[],int num
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 +197,21 @@ static struct tda10023_config tda10023_config = {
.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)
@@ -207,19 +229,49 @@ static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->mfe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->mfe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->mfe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ } else {
+ /* FE 1 DVB-T */
+ adap->mfe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->mfe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ return -ENODEV;
+ }
+
+ /* tuner is behind TDA10023 I2C-gate */
+ adap->mfe[1]->ops.i2c_gate_ctrl = adap->mfe[0]->ops.i2c_gate_ctrl;
}
+
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) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->mfe[1] == NULL)
+ fe = adap->mfe[0];
+ else
+ fe = adap->mfe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
+
return 0;
}
@@ -383,8 +435,7 @@ static struct dvb_usb_device_properties ttusb2_properties_ct3650 = {
.num_adapters = 1,
.adapter = {
{
- .streaming_ctrl = NULL,
-
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-19 23:07 ` Antti Palosaari
@ 2011-07-22 11:32 ` Antti Palosaari
2011-07-22 16:02 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-22 11:32 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
Have you had to time test these?
And about I2C adapter, I don't see why changes are needed. As far as I
understand it is already working with TDA10023 and you have done changes
for TDA10048 support. I compared TDA10048 and TDA10023 I2C functions and
those are ~similar. Both uses most typical access, for reg write {u8
REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
regards
Antti
On 07/20/2011 02:07 AM, Antti Palosaari wrote:
> On 07/19/2011 11:25 AM, Jose Alberto Reguero wrote:
>> 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ó:
>
>>>> 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.
>
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
>
> Test attached patches and try to fix if they are not working. Most
> likely not working since I don't have HW to test... I tested MFE parts
> using Anysee, so it should be working. I changed rather much your ttusb2
> and tda10048 patches, size reduced something like 50% or more. Still
> ttusb2 I2C-adapter changes made looks rather complex. Try to double
> check if those can be done easier. There is many drivers to look example
> from.
>
> DVB USB MFE is something like RFC. I know FE exclusive lock is missing,
> no need to mention that :) But other comments are welcome! I left three
> old "unneeded" pointers to struct dvb_usb_adapter to reduce changing all
> the drivers.
>
>
> regards
> Antti
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 11:32 ` Antti Palosaari
@ 2011-07-22 16:02 ` Jose Alberto Reguero
2011-07-22 16:08 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-22 16:02 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
[-- Attachment #1: Type: Text/Plain, Size: 3284 bytes --]
On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
> Have you had to time test these?
>
> And about I2C adapter, I don't see why changes are needed. As far as I
> understand it is already working with TDA10023 and you have done changes
> for TDA10048 support. I compared TDA10048 and TDA10023 I2C functions and
> those are ~similar. Both uses most typical access, for reg write {u8
> REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
>
> regards
> Antti
I just finish the testing. The changes to I2C are for the tuner tda827x. The
MFE fork fine. I need to change the code in tda10048 and ttusb2. Attached is
the patch for CT-3650 with your MFE patch.
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
Jose Alberto
>
> On 07/20/2011 02:07 AM, Antti Palosaari wrote:
> > On 07/19/2011 11:25 AM, Jose Alberto Reguero wrote:
> >> 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ó:
> >>>>
> >>>> 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.
> >
> > Signed-off-by: Antti Palosaari <crope@iki.fi>
> >
> > Test attached patches and try to fix if they are not working. Most
> > likely not working since I don't have HW to test... I tested MFE parts
> > using Anysee, so it should be working. I changed rather much your ttusb2
> > and tda10048 patches, size reduced something like 50% or more. Still
> > ttusb2 I2C-adapter changes made looks rather complex. Try to double
> > check if those can be done easier. There is many drivers to look example
> > from.
> >
> > DVB USB MFE is something like RFC. I know FE exclusive lock is missing,
> > no need to mention that :) But other comments are welcome! I left three
> > old "unneeded" pointers to struct dvb_usb_adapter to reduce changing all
> > the drivers.
> >
> >
> > regards
> > Antti
[-- Attachment #2: ttusb2-4.diff --]
[-- Type: text/x-patch, Size: 6904 bytes --]
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-22 14:48:10.526000638 +0200
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,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 +92,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 +197,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)
@@ -207,19 +229,48 @@
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->mfe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->mfe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->mfe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ } else {
+ /* FE 1 DVB-T */
+ tda10048_config.fe = adap->mfe[0];
+
+ adap->mfe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->mfe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ 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) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->mfe[1] == NULL)
+ fe = adap->mfe[0];
+ else
+ fe = adap->mfe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
+
return 0;
}
@@ -383,8 +434,7 @@
.num_adapters = 1,
.adapter = {
{
- .streaming_ctrl = NULL,
-
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
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-22 12:55:35.979000236 +0200
@@ -214,6 +214,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 },
};
@@ -478,6 +480,11 @@
dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor);
dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor);
+ /* Set the pll registers */
+ 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));
+
/* Calculate the sample frequency */
state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
state->sample_freq /= (state->pll_nfactor + 1);
@@ -707,15 +714,16 @@
struct tda10048_config *config = &state->config;
dprintk(1, "%s(%d)\n", __func__, enable);
- 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)
@@ -1123,7 +1131,7 @@
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
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-22 12:53:50.904000229 +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;
+
+ bool no_firmware;
+
+ struct dvb_frontend *fe;
};
#if defined(CONFIG_DVB_TDA10048) || \
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 16:02 ` Jose Alberto Reguero
@ 2011-07-22 16:08 ` Antti Palosaari
2011-07-22 16:25 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-22 16:08 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
>> Have you had to time test these?
>>
>> And about I2C adapter, I don't see why changes are needed. As far as I
>> understand it is already working with TDA10023 and you have done changes
>> for TDA10048 support. I compared TDA10048 and TDA10023 I2C functions and
>> those are ~similar. Both uses most typical access, for reg write {u8
>> REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
>>
>> regards
>> Antti
>
> I just finish the testing. The changes to I2C are for the tuner tda827x. The
> MFE fork fine. I need to change the code in tda10048 and ttusb2. Attached is
> the patch for CT-3650 with your MFE patch.
You still pass tda10023 fe pointer to tda10048 for I2C-gate control
which is wrong. Could you send USB sniff I can look what there really
happens. If you have raw SniffUSB2 logs I wish to check those, other
logs are welcome too if no raw SniffUSB2 available.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 16:08 ` Antti Palosaari
@ 2011-07-22 16:25 ` Jose Alberto Reguero
2011-07-22 16:46 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-22 16:25 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
> > On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
> >> Have you had to time test these?
> >>
> >> And about I2C adapter, I don't see why changes are needed. As far as I
> >> understand it is already working with TDA10023 and you have done changes
> >> for TDA10048 support. I compared TDA10048 and TDA10023 I2C functions and
> >> those are ~similar. Both uses most typical access, for reg write {u8
> >> REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
> >>
> >> regards
> >> Antti
> >
> > I just finish the testing. The changes to I2C are for the tuner tda827x.
> > The MFE fork fine. I need to change the code in tda10048 and ttusb2.
> > Attached is the patch for CT-3650 with your MFE patch.
>
> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
> which is wrong. Could you send USB sniff I can look what there really
> happens. If you have raw SniffUSB2 logs I wish to check those, other
> logs are welcome too if no raw SniffUSB2 available.
>
Youre chnage don't work. You need to change the function i2c gate of tda1048
for the one of tda1023, but the parameter of this function must be the fe
pointer of tda1023. If this is a problem, I can duplicate tda1023 i2c gate in
ttusb2 code and pass it to the tda10048. It is done this way in the first patch
of this thread.
Jose Alberto
> regards
> Antti
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 16:25 ` Jose Alberto Reguero
@ 2011-07-22 16:46 ` Antti Palosaari
[not found] ` <201107222012.20711.jareguero@telefonica.net>
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-22 16:46 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
> On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
>> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
>>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
>>>> Have you had to time test these?
>>>>
>>>> And about I2C adapter, I don't see why changes are needed. As far as I
>>>> understand it is already working with TDA10023 and you have done changes
>>>> for TDA10048 support. I compared TDA10048 and TDA10023 I2C functions and
>>>> those are ~similar. Both uses most typical access, for reg write {u8
>>>> REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
>>>>
>>>> regards
>>>> Antti
>>>
>>> I just finish the testing. The changes to I2C are for the tuner tda827x.
>>> The MFE fork fine. I need to change the code in tda10048 and ttusb2.
>>> Attached is the patch for CT-3650 with your MFE patch.
>>
>> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
>> which is wrong. Could you send USB sniff I can look what there really
>> happens. If you have raw SniffUSB2 logs I wish to check those, other
>> logs are welcome too if no raw SniffUSB2 available.
>>
>
> Youre chnage don't work. You need to change the function i2c gate of tda1048
> for the one of tda1023, but the parameter of this function must be the fe
> pointer of tda1023. If this is a problem, I can duplicate tda1023 i2c gate in
> ttusb2 code and pass it to the tda10048. It is done this way in the first patch
> of this thread.
Yes I now see why it cannot work - since FE is given as a parameter to
i2c_gate_ctrl it does not see correct priv and used I2C addr is read
from priv. You must implement own i2c_gate_ctrl in ttusb2 driver.
Implement own ct3650_i2c_gate_ctrl and override tda10048 i2c_gate_ctrl
using that. Then call tda10023 i2c_gate_ctrl but instead of tda10048 FE
use td10023 FE. Something like
static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
{
return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE, enable);
}
/* tuner is behind TDA10023 I2C-gate */
adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
Could you still send USB logs? I don't see it correct behaviour you need
to change I2C-adaper when same tuner is used for DVB-T because it was
already working in DVB-C mode.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
[not found] ` <201107222012.20711.jareguero@telefonica.net>
@ 2011-07-22 21:49 ` Jose Alberto Reguero
2011-07-22 22:23 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-22 21:49 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
[-- Attachment #1: Type: Text/Plain, Size: 3096 bytes --]
On Viernes, 22 de Julio de 2011 20:12:20 Jose Alberto Reguero escribió:
> On Viernes, 22 de Julio de 2011 18:46:24 Antti Palosaari escribió:
> > On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
> > > On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
> > >> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
> > >>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
> > >>>> Have you had to time test these?
> > >>>>
> > >>>> And about I2C adapter, I don't see why changes are needed. As far as
> > >>>> I understand it is already working with TDA10023 and you have done
> > >>>> changes for TDA10048 support. I compared TDA10048 and TDA10023 I2C
> > >>>> functions and those are ~similar. Both uses most typical access,
> > >>>> for reg write {u8 REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
> > >>>>
> > >>>> regards
> > >>>> Antti
> > >>>
> > >>> I just finish the testing. The changes to I2C are for the tuner
> > >>> tda827x. The MFE fork fine. I need to change the code in tda10048 and
> > >>> ttusb2. Attached is the patch for CT-3650 with your MFE patch.
> > >>
> > >> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
> > >> which is wrong. Could you send USB sniff I can look what there really
> > >> happens. If you have raw SniffUSB2 logs I wish to check those, other
> > >> logs are welcome too if no raw SniffUSB2 available.
> > >
> > > Youre chnage don't work. You need to change the function i2c gate of
> > > tda1048 for the one of tda1023, but the parameter of this function must
> > > be the fe pointer of tda1023. If this is a problem, I can duplicate
> > > tda1023 i2c gate in ttusb2 code and pass it to the tda10048. It is done
> > > this way in the first patch of this thread.
> >
> > Yes I now see why it cannot work - since FE is given as a parameter to
> > i2c_gate_ctrl it does not see correct priv and used I2C addr is read
> > from priv. You must implement own i2c_gate_ctrl in ttusb2 driver.
> > Implement own ct3650_i2c_gate_ctrl and override tda10048 i2c_gate_ctrl
> > using that. Then call tda10023 i2c_gate_ctrl but instead of tda10048 FE
> > use td10023 FE. Something like
> >
> > static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> > {
> >
> > return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE, enable);
> >
> > }
> >
> > /* tuner is behind TDA10023 I2C-gate */
> > adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
> >
> >
> > Could you still send USB logs? I don't see it correct behaviour you need
> > to change I2C-adaper when same tuner is used for DVB-T because it was
> > already working in DVB-C mode.
> >
> > regards
> > Antti
>
> Thanks, I try to implement that. I attach a processed log. It prints the
> first line of a usb command and the first line of the returned byes. If
> you want the full log I can upload it where you want.
>
> Jose Alberto
New version with Antti's sugestion.
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
Jose Alberto
[-- Attachment #2: ttusb2-5.diff --]
[-- Type: text/x-patch, Size: 6302 bytes --]
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-22 22:15:20.483271578 +0200
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,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 +92,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 +197,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)
@@ -203,23 +225,60 @@
return 0;
}
+static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
+{
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+
+ return adap->mfe[0]->ops.i2c_gate_ctrl(adap->mfe[0], enable);
+}
+
static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->mfe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->mfe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->mfe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ } else {
+ adap->mfe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->mfe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ return -ENODEV;
+ }
+
+ /* tuner is behind TDA10023 I2C-gate */
+ adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
+
}
+
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) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->mfe[1] == NULL)
+ fe = adap->mfe[0];
+ else
+ fe = adap->mfe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
+
return 0;
}
@@ -383,8 +442,7 @@
.num_adapters = 1,
.adapter = {
{
- .streaming_ctrl = NULL,
-
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
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-22 22:35:32.014271650 +0200
@@ -214,6 +214,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 },
};
@@ -478,6 +480,11 @@
dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor);
dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor);
+ /* Set the pll registers */
+ 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));
+
/* Calculate the sample frequency */
state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
state->sample_freq /= (state->pll_nfactor + 1);
@@ -1123,7 +1130,7 @@
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
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-22 22:15:33.841271580 +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,8 @@
/* Disable I2C gate access */
u8 disable_gate_access;
+
+ bool no_firmware;
};
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 21:49 ` Jose Alberto Reguero
@ 2011-07-22 22:23 ` Antti Palosaari
2011-07-23 8:26 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-22 22:23 UTC (permalink / raw)
To: Jose Alberto Reguero; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
On 07/23/2011 12:49 AM, Jose Alberto Reguero wrote:
> On Viernes, 22 de Julio de 2011 20:12:20 Jose Alberto Reguero escribió:
>> On Viernes, 22 de Julio de 2011 18:46:24 Antti Palosaari escribió:
>>> On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
>>>> On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
>>>>> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
>>>>>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
>>>>>>> Have you had to time test these?
>>>>>>>
>>>>>>> And about I2C adapter, I don't see why changes are needed. As far as
>>>>>>> I understand it is already working with TDA10023 and you have done
>>>>>>> changes for TDA10048 support. I compared TDA10048 and TDA10023 I2C
>>>>>>> functions and those are ~similar. Both uses most typical access,
>>>>>>> for reg write {u8 REG, u8 VAL} and for reg read {u8 REG}/{u8 VAL}.
>>>>>>>
>>>>>>> regards
>>>>>>> Antti
>>>>>>
>>>>>> I just finish the testing. The changes to I2C are for the tuner
>>>>>> tda827x. The MFE fork fine. I need to change the code in tda10048 and
>>>>>> ttusb2. Attached is the patch for CT-3650 with your MFE patch.
>>>>>
>>>>> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
>>>>> which is wrong. Could you send USB sniff I can look what there really
>>>>> happens. If you have raw SniffUSB2 logs I wish to check those, other
>>>>> logs are welcome too if no raw SniffUSB2 available.
>>>>
>>>> Youre chnage don't work. You need to change the function i2c gate of
>>>> tda1048 for the one of tda1023, but the parameter of this function must
>>>> be the fe pointer of tda1023. If this is a problem, I can duplicate
>>>> tda1023 i2c gate in ttusb2 code and pass it to the tda10048. It is done
>>>> this way in the first patch of this thread.
>>>
>>> Yes I now see why it cannot work - since FE is given as a parameter to
>>> i2c_gate_ctrl it does not see correct priv and used I2C addr is read
>>> from priv. You must implement own i2c_gate_ctrl in ttusb2 driver.
>>> Implement own ct3650_i2c_gate_ctrl and override tda10048 i2c_gate_ctrl
>>> using that. Then call tda10023 i2c_gate_ctrl but instead of tda10048 FE
>>> use td10023 FE. Something like
>>>
>>> static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
>>> {
>>>
>>> return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE, enable);
>>>
>>> }
>>>
>>> /* tuner is behind TDA10023 I2C-gate */
>>> adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
>>>
>>>
>>> Could you still send USB logs? I don't see it correct behaviour you need
>>> to change I2C-adaper when same tuner is used for DVB-T because it was
>>> already working in DVB-C mode.
>>>
>>> regards
>>> Antti
>>
>> Thanks, I try to implement that. I attach a processed log. It prints the
>> first line of a usb command and the first line of the returned byes. If
>> you want the full log I can upload it where you want.
>>
>> Jose Alberto
>
> New version with Antti's sugestion.
GOOD! As you can see implementing things correctly drops also much lines
of code! No more ugly hacks in TDA10048 driver.
But now you must fix that I2C-adapter. I looked sniffs and tda827x
driver. I2C is rather clear. tda827x uses a little bit unusual I2C read.
Normally reads are done as I2C write+read combination, that tuner, as
many other NXP tuners, uses only single read and it is starting always
from reg "0".
It looked for my eyes that it will never do read operation as in read
there is num = 1, msg[0].flags = I2C_M_RD
ttusb2_i2c_xfer():
for (i = 0; i < num; i++) {
read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
But in the case it have been working for DVB-C I don't understand why it
does not work for DVB-T. And thus I really suspect your changes to
I2C-adapter are not needed. So whats the problem using original I2C
adapter? What does it print when debugs are enabled. Is there some
errors in log?
Also looking from sniffs, it seems that this could be wrong:
(rlen > 0 && r[3] != rlen)) {
warn("there might have been an error during control message transfer.
(rlen = %d, was %d)",rlen,r[3]);
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-22 22:23 ` Antti Palosaari
@ 2011-07-23 8:26 ` Jose Alberto Reguero
2011-07-23 9:42 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-23 8:26 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky
On Sábado, 23 de Julio de 2011 00:23:05 Antti Palosaari escribió:
> On 07/23/2011 12:49 AM, Jose Alberto Reguero wrote:
> > On Viernes, 22 de Julio de 2011 20:12:20 Jose Alberto Reguero escribió:
> >> On Viernes, 22 de Julio de 2011 18:46:24 Antti Palosaari escribió:
> >>> On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
> >>>> On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
> >>>>> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
> >>>>>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
> >>>>>>> Have you had to time test these?
> >>>>>>>
> >>>>>>> And about I2C adapter, I don't see why changes are needed. As far
> >>>>>>> as I understand it is already working with TDA10023 and you have
> >>>>>>> done changes for TDA10048 support. I compared TDA10048 and
> >>>>>>> TDA10023 I2C functions and those are ~similar. Both uses most
> >>>>>>> typical access, for reg write {u8 REG, u8 VAL} and for reg read
> >>>>>>> {u8 REG}/{u8 VAL}.
> >>>>>>>
> >>>>>>> regards
> >>>>>>> Antti
> >>>>>>
> >>>>>> I just finish the testing. The changes to I2C are for the tuner
> >>>>>> tda827x. The MFE fork fine. I need to change the code in tda10048
> >>>>>> and ttusb2. Attached is the patch for CT-3650 with your MFE patch.
> >>>>>
> >>>>> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
> >>>>> which is wrong. Could you send USB sniff I can look what there really
> >>>>> happens. If you have raw SniffUSB2 logs I wish to check those, other
> >>>>> logs are welcome too if no raw SniffUSB2 available.
> >>>>
> >>>> Youre chnage don't work. You need to change the function i2c gate of
> >>>> tda1048 for the one of tda1023, but the parameter of this function
> >>>> must be the fe pointer of tda1023. If this is a problem, I can
> >>>> duplicate tda1023 i2c gate in ttusb2 code and pass it to the
> >>>> tda10048. It is done this way in the first patch of this thread.
> >>>
> >>> Yes I now see why it cannot work - since FE is given as a parameter to
> >>> i2c_gate_ctrl it does not see correct priv and used I2C addr is read
> >>> from priv. You must implement own i2c_gate_ctrl in ttusb2 driver.
> >>> Implement own ct3650_i2c_gate_ctrl and override tda10048 i2c_gate_ctrl
> >>> using that. Then call tda10023 i2c_gate_ctrl but instead of tda10048 FE
> >>> use td10023 FE. Something like
> >>>
> >>> static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> >>> {
> >>>
> >>> return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE,
> >>> enable);
> >>>
> >>> }
> >>>
> >>> /* tuner is behind TDA10023 I2C-gate */
> >>> adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
> >>>
> >>>
> >>> Could you still send USB logs? I don't see it correct behaviour you
> >>> need to change I2C-adaper when same tuner is used for DVB-T because it
> >>> was already working in DVB-C mode.
> >>>
> >>> regards
> >>> Antti
> >>
> >> Thanks, I try to implement that. I attach a processed log. It prints the
> >> first line of a usb command and the first line of the returned byes. If
> >> you want the full log I can upload it where you want.
> >>
> >> Jose Alberto
> >
> > New version with Antti's sugestion.
>
> GOOD! As you can see implementing things correctly drops also much lines
> of code! No more ugly hacks in TDA10048 driver.
>
> But now you must fix that I2C-adapter. I looked sniffs and tda827x
> driver. I2C is rather clear. tda827x uses a little bit unusual I2C read.
> Normally reads are done as I2C write+read combination, that tuner, as
> many other NXP tuners, uses only single read and it is starting always
> from reg "0".
>
> It looked for my eyes that it will never do read operation as in read
> there is num = 1, msg[0].flags = I2C_M_RD
>
> ttusb2_i2c_xfer():
> for (i = 0; i < num; i++) {
> read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
>
> But in the case it have been working for DVB-C I don't understand why it
> does not work for DVB-T. And thus I really suspect your changes to
> I2C-adapter are not needed. So whats the problem using original I2C
> adapter? What does it print when debugs are enabled. Is there some
> errors in log?
>
> Also looking from sniffs, it seems that this could be wrong:
> (rlen > 0 && r[3] != rlen)) {
> warn("there might have been an error during control message
transfer.
> (rlen = %d, was %d)",rlen,r[3]);
>
>
> regards
> Antti
The problem is in i2c read in tda827x_probe_version. Without the fix sometimes,
when changing the code the tuner is detected as tda827xo instead of tda827xa.
That is because the variable where i2c read should store the value is
initialized, and sometimes it works.
Jose Alberto
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 8:26 ` Jose Alberto Reguero
@ 2011-07-23 9:42 ` Antti Palosaari
2011-07-23 10:21 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-23 9:42 UTC (permalink / raw)
To: Jose Alberto Reguero
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
> On Sábado, 23 de Julio de 2011 00:23:05 Antti Palosaari escribió:
>> On 07/23/2011 12:49 AM, Jose Alberto Reguero wrote:
>>> On Viernes, 22 de Julio de 2011 20:12:20 Jose Alberto Reguero escribió:
>>>> On Viernes, 22 de Julio de 2011 18:46:24 Antti Palosaari escribió:
>>>>> On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
>>>>>> On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
>>>>>>> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
>>>>>>>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
>>>>>>>>> Have you had to time test these?
>>>>>>>>>
>>>>>>>>> And about I2C adapter, I don't see why changes are needed. As far
>>>>>>>>> as I understand it is already working with TDA10023 and you have
>>>>>>>>> done changes for TDA10048 support. I compared TDA10048 and
>>>>>>>>> TDA10023 I2C functions and those are ~similar. Both uses most
>>>>>>>>> typical access, for reg write {u8 REG, u8 VAL} and for reg read
>>>>>>>>> {u8 REG}/{u8 VAL}.
>>>>>>>>>
>>>>>>>>> regards
>>>>>>>>> Antti
>>>>>>>>
>>>>>>>> I just finish the testing. The changes to I2C are for the tuner
>>>>>>>> tda827x. The MFE fork fine. I need to change the code in tda10048
>>>>>>>> and ttusb2. Attached is the patch for CT-3650 with your MFE patch.
>>>>>>>
>>>>>>> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
>>>>>>> which is wrong. Could you send USB sniff I can look what there really
>>>>>>> happens. If you have raw SniffUSB2 logs I wish to check those, other
>>>>>>> logs are welcome too if no raw SniffUSB2 available.
>>>>>>
>>>>>> Youre chnage don't work. You need to change the function i2c gate of
>>>>>> tda1048 for the one of tda1023, but the parameter of this function
>>>>>> must be the fe pointer of tda1023. If this is a problem, I can
>>>>>> duplicate tda1023 i2c gate in ttusb2 code and pass it to the
>>>>>> tda10048. It is done this way in the first patch of this thread.
>>>>>
>>>>> Yes I now see why it cannot work - since FE is given as a parameter to
>>>>> i2c_gate_ctrl it does not see correct priv and used I2C addr is read
>>>>> from priv. You must implement own i2c_gate_ctrl in ttusb2 driver.
>>>>> Implement own ct3650_i2c_gate_ctrl and override tda10048 i2c_gate_ctrl
>>>>> using that. Then call tda10023 i2c_gate_ctrl but instead of tda10048 FE
>>>>> use td10023 FE. Something like
>>>>>
>>>>> static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
>>>>> {
>>>>>
>>>>> return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE,
>>>>> enable);
>>>>>
>>>>> }
>>>>>
>>>>> /* tuner is behind TDA10023 I2C-gate */
>>>>> adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
>>>>>
>>>>>
>>>>> Could you still send USB logs? I don't see it correct behaviour you
>>>>> need to change I2C-adaper when same tuner is used for DVB-T because it
>>>>> was already working in DVB-C mode.
>>>>>
>>>>> regards
>>>>> Antti
>>>>
>>>> Thanks, I try to implement that. I attach a processed log. It prints the
>>>> first line of a usb command and the first line of the returned byes. If
>>>> you want the full log I can upload it where you want.
>>>>
>>>> Jose Alberto
>>>
>>> New version with Antti's sugestion.
>>
>> GOOD! As you can see implementing things correctly drops also much lines
>> of code! No more ugly hacks in TDA10048 driver.
>>
>> But now you must fix that I2C-adapter. I looked sniffs and tda827x
>> driver. I2C is rather clear. tda827x uses a little bit unusual I2C read.
>> Normally reads are done as I2C write+read combination, that tuner, as
>> many other NXP tuners, uses only single read and it is starting always
>> from reg "0".
>>
>> It looked for my eyes that it will never do read operation as in read
>> there is num = 1, msg[0].flags = I2C_M_RD
>>
>> ttusb2_i2c_xfer():
>> for (i = 0; i< num; i++) {
>> read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
>>
>> But in the case it have been working for DVB-C I don't understand why it
>> does not work for DVB-T. And thus I really suspect your changes to
>> I2C-adapter are not needed. So whats the problem using original I2C
>> adapter? What does it print when debugs are enabled. Is there some
>> errors in log?
>>
>> Also looking from sniffs, it seems that this could be wrong:
>> (rlen> 0&& r[3] != rlen)) {
>> warn("there might have been an error during control message
> transfer.
>> (rlen = %d, was %d)",rlen,r[3]);
>>
>>
>> regards
>> Antti
>
> The problem is in i2c read in tda827x_probe_version. Without the fix sometimes,
> when changing the code the tuner is detected as tda827xo instead of tda827xa.
> That is because the variable where i2c read should store the value is
> initialized, and sometimes it works.
struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
.buf = &data, .len = 1 };
rc = tuner_transfer(fe, &msg, 1);
:-( Could you read what I write. It is a little bit annoying to find out
everything for you. You just answer every time something like it does
not work and I should always find out what's problem.
As I pointed out read will never work since I2C adapter supports only
read done in WRITE+READ combination. Driver uses read which is single
READ without write.
You should implement new read. You can look example from af9015 or other
drivers using tda827x
This have been never worked thus I Cc Guy Martin who have added DVB-C
support for that device.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 9:42 ` Antti Palosaari
@ 2011-07-23 10:21 ` Jose Alberto Reguero
2011-07-23 10:37 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-23 10:21 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
> On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
> > On Sábado, 23 de Julio de 2011 00:23:05 Antti Palosaari escribió:
> >> On 07/23/2011 12:49 AM, Jose Alberto Reguero wrote:
> >>> On Viernes, 22 de Julio de 2011 20:12:20 Jose Alberto Reguero escribió:
> >>>> On Viernes, 22 de Julio de 2011 18:46:24 Antti Palosaari escribió:
> >>>>> On 07/22/2011 07:25 PM, Jose Alberto Reguero wrote:
> >>>>>> On Viernes, 22 de Julio de 2011 18:08:39 Antti Palosaari escribió:
> >>>>>>> On 07/22/2011 07:02 PM, Jose Alberto Reguero wrote:
> >>>>>>>> On Viernes, 22 de Julio de 2011 13:32:53 Antti Palosaari escribió:
> >>>>>>>>> Have you had to time test these?
> >>>>>>>>>
> >>>>>>>>> And about I2C adapter, I don't see why changes are needed. As far
> >>>>>>>>> as I understand it is already working with TDA10023 and you have
> >>>>>>>>> done changes for TDA10048 support. I compared TDA10048 and
> >>>>>>>>> TDA10023 I2C functions and those are ~similar. Both uses most
> >>>>>>>>> typical access, for reg write {u8 REG, u8 VAL} and for reg read
> >>>>>>>>> {u8 REG}/{u8 VAL}.
> >>>>>>>>>
> >>>>>>>>> regards
> >>>>>>>>> Antti
> >>>>>>>>
> >>>>>>>> I just finish the testing. The changes to I2C are for the tuner
> >>>>>>>> tda827x. The MFE fork fine. I need to change the code in tda10048
> >>>>>>>> and ttusb2. Attached is the patch for CT-3650 with your MFE patch.
> >>>>>>>
> >>>>>>> You still pass tda10023 fe pointer to tda10048 for I2C-gate control
> >>>>>>> which is wrong. Could you send USB sniff I can look what there
> >>>>>>> really happens. If you have raw SniffUSB2 logs I wish to check
> >>>>>>> those, other logs are welcome too if no raw SniffUSB2 available.
> >>>>>>
> >>>>>> Youre chnage don't work. You need to change the function i2c gate of
> >>>>>> tda1048 for the one of tda1023, but the parameter of this function
> >>>>>> must be the fe pointer of tda1023. If this is a problem, I can
> >>>>>> duplicate tda1023 i2c gate in ttusb2 code and pass it to the
> >>>>>> tda10048. It is done this way in the first patch of this thread.
> >>>>>
> >>>>> Yes I now see why it cannot work - since FE is given as a parameter
> >>>>> to i2c_gate_ctrl it does not see correct priv and used I2C addr is
> >>>>> read from priv. You must implement own i2c_gate_ctrl in ttusb2
> >>>>> driver. Implement own ct3650_i2c_gate_ctrl and override tda10048
> >>>>> i2c_gate_ctrl using that. Then call tda10023 i2c_gate_ctrl but
> >>>>> instead of tda10048 FE use td10023 FE. Something like
> >>>>>
> >>>>> static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> >>>>> {
> >>>>>
> >>>>> return adap->mfe[0]->ops.i2c_gate_ctrl(POINTER_TO_TDA10023_FE,
> >>>>> enable);
> >>>>>
> >>>>> }
> >>>>>
> >>>>> /* tuner is behind TDA10023 I2C-gate */
> >>>>> adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
> >>>>>
> >>>>>
> >>>>> Could you still send USB logs? I don't see it correct behaviour you
> >>>>> need to change I2C-adaper when same tuner is used for DVB-T because
> >>>>> it was already working in DVB-C mode.
> >>>>>
> >>>>> regards
> >>>>> Antti
> >>>>
> >>>> Thanks, I try to implement that. I attach a processed log. It prints
> >>>> the first line of a usb command and the first line of the returned
> >>>> byes. If you want the full log I can upload it where you want.
> >>>>
> >>>> Jose Alberto
> >>>
> >>> New version with Antti's sugestion.
> >>
> >> GOOD! As you can see implementing things correctly drops also much lines
> >> of code! No more ugly hacks in TDA10048 driver.
> >>
> >> But now you must fix that I2C-adapter. I looked sniffs and tda827x
> >> driver. I2C is rather clear. tda827x uses a little bit unusual I2C read.
> >> Normally reads are done as I2C write+read combination, that tuner, as
> >> many other NXP tuners, uses only single read and it is starting always
> >> from reg "0".
> >>
> >> It looked for my eyes that it will never do read operation as in read
> >> there is num = 1, msg[0].flags = I2C_M_RD
> >>
> >> ttusb2_i2c_xfer():
> >> for (i = 0; i< num; i++) {
> >>
> >> read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> >>
> >> But in the case it have been working for DVB-C I don't understand why it
> >> does not work for DVB-T. And thus I really suspect your changes to
> >> I2C-adapter are not needed. So whats the problem using original I2C
> >> adapter? What does it print when debugs are enabled. Is there some
> >> errors in log?
> >>
> >> Also looking from sniffs, it seems that this could be wrong:
> >> (rlen> 0&& r[3] != rlen)) {
> >> warn("there might have been an error during control message
> >
> > transfer.
> >
> >> (rlen = %d, was %d)",rlen,r[3]);
> >>
> >>
> >> regards
> >> Antti
> >
> > The problem is in i2c read in tda827x_probe_version. Without the fix
> > sometimes, when changing the code the tuner is detected as tda827xo
> > instead of tda827xa. That is because the variable where i2c read should
> > store the value is initialized, and sometimes it works.
>
> struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
> .buf = &data, .len = 1 };
>
> rc = tuner_transfer(fe, &msg, 1);
>
> :-( Could you read what I write. It is a little bit annoying to find out
>
> everything for you. You just answer every time something like it does
> not work and I should always find out what's problem.
>
> As I pointed out read will never work since I2C adapter supports only
> read done in WRITE+READ combination. Driver uses read which is single
> READ without write.
>
> You should implement new read. You can look example from af9015 or other
> drivers using tda827x
>
> This have been never worked thus I Cc Guy Martin who have added DVB-C
> support for that device.
>
>
> regards
> Antti
I don't understand you. I think that you don' see the fix, but the old code.
Old code:
read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
Fix:
read1 = i+1 < num && (msg[i+1].flags & I2C_M_RD); for the tda10023 and tda10048
read2 = msg[i].flags & I2C_M_RD; for the tda827x
Jose Alberto
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 10:21 ` Jose Alberto Reguero
@ 2011-07-23 10:37 ` Antti Palosaari
2011-07-23 15:41 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-23 10:37 UTC (permalink / raw)
To: Jose Alberto Reguero
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On 07/23/2011 01:21 PM, Jose Alberto Reguero wrote:
> On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
>> On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
>>> The problem is in i2c read in tda827x_probe_version. Without the fix
>>> sometimes, when changing the code the tuner is detected as tda827xo
>>> instead of tda827xa. That is because the variable where i2c read should
>>> store the value is initialized, and sometimes it works.
>>
>> struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
>> .buf =&data, .len = 1 };
>>
>> rc = tuner_transfer(fe,&msg, 1);
>>
>> :-( Could you read what I write. It is a little bit annoying to find out
>>
>> everything for you. You just answer every time something like it does
>> not work and I should always find out what's problem.
>>
>> As I pointed out read will never work since I2C adapter supports only
>> read done in WRITE+READ combination. Driver uses read which is single
>> READ without write.
>>
>> You should implement new read. You can look example from af9015 or other
>> drivers using tda827x
>>
>> This have been never worked thus I Cc Guy Martin who have added DVB-C
>> support for that device.
>>
>>
>> regards
>> Antti
>
> I don't understand you. I think that you don' see the fix, but the old code.
> Old code:
>
> read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
>
> Fix:
>
> read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); for the tda10023 and tda10048
> read2 = msg[i].flags& I2C_M_RD; for the tda827x
>
> Jose Alberto
First of all I must apologize of blaming you about that I2C adapter,
sorry, I should going to shame now. It was me who doesn't read your
changes as should :/
Your changes are logically OK and implements correctly single reading as
needed. Some comments still;
* consider renaming read1 and read2 for example write_read and read
* obuf[1] contains WRITE len. your code sets that now as READ len.
Probably it should be 0 always in single write since no bytes written.
* remove useless checks from end of the "if (foo) if (foo)";
if (read1 || read2) {
if (read1) {
[...]
} else if (read2)
If you store some variables at the beginning, olen, ilen, obuf, ibuf,
you can increase i++ for write+read and rest of the code in function can
be same (no more if read or write + read). But maybe it is safe to keep
closer original than change such much.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 10:37 ` Antti Palosaari
@ 2011-07-23 15:41 ` Jose Alberto Reguero
2011-07-23 17:47 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-23 15:41 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On Sábado, 23 de Julio de 2011 12:37:53 Antti Palosaari escribió:
> On 07/23/2011 01:21 PM, Jose Alberto Reguero wrote:
> > On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
> >> On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
> >>> The problem is in i2c read in tda827x_probe_version. Without the fix
> >>> sometimes, when changing the code the tuner is detected as tda827xo
> >>> instead of tda827xa. That is because the variable where i2c read should
> >>> store the value is initialized, and sometimes it works.
> >>
> >> struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
> >>
> >> .buf =&data, .len = 1 };
> >>
> >> rc = tuner_transfer(fe,&msg, 1);
> >>
> >> :-( Could you read what I write. It is a little bit annoying to find out
> >>
> >> everything for you. You just answer every time something like it does
> >> not work and I should always find out what's problem.
> >>
> >> As I pointed out read will never work since I2C adapter supports only
> >> read done in WRITE+READ combination. Driver uses read which is single
> >> READ without write.
> >>
> >> You should implement new read. You can look example from af9015 or other
> >> drivers using tda827x
> >>
> >> This have been never worked thus I Cc Guy Martin who have added DVB-C
> >> support for that device.
> >>
> >>
> >> regards
> >> Antti
> >
> > I don't understand you. I think that you don' see the fix, but the old
> > code. Old code:
> >
> > read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> >
> > Fix:
> >
> > read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); for the tda10023 and
> > tda10048 read2 = msg[i].flags& I2C_M_RD; for the tda827x
> >
> > Jose Alberto
>
> First of all I must apologize of blaming you about that I2C adapter,
> sorry, I should going to shame now. It was me who doesn't read your
> changes as should :/
>
> Your changes are logically OK and implements correctly single reading as
> needed. Some comments still;
> * consider renaming read1 and read2 for example write_read and read
> * obuf[1] contains WRITE len. your code sets that now as READ len.
> Probably it should be 0 always in single write since no bytes written.
> * remove useless checks from end of the "if (foo) if (foo)";
> if (read1 || read2) {
> if (read1) {
> [...]
> } else if (read2)
>
> If you store some variables at the beginning, olen, ilen, obuf, ibuf,
> you can increase i++ for write+read and rest of the code in function can
> be same (no more if read or write + read). But maybe it is safe to keep
> closer original than change such much.
>
>
> regards
> Antti
There are a second i2c read, but less important.It is in:
tda827xa_set_params
............
buf[0] = 0xa0;
buf[1] = 0x40;
msg.len = 2;
rc = tuner_transfer(fe, &msg, 1);
if (rc < 0)
goto err;
msleep(11);
msg.flags = I2C_M_RD;
rc = tuner_transfer(fe, &msg, 1);
if (rc < 0)
goto err;
msg.flags = 0;
buf[1] >>= 4;
............
I supposed that buf[0] is the register to read and they read the value in
buf[1]. The code now seem to work ok but perhaps is wrong.
Jose Alberto
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 15:41 ` Jose Alberto Reguero
@ 2011-07-23 17:47 ` Antti Palosaari
2011-07-23 21:45 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-23 17:47 UTC (permalink / raw)
To: Jose Alberto Reguero
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On 07/23/2011 06:41 PM, Jose Alberto Reguero wrote:
> On Sábado, 23 de Julio de 2011 12:37:53 Antti Palosaari escribió:
>> On 07/23/2011 01:21 PM, Jose Alberto Reguero wrote:
>>> On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
>>>> On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
>>>>> The problem is in i2c read in tda827x_probe_version. Without the fix
>>>>> sometimes, when changing the code the tuner is detected as tda827xo
>>>>> instead of tda827xa. That is because the variable where i2c read should
>>>>> store the value is initialized, and sometimes it works.
>>>>
>>>> struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
>>>>
>>>> .buf =&data, .len = 1 };
>>>>
>>>> rc = tuner_transfer(fe,&msg, 1);
>>>>
>>>> :-( Could you read what I write. It is a little bit annoying to find out
>>>>
>>>> everything for you. You just answer every time something like it does
>>>> not work and I should always find out what's problem.
>>>>
>>>> As I pointed out read will never work since I2C adapter supports only
>>>> read done in WRITE+READ combination. Driver uses read which is single
>>>> READ without write.
>>>>
>>>> You should implement new read. You can look example from af9015 or other
>>>> drivers using tda827x
>>>>
>>>> This have been never worked thus I Cc Guy Martin who have added DVB-C
>>>> support for that device.
>>>>
>>>>
>>>> regards
>>>> Antti
>>>
>>> I don't understand you. I think that you don' see the fix, but the old
>>> code. Old code:
>>>
>>> read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
>>>
>>> Fix:
>>>
>>> read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); for the tda10023 and
>>> tda10048 read2 = msg[i].flags& I2C_M_RD; for the tda827x
>>>
>>> Jose Alberto
>>
>> First of all I must apologize of blaming you about that I2C adapter,
>> sorry, I should going to shame now. It was me who doesn't read your
>> changes as should :/
>>
>> Your changes are logically OK and implements correctly single reading as
>> needed. Some comments still;
>> * consider renaming read1 and read2 for example write_read and read
>> * obuf[1] contains WRITE len. your code sets that now as READ len.
>> Probably it should be 0 always in single write since no bytes written.
>> * remove useless checks from end of the "if (foo) if (foo)";
>> if (read1 || read2) {
>> if (read1) {
>> [...]
>> } else if (read2)
>>
>> If you store some variables at the beginning, olen, ilen, obuf, ibuf,
>> you can increase i++ for write+read and rest of the code in function can
>> be same (no more if read or write + read). But maybe it is safe to keep
>> closer original than change such much.
>>
>>
>> regards
>> Antti
>
> There are a second i2c read, but less important.It is in:
>
> tda827xa_set_params
>
> ............
> buf[0] = 0xa0;
> buf[1] = 0x40;
> msg.len = 2;
> rc = tuner_transfer(fe,&msg, 1);
> if (rc< 0)
> goto err;
>
> msleep(11);
> msg.flags = I2C_M_RD;
> rc = tuner_transfer(fe,&msg, 1);
> if (rc< 0)
> goto err;
> msg.flags = 0;
>
> buf[1]>>= 4;
> ............
> I supposed that buf[0] is the register to read and they read the value in
> buf[1]. The code now seem to work ok but perhaps is wrong.
This one is as translated to "normal" C we usually use;
write_reg(0xa0, 0x40); // write one reg
read_regs(2); // read 2 regs
example from the sniff
AA B0 31 05 C2 02 00 A0 40 ª°1.Â.. @
55 B0 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC U°1.Â..JD....q¬ì
AA B1 31 05 C2 02 00 30 11 ª±1.Â..0.
55 B1 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC U±1.Â..JD....q¬ì
AA USB direction to device
B1 USB msg seq
31 USB cmd
05 USB data len (4+5=9, 4=hdr len, 5=data len, 9=total)
C2 I2C addr (addr << 1)
02 I2C write len
00 I2C read len
30 I2C data [0]
11 I2C data [1]
So it seems actually to write 30 11 and then read 4a 44 as reply. But if
you read driver code it does not write "30 11" instead just reads. Maybe
buggy I2C adap implementation or buggy tuner driver (Linux driver or
driver where sniff taken). Try to read without write and with write and
compare if there is any difference.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 17:47 ` Antti Palosaari
@ 2011-07-23 21:45 ` Jose Alberto Reguero
2011-07-27 19:22 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-23 21:45 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
[-- Attachment #1: Type: Text/Plain, Size: 5006 bytes --]
On Sábado, 23 de Julio de 2011 19:47:27 Antti Palosaari escribió:
> On 07/23/2011 06:41 PM, Jose Alberto Reguero wrote:
> > On Sábado, 23 de Julio de 2011 12:37:53 Antti Palosaari escribió:
> >> On 07/23/2011 01:21 PM, Jose Alberto Reguero wrote:
> >>> On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
> >>>> On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
> >>>>> The problem is in i2c read in tda827x_probe_version. Without the fix
> >>>>> sometimes, when changing the code the tuner is detected as tda827xo
> >>>>> instead of tda827xa. That is because the variable where i2c read
> >>>>> should store the value is initialized, and sometimes it works.
> >>>>
> >>>> struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,
> >>>>
> >>>> .buf =&data, .len = 1 };
> >>>>
> >>>> rc = tuner_transfer(fe,&msg, 1);
> >>>>
> >>>> :-( Could you read what I write. It is a little bit annoying to find
> >>>> :out
> >>>>
> >>>> everything for you. You just answer every time something like it does
> >>>> not work and I should always find out what's problem.
> >>>>
> >>>> As I pointed out read will never work since I2C adapter supports only
> >>>> read done in WRITE+READ combination. Driver uses read which is single
> >>>> READ without write.
> >>>>
> >>>> You should implement new read. You can look example from af9015 or
> >>>> other drivers using tda827x
> >>>>
> >>>> This have been never worked thus I Cc Guy Martin who have added DVB-C
> >>>> support for that device.
> >>>>
> >>>>
> >>>> regards
> >>>> Antti
> >>>
> >>> I don't understand you. I think that you don' see the fix, but the old
> >>> code. Old code:
> >>>
> >>> read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> >>>
> >>> Fix:
> >>>
> >>> read1 = i+1< num&& (msg[i+1].flags& I2C_M_RD); for the tda10023
> >>> and tda10048 read2 = msg[i].flags& I2C_M_RD; for the tda827x
> >>>
> >>> Jose Alberto
> >>
> >> First of all I must apologize of blaming you about that I2C adapter,
> >> sorry, I should going to shame now. It was me who doesn't read your
> >> changes as should :/
> >>
> >> Your changes are logically OK and implements correctly single reading as
> >> needed. Some comments still;
> >> * consider renaming read1 and read2 for example write_read and read
> >> * obuf[1] contains WRITE len. your code sets that now as READ len.
> >> Probably it should be 0 always in single write since no bytes written.
> >> * remove useless checks from end of the "if (foo) if (foo)";
> >> if (read1 || read2) {
> >>
> >> if (read1) {
> >>
> >> [...]
> >>
> >> } else if (read2)
> >>
> >> If you store some variables at the beginning, olen, ilen, obuf, ibuf,
> >> you can increase i++ for write+read and rest of the code in function can
> >> be same (no more if read or write + read). But maybe it is safe to keep
> >> closer original than change such much.
> >>
> >>
> >> regards
> >> Antti
> >
> > There are a second i2c read, but less important.It is in:
> >
> > tda827xa_set_params
> >
> > ............
> >
> > buf[0] = 0xa0;
> > buf[1] = 0x40;
> > msg.len = 2;
> > rc = tuner_transfer(fe,&msg, 1);
> > if (rc< 0)
> >
> > goto err;
> >
> > msleep(11);
> > msg.flags = I2C_M_RD;
> > rc = tuner_transfer(fe,&msg, 1);
> > if (rc< 0)
> >
> > goto err;
> >
> > msg.flags = 0;
> >
> > buf[1]>>= 4;
> >
> > ............
> > I supposed that buf[0] is the register to read and they read the value in
> > buf[1]. The code now seem to work ok but perhaps is wrong.
>
> This one is as translated to "normal" C we usually use;
> write_reg(0xa0, 0x40); // write one reg
> read_regs(2); // read 2 regs
>
> example from the sniff
> AA B0 31 05 C2 02 00 A0 40 ª°1.Â.. @
> 55 B0 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC U°1.Â..JD....q¬ì
> AA B1 31 05 C2 02 00 30 11 ª±1.Â..0.
> 55 B1 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC U±1.Â..JD....q¬ì
>
>
> AA USB direction to device
> B1 USB msg seq
> 31 USB cmd
> 05 USB data len (4+5=9, 4=hdr len, 5=data len, 9=total)
> C2 I2C addr (addr << 1)
> 02 I2C write len
> 00 I2C read len
> 30 I2C data [0]
> 11 I2C data [1]
>
> So it seems actually to write 30 11 and then read 4a 44 as reply. But if
> you read driver code it does not write "30 11" instead just reads. Maybe
> buggy I2C adap implementation or buggy tuner driver (Linux driver or
> driver where sniff taken). Try to read without write and with write and
> compare if there is any difference.
>
>
> regards
> Antti
Read without write work as with write. Attached updated patch.
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
Jose Alberto
[-- Attachment #2: ttusb2-6.diff --]
[-- Type: text/x-patch, Size: 6417 bytes --]
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-23 23:12:29.341385243 +0200
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,7 @@
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
static u8 obuf[60], ibuf[60];
- int i,read;
+ int i, write_read, read;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
@@ -91,28 +92,35 @@
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);
+ write_read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
+ read = msg[i].flags & I2C_M_RD;
- obuf[0] = (msg[i].addr << 1) | read;
- obuf[1] = msg[i].len;
+ obuf[0] = (msg[i].addr << 1) | (write_read | read);
+ if (read)
+ obuf[1] = 0;
+ else
+ obuf[1] = msg[i].len;
/* read request */
- if (read)
+ if (write_read)
obuf[2] = msg[i+1].len;
+ else if (read)
+ 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);
+ if (write_read) {
+ memcpy(msg[i+1].buf, &ibuf[3], msg[i+1].len);
i++;
- }
+ } else if (read)
+ memcpy(msg[i].buf, &ibuf[3], msg[i].len);
}
mutex_unlock(&d->i2c_mutex);
@@ -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)
@@ -203,23 +226,60 @@
return 0;
}
+static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
+{
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+
+ return adap->mfe[0]->ops.i2c_gate_ctrl(adap->mfe[0], enable);
+}
+
static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->mfe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->mfe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->mfe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ } else {
+ adap->mfe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->mfe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ return -ENODEV;
+ }
+
+ /* tuner is behind TDA10023 I2C-gate */
+ adap->mfe[1]->ops.i2c_gate_ctrl = ct3650_i2c_gate_ctrl;
+
}
+
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) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->mfe[1] == NULL)
+ fe = adap->mfe[0];
+ else
+ fe = adap->mfe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
+
return 0;
}
@@ -383,8 +443,7 @@
.num_adapters = 1,
.adapter = {
{
- .streaming_ctrl = NULL,
-
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
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-22 22:35:32.014271650 +0200
@@ -214,6 +214,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 },
};
@@ -478,6 +480,11 @@
dprintk(1, "- pll_nfactor = %d\n", state->pll_nfactor);
dprintk(1, "- pll_pfactor = %d\n", state->pll_pfactor);
+ /* Set the pll registers */
+ 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));
+
/* Calculate the sample frequency */
state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);
state->sample_freq /= (state->pll_nfactor + 1);
@@ -1123,7 +1130,7 @@
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
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-22 22:15:33.841271580 +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,8 @@
/* Disable I2C gate access */
u8 disable_gate_access;
+
+ bool no_firmware;
};
#if defined(CONFIG_DVB_TDA10048) || \
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-23 21:45 ` Jose Alberto Reguero
@ 2011-07-27 19:22 ` Antti Palosaari
2011-07-28 19:25 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-07-27 19:22 UTC (permalink / raw)
To: Jose Alberto Reguero
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> Read without write work as with write. Attached updated patch.
> ttusb2-6.diff
> - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> + read = msg[i].flags& I2C_M_RD;
ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing any
random bytes in case of single read. Good!
> + .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,
> +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
> { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
> + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
> + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
> + /* Set the pll registers */
> + 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));
This if only issue can have effect to functionality and I want double
check. I see few things.
1) clock (and PLL) settings should be done generally only once at
.init() and probably .sleep() in case of needed for sleep. Something
like start clock in init, stop clock in sleep. It is usually very first
thing to set before other. Now it is in wrong place - .set_frontend().
2) Those clock settings seem somehow weird. As you set different PLL M
divider for 6 MHz bandwidth than others. Have you looked those are
really correct? I suspect there could be some other Xtal than 16MHz and
thus those are wrong. Which Xtals there is inside device used? There is
most likely 3 Xtals, one for each chip. It is metal box nearest to chip.
Ran checkpatch.pl also to find out style issues before send patch.
I have send new version, hopefully final, of MFE. It changes array name
from adap->mfe to adap-fe. You should also update that.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-27 19:22 ` Antti Palosaari
@ 2011-07-28 19:25 ` Jose Alberto Reguero
2011-08-02 19:21 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-07-28 19:25 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
> On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> > Read without write work as with write. Attached updated patch.
> >
> > ttusb2-6.diff
> >
> > - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > + read = msg[i].flags& I2C_M_RD;
>
> ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing any
> random bytes in case of single read. Good!
>
> > + .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,
> >
> >
> > +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
>
> cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
>
> > { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
> >
> > + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
> > + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
> >
> > + /* Set the pll registers */
> > + 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));
>
> This if only issue can have effect to functionality and I want double
> check. I see few things.
>
> 1) clock (and PLL) settings should be done generally only once at
> .init() and probably .sleep() in case of needed for sleep. Something
> like start clock in init, stop clock in sleep. It is usually very first
> thing to set before other. Now it is in wrong place - .set_frontend().
>
> 2) Those clock settings seem somehow weird. As you set different PLL M
> divider for 6 MHz bandwidth than others. Have you looked those are
> really correct? I suspect there could be some other Xtal than 16MHz and
> thus those are wrong. Which Xtals there is inside device used? There is
> most likely 3 Xtals, one for each chip. It is metal box nearest to chip.
I left 6MHz like it was before in the driver. I try to do other way, allowing
to put different PLL in config that the default ones of the driver and
initialize it in init.
Jose Alberto
>
>
> Ran checkpatch.pl also to find out style issues before send patch.
>
> I have send new version, hopefully final, of MFE. It changes array name
> from adap->mfe to adap-fe. You should also update that.
>
>
> regards
> Antti
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-07-28 19:25 ` Jose Alberto Reguero
@ 2011-08-02 19:21 ` Jose Alberto Reguero
2011-08-08 10:35 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-08-02 19:21 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
[-- Attachment #1: Type: Text/Plain, Size: 3095 bytes --]
On Jueves, 28 de Julio de 2011 21:25:01 Jose Alberto Reguero escribió:
> On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
> > On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> > > Read without write work as with write. Attached updated patch.
> > >
> > > ttusb2-6.diff
> > >
> > > - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > > + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > > + read = msg[i].flags& I2C_M_RD;
> >
> > ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing any
> > random bytes in case of single read. Good!
> >
> > > + .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,
> > >
> > >
> > > +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> >
> > cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
> >
> > > { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
> > >
> > > + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
> > > + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
> > >
> > > + /* Set the pll registers */
> > > + 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));
> >
> > This if only issue can have effect to functionality and I want double
> > check. I see few things.
> >
> > 1) clock (and PLL) settings should be done generally only once at
> > .init() and probably .sleep() in case of needed for sleep. Something
> > like start clock in init, stop clock in sleep. It is usually very first
> > thing to set before other. Now it is in wrong place - .set_frontend().
> >
> > 2) Those clock settings seem somehow weird. As you set different PLL M
> > divider for 6 MHz bandwidth than others. Have you looked those are
> > really correct? I suspect there could be some other Xtal than 16MHz and
> > thus those are wrong. Which Xtals there is inside device used? There is
> > most likely 3 Xtals, one for each chip. It is metal box nearest to chip.
>
> I left 6MHz like it was before in the driver. I try to do other way,
> allowing to put different PLL in config that the default ones of the
> driver and initialize it in init.
>
> Jose Alberto
>
Attached new version of the patch. Adding tda827x lna and doing tda10048 pll
in other way.
Jose Alberto
> > Ran checkpatch.pl also to find out style issues before send patch.
> >
> > I have send new version, hopefully final, of MFE. It changes array name
> > from adap->mfe to adap-fe. You should also update that.
> >
> >
> > 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
[-- Attachment #2: ttusb2-7.diff --]
[-- Type: text/x-patch, Size: 7990 bytes --]
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-08-01 05:45:24.000000000 +0200
+++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-08-02 21:09:18.363001064 +0200
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,7 @@
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
static u8 obuf[60], ibuf[60];
- int i,read;
+ int i, write_read, read;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
@@ -91,28 +92,35 @@
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);
+ write_read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
+ read = msg[i].flags & I2C_M_RD;
- obuf[0] = (msg[i].addr << 1) | read;
- obuf[1] = msg[i].len;
+ obuf[0] = (msg[i].addr << 1) | (write_read | read);
+ if (read)
+ obuf[1] = 0;
+ else
+ obuf[1] = msg[i].len;
/* read request */
- if (read)
+ if (write_read)
obuf[2] = msg[i+1].len;
+ else if (read)
+ 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);
+ if (write_read) {
+ memcpy(msg[i+1].buf, &ibuf[3], msg[i+1].len);
i++;
- }
+ } else if (read)
+ memcpy(msg[i].buf, &ibuf[3], msg[i].len);
}
mutex_unlock(&d->i2c_mutex);
@@ -190,6 +198,25 @@
.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,
+ .set_pll = true ,
+ .pll_m = 5,
+ .pll_n = 3,
+ .pll_p = 0,
+};
+
+static struct tda827x_config tda827x_config = {
+ .config = 3,
+};
+
static int ttusb2_frontend_tda10086_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev,0,3) < 0)
@@ -203,20 +230,71 @@
return 0;
}
+static int ttusb2_ct3650_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
+{
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+
+ return adap->fe[0]->ops.i2c_gate_ctrl(adap->fe[0], enable);
+}
+
+static int ttusb2_tuner_callback(void *priv, int component, int cmd, int enable)
+{
+ struct dvb_usb_device *d = priv;
+ u8 b[2];
+
+
+ b[0] = 0x02;
+ b[1] = enable ? 0 : 1;
+
+ return ttusb2_msg(d, 0x03, b, 2, NULL, 0);
+}
+
static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe[0] = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->fe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->fe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->fe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ adap->dev->i2c_adap.algo_data = adap->dev;
+ adap->fe[0]->callback = ttusb2_tuner_callback;
+ } else {
+ adap->fe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->fe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ return -ENODEV;
+ }
+
+ /* tuner is behind TDA10023 I2C-gate */
+ adap->fe[1]->ops.i2c_gate_ctrl = ttusb2_ct3650_i2c_gate_ctrl;
+ adap->fe[1]->callback = ttusb2_tuner_callback;
+
}
+
return 0;
}
static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap)
{
- if (dvb_attach(tda827x_attach, adap->fe[0], 0x61, &adap->dev->i2c_adap, NULL) == NULL) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->fe[1] == NULL)
+ fe = adap->fe[0];
+ else
+ fe = adap->fe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
@@ -385,6 +463,7 @@
{
.streaming_ctrl = NULL,
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
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-08-02 18:46:19.165419143 +0200
@@ -206,15 +206,16 @@
static struct pll_tab {
u32 clk_freq_khz;
u32 if_freq_khz;
- u8 m, n, p;
} pll_tab[] = {
- { TDA10048_CLK_4000, TDA10048_IF_36130, 10, 0, 0 },
- { TDA10048_CLK_16000, TDA10048_IF_3300, 10, 3, 0 },
- { TDA10048_CLK_16000, TDA10048_IF_3500, 10, 3, 0 },
- { 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_36130, 10, 3, 0 },
+ { TDA10048_CLK_4000, TDA10048_IF_36130 },
+ { TDA10048_CLK_16000, TDA10048_IF_3300 },
+ { TDA10048_CLK_16000, TDA10048_IF_3500 },
+ { TDA10048_CLK_16000, TDA10048_IF_3800 },
+ { TDA10048_CLK_16000, TDA10048_IF_4000 },
+ { TDA10048_CLK_16000, TDA10048_IF_4300 },
+ { TDA10048_CLK_16000, TDA10048_IF_4500 },
+ { TDA10048_CLK_16000, TDA10048_IF_5000 },
+ { TDA10048_CLK_16000, TDA10048_IF_36130 },
};
static int tda10048_writereg(struct tda10048_state *state, u8 reg, u8 data)
@@ -460,9 +461,6 @@
state->freq_if_hz = pll_tab[i].if_freq_khz * 1000;
state->xtal_hz = pll_tab[i].clk_freq_khz * 1000;
- state->pll_mfactor = pll_tab[i].m;
- state->pll_nfactor = pll_tab[i].n;
- state->pll_pfactor = pll_tab[i].p;
break;
}
}
@@ -781,6 +779,10 @@
dprintk(1, "%s()\n", __func__);
+ /* PLL */
+ init_tab[4].data = (u8)(state->pll_mfactor);
+ init_tab[5].data = (u8)(state->pll_nfactor) | 0x40;
+
/* Apply register defaults */
for (i = 0; i < ARRAY_SIZE(init_tab); i++)
tda10048_writereg(state, init_tab[i].reg, init_tab[i].data);
@@ -1123,7 +1125,7 @@
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
@@ -1135,6 +1137,17 @@
sizeof(struct dvb_frontend_ops));
state->frontend.demodulator_priv = state;
+ /* set pll */
+ if (config->set_pll) {
+ state->pll_mfactor = config->pll_m;
+ state->pll_nfactor = config->pll_n;
+ state->pll_pfactor = config->pll_p;
+ } else {
+ state->pll_mfactor = 10;
+ state->pll_nfactor = 3;
+ state->pll_pfactor = 0;
+ }
+
/* Establish any defaults the the user didn't pass */
tda10048_establish_defaults(&state->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-08-01 12:11:16.091956357 +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,13 @@
/* Disable I2C gate access */
u8 disable_gate_access;
+
+ bool no_firmware;
+
+ bool set_pll;
+ u8 pll_m;
+ u8 pll_p;
+ u8 pll_n;
};
#if defined(CONFIG_DVB_TDA10048) || \
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-08-02 19:21 ` Jose Alberto Reguero
@ 2011-08-08 10:35 ` Jose Alberto Reguero
2011-08-08 21:44 ` Antti Palosaari
0 siblings, 1 reply; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-08-08 10:35 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
[-- Attachment #1: Type: Text/Plain, Size: 3417 bytes --]
On Martes, 2 de Agosto de 2011 21:21:13 Jose Alberto Reguero escribió:
> On Jueves, 28 de Julio de 2011 21:25:01 Jose Alberto Reguero escribió:
> > On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
> > > On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> > > > Read without write work as with write. Attached updated patch.
> > > >
> > > > ttusb2-6.diff
> > > >
> > > > - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > > > + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> > > > + read = msg[i].flags& I2C_M_RD;
> > >
> > > ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing
> > > any random bytes in case of single read. Good!
> > >
> > > > + .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,
> > > >
> > > >
> > > > +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> > >
> > > cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
> > >
> > > > { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
> > > >
> > > > + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
> > > > + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
> > > >
> > > > + /* Set the pll registers */
> > > > + 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));
> > >
> > > This if only issue can have effect to functionality and I want double
> > > check. I see few things.
> > >
> > > 1) clock (and PLL) settings should be done generally only once at
> > > .init() and probably .sleep() in case of needed for sleep. Something
> > > like start clock in init, stop clock in sleep. It is usually very first
> > > thing to set before other. Now it is in wrong place - .set_frontend().
> > >
> > > 2) Those clock settings seem somehow weird. As you set different PLL M
> > > divider for 6 MHz bandwidth than others. Have you looked those are
> > > really correct? I suspect there could be some other Xtal than 16MHz and
> > > thus those are wrong. Which Xtals there is inside device used? There is
> > > most likely 3 Xtals, one for each chip. It is metal box nearest to
> > > chip.
> >
> > I left 6MHz like it was before in the driver. I try to do other way,
> > allowing to put different PLL in config that the default ones of the
> > driver and initialize it in init.
> >
> > Jose Alberto
>
> Attached new version of the patch. Adding tda827x lna and doing tda10048
> pll in other way.
>
> Jose Alberto
Another version, without tda827x lna. It don't improve anything.
Jose Alberto
>
> > > Ran checkpatch.pl also to find out style issues before send patch.
> > >
> > > I have send new version, hopefully final, of MFE. It changes array name
> > > from adap->mfe to adap-fe. You should also update that.
> > >
> > >
> > > 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
[-- Attachment #2: ttusb2-8.diff --]
[-- Type: text/x-patch, Size: 7617 bytes --]
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-08-01 05:45:24.000000000 +0200
+++ linux.new/drivers/media/dvb/dvb-usb/ttusb2.c 2011-08-08 12:22:59.624061045 +0200
@@ -30,6 +30,7 @@
#include "tda826x.h"
#include "tda10086.h"
#include "tda1002x.h"
+#include "tda10048.h"
#include "tda827x.h"
#include "lnbp21.h"
@@ -82,7 +83,7 @@
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
static u8 obuf[60], ibuf[60];
- int i,read;
+ int i, write_read, read;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
@@ -91,28 +92,35 @@
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);
+ write_read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
+ read = msg[i].flags & I2C_M_RD;
- obuf[0] = (msg[i].addr << 1) | read;
- obuf[1] = msg[i].len;
+ obuf[0] = (msg[i].addr << 1) | (write_read | read);
+ if (read)
+ obuf[1] = 0;
+ else
+ obuf[1] = msg[i].len;
/* read request */
- if (read)
+ if (write_read)
obuf[2] = msg[i+1].len;
+ else if (read)
+ 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);
+ if (write_read) {
+ memcpy(msg[i+1].buf, &ibuf[3], msg[i+1].len);
i++;
- }
+ } else if (read)
+ memcpy(msg[i].buf, &ibuf[3], msg[i].len);
}
mutex_unlock(&d->i2c_mutex);
@@ -190,6 +198,25 @@
.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,
+ .set_pll = true ,
+ .pll_m = 5,
+ .pll_n = 3,
+ .pll_p = 0,
+};
+
+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)
@@ -203,20 +230,56 @@
return 0;
}
+static int ttusb2_ct3650_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
+{
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+
+ return adap->fe[0]->ops.i2c_gate_ctrl(adap->fe[0], enable);
+}
+
static int ttusb2_frontend_tda10023_attach(struct dvb_usb_adapter *adap)
{
if (usb_set_interface(adap->dev->udev, 0, 3) < 0)
err("set interface to alts=3 failed");
- if ((adap->fe[0] = dvb_attach(tda10023_attach, &tda10023_config, &adap->dev->i2c_adap, 0x48)) == NULL) {
- deb_info("TDA10023 attach failed\n");
- return -ENODEV;
+
+ if (adap->fe[0] == NULL) {
+ /* FE 0 DVB-C */
+ adap->fe[0] = dvb_attach(tda10023_attach,
+ &tda10023_config, &adap->dev->i2c_adap, 0x48);
+
+ if (adap->fe[0] == NULL) {
+ deb_info("TDA10023 attach failed\n");
+ return -ENODEV;
+ }
+ } else {
+ adap->fe[1] = dvb_attach(tda10048_attach,
+ &tda10048_config, &adap->dev->i2c_adap);
+
+ if (adap->fe[1] == NULL) {
+ deb_info("TDA10048 attach failed\n");
+ return -ENODEV;
+ }
+
+ /* tuner is behind TDA10023 I2C-gate */
+ adap->fe[1]->ops.i2c_gate_ctrl = ttusb2_ct3650_i2c_gate_ctrl;
+
}
+
return 0;
}
static int ttusb2_tuner_tda827x_attach(struct dvb_usb_adapter *adap)
{
- if (dvb_attach(tda827x_attach, adap->fe[0], 0x61, &adap->dev->i2c_adap, NULL) == NULL) {
+ struct dvb_frontend *fe;
+
+ /* MFE: select correct FE to attach tuner since that's called twice */
+ if (adap->fe[1] == NULL)
+ fe = adap->fe[0];
+ else
+ fe = adap->fe[1];
+
+ /* attach tuner */
+ if (dvb_attach(tda827x_attach, fe, 0x61, &adap->dev->i2c_adap, &tda827x_config) == NULL) {
printk(KERN_ERR "%s: No tda827x found!\n", __func__);
return -ENODEV;
}
@@ -385,6 +448,7 @@
{
.streaming_ctrl = NULL,
+ .num_frontends = 2,
.frontend_attach = ttusb2_frontend_tda10023_attach,
.tuner_attach = ttusb2_tuner_tda827x_attach,
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-08-07 22:23:01.574897364 +0200
@@ -206,15 +206,16 @@
static struct pll_tab {
u32 clk_freq_khz;
u32 if_freq_khz;
- u8 m, n, p;
} pll_tab[] = {
- { TDA10048_CLK_4000, TDA10048_IF_36130, 10, 0, 0 },
- { TDA10048_CLK_16000, TDA10048_IF_3300, 10, 3, 0 },
- { TDA10048_CLK_16000, TDA10048_IF_3500, 10, 3, 0 },
- { 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_36130, 10, 3, 0 },
+ { TDA10048_CLK_4000, TDA10048_IF_36130 },
+ { TDA10048_CLK_16000, TDA10048_IF_3300 },
+ { TDA10048_CLK_16000, TDA10048_IF_3500 },
+ { TDA10048_CLK_16000, TDA10048_IF_3800 },
+ { TDA10048_CLK_16000, TDA10048_IF_4000 },
+ { TDA10048_CLK_16000, TDA10048_IF_4300 },
+ { TDA10048_CLK_16000, TDA10048_IF_4500 },
+ { TDA10048_CLK_16000, TDA10048_IF_5000 },
+ { TDA10048_CLK_16000, TDA10048_IF_36130 },
};
static int tda10048_writereg(struct tda10048_state *state, u8 reg, u8 data)
@@ -460,9 +461,6 @@
state->freq_if_hz = pll_tab[i].if_freq_khz * 1000;
state->xtal_hz = pll_tab[i].clk_freq_khz * 1000;
- state->pll_mfactor = pll_tab[i].m;
- state->pll_nfactor = pll_tab[i].n;
- state->pll_pfactor = pll_tab[i].p;
break;
}
}
@@ -781,6 +779,10 @@
dprintk(1, "%s()\n", __func__);
+ /* PLL */
+ init_tab[4].data = (u8)(state->pll_mfactor);
+ init_tab[5].data = (u8)(state->pll_nfactor) | 0x40;
+
/* Apply register defaults */
for (i = 0; i < ARRAY_SIZE(init_tab); i++)
tda10048_writereg(state, init_tab[i].reg, init_tab[i].data);
@@ -1123,7 +1125,7 @@
/* setup the state and clone the config */
memcpy(&state->config, config, sizeof(*config));
state->i2c = i2c;
- state->fwloaded = 0;
+ state->fwloaded = config->no_firmware;
state->bandwidth = BANDWIDTH_8_MHZ;
/* check if the demod is present */
@@ -1135,6 +1137,17 @@
sizeof(struct dvb_frontend_ops));
state->frontend.demodulator_priv = state;
+ /* set pll */
+ if (config->set_pll) {
+ state->pll_mfactor = config->pll_m;
+ state->pll_nfactor = config->pll_n;
+ state->pll_pfactor = config->pll_p;
+ } else {
+ state->pll_mfactor = 10;
+ state->pll_nfactor = 3;
+ state->pll_pfactor = 0;
+ }
+
/* Establish any defaults the the user didn't pass */
tda10048_establish_defaults(&state->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-08-01 12:11:16.091956357 +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,13 @@
/* Disable I2C gate access */
u8 disable_gate_access;
+
+ bool no_firmware;
+
+ bool set_pll;
+ u8 pll_m;
+ u8 pll_p;
+ u8 pll_n;
};
#if defined(CONFIG_DVB_TDA10048) || \
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-08-08 10:35 ` Jose Alberto Reguero
@ 2011-08-08 21:44 ` Antti Palosaari
2011-08-09 19:45 ` Jose Alberto Reguero
0 siblings, 1 reply; 28+ messages in thread
From: Antti Palosaari @ 2011-08-08 21:44 UTC (permalink / raw)
To: Jose Alberto Reguero
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
Reviewed-by: Antti Palosaari <crope@iki.fi>
It looks just fine.
regards
Antti
On 08/08/2011 01:35 PM, Jose Alberto Reguero wrote:
> On Martes, 2 de Agosto de 2011 21:21:13 Jose Alberto Reguero escribió:
>> On Jueves, 28 de Julio de 2011 21:25:01 Jose Alberto Reguero escribió:
>>> On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
>>>> On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
>>>>> Read without write work as with write. Attached updated patch.
>>>>>
>>>>> ttusb2-6.diff
>>>>>
>>>>> - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
>>>>> + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
>>>>> + read = msg[i].flags& I2C_M_RD;
>>>>
>>>> ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing
>>>> any random bytes in case of single read. Good!
>>>>
>>>>> + .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,
>>>>>
>>>>>
>>>>> +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
>>>>
>>>> cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
>>>>
>>>>> { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
>>>>>
>>>>> + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
>>>>> + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
>>>>>
>>>>> + /* Set the pll registers */
>>>>> + 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));
>>>>
>>>> This if only issue can have effect to functionality and I want double
>>>> check. I see few things.
>>>>
>>>> 1) clock (and PLL) settings should be done generally only once at
>>>> .init() and probably .sleep() in case of needed for sleep. Something
>>>> like start clock in init, stop clock in sleep. It is usually very first
>>>> thing to set before other. Now it is in wrong place - .set_frontend().
>>>>
>>>> 2) Those clock settings seem somehow weird. As you set different PLL M
>>>> divider for 6 MHz bandwidth than others. Have you looked those are
>>>> really correct? I suspect there could be some other Xtal than 16MHz and
>>>> thus those are wrong. Which Xtals there is inside device used? There is
>>>> most likely 3 Xtals, one for each chip. It is metal box nearest to
>>>> chip.
>>>
>>> I left 6MHz like it was before in the driver. I try to do other way,
>>> allowing to put different PLL in config that the default ones of the
>>> driver and initialize it in init.
>>>
>>> Jose Alberto
>>
>> Attached new version of the patch. Adding tda827x lna and doing tda10048
>> pll in other way.
>>
>> Jose Alberto
>
> Another version, without tda827x lna. It don't improve anything.
>
> Jose Alberto
>
>>
>>>> Ran checkpatch.pl also to find out style issues before send patch.
>>>>
>>>> I have send new version, hopefully final, of MFE. It changes array name
>>>> from adap->mfe to adap-fe. You should also update that.
>>>>
>>>>
>>>> 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
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] add support for the dvb-t part of CT-3650 v3
2011-08-08 21:44 ` Antti Palosaari
@ 2011-08-09 19:45 ` Jose Alberto Reguero
0 siblings, 0 replies; 28+ messages in thread
From: Jose Alberto Reguero @ 2011-08-09 19:45 UTC (permalink / raw)
To: Antti Palosaari
Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Guy Martin
On Lunes, 8 de Agosto de 2011 23:44:43 Antti Palosaari escribió:
> Reviewed-by: Antti Palosaari <crope@iki.fi>
>
> It looks just fine.
>
> regards
> Antti
>
Forgot the Signed-off-by
Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>
Jose Alberto
> On 08/08/2011 01:35 PM, Jose Alberto Reguero wrote:
> > On Martes, 2 de Agosto de 2011 21:21:13 Jose Alberto Reguero escribió:
> >> On Jueves, 28 de Julio de 2011 21:25:01 Jose Alberto Reguero escribió:
> >>> On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
> >>>> On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> >>>>> Read without write work as with write. Attached updated patch.
> >>>>>
> >>>>> ttusb2-6.diff
> >>>>>
> >>>>> - read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> >>>>> + write_read = i+1< num&& (msg[i+1].flags& I2C_M_RD);
> >>>>> + read = msg[i].flags& I2C_M_RD;
> >>>>
> >>>> ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing
> >>>> any random bytes in case of single read. Good!
> >>>>
> >>>>> + .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,
> >>>>>
> >>>>>
> >>>>> +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> >>>>
> >>>> cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
> >>>>
> >>>>> { TDA10048_CLK_16000, TDA10048_IF_4000, 10, 3, 0 },
> >>>>>
> >>>>> + { TDA10048_CLK_16000, TDA10048_IF_4500, 5, 3, 0 },
> >>>>> + { TDA10048_CLK_16000, TDA10048_IF_5000, 5, 3, 0 },
> >>>>>
> >>>>> + /* Set the pll registers */
> >>>>> + 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));
> >>>>
> >>>> This if only issue can have effect to functionality and I want double
> >>>> check. I see few things.
> >>>>
> >>>> 1) clock (and PLL) settings should be done generally only once at
> >>>> .init() and probably .sleep() in case of needed for sleep. Something
> >>>> like start clock in init, stop clock in sleep. It is usually very
> >>>> first thing to set before other. Now it is in wrong place -
> >>>> .set_frontend().
> >>>>
> >>>> 2) Those clock settings seem somehow weird. As you set different PLL M
> >>>> divider for 6 MHz bandwidth than others. Have you looked those are
> >>>> really correct? I suspect there could be some other Xtal than 16MHz
> >>>> and thus those are wrong. Which Xtals there is inside device used?
> >>>> There is most likely 3 Xtals, one for each chip. It is metal box
> >>>> nearest to chip.
> >>>
> >>> I left 6MHz like it was before in the driver. I try to do other way,
> >>> allowing to put different PLL in config that the default ones of the
> >>> driver and initialize it in init.
> >>>
> >>> Jose Alberto
> >>
> >> Attached new version of the patch. Adding tda827x lna and doing tda10048
> >> pll in other way.
> >>
> >> Jose Alberto
> >
> > Another version, without tda827x lna. It don't improve anything.
> >
> > Jose Alberto
> >
> >>>> Ran checkpatch.pl also to find out style issues before send patch.
> >>>>
> >>>> I have send new version, hopefully final, of MFE. It changes array
> >>>> name from adap->mfe to adap-fe. You should also update that.
> >>>>
> >>>>
> >>>> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-08-09 19:46 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <201106070205.08118.jareguero@telefonica.net>
[not found] ` <201107070057.06317.jareguero@telefonica.net>
2011-07-13 12:41 ` [PATCH] add support for the dvb-t part of CT-3650 v2 Mauro Carvalho Chehab
2011-07-14 20:00 ` [PATCH] add support for the dvb-t part of CT-3650 v3 Jose Alberto Reguero
2011-07-18 20:28 ` Antti Palosaari
2011-07-18 21:31 ` Michael Krufky
[not found] ` <201107190100.16802.jareguero@telefonica.net>
2011-07-18 23:44 ` Antti Palosaari
2011-07-19 8:25 ` Jose Alberto Reguero
2011-07-19 23:07 ` Antti Palosaari
2011-07-22 11:32 ` Antti Palosaari
2011-07-22 16:02 ` Jose Alberto Reguero
2011-07-22 16:08 ` Antti Palosaari
2011-07-22 16:25 ` Jose Alberto Reguero
2011-07-22 16:46 ` Antti Palosaari
[not found] ` <201107222012.20711.jareguero@telefonica.net>
2011-07-22 21:49 ` Jose Alberto Reguero
2011-07-22 22:23 ` Antti Palosaari
2011-07-23 8:26 ` Jose Alberto Reguero
2011-07-23 9:42 ` Antti Palosaari
2011-07-23 10:21 ` Jose Alberto Reguero
2011-07-23 10:37 ` Antti Palosaari
2011-07-23 15:41 ` Jose Alberto Reguero
2011-07-23 17:47 ` Antti Palosaari
2011-07-23 21:45 ` Jose Alberto Reguero
2011-07-27 19:22 ` Antti Palosaari
2011-07-28 19:25 ` Jose Alberto Reguero
2011-08-02 19:21 ` Jose Alberto Reguero
2011-08-08 10:35 ` Jose Alberto Reguero
2011-08-08 21:44 ` Antti Palosaari
2011-08-09 19:45 ` Jose Alberto Reguero
2011-07-16 11:38 ` [PATCH] improve recection with limits frecuenies and tda827x Jose Alberto Reguero
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).