Mailing List archive

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

[linux-dvb] Re: refactoring



> > 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.

You lose portability yes.  I don't care much, especially in case of the
cx88 driver which is linux specific anyway.  I want to be able to write
a *good* linux driver, with full suspend+sysfs support, and I simply
have to use the kernel i2c infrastructure for that.

Note however that the frontend interface I've suggested doesn't has any
linux kernel API dependencies in there.  The very same interface can be
used on BSD or MacOS-X as well.  If you want write portable drivers, you
can.  The suggested dvb frontend interface will not stop you.

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

No, it is a bad designed interface.  I'll drop the "more professional"
into the waste basket, thats just a buzzword, and I don't want to play
buzzword bingo here.

Andrew's approach might be easier to understand on a first sight.  Look
again, it's not.  You have one struct with both config info and state
info mixed in there.  It's not clear who has to setup stuff:  It is the
dvb adapter driver or is it the frontend driver?  Same for releasing the
stuff.

I have both separated.  That makes it possible to have static allocated,
read/only config data structs.  And it's also pretty clear who has to
maintain what:  The dvb adapter driver's job is to care about the config
data, and it is the frontend drivers job to maintain its own state info.
It is next to impossible to get it wrong.  And that is a big advantage
in code maintainability.

Maybe that isn't a big problem now when writing the stuff new from
scratch and know exactly how you have designed it.  But it is a problem
if someone else looks at the code or even if you look at it yourself
again one year later.

> >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.

This is wrong.  It probably works fine for small systems, but the more
complex things become the more horrible it is to have state information
statically allocated.

  Gerd

-- 
return -ENOSIG;




Home | Main Index | Thread Index