On 5/15/07, Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote:
Hi Michael and others,
> I know we've been over this, but I need to state my feelings on the
> matter, otherwise I would have to simply keep quiet, and that doesn't
> help the situation for anybody. I _strongly_ feel that the core changes
> being proposed here will hurt the integrity of the dvb subsystem.
I don't think they will hurt the integrity of the subsystem, but a large
amount of changes can really affect the driver stability, even being
trivial changes. Such patch series, if applied, will need acks for the
active driver maintainers that should make sure this won't break any other
driver.
I would, instead, use a different approach, without losing the required
functionalities for xc3028.
To give my $2 cents of contribution, I've sent a modified version of dvb
integration for xc3028 sometime ago to Markus, as a sugestion.
It should be noticed that I didn't tested the patchset (as currently I
don't have em288x+xc3028 DVB hardware). If someone wants to take a look,
the patch series it is available (at quilt format) at:
http://linuxtv.org/~mchehab/mrec2/
The required changes on DVB are minimal (just adding a few newer fields
at frontend core). Also, there's no need to change any stuff on dvb or v4l
drivers. So, it wouln't break any existing drivers.
The diffstat for the core changes is:
linux/drivers/media/Kconfig | 2
linux/drivers/media/Makefile | 1
linux/drivers/media/dvb/dvb-core/dvb_frontend.h | 29 ++
linux/drivers/media/video/tuner-core.c | 156 ++++++++++++++-
linux/include/media/tuner.h | 6
linux/include/media/v4l2-common.h | 2
v4l/Makefile | 4
v4l/scripts/gentree.pl | 1
All API changes on DVB are at the first patch:
http://linuxtv.org/~mchehab/mrec2/hg_mrec_01.patch
It should be noticed that the comments at the patches may not be correct
anymore, since I've folded some patches, and modified some API calls on
Markus original series to be compatible with the way I did the integration
on DVB frontend. If we go to this path, some work is still required.
> I do frown upon code duplication, however, in this case it is a safer
> alternative to the one currently on the table. Earlier versions of the
> xc3028 tuner driver were bound to the video4linux tuner.ko module, for
> the sake of tuning analog television stations. There was a wrapper
> present inside the em2880-dvb driver that allowed the dvb subsystem to
> access the xc3028 via tuner.ko. I am not a big fan of this solution,
> however, it does not touch any core subsystem code. DVB-only devices
> can use a separate module in order to access the xc3028 without imposing
> dependencies on the v4l core.
Duplicating a driver is not a solution, just a terrible hack. We should
focus on a way to use the same tuner driver for both Analog and Digital
TV.
If I compare that solution with the solution I provided your one is
only half way done, you add a dependency for a structure which will
never be fully used (only 1 member of dvb_frontend, dvb_tuner_ops will
be used).
If you look at v4l_dvb_tuner_ops it's clear what it intends to be and
in no way it adds extra struct definitions which do not belong there,
if you look at dvb_frontend in tuner-core.c it has nothing to do with
the tuner, it also contains the callbacks for the digital demod.
It also requires all the dvb headers.
#include "dvb_frontend.h"
#include <linux/dvb/frontend.h>
#include "dvbdev.h"
dvbdev.h is not needed at all either, even if gcc might wipe out the
defined functions because they're not used.
We shouldn't care about hacks to keep the noise on the ML low, put the
technical aspect (which includes a solution for all the requirements)
infront of everything then I might agree with your patch.
Markus
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
