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

[Manu Abraham <abraham.manu@gmail.com>]

hermann pitton wrote:
> Am Freitag, den 20.04.2007, 02:51 +0400 schrieb Manu Abraham:
>> Markus Rechberger wrote:
>>> On 4/20/07, Manu Abraham <abraham.manu@gmail.com> wrote:
>>>> hermann pitton wrote:
>>>>> Am Freitag, den 20.04.2007, 00:55 +0400 schrieb Manu Abraham:
>>>>>> 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@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.
>>>>>> Following a project's coding styles and conventions is "respecting" a
>>>>>> project
>>>>>>
>>>>>> Manu
>>>>>>
>>>>> Hi,
>>>>>
>>>>> the other natural place for this should be the LKML to get more _good_
>>>>> arguments, instead of hanging soon in some "respect" stuff again.
>>>>
>>>> DVB drivers generally have device addresses such as tuner_addresses and
>>>> demod_adresses defined in a config struct least to prevent them from
>>>> being global, wherever the header is included, since the very same
>>>> device can have multiple addresses and so on, which are non-probable
>>>> since being behind a repeater which is switched by a demod (private) and
>>>> hence.
>>>>
>>>> Those are some of the reasons to follow a certain coding
>>>> style/conventions. They are _not_ for fun.
>>>>
>>> cat *priv.h says something else too...
>>> there are also many global register defines in DVB drivers, they just
>>> don't include the register value in the define name.
>>
>> *_priv.h from what i understand means private .. i don't know what you
>> make out from that.
>>
>>
>> HTH,
>> Manu
> 
> ;)
> 
> That means that I had to post the actual headers to every single tester

If you use a private header as a public header, of course yes. But that
is not what private explicitly means.
It _is_ indeed wrong to use a private header as a public header _even_
for workarounds.

HTH,
Manu
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/