[linux-dvb] [PATCH] Add support for LGDT3303 VSB/QAM Frontend (LG 5th Gen Chipset)

Andreas Oberritter obi at linuxtv.org
Sat Jul 9 18:33:55 CEST 2005


Hi,

On Sat, 2005-07-09 at 11:38 -0400, Taylor Jacob wrote:
> This patch adds support for the LGDT3303 VSB/QAM chipset.  I used the same SNR
> code as the LGDT3302 chipset that was recently added to cvs.  Any questions
> please feel free to ask.  This will add support for the BBTI Air2PC card due to
> ship very soon that utilizes this chipset.

Sending the Files inline would have made commenting easier. Some
"standard" comments follow below:

> /*
>     Support for BBTI Technisat Air2PC - ATSC - 3rd Gen 
> 
>     Copyright (C) 2005 Taylor Jacob <rtjacob at earthlink.net>
> 
>     This program is free software; you can redistribute it and/or modify
>     it under the terms of the GNU General Public License as published by
>     the Free Software Foundation; either version 2 of the License, or
>     (at your option) any later version.
> 
>     This program is distributed in the hope that it will be useful,
>     but WITHOUT ANY WARRANTY; without even the implied warranty of
>     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>     GNU General Public License for more details.
> 
>     You should have received a copy of the GNU General Public License
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> 
> */
> 
> #include <linux/init.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <asm/div64.h>
> 
> #include "dvb_frontend.h"
> #include "lgdt3303.h"
> 
> struct lgdt3303_state {
> 
> 	struct i2c_adapter* i2c;
> 	struct dvb_frontend_ops ops;
> 	const struct lgdt3303_config* config;
> 	struct dvb_frontend frontend;
> 	u8 current_modulation;
> 
> 	/* demodulator private data */
> 	u8 initialised:1;

Don't use bit fields if not necessary.

> };
> 
> static int debug;
> #define dprintk(args...) \
> 	do { \
> 		if (debug) printk(KERN_DEBUG "lgdt3303: " args); \
> 	} while (0)
> 
> static int i2c_writebytes (struct lgdt3303_state* state, u8 reg, u8 *buf, u8 len)
> {
> 	/* probbably a much better way or doing this */
> 	u8 buf2 [256],x;

Should probably be "u8 buf2[256], x;".

> 	int err;
> 	struct i2c_msg msg = { .addr = state->config->demod_address, .flags = 0, .buf = buf2, .len = len + 1 };
> 
> 	buf2[0] = reg;
> 	for (x = 0 ; x < len ; x++)
> 		buf2[x+1] = buf[x];

Use memcpy().

> 
> 	if ((err = i2c_transfer (state->i2c, &msg, 1)) != 1) {
> 		printk ("%s: i2c write error (addr %02x, err == %i)\n",
> 			__FUNCTION__, state->config->demod_address, err);
> 		return -EREMOTEIO;

"err" should be returned if it is negative.

> 	}
> 
> 	return 0;
> }
> 
> static int i2c_tunerwritebytes (struct lgdt3303_state* state, u8 *buf, u8 len)
> {
> 	/* probbably a much better way or doing this */
> 	int err;
> 	struct i2c_msg msg = { .addr = state->config->tuner_address, .flags = 0, .buf = buf, .len = len };
> 
> 	if ((err = i2c_transfer (state->i2c, &msg, 1)) != 1) {
> 		printk ("%s: i2c write error (addr %02x, err == %i)\n",
> 			__FUNCTION__, state->config->demod_address, err);
> 		return -EREMOTEIO;

same as above

> 	}
> 
> 	return 0;
> }
> 
> static u8 i2c_readbytes (struct lgdt3303_state* state, u8 reg, u8* buf, u8 len)
> {
> 	u8 reg2 [] = { reg };
> 
> 	struct i2c_msg msg [] = { { .addr = state->config->demod_address, .flags = 0, .buf = reg2, .len = 1 },
> 			{ .addr = state->config->demod_address, .flags = I2C_M_RD, .buf = buf, .len = len } };
> 
> 	int err;
> 
> 	if ((err = i2c_transfer (state->i2c, msg, 2)) != 2) {
> 		printk ("%s: i2c read error (addr %02x, err == %i)\n",
> 			__FUNCTION__, state->config->demod_address, err);
> 		return -EREMOTEIO;

again

> 	}
> 
> 	return 0;
> }
> 
> static void lgdt3303_soft_reset(struct lgdt3303_state* state)
> {
> 	u8 buf;
> 	dprintk("%s\n", __FUNCTION__);
> 
> 	buf = 0x00;
> 	i2c_writebytes(state,0x02,&buf,1);

Please use some white space to separate parameters, also in many other places.

> 
> 	buf = 0x01;
> 	i2c_writebytes(state,0x02,&buf,1);
> 
> 	return;

No need for that return.

> }
> 
> static int lgdt3303_setup_frontend_parameters (struct dvb_frontend* fe,
> 					     struct dvb_frontend_parameters *p)

