[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070712123917.5d54fbaa@freepuppy.rosehill.hemminger.net>
Date: Thu, 12 Jul 2007 12:39:17 -0700
From: Stephen Hemminger <shemminger@...ux-foundation.org>
To: Masakazu Mokuno <mokuno@...sony.co.jp>
Cc: Jeff Garzik <jeff@...zik.org>, netdev@...r.kernel.org,
cbe-oss-dev@...abs.org
Subject: Re: [PATCH] ps3: gigabit ethernet driver for PS3, take3
On Thu, 05 Jul 2007 20:11:16 +0900
Masakazu Mokuno <mokuno@...sony.co.jp> wrote:
> Hi,
>
> This is the third submission of the network driver for PS3.
> The differences from the previous one are:
>
> - renamed source file names so that their prefix can match
> with the module name
> - added cbe-oss-dev@...abs.org line for MAINTAINER file
> - changed some in copyright comments
>
> If there are no more comments, please apply for 2.6.23.
>
This isn't a show stopper. But could you consider integrating vlan
support with existing VLAN acceleration hooks?
Please remove GELIC_NET_GET_MODE/GELIC_NET_SET_MODE, they seem
to be ioctl's left over from an earlier version.
You don't need a net_device_stats element in gelic_net_card because
it is already included in net_device structure in recent kernels.
Why make ethtool support conditional. It should always be enabled.
Don't call netif_poll_enable() in net_open. It is unnecessary and
racy.
It is better to use device name (ie eth0) as name of irq for request_irq()
because the user level IRQ balancer has some heuristics that know that
names of form "ethX" are network devices. May not matter on PS3 because
it isn't true SMP.
You are wasting memory by always allocating full maximum receive size buffers.
Your max mtu is 2308 so the allocation will end up being rounded up to 4K
so that costs twice as much memory as allocating a standard 1518 (or 1536)
size Ethernet MTU buffer.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists