lkml.org 
[lkml]   [2011]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] net: add Faraday FTMAC100 10/100 Ethernet driver
From
Date
Le jeudi 13 janvier 2011 à 19:49 +0800, Po-Yu Chuang a écrit :
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>
> FTMAC100 Ethernet Media Access Controller supports 10/100 Mbps and
> MII. This driver has been working on some ARM/NDS32 SoC including
> Faraday A320 and Andes AG101.
>
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> drivers/net/Kconfig | 9 +
> drivers/net/Makefile | 1 +
> drivers/net/ftmac100.c | 1341 ++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/ftmac100.h | 180 +++++++
> 4 files changed, 1531 insertions(+), 0 deletions(-)
> create mode 100644 drivers/net/ftmac100.c
> create mode 100644 drivers/net/ftmac100.h

Hi

1) please use netdev_alloc_skb_ip_align() instead of dev_alloc_skb() +
skb_reserve(skb, NET_IP_ALIGN);

2) Dont include a "struct net_device_stats stats" in struct ftmac100,
you can use the one included in struct net_device
(You can then remove ftmac100_get_stats())

3) Dont "netdev->last_rx = jiffies;" : This is not driver job.
Ditto for trans_start

4) You must comment why wmb() is needed in ftmac100_xmit()

5) Why isnt NAPI used too for TX completions ?
BTW, I am not sure we want a define. All new drivers must use NAPI.

6) Use is_valid_ether_addr() instead of is_zero_ether_addr() in
ftmac100_probe()

7) "static struct ethtool_ops ftmac100_ethtool_ops" should be const :
"static const struct ethtool_ops ftmac100_ethtool_ops"
Ditto for :
"static struct net_device_ops ftmac100_netdev_ops"


8) Why an interrupt handler (ftmac100_interrupt()) needs to block
interrupts with spin_lock_irqsave(&priv->hw_lock, flags); ?

9) Instead of dev_info(&netdev->dev ...) , please consider netdev_info()


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

\
 
 \ /
  Last update: 2011-01-13 15:25    [W:0.257 / U:6.124 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site