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@xxxxxxxxx> 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@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. >>>>>>>> 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. Manu _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
