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

Michael Krufky mkrufky at linuxtv.org
Thu Apr 19 22:59:38 CEST 2007


Mauro Carvalho Chehab 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 at freenet.de>
>>> 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.
>   
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
-- 
Michael Krufky





More information about the linux-dvb mailing list