Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[linux-dvb] Re: refactoring



Gerd Knorr wrote:


I have left it using the kernel i2c_adapter stuff, and i2c_transfer() functions as before.

Which is a bad idea.

why? please elaborate.

 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.

no. that's just the reason why it's not portable, not testable in a userspace mockup and hardly portable to other OSes like MacOS-X and BSD. The new approach without dependencies to kernel APIs is much nicer.

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.

I have to say that I prefer Andrew's approach much more, it's simply more professional + easier to understand and maintain.


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.

Please calm down. Not allocating dynamic memory but using static data instead is always a good thing.

Holger





Home | Main Index | Thread Index