On Thursday 06 July 2006 12:37, Trent Piepho wrote: > On Thu, 6 Jul 2006, Andrew de Quincey wrote: > > On Thursday 06 July 2006 01:57, Trent Piepho wrote: > > > Or, just change the one function in dvb-bt8xx.c that calls > > > cx24110_pll_write() to use fe->ops.write() directly and get rid of the > > > whole cx24110_pll_write() function. > > I looked into this a bit more, and the only card driver that uses cx24110 > is dvb-bt8xx. cx24110_pll_write() is only called from one function, > cx24108_tuner_set_params() which is in dvb-bt8xx.c. > > It seems like it would make the most sense to move that function from > dvb-bt8xx.c to cx24110.c. It looks like it's programming the cx24110 chip > and any card using a cx24110 would need it, so conceptually it belongs with > the cx24110 code and not with the bt8xx code. It's also more efficient, > since the bt8xx driver won't include that code for cards that use different > frontends. Yeah; conexant demods always seem to be in matched pairs with conexant PLLs... the cx24123 is like that too. So it probably makes sense for that demod. I'm gonna leave that as it is for the moment though: This sort of change will be part of the tuner refactoring stage 2, which I will be starting once we sort out the DVB attach stuff. > I haven't looked into it, but maybe the three other ops.write() users > aren't really necessary in the same manner. The tda1004x needs them for sure - the demodulator has some GPIOs on it that some boards need to tweak external SAW filtering hardware. This could be moved into the config structure though. The stv0299 needs it as well as some boards need different settings for the symbol rate stuff. Although I've been wondering recently if it would be possible to make that piece of code internal to the demod, or dependant on some stuff in the config structure. Changing the stv0299 symbol rate stuff is difficult though; so many boards use that demod in different configurations. > > > In the Kconfig patch, I'd get rid of the extra checks for the kernel > > > version. The requirement of kernel 2.6.17 in versions.txt for > > > DVB_CORE_ATTACH will take of everything. What's the point of > > > versions.txt if you're just going to stick a version check in the code > > > anyway? > > > > I thought about that, but remembered someone could do "make kernel-links" > > against a pre-2.6.17 kernel - and that will bypass versions.txt - so I > > left the version check in. > > That's a good point, I hadn't thought of that. Still, there are plenty of > drivers which will crash on older kernels and they don't have version > checks. yeah you're quite right :) > > > Only allowing FE customization when DVB_CORE_ATTACH is on seems > > > backward. Customizing which frontends are built makes the most sense > > > when you still have static dependencies or are not even using modules. > > > There is no reason you can't customize both with and without > > > dvb_attach, so why disable it? > > > > For static building, would that not require spreading #ifdefs everywhere > > throughout the code? I really don't want to do that. > > For the drivers which support the largest number of frontends, the #ifdefs > are already there, so it's more a matter of not removing them rather than > adding new ones. Actually, most of them were there before customizing > front-ends was possible, for some other reason that probably no longer > exists. > > All the #ifdefs around the front-end header files aren't needed for > front-end customization. I don't know why they're there. > > Your idea to put empty static inline versions of the attach functions in > the front-end header files when that front-end is turned off would get rid > of the rest of the #ifdefs. It's slightly less efficient since the drivers > would include front-end specific code that won't be used and would have > been #ifdefed out. Some functionality could be added though, the empty > attach functions could printk "Front-end foo support was disabled at > compile time" or something like that. > > Will dvb_attach() work on a static inline non-EXPORT_SYMBOLed function? I will check that... most likely not, but I can maybe come up with a workaround. If its not too nasty that is. As for the ifdefs - one of the reasons for starting the attach stuff was to remove the need for ifdefs but also allow users to configure demods. I don't really like having code littered with ifdefs because it is hard to read. LKML policy says the same. I know it will increase the size of those drivers temporarily, but tuner-refactoring stage 2 will sort that. Also, the size increase will be quite small. _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
