[linux-dvb] Re: refactoring

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

On Saturday 09 Oct 2004 13:12, Gerd Knorr wrote:
> On Sat, Oct 09, 2004 at 01:27:22PM +0200, Johannes Stezenbach wrote:
> > Gerd Knorr wrote:
> > > struct dvb_fe* create_<fename>(/* whatever is needed for the fe in
> > > question */);
> >
> > I would prefere this to the current code.
> >
> > However, looking at av7110.c:frontend_init() I think that
> > create_<fename>() would get too much paramters, which would make the code
> > unreadable.
> >
> > We could define a struct <fename>_config and pass that to
> > create_<fename> to get around this.
>
> We can decide that even on a frontend-by-frontend base as
> create_<fename>() and the args passed to are frontend specific anyway.
> For frontends which need lots of parameters it certainly makes sense to
> have a config struct.  For the few ones needed for cx22702 I probably
> wouldn't do it that way.

So, prototyping this for at76c651:

at76c651.h:

struct at76c651_config
{
        /* the i2c adapter to use */
        struct i2c_client* i2c;

        /* PLL maintenance */
        int (*pll_init)(struct dvb_frontend* fe);
        int (*pll_set)(struct dvb_frontend* fe, struct 
dvb_frontend_parameters* params);
};

extern int at76c651_attach(struct dvb_frontend* fe, struct at76c651_config* 
config);
extern const struct dvb_frontend_ops at76c651_ops;


at76c651.c:

struct at76c651_state {

        struct at76c651_config* config;

        /* revision of the chip */
        u8 revision;

        /* last QAM value set */
        u8 qam;
};
....

The *_state is the private data to the demod. I can't decide whether it should 
be a pointer to the config, or whether it should be an instance of it, and 
just memcpy whatever is passed in to the *_attach() function.