[linux-dvb] Re: refactoring

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

On Monday 11 Oct 2004 10:17, Gerd Knorr wrote:
> > > > struct dvb_fe {
> > > >  struct fe_api  *api;
> > > >  struct dvb_adapter *dvb;
> > > >  void            *priv;
> > > > };
> >
> > Issue: We need two void* in dvb_frontend.
> >
> > One for the demodulator to store its private state+config in. Another for
> > the card driver to which the demodulator is attached to store something.
>
> Yep, I was thinking that we might need that well while looking at ...
>
> >         /* PLL maintenance */
> >         int (*pll_init)(struct dvb_frontend* fe);
>
> ... these prototypes ;)
>
> > My current prototyped XXX_attach() function is:
> >
> > extern int at76c651_attach(struct at76c651_config* config,
> >                            struct dvb_frontend_ops* ops,
> >                            struct dvb_adapter* dvb,
> >       void* adapter_data,
> >                            struct dvb_frontend** fe);
>
> Hmm, giving this I'm not fully sure it is a good idea to have the fe
> (demod) drivers allocate struct dvb_frontend.  Maybe we should do it
> this way:
>
> struct dvb_frontend {
>  /* general info */
>  struct dvb_frontent_info *info;
>  struct dvb_adapter       *dvb;
> #ifdef __linux__
>  struct device            *dev;              /* sysfs */
> #endif
>
>  /* to be filled by the demod driver */
>  struct demod_ops  *demod_ops;
>  void              *demod_priv;
>
>  /* to be filled by the pll driver */
>  struct pll_ops    *pll_ops;
>         void              *pll_priv;
> }
>
> Then have the adapter driver allocate + free dvb_frontend and pass it
> down to the *_attach function for filling in the missing informations.
> Same goes for the pll stuff.  If the dvb adapter driver implements that
> directly -- fine, then it can just fill in the stuff itself.  But the
> pll stuff could also live in a separate module, where the adapter driver
> calles some init function and passes in the dvb_frontend struct to get
> the pll_* fields initialized.

I think having separate pll_ops/pll_priv is going a bit far - they're just too 
simple. Its unlikely they can ever be shared really.

Actually, what I ended up doing was having dvb_register_frontend() allocate 
the dvb_frontend structure and return it - the XXX_attach() functions just 
pass the relevant values in to it.

If we do it as above, we're relying on the user reading comments to determine 
what they should initialise in the structure - we've already had a long 
discussion about that being bad :)