Michael Krufky wrote:
> For the past few months, I've been working on refactoring the analog tuner.ko 
> module, such that all hardware-specific code can be separated into 
> dvb_frontend style tuner modules.
> 
> This allows for a single module to be used by both the v4l2 tuner interface 
> via the tuner.ko i2c_client driver, and directly by the dvb subsystem's 
> tuning system.
> 
> This refactoring process has zero impact to the way that v4l and dvb 
> functions.
> 
> I have completed phase one of the refactoring process, and now it is ready 
> for testing and review.
> 
> http://linuxtv.org/hg/~mkrufky/tuner-refactor-phase-1
> 
> A brief description of the individual changesets follows:
> 
> - tuner: kill i2c_client interface to tuner sub-drivers
> 
> This changeset removes the i2c_client interface between tuner.ko and the 
> tuner sub-drivers.
> 
> The i2c_client interface to tuner.ko, itself, remains the same as it has been 
> -- this is only an internal change that affects the interaction between 
> tuner.ko and the hardware-specific code.
> 
> Some helper functions and macros were added in this changeset, in order to 
> ease the conversion process, without causing headaches or breakage. (see 
> tuner-i2c.c)  We can remove these extra structs and helper functions after 
> the refactoring process is complete.
> 
> - hybrid tuner refactoring core changes, phase 1
> 
> This changeset contains the more interesting work, where tuner-core is 
> altered to support attachment of dvb_frontend style tuner modules.  An 
> additional method "set_analog_params" was added to struct dvb_tuner_ops, so 
> as to avoid altering the DVB subsystem userspace API headers.  This change 
> does not create any dependency of the DVB subsystem on V4L, nor does it 
> create any dependency of the V4L subsystem on DVB.
> 

It looks fine, althought one aspect i would like to mention:


http://linuxtv.org/hg/~mkrufky/tuner-refactor-phase-1/file/2813710f99ba/linux/drivers/media/dvb/dvb-core/dvb_frontend.h

       67 struct analog_parameters {
       68       unsigned int frequency;
       69       unsigned int mode;
       70       unsigned int audmode;
       71       __u64 std;
       72 };

       84       int (*set_analog_params)(struct dvb_frontend *fe, struct
analog_parameters *p);


Rather than having the analog operations/data structures into
dvb_frontend (which is supposed to be purely digital for anyone reading
the code), you can move the operations/struct into the hybrid tuner
header (below) where the operations are really meant for.


http://linuxtv.org/hg/~mkrufky/tuner-refactor-phase-1/file/2813710f99ba/linux/drivers/media/video/tuner-driver.h

       28 #include "dvb_frontend.h"


This would allow not to add in dvb_frontend header into the tuner header
unnecessarily as well.


The whole point being if we keep adding all stuff to dvb_frontend, in
the end it ends up with lot of stuff which aren't directly related.


Looks fine otherwise.

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Reply via email to