Your code mixes foo* bar and foo *bar style. The latter is more common in kernel
code. Please try to keep it consistent.

> {
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	u32 freq = 0;
> 	u16 tunerfreq = 0;
> 	u8 buf[6];
> 
> 	freq = 44000000 + p->frequency;
> 
> 
> 	tunerfreq = freq / 62500;
> 
> 	buf[0] = (tunerfreq >> 8) & 0x7F;
> 	buf[1] = (tunerfreq & 0xFF);
> 	buf[2] = 0x86;
> 
> 	if (p->frequency < 160000000) {
> 		buf[3] = 0x01;
> 	} else if (p->frequency > 445000000) {
> 		buf[3] = 0x04;
> 	} else {
> 		buf[3] = 0x02;
> 	}
> 
> 	/* Cut and paste from BBTI Windows driver Important? */
> 	buf[4] = buf[2] | 0x18;
> 	buf[5] = 0x50;
> 
> 	i2c_tunerwritebytes(state,buf,4);
> 
> 	i2c_tunerwritebytes(state,&buf[4],2);
> 
> 	/* invert clock */
> 	buf[0] = 0xF3;
> 	i2c_writebytes(state,0x87,buf,1);
> 
> 	state->current_modulation = p->u.vsb.modulation;
> 
> 	switch (p->u.vsb.modulation)
> 	{
> 		case VSB_8:
> 				buf[0] = 0x03;
> 				i2c_writebytes(state,0x00,buf,1);

One level of indentation could probably be removed.

> 				buf[0] = 0x40;
> 				i2c_writebytes(state,0x0d,buf,1);
> 				buf[0] = 0x87;
> 				i2c_writebytes(state,0x0e,buf,1);
> 				buf[0] = 0x8e;
> 				i2c_writebytes(state,0x0f,buf,1);
> 				buf[0] = 0x01;
> 				i2c_writebytes(state,0x10,buf,1);
> 				buf[0] = 0x88;
> 				i2c_writebytes(state,0x47,buf,1);
> 				buf[0] = 0x14;
> 				i2c_writebytes(state,0x4c,buf,1);
> 				break;
> 
> 		case QAM_64:
> 				buf[0] = 0x00;
> 				i2c_writebytes(state,0x00,buf,1);
> 				i2c_writebytes(state,0x0d,buf,1);
> 				i2c_writebytes(state,0x0e,buf,1);
> 				i2c_writebytes(state,0x0f,buf,1);
> 				i2c_writebytes(state,0x10,buf,1);
> 				buf[0] = 0x63;
> 				i2c_writebytes(state,0x51,buf,1);
> 				buf[0] = 0x66;
> 				i2c_writebytes(state,0x47,buf,1);
> 				buf[0] = 0x66;
> 				i2c_writebytes(state,0x48,buf,1);
> 				buf[0] = 0x1a;
> 				i2c_writebytes(state,0x4d,buf,1);
> 				buf[0] = 0x14;
> 				i2c_writebytes(state,0x4c,buf,1);
> 				buf[0] = 0x08;
> 				i2c_writebytes(state,0x49,buf,1);
> 				buf[0] = 0x9b;
> 				i2c_writebytes(state,0x4a,buf,1);
> 				break;
> 
> 		case QAM_256:
> 				buf[0] = 0x01;
> 				i2c_writebytes(state,0x00,buf,1);
> 				buf[0] = 0x00;
> 				i2c_writebytes(state,0x0d,buf,1);
> 				i2c_writebytes(state,0x0e,buf,1);
> 				i2c_writebytes(state,0x0f,buf,1);
> 				i2c_writebytes(state,0x10,buf,1);
> 				buf[0] = 0x63;
> 				i2c_writebytes(state,0x51,buf,1);
> 				buf[0] = 0x66;
> 				i2c_writebytes(state,0x47,buf,1);
> 				buf[0] = 0x66;
> 				i2c_writebytes(state,0x48,buf,1);
> 				buf[0] = 0x1a;
> 				i2c_writebytes(state,0x4d,buf,1);
> 				buf[0] = 0x14;
> 				i2c_writebytes(state,0x4c,buf,1);
> 				buf[0] = 0x08;
> 				i2c_writebytes(state,0x49,buf,1);
> 				buf[0] = 0x9b;
> 				i2c_writebytes(state,0x4a,buf,1);
> 				break;
> 		default:
> 				break;
> 	}
> 
> 	lgdt3303_soft_reset(state);
> 
> 	return 0;
> }
> 
> static int lgdt3303_read_status(struct dvb_frontend* fe, fe_status_t* status)
> {
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	u8 lock;
> 	*status = 0;
> 
> 	i2c_readbytes(state,0x1C,&lock,1);
> 	if (lock & 0x80) {
> 		*status |= FE_HAS_SIGNAL;
> 		*status |= FE_HAS_CARRIER;
> 	}
> 
> 	i2c_readbytes(state,0x58,&lock,1);
> 	if (lock & 0x01) {
> 		*status |= FE_HAS_VITERBI;
> 	}
> 
> 	if (lock & 0x02) {
> 		*status |= FE_HAS_SYNC;
> 		*status |= FE_HAS_LOCK;
> 	}
> 	return 0;
> }
> 
> static int lgdt3303_read_ber(struct dvb_frontend* fe, u32* ber)
> {
> 
> 	/* Not implimented in frontend */

implemented

> 	*ber = 0;
> 
> 	return 0;
> }
> 
> static int lgdt3303_read_signal_strength(struct dvb_frontend* fe, u16* strength)
> {
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	u8 b[] = {0,0,0};

Only two bytes are needed.

> 	u16 temp = 0;
> 
> 	i2c_readbytes(state,0x52,&b[0],1);
> 	i2c_readbytes(state,0x54,&b[1],1);
> 
> 	/* This math was given from example from the windows bbti driver */
> 	temp = 1700 - ((b[0] & 0x07) << 8 | b[1]);
> 
> 	*strength = temp * (0xFFFF/1700);
> 
> 	return 0;
> }
> 
> static int lgdt3303_read_snr(struct dvb_frontend* fe, u16* snr)
> {
> 
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	u8 b[] = {0,0,0};
> 	u32 noise = 0;
> 
> 	/* This code cut and pasted from the LGDT3302.C driver and ought
> 	 * to be put in a common place at some point */
> 
> 	/*
> 	 * Spec sheet shows formula for SNR_EQ = 10 log10(25 * 24**2 / noise)
> 	 * and SNR_PH = 10 log10(25 * 32**2 / noise) for equalizer and phase tracker
> 	 * respectively. The following tables are built on these formulas.
> 	 * The usual definition is SNR = 20 log10(signal/noise)
> 	 * If the specification is wrong the value retuned is 1/2 the actual SNR in db.
> 	 *
> 	 * This table is a an ordered list of noise values computed by the
> 	 * formula from the spec sheet such that the index into the table
> 	 * starting at 43 or 45 is the SNR value in db. There are duplicate noise
> 	 * value entries at the beginning because the SNR varies more than
> 	 * 1 db for a change of 1 digit in noise at very small values of noise.
> 	 *
> 	 * Examples from SNR_EQ table:
> 	 * noise SNR
> 	 *   0    43
> 	 *   1    42
> 	 *   2    39
> 	 *   3    37
> 	 *   4    36
> 	 *   5    35
> 	 *   6    34
> 	 *   7    33
> 	 *   8    33
> 	 *   9    32
> 	 *   10   32
> 	 *   11   31
> 	 *   12   31
> 	 *   13   30
> 	 */
> 
> 	static const u32 SNR_EQ[] =
> 		{ 1,     2,      2,      2, 3,      3,      4,     4,     5,     7,
> 		  9,     11,     13,     17, 21,     26,     33,    41,    52,    65,
> 		  81,    102,    129,    162, 204,    257,    323,   406,   511,   644,
> 		  810,   1020,   1284,   1616, 2035,   2561,   3224,  4059,  5110,  6433,
> 		  8098,  10195,  12835,  16158, 20341,  25608,  32238, 40585, 51094, 64323,
> 		  80978, 101945, 128341, 161571, 203406, 256073, 0x40000
> 		};
> 
> 	static const u32 SNR_PH[] =
> 		{ 1,     2,      2,      2,      3,      3,     4,     5,     6,     8,
> 		  10,    12,     15,     19,     23,     29, 37,    46,    58,    73,
> 		  91,    115,    144,    182,    229,    288, 362,   456,   574,   722,
> 		  909,   1144,   1440,   1813,   2282,   2873, 3617,  4553,  5732,  7216,
> 		  9084,  11436,  14396,  18124,  22817,  28724,  36161, 45524, 57312, 72151,
> 		  90833, 114351, 143960, 181235, 228161, 0x040000
> 		};
> 
> 	static u32 snr_db;  /* index into SNR_EQ[] */
> 
> 	if (state->current_modulation == VSB_8) {
> 
> 		/* Equalizer Mean-Square Error Register for VSB */
> 
> 		i2c_readbytes(state,0x6E,&b[0],1);
> 		i2c_readbytes(state,0x71,&b[1],1);
> 		i2c_readbytes(state,0x72,&b[2],1);
> 	
> 		noise = ((b[0] & 0x07) << 16) | (b[1] << 8) | b[2];
> 
> 		/*
> 		 * Look up noise value in table.
> 		 * A better search algorithm could be used...
> 		 * watch out there are duplicate entries.
> 		 */
> 		for (snr_db = 0; snr_db < sizeof(SNR_EQ); snr_db++) {
> 			if (noise < SNR_EQ[snr_db]) {
> 				*snr = (43 - snr_db) * (0xFFFF/43);
> 				
> 				break;
> 			}
> 		}
> 	} else {
> 		/* Phase Tracker Mean-Square Error Register for QAM */
> 
> 		i2c_readbytes(state,0x6F,&b[0],1);
> 		i2c_readbytes(state,0x70,&b[1],1);
> 		i2c_readbytes(state,0x71,&b[2],1);
> 		noise = ((b[0] & 7<<3) << 13) | (b[1] << 8) | b[2];
> 
> 		/* Look up noise value in table. */
> 		for (snr_db = 0; snr_db < sizeof(SNR_PH); snr_db++) {
> 			if (noise < SNR_PH[snr_db]) {
> 				*snr = (45 - snr_db) * (0xFFFF/45);
> 				break;
> 			}
> 		}
> 	}
> 
> 	return 0;
> }
> 
> static int lgdt3303_read_ucblocks(struct dvb_frontend* fe, u32* ucblocks)
> {
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	u8 b[] = {0,0,0};

Same as above.

> 
> 	i2c_readbytes(state,0x8B,&b[0],1);
> 	i2c_readbytes(state,0x8C,&b[1],1);
> 
> 	*ucblocks = (b[0] << 8) | b[1];
> 
> 	return 0;
> }
> 
> static int lgdt3303_sleep(struct dvb_frontend* fe)
> {
> 	return 0;
> }

