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

[hermann pitton <hermann-pitton@arcor.de>]

Am Freitag, den 20.04.2007, 03:42 +0400 schrieb Manu Abraham:
> hermann pitton wrote:
> > Am Freitag, den 20.04.2007, 03:19 +0400 schrieb Manu Abraham:
> >> 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
> > 
> > Forget it.
> > 
> > That is as wrong as older Fedora distros were shipping v4l2 apps like
> > tvtime, but only providing v4l1 headers on the user level.
> > 
> 
> I don't know about Fedora shipping v4l2 apps.  Forgive my ignorance. But
> it is really hopeless to include a private header for a device into
> another device. Anyway not talking about V4L1/2/n headers, but about DVB
> device (demod/tuner) private headers being included publicly. Private
> means private, i don't understand how the notion comes around that a
> private header is a public header.
> 
> It is _not_ named private for no reason.
> 

The GPL was also not named GPL for no reason.

HTH,
Hermann