[linux-dvb] [PATCH] Moving ALPS BSRV2 tuner handling code to
o.endriss at gmx.de
Sat Apr 15 19:53:45 CEST 2006
Michael Krufky wrote:
> Oliver Endriss wrote:
> >Michael Krufky wrote:
> >>Andreas Oberritter wrote:
> >>>Michael Krufky wrote:
> >>>>I think that this was a good idea, although the same exact thing could
> >>>>be done for all of the other drivers ...
> >>>I like this patch and I think it should be applied. It is a disadvantage
> >>>to have all the code and arrays duplicated in several drivers if we
> >>>could have it at a single place.
> >>>The code can still be changed to use dvb-pll afterwards without
> >>>unnecessary code duplication.
> >>I can agree to that. The same can be done for lg-h06xf, and many
> >>others. A lot of duplicated code could be removed, and I do agree that
> >>this would be a step forward. The only problem I see with this is that
> >>we'll end up with many tiny little header files just like this one,
> >>bsbe1.h and bsru6.h ... This isn't necessarily a bad thing either. I
> >>just didn't know if this is what we wanted to be doing.
> >Well, when we started with bsbe1.h and bsru6.h the patches were posted
> >on this mailing list. Nobody complained...
> Nobody complained because there is nothing wrong with it. (except for
> one flaw -- see down below) I just had a few ideas I thought we could
> talk about. Feel free to disagree with me -- IMHO, discussions like
> this are good.
> >>Would it make sense to consolidate these small files into single source.[ch] files?
> >>What do you think?
> >I don't like big files which contain lots of definitions which are not
> >needed by most drivers. That's why I'm not very happy with dvb_pll.c.
> OK. I agree to keep these separate, but dvb-pll is a good thing, and I
> wouldn't want to see that split apart. When it comes to pll
> programming, it makes sense to store the ranges in an array like we do
> in dvb-pll, and to have a single function like dvb_pll_configure to
> intepret it. I would hate to see that go away.
Well, there is no reason to change that. I can live with it.
> >When editing a file there is always a small chance to introduce a random
> >bug somewhere in the file. In theory we have to re-test a driver
> >whenever we change a file which the driver depends on...
> This might be taking the anti- dvb-pll argument a bit too far... If I
> add a new pll description to dvb-pll, the diff will clearly show that no
> other descriptions were touched. This should not be a concern.
In theory you are right, but Murphy's law applies, too. ;-)
Btw, it was not meant to be an anti- dvb-pll argument.
> >Small files will only affect those drivers which really need them.
> >I vote for one header file per tuner. It's both easier to maintain and
> >easier to understand for newbies. If you prefer we can move these files
> >to a subdirectory to keep them separate from the frontend drivers.
> >Later these definitions can be converted to use dvb_pll or whatever.
> >For now they save us hundreds of lines of duplicated code.
> >Imho this process should be continued step by step for all duplicated
> >tuner descriptions.
> This sounds good to me. I don't know if we need to actually move them
> into a different directory, or if a filename prefix would suffice (see
> my response to Andreas below). It seems to me that the decision has
> already been made to use a filename prefix -- we should rename / move
> those files sooner than later.
> Andreas Oberritter wrote:
> >I like small independent files.
> Agreed. I retract my comment about consolidating them -- independent
> files are most appropriate.
> >Btw. such a change has been proposed by me last summer and Johannes
> >suggested using a common prefix like "fe-" although I'd vote for using
> Yes, now I remember... I second that vote. I would also prefer the
> prefix "nim_".
Please choose a prefix which can be understood by someone who is not a
hardcore dvb developer. Virtuelly nobody knows what a 'nim' is. :-(
> >My old patch is still available but moved to a new URL.
> My last email was purely inquisitive. Now that you guys have responded,
> I'd like to make another point:
> If we are putting these commonly used fuctions into nim-specific header
> files, we STILL end up with duplicated binary code. Since all of the
> code is now in these small header files, the c code is no longer
> duplicated, however, these functions are still being compiled staticly
> into each driver that uses them.
Correct, see below.
> I propose the following:
> We should apply Perceval's patch, but instead of creating "bsrv2.h" ,
> that we instead call it, "nim_bsrv2.c", and then we should create a
> matching "nim_bsrv2.h" header, containing the following prototypes:
> int alps_bsrv2_pll_set(struct dvb_frontend* fe, struct i2c_adapter* i2c,
> struct dvb_frontend_parameters* params);
> struct ves1x93_config alps_bsrv2_config;
> ... Of course, the same should be applied to bsbe1.h and bsru6.h , and
> all other newly-created nim modules.
> This way, instead of the same code being statically compiled into each
> driver, we could compile them once as a module, and have that code
> reused by the other drivers.
> IMHO, this would be the best and most portable solution, truly resulting
> in the removal of such duplicated code in all senses.
> If we can all agree to this, then I will move forward and create such a
> module for the lgh06xf.
Well, I did it this way because I wanted to make the code easier to
maintain while avoiding the module overhead (Kconfig modification and
testing, module load/unload, module init/exit, usage counters etc).
The size of a tuner description is just a few bytes, and it will be even
smaller if it would be converted to use dvb_pll in the future...
Imho it is not worth the trouble...
VDR Remote Plugin available at
More information about the linux-dvb