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