Are empty functions needed?

> 
> static int lgdt3303_init(struct dvb_frontend* fe)
> {
> 	return 0;
> }
> 
> static int lgdt3303_get_tune_settings(struct dvb_frontend* fe, struct dvb_frontend_tune_settings* fesettings)
> {
> 	fesettings->min_delay_ms = 500;
> 	fesettings->step_size = 0;
> 	fesettings->max_drift = 0;
> 	return 0;
> }
> 
> static void lgdt3303_release(struct dvb_frontend* fe)
> {
> 	struct lgdt3303_state* state = fe->demodulator_priv;
> 	kfree(state);
> }
> 
> static struct dvb_frontend_ops lgdt3303_ops;
> 
> struct dvb_frontend* lgdt3303_attach(const struct lgdt3303_config* config,
> 				   struct i2c_adapter* i2c)
> {
> 	struct lgdt3303_state* state = NULL;
> 	u8 buf = 0;
> 
> 	/* allocate memory for the internal state */
> 	state = kmalloc(sizeof(struct lgdt3303_state), GFP_KERNEL);
> 	if (state == NULL) goto error;

if (state == NULL)
	return NULL;

> 
> 	/* setup the state */
> 	state->config = config;
> 	state->i2c = i2c;
> 	state->current_modulation = VSB_8;
> 	memcpy(&state->ops, &lgdt3303_ops, sizeof(struct dvb_frontend_ops));
> 
> 	i2c_readbytes(state, 0x85, &buf, 1);
> 	if (buf != 0x20) goto error;	

Please don't put the "goto" onto the same line as the "if".

> 
> 	i2c_readbytes(state, 0x86, &buf, 1);
> 	if (buf != 0x40) goto error;		
> 
> 	/* create dvb_frontend */
> 	state->frontend.ops = &state->ops;
> 	state->frontend.demodulator_priv = state;
> 	return &state->frontend;
> 
> error:
> 	kfree(state);
> 	return NULL;
> }
> 
> static struct dvb_frontend_ops lgdt3303_ops = {
> 
> 	.info = {
> 		.name = "LGDT3303 VSB/QAM frontend",
> 		.type = FE_ATSC,
> 		.frequency_min =  54000000,
> 		.frequency_max = 860000000,
>                 /* stepsize is just a guess */
> 		.frequency_stepsize = 166666,
> 		.caps = FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 | FE_CAN_FEC_3_4 |
> 			FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
> 			FE_CAN_8VSB | FE_CAN_QAM_64 | FE_CAN_QAM_256
> 	},
> 
> 	.release = lgdt3303_release,
> 
> 	.init = lgdt3303_init,
> 	.sleep = lgdt3303_sleep,

Empty function pointers should probably not be set. I don't know how dvb-core
handles that.

> 
> 	.set_frontend = lgdt3303_setup_frontend_parameters,
> 	.get_tune_settings = lgdt3303_get_tune_settings,
> 
> 	.read_status = lgdt3303_read_status,
> 	.read_ber = lgdt3303_read_ber,
> 	.read_signal_strength = lgdt3303_read_signal_strength,
> 	.read_snr = lgdt3303_read_snr,
> 	.read_ucblocks = lgdt3303_read_ucblocks,
> 
> };
> 
> module_param(debug, int, 0644);
> MODULE_PARM_DESC(debug, "Turn on/off frontend debugging (default:off).");
> 
> MODULE_DESCRIPTION("LGDT3303 ATSC (8VSB & ITU J83 AnnexB FEC QAM64/256) demodulator driver");
> MODULE_AUTHOR("Taylor Jacob");
> MODULE_LICENSE("GPL");
> 
> EXPORT_SYMBOL(lgdt3303_attach);

Regards,
Andreas





More information about the linux-dvb mailing list