Re: patch tulip-natsemi-dp83840a-phy-fix.patch added to -mm tree

[Grant Grundler]

On Fri, May 20, 2005 at 05:34:26PM -0400, Jeff Garzik wrote:
> Yes, tg3 is awful in this regard.  I have made a bit of progress by 
> moving some of the stuff into a workqueue.

ah good! At some point I should re-install the fiber bcm5701 cards
I have and see if they work better then.

> >It's totally relevant.
> >Complaints (bug reports) and patches drive change.  That's how both
> >commercial *and* non-commercial developers prioritize.
> >
> >"Ingo and the real-time crowd" are a good example of a change
> >in priorities driven by non-commercial users.
> 
> No, these are commercial users.  Embedded space really cares about this 
> stuff.

Sorry - maybe VM support for VIVT cache is a better example?
(I'm thinking sparc/parisc support)

Almost everything I think of has some form of commercial interest
either directly (embedded, HA, infiniband, Server IO, ia64/amd64, etc)
or indirectly (sponsorships at Universities or sourceforge projects).

> >Understood. But they were not the first ones. Donald Becker has a fairly
> >well known digust for use of CPU spin loops.  But we can't eliminate
> >*all* udelay just becuase of the way HW is designed and has bugs.
> >I think it would be reasonable to get rid of many udelay calls
> >(replace them with PCI read delay loops like Donald has advocated)
> >and get the rest into a context where it matters less.
> >I have no objection to people fixing those sorts of issues.
> 
> If you recognize the issue, you should object to new changes adding new 
> issues!

And ignore fixes for existing issues.
"The enemy of good is perfect"

...
> >Not true. This patch brings the tulip driver into compliance with
> >published specs and makes the driver functional for parisc/mips/ia64 users.
> 
> Ok, yes, it fixes the tulip issue AND causes work for others.

But it's not new work.

In timer.c, tulip_restart_rxtx() is called right after tulip_select_media().
tulip_restart_rxtx() has a potential 1300 usec delay loop. About
the same as the code in tulip_select_media() needs.

I now appreciate much more that a RH employee (IIRC, John Linville)
submitted that patch indirectly on my behalf. I filed the original
bugzilla and provided the patch based on work by Charlie Brett.


> >The "whatever reason" is clearly decided by the mainline kernel maintainer.
> >If we treat other people's labor as "free", then the right answer is
> >to drop the patch and wait for some subset of "we" to do the extra legwork.
> 
> Um, this is how all kernel development works :)

*nod*

> Maintainers are not supposed to merge patches into upstream, if they 
> have flaws still remaining to be corrected.

Many patches are not this black and white.
All patches have to survive at least one subjective evaluation:
	Is the cure worse than the illness?


> >Several people care if tulip phy init works right. OTOH, I'm only aware of
> >one person who specifically cares if tulip is holding the CPU hostage for
> >1 or 2 ms during the occasional phy init.
> >
> >Is being a technology purist more important than moving forward with
> >what people care about?
> 
> There will _always_ be ugly patches that get hardware going for some users.

"ugly" is subjective.  Especially in the context of tulip phy init sequence.
Anyone who finds beauty in that chunk of code is truly twisted. :^)

(It just reflects the ugly mess of HW that too many vendors shipped.
I don't ever expect tulip driver to be "clean" here.)

> Tons of changes are kept outside the kernel until they are ready.  This 
> is just one more example.
> 
> Merging code into the kernel is a big deal.  That code will have to be 
> maintained for years, maybe decades.  "when in doubt, don't merge" is 
> generally the right answer.

Agreed.

> I don't want your patch to become an issue that embedded developers must 
> address (like the tg3 example above), causing further patching and 
> headache in that area.

Certainly. No problem with that.

BTW, which embedded developers still care about tulip driver?
One could use this patch as a litmus test to see how much they care. :^)

I was expecting they'd be using gigabit NICs, Wi-Fi, GSM,
or bluetooth these days.

thanks,
grant
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/