[linux-dvb] [patch/resend] dvb pll library

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

 



Gerd Knorr wrote:
> Johannes Stezenbach <js@xxxxxxxxxxx> writes:
> 
> > > Whats the status on the cx22702 driver btw.?
> > 
> > Well, you know my position. If you send me what I asked for
> > it would be in CVS immediately. Now, I tried to get Michael
> > and Andrew to back up either your or my position so we
> > could reach a decision, but it seems they are not willing
> > to say something.
> 
> Michael askd me to send him the files, I did, but never heared back
> from him since.  Maybe he is just busy, I've also havn't seen anything
> on the list from him recently.  But I through I'll ask for the status
> while I'm submitting patches anyway ...

Yeah, his new employer keeps him busy...

> > Just in case you forgot: My problem is not that your code sucks, but
> > simply that it is different from all other frontend drivers.
> 
> Well, the _interface_ to the other modules (drivers + dvb core) and is
> exactly the same now.  The only minor difference are new entries in
> the cx22702_config struct due to the switchover to dvb-pll.
> 
> The _internals_ are slightly different, yes.  I still think that it is
> better register within the i2c core.  But whenever I do or not doesn't
> change the module behavior at all.
> 
> IMHO it's just nicer for some things, for example for being visible in
> sysfs.  Or -- just found that today -- not needing ugly hacks for
> firmware loading like the one in tda1004x.c currently.  You could just
> use i2c_client->dev and call request_firmware() directly.

Yeah, sounds good. The way you try to force your improvements
upon us is still wrong, IMHO.

> > I see maintainability issues,
> 
> Just because I'm registering my i2c clients within the i2c core?
> Thats the _only_ difference remaining.

The other difference is your use of dvb-pll. I think it was a conscious
decision to move PLL handling out of the frontend drivers (thus they are
now just "demodulator drivers") and into the adapter drivers. I would
have expected that you use dvb-pll from the cx88 driver, and not add
PLL handling back to cx22702. So in essence the usage of the
cx22702 driver in the adapter drivers is different from the
usage of the other demodulator drivers.

> > but also social problems with the people who were involved in the
> > discussions that led to the current frontend design.
> 
> The discussion was about the frontend _interface_ design, i.e. how the
> modules talk to each other, how they are configured and so on.  I
> can't remember having discussed any strict coding guidelines for the
> driver internals.

I remember it different, but could be wrong (and I don't have the
time to re-read the giant "refactoring" and "CX88 i2c issue w/
DVB tuners" threads to find out). Anyway, Andrew invested lots
of time to rework the drivers according to what he thought the
concensus was, and now you come and try to invalidate (part of)
his work.

I don't get why you can't just send a small patch to get the
cx22702 driver to work, and then in a second step start a discussion
to convert *all* frontend drivers to use i2c_client, outlining
the improvements that would make for all. (The first step
shouldn't be too difficult given that you claim that the use
of i2c_client is just an internal implementation thing.)

If we'd used the energy we put into writing all these mails
for coding, the second step would probably be done by now.

> > An easy way out: You could get a CVS account and commit it
> > yourself. That way you would take direct responsibility for
> > your deeds.
> 
> I don't see how that makes a difference.  Either I care about that
> baby or I don't, whenever I do by mailing patches or by commiting
> myself doesn't really matter I think.  Well, committing directly would
> bypass peer review, I don't think that is good (look at the mt352
> discussion for example ...), you guys know the dvb internals much
> better than I do.

Well, I get the feeling that you try to ignore my peer review (in
the cx22702 case).
With a CVS account you could still post your patches for review before
comitting them, but our lameness wouldn't keep you from proceeding.


Johannes



[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux