Manu Abraham wrote: > 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. > Manu, tuner-driver.h will be entirely deleted at the end of the refactoring process. If you notice, inside the new dvb_frontend style tuners (post-conversion) these source files no longer include "tuner-driver.h" -- All of the contents of tuner-driver.h is for the old tuner.ko internal api, which has been replaced by the dvb_frontend internal api. This file will be deleted after tda9887 goes through analog if demod refactoring. The "struct tuner" declaration will be moved to the top of tuner-core.c We cannot move struct analog_parameters out of dvb_frontend.h , since the new set_analog_params method must be a part of the dvb_tuner_ops structure. This is a very small price to pay, for quite a large gain, without creating any dependencies of the DVB subsystem onto V4L, and also without creating any dependencies of the V4L subsystem onto DVB. The idea of my refactoring work, was to add the analog tuning capability to the dvb_frontend style tuner modules, allowing us to have a single module that can receive tuning commands for both analog and digital. Since we are adding this capability to the dvb_frontend internal API, naturally, this must all appear within dvb_frontend.h -- There are already dvb_frontend tuner modules in the frontends/ tree whose hardware is capable of tuning analog, but have not been able to thus far, due to code limitations. The change described above gets us past those limitations. There is no need for this desire to separate this code into separate headers. After all, we are dealing with, potentially, a single piece of hardware that will have to be able to tune both analog and digital. This is one thing that we are not going to agree on, unless you can show a patch on top of my tree that will satisfy your requirements. Otherwise, I hope that you can get past this small issue. Regards, Mike _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
