[linux-dvb] Re: Proposal for new frontend architecture

[Andrew de Quincey <adq_dvb@lidskialf.net>]

On Wednesday 22 Sep 2004 23:02, Wolfgang Wegner wrote:
> On Wed, Sep 22, 2004 at 10:09:14PM +0100, Andrew de Quincey wrote:
> > Have you looked at stv0299.c? It supports about 5 different
> > tinboxes/boards, each of which uses the stv0299 chip. They all require
> > different PLLs and have different settings. The code is spaghetti because
> > of this.
>
> [...]
> Now i had a look at it.
> As far as i can see, the demod code is almost clean.
> What differs is the initialisation and the symbolrate setting - IIRC
> this is what could be configurable by simply having some configurable
> parameters for a generic STV0299:
> - crystal frequency
> - AGC usage (tuner/if/tuner+if, negative/positive)
> - what did i miss?
>
> The main differences are, as you already said, in PLL code.
> Why not leave STV0299 as a real "demod driver", which gets included/used
> by a frontend driver knowing how to configure the variable parameters
> and how to set up the PLL? Or have a wrapper for both of them, in case
> it makes sense to re-use the PLL driver, too?
>
> Something like this:
>
> demod_driver functionality:
> - demod_struct *demod_create(i2c_functions)
>   allocates and initialises a struct with demod parameters.
>   tries to access i2c bus (open() if necessary/applicable),
>   i2c functions are stored in demod struct.
> - demod_init(demod_struct *demod)
>   initialises the demod part, including firmware loading etc.
> - demod_activate_pll_i2c(on/off)
>   activates i2c repeater to access PLL device
> - demod_set_channel_parms(DSD/demod parameters)
> - other demod handling functions
> - demod_release(demod_struct *demod)
>   resets and disables demod part, frees struct with parameters
>
> pll_driver functionality:
> - pll_struct *pll_create(i2c_functions, demod_activate_pll_i2c)
>   same as for demod
> - pll_init(pll_struct *pll)
>   (if needed - not sure)
> - pll_set_frequency(pll_struct *pll, frequency)
> - pll_release(pll_struct *pll)
>
> frontend_driver functionality:
> - frontend_struct *frontend_create(i2c_functions)
>   calls demod_create and pll_create
> - frontend_init(frontend_struct *fe)
>   configures parameters of demod and pll, then calls demod_init and
> pll_init - maybe included in frontend_create to allow easier auto-probing,
> see below - frontend_set_transponder(frontend_struct *fe,
> delivery_system_descriptor) calls demod_activate_pll_i2c,
> demod_pll_set_frequency and
>   demod_set_channel_parms
> - frontend_get_status(frontend_struct *fe)
>   get lock state of demod or pll (if demod unlocked)
> - frontend_release(frontend_struct *fe)
>   releases pll, demod and own memory
>
> of course, this is just the rough outline to make clear what i
> was thinking about.
>
> Like this, an additional layer is introduced (what i called
> "frontend" here), but the demod itself should stay quite clean
> while still being configurable and re-usable. Or you think this
> already to be code bloat? The "frontend" layer could then be
> implemented for each card (thus including knowledge about
> i2c adapter and i2c address of the devices), to allow simply
> calling some frontend_create()s for all the frontends expected
> for this device id until one succeeds.
>
> Also, like this, i do not see where a dependency on a special
> i2c adapter is as long as a wrapper can be provided to have a
> general API for them.
>
> Did i think too complicated about all this?

I think maybe - I have a tendency to do that as well though.

I think everyone agrees that what are currently the "frontend drivers" should 
from now on only control the demodulators.

If you look back in the list, I proposed having a "pll library", which is very 
similar to your pll_driver idea. However, as everyone pointed out, most PLLs 
are only about 20 lines of code - so having an entire driver structure for 
them is overkill... I agree with this (now :)

In the end, I think we should keep it as simple as possible - having 
demodulator drivers since they are generally substantial pieces of code, and 
just making the card drivers implement the PLLs/glue logic themselves is 
simplest, and leads to the best balance of code bloat vs. code nastiness we 
can hope for.

For example, on some cards (Hauppauge TDA10045 DVB-T) there is a third chip 
between the tinbox and the demod - the SAW filter. This has to be set up 
correctly as well - yet its only relevant to that particular card 
implementation, so there's little point in developing a specific frontend 
driver for that combination of three.