On 4/19/07, Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote:
Em Qui, 2007-04-19 às 16:41 -0400, Michael Krufky escreveu: > Marco Gittler wrote: > > this patch has applied the hints from mkrufky (dvb_attach, > > firmware-naming) > > and also one working rewrite of the i2c addresses stuff to fit the > > kernel i2c reqs. > > > > Signed-off-by: Marco Gittler<g.marco@xxxxxxxxxx> > > diff -r c8b73ec18b42 linux/drivers/media/dvb/dvb-usb/opera1.c > > --- a/linux/drivers/media/dvb/dvb-usb/opera1.c Thu Apr 19 12:04:50 2007 -0300 > > +++ b/linux/drivers/media/dvb/dvb-usb/opera1.c Thu Apr 19 20:38:01 2007 +0200 > > @@ -25,6 +25,13 @@ > > #define REG_20_SYMBOLRATE_BYTE1 0x20 > > #define REG_21_SYMBOLRATE_BYTE2 0x21 > > > > +#define ADDR_C0_TUNER (0xc0>>1) > > +#define ADDR_D0_PLL (0xd0>>1) > > > I don't like these two #define's. These i2c addresses need only be > specified once, in the config structs / frontendfoo_attach calls for the > tuner / demod. > > Better to just put them in as constants like all of the other dvb drivers. I prefer the way it is. We should really avoid having magic numbers inside the code. The alias here helps to know that 0x60 is tuner addres and 0x68 the pll.
I agree the defines saved alot time when I did some modifications in several drivers a while ago, there is no reason at all to use hex values only (maybe instead obfuscating NDA code) I also converted the em28xx to use such defines which try to describe the registers, it's much easier to look up certain register functions now. The reason that this would be against codingstyle vanishs if someone thinks about having to look up every register in the specs all the time. Markus _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
