[linux-dvb] Re: [video4linux-cvs] [hg:v4l-dvb] Add support for Opera S1- DVB-USB

Mauro Carvalho Chehab mchehab at infradead.org
Fri Apr 20 11:35:31 CEST 2007


Argh! Too much flood for two simple defines!

IMO, we should really avoid all those magic numbers at this driver. Ok,
we don't have docs for opera1, but, as soon as we discover the reasons
for each magic, we should use, instead, an alias. It is not clear at all
what each magic number do inside this driver. I sincerely doubt that any
one that argued against documenting the magic can understand what all
those binary code magic inside the driver does, without a datasheet. We
really should avoid this.

At least with those few defines, I have a hint for regs 0x1f to 0x21,
and i2c regs 0xc0 and 0xd0 (8-bit notation).

However, the final decision of proper documenting those magic belongs to
the driver author, since he is the one that knows more about the magic
numbers.

Just my two cents.

Cheers,
Mauro.

> >>>  #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 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.
> >   
> The problem, Mauro, is that the driver currently supports one device. 
> When more devices need to use this driver, with different tuners and
> different demods using different i2c addresses, these #defines will no
> longer be practical.
> 
> DVB has a codingstyle that is different than your taste, Mauro.  You
> need to accept that some things cant be perfectly the way that you would
> like.
> 
> If you look through the other dvb-usb drivers, you'll notice that there
> are various tuner_attach and frontend_attach functions that are
> sometimes used by more than one dvb_usb_device_properties struct.  The
> i2c addresses should be specified in the frontendfoo_config structs,
> since that is how is done for all dvb drivers.  It is very important
> that DVB codingstyle remain intact, otherwise, having code that doesn't
> conform to the other drivers becomes a maintenance problem.
> 
> If you want this to be changed in _this_ driver, then _all_ drivers
> should be changed.  DO NOT change _all_ drivers -- this is an
> established CodingStyle, and it should be respected.  It is not
> appropriate for you to make decisions with regards to changing the
> established CodingStyle of the DVB subsystem.
> 
> >> Regards,
> >>
> >> Mike Krufky
-- 
Cheers,
Mauro




More information about the linux-dvb mailing list