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.
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.
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
"nim_".
Yes, now I remember... I second that vote. I would also prefer the
prefix "nim_".
http://thread.gmane.org/gmane.linux.drivers.dvb/19261/focus=19261
My old patch is still available but moved to a new URL.
http://www.saftware.de/patches/frontend.diff
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.
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.
Comments?
Cheers,
Michael Krufky
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
