[linux-dvb] Re: refactoring

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

On Friday 08 Oct 2004 15:06, Gerd Knorr wrote:
> > > Oh, no, not *this* again please.  What the heck is the reason to not
> > > use the kernel i2c stuff?  I think we had discussed that ...
> >
> > I have removed the kernel i2c_driver structures because we do not need
> > it.
>
> We do.  Well, we don't really have to, but it would make several things
> easier when we'll use the existing infrastructure

What things actually? I couldn't see anything beyond the firmware thing, and 
that is gone now.

> > I have left it using the kernel i2c_adapter stuff, and i2c_transfer()
> > functions as before.
>
> Which is a bad idea.  Have you actually looked at my cx22702 version?
> There is much generic, reusable code in there.  And it is generic
> *because* it makes heavy use of the kernl i2c infrastructure.  It
> creates and registeres i2c_client structs for both demod + pll and then
> passes around just a i2c_client ptr (instead of a fe-specific struct) to
> many functions, like this one here:
>
>    static int pll_dtt759x_set_tv_freq(struct i2c_client *c, u32 freq,
>                  int bandwidth);
>
> It's absolutely no problem to move that to some other -- say -- pll-lib
> module and then have other frontend driver reuse it.

We decided that that was not worth it. So I did not bother to implement 
support for it.

> > > And free the drivers private data.  And any other cleanups the driver
> > > migth have to do depending on the hardware.  We certainly need a
> > > detach function.
> >
> > There is no private data any longer.
>
> Yes, I've seen the code now.  Thats is a serve design bug.  The drivers
> private data should not be public.  And it should be allocated and freed
> by the driver itself and not by someone else.

My take on this was that the demod drivers are meant to be as simple and as 
small as possible. I didn't really see the point in hiding the private data 
with this in mind.