Subject: Re: ppc405 enet changes (fwd)

From: andrew may <[email-masked]>
Date: Tue, 4 Sep 2001 15:26:08 -0700

On Tue, Sep 04, 2001 at 06:38:02PM -0400, Dan Malek wrote:
> John Tyner wrote:
> >
> > Is this patch going to be applied to the kernel? I haven't seen it in the
> > logs...
>
> I dunno.....from the code it doesn't look like you allow multiple
> device support (probe is only done once for one device).  If you
> have something that actually enables a new feature, add that too.
> Otherwise, it just looks like you added some additional indirection
> that doesn't add anything new and just adds overhead to the existing
> driver.  I'm also trying to determine why your patch seems to add
> things to the driver that should already be there.

Well we don't have the hardware to do multiple devices. I just know
that is possible in the future. I would prefer to go through the
pain of removing the static's now rather than later.

If you want to argue that we are just adding more redirection I will
be happy to send a patch to remove the entire struct ppc405_enet_private
to remove some more "useless" redirection. One of my biggest gripes
with the driver is that half of the stuff is kept as static and the
other half in the private sturct, for example ep_xmit_skb is in the
private stuct and ppc405_skb_rx is static. With things like this it
makes the entire driver hard to follow. I can't really think of an
explanation on why this is done in the driver. My only guess would
be that the person that did the driver has not done a linux network
driver before. There are other little clues throughout the driver
that point to this as well.

> We don't just blindly take stuff from people and patch it.  We
> actually have to believe it adds some benefits, we have to test
> it, and then it gets checked in.  This doesn't happen overnight.

No I don't expect the patch to go in quickly. I just wanted John
to start the process off. This patch does not do much to fix anything
but I would like to make any future changes off the new variable
names rather than what is in there now. Since this patch touches
so much stuff I think it would be better to keep any real changes
out of it until it gets tested.

So if you want to point to specific code that you don't like let
us know.

--
Thanks

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/