[PATCH] net/enc28j60: low power mode
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle; this
low power mode cuts that power usage by a factor of around 100.
This version provides a generic routine to poll a register until
its masked value equals some value ... e.g. bit set or cleared.
It's basically what the previous wait_phy_ready() did, but this
version is generalized to support the handshaking needed to
enter and exit low power mode.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Signed-off-by: Claudio Lanconelli [EMAIL PROTECTED]
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -400,26 +400,31 @@ enc28j60_packet_write(struct enc28j60_ne
mutex_unlock(priv-lock);
}
-/*
- * Wait until the PHY operation is complete.
- */
-static int wait_phy_ready(struct enc28j60_net *priv)
+static unsigned long msec20_to_jiffies;
+
+static int poll_ready(struct enc28j60_net *priv, u8 reg, u8 mask, u8 val)
{
- unsigned long timeout = jiffies + 20 * HZ / 1000;
- int ret = 1;
+ unsigned long timeout = jiffies + msec20_to_jiffies;
/* 20 msec timeout read */
- while (nolock_regb_read(priv, MISTAT) MISTAT_BUSY) {
+ while ((nolock_regb_read(priv, reg) mask) != val) {
if (time_after(jiffies, timeout)) {
if (netif_msg_drv(priv))
-printk(KERN_DEBUG DRV_NAME
- : PHY ready timeout!\n);
- ret = 0;
- break;
+dev_dbg(priv-spi-dev,
+ reg %02x ready timeout!\n, reg);
+ return -ETIMEDOUT;
}
cpu_relax();
}
- return ret;
+ return 0;
+}
+
+/*
+ * Wait until the PHY operation is complete.
+ */
+static int wait_phy_ready(struct enc28j60_net *priv)
+{
+ return poll_ready(priv, MISTAT, MISTAT_BUSY, 0) ? 0 : 1;
}
/*
@@ -594,6 +599,32 @@ static void nolock_txfifo_init(struct en
nolock_regw_write(priv, ETXNDL, end);
}
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive. (However, we
+ * can't stay in lowpower mode during suspend with WOL active.)
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ if (netif_msg_drv(priv))
+ dev_dbg(priv-spi-dev, %s power...\n,
+is_low ? low : high);
+
+ mutex_lock(priv-lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ poll_ready(priv, ESTAT, ESTAT_RXBUSY, 0);
+ poll_ready(priv, ECON1, ECON1_TXRTS, 0);
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ poll_ready(priv, ESTAT, ESTAT_CLKRDY, ESTAT_CLKRDY);
+ /* caller sets ECON1_RXEN */
+ }
+ mutex_unlock(priv-lock);
+}
+
static int enc28j60_hw_init(struct enc28j60_net *priv)
{
u8 reg;
@@ -612,8 +643,8 @@ static int enc28j60_hw_init(struct enc28
priv-tx_retry_count = 0;
priv-max_pk_counter = 0;
priv-rxfilter = RXFILTER_NORMAL;
- /* enable address auto increment */
- nolock_regb_write(priv, ECON2, ECON2_AUTOINC);
+ /* enable address auto increment and voltage regulator powersave */
+ nolock_regb_write(priv, ECON2, ECON2_AUTOINC | ECON2_VRPS);
nolock_rxfifo_init(priv, RXSTART_INIT, RXEND_INIT);
nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
@@ -690,7 +721,7 @@ static int enc28j60_hw_init(struct enc28
static void enc28j60_hw_enable(struct enc28j60_net *priv)
{
- /* enable interrutps */
+ /* enable interrupts */
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME : %s() enabling interrupts.\n,
__FUNCTION__);
@@ -726,15 +757,12 @@ enc28j60_setlink(struct net_device *ndev
int ret = 0;
if (!priv-hw_enable) {
- if (autoneg == AUTONEG_DISABLE speed == SPEED_10) {
+ /* link is in low power mode now; duplex setting
+ * will take effect on next enc28j60_hw_init().
+ */
+ if (autoneg == AUTONEG_DISABLE speed == SPEED_10)
priv-full_duplex = (duplex == DUPLEX_FULL);
- if (!enc28j60_hw_init(priv)) {
-if (netif_msg_drv(priv))
- dev_err(ndev-dev,
- hw_reset() failed\n);
-ret = -EINVAL;
- }
- } else {
+ else {
if (netif_msg_link(priv))
dev_warn(ndev-dev,
unsupported link setting\n);
@@ -1307,8 +1335,9 @@ static int enc28j60_net_open(struct net_
}
return -EADDRNOTAVAIL;
}
- /* Reset the hardware here */
+ /* Take it out of low power mode and reset the hardware here */
enc28j60_hw_disable(priv);
+ enc28j60_lowpower(priv, false);
if (!enc28j60_hw_init(priv)) {
if (netif_msg_ifup(priv))
dev_err(dev-dev, hw_reset() failed\n);
@@ -1337,6 +1365,7 @@ static int enc28j60_net_close(struct net
printk(KERN_DEBUG DRV_NAME : %s() enter\n, __FUNCTION__);
enc28j60_hw_disable(priv);
+ enc28j60_lowpower(priv, true);
netif_stop_queue(dev);
return 0;
@@ -1537,6 +1566,8 @@ static int __devinit enc28j60_probe(stru
dev-watchdog_timeo = TX_TIMEOUT;
SET_ETHTOOL_OPS(dev, enc28j60_ethtool_ops);
+ enc28j60_lowpower(priv, true);
+
ret = register_netdev(dev);
if (ret) {
if (netif_msg_probe(priv))
@@ -1582,6 +1613,8 @@ static struct spi_driver
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
David Brownell wrote: On Monday 11 February 2008, Claudio Lanconelli wrote: I have tried your latest patch. Only after the following change it works fine (no more rx errors during ifconfig up). Hmm, what chip rev do you have? Different errata and all. ISTR mine is rev4; so, not the most current, but not the oldest version either. I use the same revision. I added enc28j60_lowpower(false) just before enc28j60_hw_init() Hmm, I'd have expected it would go best *before* that, but what you include below shows it going *after* ... If there's some problem where reset doesn't work correctly in low power mode, who knows what else would need manual resetting. I don't know why it needs low power resume before reset. I read in the errata tath clkready bit after reset doesn't work reliably. May be something related to this, but undocumented. Better yet, since I can't reproduce the problem, why don't you just update my latest patch with the relevant version of this tweak, and then forward it as From: me and with both our signoffs. That's the usual way to cope with this type of tweaking. (Not all updates to your driver should need your signoff, but then most patches shouldn't need very many iterations either.) Done -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
David Brownell wrote:
and in the enc28j60_net_close() after enc28j60_hw_disable().
Probably we don't need to set_lowpower(false) in enc28j60_net_open() since
it performs a soft reset with enc28j60_hw_init() (not sure).
The current patch sets the device in low power mode in hw_disable(),
and takes it out of that mode in hw_enable(). I can move them; and
the only soft thing about this chip's reset is when it starts from
a protocol command not the reset command.
I want to mean the reset software command. It's functionally
equivalent to hardware system reset, but it seems to need exit low
power mode to work flawlessly.
I have tried your latest patch. Only after the following change it
works fine (no more rx errors during ifconfig up).
I added enc28j60_lowpower(false) just before enc28j60_hw_init()
@@ -1318,8 +1347,9 @@
}
return -EADDRNOTAVAIL;
}
-/* Reset the hardware here */
+/* Reset the hardware here (and take it out of low power mode) */
enc28j60_hw_disable(priv);
+enc28j60_lowpower(priv, false);
if (!enc28j60_hw_init(priv)) {
if (netif_msg_ifup(priv))
dev_err(dev-dev, hw_reset() failed\n);
With this addition you can add Acked-by line.
Thank you.
After a couple of :
ifconfig eth0 down
(wait just 1 second)
ifconfig eth0 up
the network is frozen.
If I do another
ifconfig eth0 down
(wait just 1 second)
ifconfig eth0 up
restarts.
It's random, no rule.
I write a shell loop to do that, and added a ping -c2 too.
If that was done before the sleep 1 no packets flowed.
Afterwards, no problem -- ever.
(And outside the loop, ethool -s eth1 duplex full.)
I forgot to tell that during my test I have a web server running on the
board and
a client continuously requesting a page.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix
David Brownell wrote:
Prevent oops on enc28j60 packet RX: make sure buffers are aligned.
Not all architectures support unaligned accesses in kernel space.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Acked-by: Claudio Lanconelli [EMAIL PROTECTED]
---
drivers/net/enc28j60.c |3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/net/enc28j60.c2008-02-06 09:29:00.0 -0800
+++ b/drivers/net/enc28j60.c2008-02-06 09:30:03.0 -0800
@@ -900,7 +900,7 @@ static void enc28j60_hw_rx(struct net_de
if (RSV_GETBIT(rxstat, RSV_LENCHECKERR))
ndev-stats.rx_frame_errors++;
} else {
- skb = dev_alloc_skb(len);
+ skb = dev_alloc_skb(len + NET_IP_ALIGN);
if (!skb) {
if (netif_msg_rx_err(priv))
dev_err(ndev-dev,
@@ -908,6 +908,7 @@ static void enc28j60_hw_rx(struct net_de
ndev-stats.rx_dropped++;
} else {
skb-dev = ndev;
+ skb_reserve(skb, NET_IP_ALIGN);
/* copy the packet from the receive buffer */
enc28j60_mem_read(priv, priv-next_pk_ptr + sizeof(rsv),
len, skb_put(skb, len));
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: low power mode
David Brownell wrote: The driver seemed to already have some goofage there: # ifconfig eth1 up net eth1: link down net eth1: link down net eth1: normal mode net eth1: multicast mode net eth1: multicast mode # Without low power patch I have: # ifconfig eth0 up net eth0: link down net eth0: normal mode net eth0: multicast mode net eth0: multicast mode # net eth0: link up - Half duplex I'd normally expect it not to go down when told to go up, and then only to do the multicast thing once ... multicast is called by network stacks, no control by the driver. The driver just print message. I don't know why enc28j60_set_multicast_list() are called more than once. When you do an ifconfig up the driver reset the chip, so you see link down before link up message. In such cases If I dump the counters with ifconfig I got rx error counter 0 and the RX and TX packets counters are freezed. Actually I don't know what causes the freeze, it needs investigation. The cause can be the rx error condition or the power down/up commands. May be receiving packets while it's going to wakeup causes problems. The enc28j60_setlink() was odd too. It insists the link be down before changing duplex, then brings the link up ... so I had to put it down again to maintain the lowpower if not up invariant. But the way it brings the link up is to enc28j60_hw_init(), which it also does when bringing the link up. So there's no need to bring the link up when changing duplex ... I don't follow you anymore. To change duplex mode the link must be down. enc28j60_setlink() reinitialize the chip with new duplex mode, enc28j60_hw_init() never brings link up. I propose you to add set_lowpower(true) in the enc28j60_probe() and in the enc28j60_net_close() after enc28j60_hw_disable(). Probably we don't need to set_lowpower(false) in enc28j60_net_open() since it performs a soft reset with enc28j60_hw_init(). Do you agree? use if(netif_msg_drv(priv)) ... Doing so we can switch on/off messages runtime using ethtool. OK, although there's still a role for -DDEBUG compile-time mesage removal. It's ok to use if(netif_msg_drv(priv)) dev_dbv(... This driver also abuses __FUNCTION__ (general policy: don't use it) Why? Then there should be a single routine for all such busy-wait loops, so each such usage doesn't need to be open-coded. Less space, and more obviously correct. I'll add one and make phy_read() use it too. Ok -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
David Brownell wrote: How long did that take? I did about four dozen ifconfig eth1 up sleep 3 ifconfig eth1 down cycles ... it worked fine. The sleep was to let the link negotiation complete. After a couple of : ifconfig eth0 down (wait just 1 second) ifconfig eth0 up the network is frozen. If I do another ifconfig eth0 down (wait just 1 second) ifconfig eth0 up restarts. It's random, no rule. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: low power mode
Sorry,
let me repeat what I said in previous mail.
I propose you to add set_lowpower(true) in the enc28j60_probe() and in
the enc28j60_net_close() after enc28j60_hw_disable().
Probably we don't need to set_lowpower(false) in enc28j60_net_open() since
it performs a soft reset with enc28j60_hw_init() (not sure).
Furthermore, as you suggested, we also need to remove hw_init() from the
setlink()
because hw_init() is called when we bring link up.
--- enc28j60.c 20 Dec 2007 10:47:01 - 1.22
+++ enc28j60.c 7 Feb 2008 11:07:20 -
@@ -740,12 +740,6 @@
if (!priv-hw_enable) {
if (autoneg == AUTONEG_DISABLE speed == SPEED_10) {
priv-full_duplex = (duplex == DUPLEX_FULL);
- if (!enc28j60_hw_init(priv)) {
- if (netif_msg_drv(priv))
- dev_err(ndev-dev,
- hw_reset() failed\n);
- ret = -EINVAL;
- }
} else {
if (netif_msg_link(priv))
dev_warn(ndev-dev,
Can you update your low power patch with these modifications?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: section fix
David Brownell wrote:
Minor bugfixes to the enc28j60 driver ... wrong section marking and
indentation, and bogus use of spi_bus_type. (Setting the bus type
of a driver is a SPI framework responsibility.)
Signed-off-by: David Brownell [EMAIL PROTECTED]
Acked-by: Claudio Lanconelli [EMAIL PROTECTED]
---
Bonus patch.
drivers/net/enc28j60.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -1555,7 +1555,7 @@ error_alloc:
return ret;
}
-static int enc28j60_remove(struct spi_device *spi)
+static int __devexit enc28j60_remove(struct spi_device *spi)
{
struct enc28j60_net *priv = dev_get_drvdata(spi-dev);
@@ -1572,9 +1572,8 @@ static int enc28j60_remove(struct spi_de
static struct spi_driver enc28j60_driver = {
.driver = {
.name = DRV_NAME,
- .bus = spi_bus_type,
.owner = THIS_MODULE,
- },
+},
.probe = enc28j60_probe,
.remove = __devexit_p(enc28j60_remove),
};
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
David Brownell wrote:
From: David Brownell [EMAIL PROTECTED]
Prevent unaligned packet oops on enc28j60 packet RX.
How can I reproduce the unaligned packet oops? Did you use any utilities
to force this condition?
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle. Low
power mode cuts that power usage by a factor of around 100.
Good idea, but with your patch applied, after some ifconfig down -
ifconfig up cycle, the
enc28j60 is left in an unknown state and it doesn' Rx/Tx anything.
In such cases If I dump the counters with ifconfig I got rx error
counter 0 and the RX and TX packets counters are freezed.
Actually I don't know what causes the freeze, it needs investigation.
The cause can be the rx error condition or the power down/up commands.
May be receiving packets while it's going to wakeup causes problems.
Can you split the patch in 2 parts, unaligned rx and power save?
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive.
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ int tmp;
+
+ dev_dbg(priv-spi-dev, %s power...\n, is_low ? low : high);
use
if(netif_msg_drv(priv)) printk(...
instead of dev_dbg(), please.
Doing so we can switch on/off messages runtime using ethtool.
+
+ mutex_lock(priv-lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ for (;;) {
+ tmp = nolock_regb_read(priv, ESTAT);
+ if (!(tmp ESTAT_RXBUSY))
+ break;
+ }
Avoid infinite waiting loops, please.
Look at enc28j60_phy_read() for example.
+ for (;;) {
+ tmp = nolock_regb_read(priv, ECON1);
+ if (!(tmp ECON1_TXRTS))
+ break;
+ }
idem
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ for (;;) {
+ tmp = nolock_regb_read(priv, ESTAT);
+ if (tmp ESTAT_CLKRDY)
+ break;
+ }
idem
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][resend] add driver for enc28j60 ethernet chip
This patch add support for Microchip enc28j60 10Mbps Ethernet chip used in
embedded systems
due to its cheap SPI interface.
This 2nd version include changes from previous comments by Jeff and Stephen,
all but NAPI, see comments below at this regard.
I resend the patch because I didn't receive any feedback.
Changes to 1st version:
- use mutex instead of semaphore
- add carrier detect handling
- add ethtool support
- set_multicast_list when the interface is up using a workqueue
- add restart_work to reset the chip in case of tx_timeout
- removed kmalloc() for spi_transfer_buf (array defined in the priv struct)
Jeff Garzik wrote:
comments:
* Why do interrupt work in a kernel thread? Your comment says you
cannot, but does not explain.
The enc28j60 is a SPI to Ethernet adapter, so we cannot access register
with simple in() out() instructions, but we need to use the SPI
subsystem. The spi_sync() basic operation to read/write a register is a
blocking operation, so can't be done in interrupt context.
Since every basic operation like read interrupt flag register call
spi_sync() we need the work queue for almost everything.
* should use NAPI
For the reason I just explained I don't think NAPI is a viable way for
enc28j60.
Furthermore enc28j60 is a 10Mb only device and probably don't suffer to
interrupt overload.
I'm waiting for any comments, please.
Cheers,
Claudio Lanconelli
Signed-off-by: Claudio Lanconelli [EMAIL PROTECTED]
diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
new file mode 100644
index 000..0809a6a
--- /dev/null
+++ b/drivers/net/enc28j60.c
@@ -0,0 +1,1600 @@
+/*
+ * Microchip ENC28J60 ethernet driver (MAC + PHY)
+ *
+ * Copyright (C) 2007 Eurek srl
+ * Author: Claudio Lanconelli [EMAIL PROTECTED]
+ * based on enc28j60.c written by David Anders for 2.4 kernel version
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * $Id: enc28j60.c,v 1.22 2007/12/20 10:47:01 claudio Exp $
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/types.h
+#include linux/fcntl.h
+#include linux/interrupt.h
+#include linux/slab.h
+#include linux/string.h
+#include linux/errno.h
+#include linux/init.h
+#include linux/netdevice.h
+#include linux/etherdevice.h
+#include linux/ethtool.h
+#include linux/tcp.h
+#include linux/skbuff.h
+#include linux/delay.h
+#include linux/spi/spi.h
+
+#include enc28j60_hw.h
+
+#define DRV_NAME enc28j60
+#define DRV_VERSION 1.01
+
+#define SPI_OPLEN 1
+
+#define ENC28J60_MSG_DEFAULT \
+ (NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN | NETIF_MSG_LINK)
+
+/* Buffer size required for the largest SPI transfer (i.e., reading a
+ * frame). */
+#define SPI_TRANSFER_BUF_LEN (4 + MAX_FRAMELEN)
+
+#define TX_TIMEOUT (4 * HZ)
+
+/* Max TX retries in case of collision as suggested by errata datasheet */
+#define MAX_TX_RETRYCOUNT 16
+
+enum {
+ RXFILTER_NORMAL,
+ RXFILTER_MULTI,
+ RXFILTER_PROMISC
+};
+
+/* Driver local data */
+struct enc28j60_net {
+ struct net_device *netdev;
+ struct spi_device *spi;
+ struct mutex lock;
+ struct sk_buff *tx_skb;
+ struct work_struct tx_work;
+ struct work_struct irq_work;
+ struct work_struct setrx_work;
+ struct work_struct restart_work;
+ u8 bank; /* current register bank selected */
+ u16 next_pk_ptr; /* next packet pointer within FIFO */
+ u16 max_pk_counter; /* statistics: max packet counter */
+ u16 tx_retry_count;
+ bool hw_enable;
+ bool full_duplex;
+ int rxfilter;
+ u32 msg_enable;
+ u8 spi_transfer_buf[SPI_TRANSFER_BUF_LEN];
+};
+
+/* use ethtool to change the level for any given device */
+static struct {
+ u32 msg_enable;
+} debug = { -1 };
+
+/*
+ * SPI read buffer
+ * wait for the SPI transfer and copy received data to destination
+ */
+static int
+spi_read_buf(struct enc28j60_net *priv, int len, u8 *data)
+{
+ u8 *rx_buf = priv-spi_transfer_buf + 4;
+ u8 *tx_buf = priv-spi_transfer_buf;
+ struct spi_transfer t = {
+ .tx_buf = tx_buf,
+ .rx_buf = rx_buf,
+ .len = SPI_OPLEN + len,
+ };
+ struct spi_message msg;
+ int ret;
+
+ tx_buf[0] = ENC28J60_READ_BUF_MEM;
+ tx_buf[1] = tx_buf[2] = tx_buf[3] = 0; /* don't care */
+
+ spi_message_init(msg);
+ spi_message_add_tail(t, msg);
+ ret = spi_sync(priv-spi, msg);
+ if (ret == 0) {
+ memcpy(data, rx_buf[SPI_OPLEN], len);
+ ret = msg.status;
+ }
+ if (ret netif_msg_drv(priv))
+ printk(KERN_DEBUG DRV_NAME : %s() failed: ret = %d\n,
+ __FUNCTION__, ret);
+
+ return ret;
+}
+
+/*
+ * SPI write buffer
+ */
+static int spi_write_buf(struct enc28j60_net *priv, int len,
+ const u8 *data)
+{
+ int ret;
+
+ if (len SPI_TRANSFER_BUF_LEN - 1 || len = 0)
+ ret = -EINVAL;
+ else {
+ priv-spi_transfer_buf[0] = ENC28J60_WRITE_BUF_MEM;
+ memcpy(priv-spi_transfer_buf[1], data, len);
+ ret = spi_write(priv-spi, priv-spi_transfer_buf
Re: [PATCH 1/2] add driver for enc28j60 ethernet chip
Hi Stephen,
thank you for your suggestions.
I already applied trivial fixes, but I have questions on some points,
see inline.
Stephen Hemminger wrote:
General comments:
* device driver does no carrier detection. This makes it useless
for bridging, bonding, or any form of failover.
* use msglevel method (via ethtool) to control debug messages
rather than kernel configuration. This allows enabling debugging
without recompilation which is important in distributions.
* Please add ethtool support
* Consider using NAPI
Can you point me to a possibly simple driver that uses ethtool and NAPI?
Or other example that I can use for reference.
May be the skeleton should be updated.
* use netdev_priv(netdev) rather than netdev-priv
I can't find where I used netdev-priv, may be do you mean priv-netdev?
My comments:
diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
new file mode 100644
index 000..6182473
--- /dev/null
+++ b/drivers/net/enc28j60.c
@@ -0,0 +1,1400 @@
+/*
+ * Microchip ENC28J60 ethernet driver (MAC + PHY)
+ *
+ * Copyright (C) 2007 Eurek srl
+ * Author: Claudio Lanconelli [EMAIL PROTECTED]
+ * based on enc28j60.c written by David Anders for 2.4 kernel version
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * $Id: enc28j60.c,v 1.10 2007/12/10 16:59:37 claudio Exp $
+ */
+
+#include linux/autoconf.h
Use msglvl instead see netdevice.h
Ok
+
+#if CONFIG_ENC28J60_DBGLEVEL 1
+# define VERBOSE_DEBUG
+#endif
+#if CONFIG_ENC28J60_DBGLEVEL 0
+# define DEBUG
+#endif
+
...
+
+#define MY_TX_TIMEOUT ((500*HZ)/1000)
That is a really short TX timeout, should be 2 seconds at least not 1/2 sec.
Having it less than a second causes increased wakeups.
Ok
+
+/* Max TX retries in case of collision as suggested by errata datasheet */
+#define MAX_TX_RETRYCOUNT 16
+
+/* Driver local data */
+struct enc28j60_net_local {
Rename something shorter like enc28j60_net or just enc28j60?
Ok, renamed enc28j60_net
+ struct net_device_stats stats;
net_device_stats are now in net_device.
+ struct net_device *netdev;
+ struct spi_device *spi;
+ struct semaphore semlock; /* protect spi_transfer_buf */
Use mutex (or spin_lock) rather than semaphore
Ok
+ uint8_t *spi_transfer_buf;
+ struct sk_buff *tx_skb;
+ struct work_struct tx_work;
+ struct work_struct irq_work;
Not sure why you need to have workqueue's for
tx_work and irq_work, rather than using a spin_lock
and doing directly.
I need irq_work for sure because it needs to go sleep. Any
access to enc28j60 registers are through SPI blocking transaction,
spi_sync().
I'm not sure if the hard_start_xmit() can go sleep, so I used a work
queue to tx too.
+ int bank; /* current register bank selected */
bank is really unsigned.
+ uint16_t next_pk_ptr; /* next packet pointer within FIFO */
+ int max_pk_counter; /* statistics: max packet counter */
+ int tx_retry_count;
these are used as unsigned.
+ int hw_enable;
+};
+
+/* Selects Full duplex vs. Half duplex mode */
+static int full_duplex = 0;
Use ethtool for this.
Ok
+
+static int enc28j60_send_packet(struct sk_buff *skb, struct net_device *dev);
+static int enc28j60_net_close(struct net_device *dev);
+static struct net_device_stats *enc28j60_net_get_stats(struct net_device *dev);
+static void enc28j60_set_multicast_list(struct net_device *dev);
+static void enc28j60_net_tx_timeout(struct net_device *ndev);
+
+static int enc28j60_chipset_init(struct net_device *dev);
+static void enc28j60_hw_disable(struct enc28j60_net_local *priv);
+static void enc28j60_hw_enable(struct enc28j60_net_local *priv);
+static void enc28j60_hw_rx(struct enc28j60_net_local *priv);
+static void enc28j60_hw_tx(struct enc28j60_net_local *priv);
If you order functions correctly in code, you don't have to waste lots
of space with all these forward declarations.
...
Ok
+ const char *msg);
+
+/*
+ * SPI read buffer
+ * wait for the SPI transfer and copy received data to destination
+ */
+static int
+spi_read_buf(struct enc28j60_net_local *priv, int len, uint8_t *data)
+{
+ uint8_t *rx_buf;
+ uint8_t *tx_buf;
+ struct spi_transfer t;
+ struct spi_message msg;
+ int ret, slen;
+
+ slen = 1;
+ memset(t, 0, sizeof(t));
+ t.tx_buf = tx_buf = priv-spi_transfer_buf;
+ t.rx_buf = rx_buf = priv-spi_transfer_buf + 4;
+ t.len = slen + len;
If you use structure initializer you can avoid having to do
the memset
Ok
+
+ down(priv-semlock);
+ tx_buf[0] = ENC28J60_READ_BUF_MEM;
+ tx_buf[1] = tx_buf[2] = tx_buf[3] = 0; /* don't care
[PATCH 1/2] add driver for enc28j60 ethernet chip
These patches add support for Microchip enc28j60 ethernet chip
controlled via SPI.
I tested it on my custom board (S162) with ARM9 s3c2442 SoC.
Any comments are welcome.
Signed-off-by: Claudio Lanconelli [EMAIL PROTECTED]
drivers/net/enc28j60.c| 1400 +
drivers/net/enc28j60_hw.h | 303 ++
2 files changed, 1703 insertions(+), 0 deletions(-)
diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
new file mode 100644
index 000..6182473
--- /dev/null
+++ b/drivers/net/enc28j60.c
@@ -0,0 +1,1400 @@
+/*
+ * Microchip ENC28J60 ethernet driver (MAC + PHY)
+ *
+ * Copyright (C) 2007 Eurek srl
+ * Author: Claudio Lanconelli [EMAIL PROTECTED]
+ * based on enc28j60.c written by David Anders for 2.4 kernel version
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * $Id: enc28j60.c,v 1.10 2007/12/10 16:59:37 claudio Exp $
+ */
+
+#include linux/autoconf.h
+
+#if CONFIG_ENC28J60_DBGLEVEL 1
+# define VERBOSE_DEBUG
+#endif
+#if CONFIG_ENC28J60_DBGLEVEL 0
+# define DEBUG
+#endif
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/types.h
+#include linux/fcntl.h
+#include linux/interrupt.h
+#include linux/slab.h
+#include linux/string.h
+#include linux/spinlock.h
+#include linux/errno.h
+#include linux/init.h
+#include linux/netdevice.h
+#include linux/etherdevice.h
+#include linux/skbuff.h
+#include linux/bitops.h
+#include linux/delay.h
+#include linux/spi/spi.h
+#include asm/semaphore.h
+
+#include enc28j60_hw.h
+
+/* Buffer size required for the largest SPI transfer (i.e., reading a
+ * frame). */
+#define SPI_TRANSFER_BUF_LEN (4 + MAX_FRAMELEN)
+
+#define MY_TX_TIMEOUT ((500*HZ)/1000)
+
+/* Max TX retries in case of collision as suggested by errata datasheet */
+#define MAX_TX_RETRYCOUNT 16
+
+/* Driver local data */
+struct enc28j60_net_local {
+ struct net_device_stats stats;
+
+ struct net_device *netdev;
+ struct spi_device *spi;
+ struct semaphore semlock; /* protect spi_transfer_buf */
+ uint8_t *spi_transfer_buf;
+ struct sk_buff *tx_skb;
+ struct work_struct tx_work;
+ struct work_struct irq_work;
+ int bank; /* current register bank selected */
+ uint16_t next_pk_ptr; /* next packet pointer within FIFO */
+ int max_pk_counter; /* statistics: max packet counter */
+ int tx_retry_count;
+ int hw_enable;
+};
+
+/* Selects Full duplex vs. Half duplex mode */
+static int full_duplex = 0;
+
+static int enc28j60_send_packet(struct sk_buff *skb, struct net_device *dev);
+static int enc28j60_net_close(struct net_device *dev);
+static struct net_device_stats *enc28j60_net_get_stats(struct net_device *dev);
+static void enc28j60_set_multicast_list(struct net_device *dev);
+static void enc28j60_net_tx_timeout(struct net_device *ndev);
+
+static int enc28j60_chipset_init(struct net_device *dev);
+static void enc28j60_hw_disable(struct enc28j60_net_local *priv);
+static void enc28j60_hw_enable(struct enc28j60_net_local *priv);
+static void enc28j60_hw_rx(struct enc28j60_net_local *priv);
+static void enc28j60_hw_tx(struct enc28j60_net_local *priv);
+
+/* Basic SPI operations */
+static int spi_read_buf(struct enc28j60_net_local *priv, int len,
+ uint8_t *data);
+static int spi_write_buf(struct enc28j60_net_local *priv, int len,
+ const uint8_t * data);
+static uint8_t spi_read_op(struct enc28j60_net_local *priv, uint8_t op,
+ uint8_t addr);
+static int spi_write_op(struct enc28j60_net_local *priv, uint8_t op,
+ uint8_t addr, uint8_t val);
+
+/* Low level routines */
+
+/* utility function for register access routines */
+static void enc28j60_set_bank(struct enc28j60_net_local *priv, uint8_t address);
+
+static inline int enc28j60_regb_read(struct enc28j60_net_local *priv,
+ uint8_t address);
+static inline int enc28j60_regw_read(struct enc28j60_net_local *priv,
+ uint8_t address);
+static inline void enc28j60_regb_write(struct enc28j60_net_local *priv,
+ uint8_t address, uint8_t data);
+static inline void enc28j60_regw_write(struct enc28j60_net_local *priv,
+ uint8_t address, uint16_t data);
+
+static inline void enc28j60_reg_bfset(struct enc28j60_net_local *priv,
+ uint8_t reg, uint8_t mask);
+static inline void enc28j60_reg_bfclr(struct enc28j60_net_local *priv,
+ uint8_t reg, uint8_t mask);
+
+static void enc28j60_soft_reset(struct enc28j60_net_local *priv);
+
+/* debug routines */
+static void dump_packet(struct enc28j60_net_local *priv, const char *msg,
+ int len, const char *data);
+static void enc28j60_dump_tsv(struct enc28j60_net_local *priv, const char *msg,
+ uint8_t tsv[TSV_SIZE]);
+static void enc28j60_dump_rsv(struct enc28j60_net_local *priv, const char *msg,
+ uint16_t pk_ptr, int len, uint16_t sts);
+static void enc28j60_dump_regs(struct enc28j60_net_local *priv
[PATCH 2/2] add driver for enc28j60 ethernet chip
Second part contains changes on Kconfig and Makefile on dir drivers/net Signed-off-by: Claudio Lanconelli [EMAIL PROTECTED] drivers/net/Kconfig | 26 ++ drivers/net/Makefile |1 + 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index d9107e5..e643e0f 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -922,6 +922,32 @@ config DM9000 To compile this driver as a module, choose M here. The module will be called dm9000. +config ENC28J60 + tristate ENC28J60 support + depends on EXPERIMENTAL SPI NET_ETHERNET + select CRC32 + select MII + ---help--- + Support for the Microchip EN28J60 ethernet chip. + + To compile this driver as a module, choose M here and read + file:Documentation/networking/net-modules.txt. The module will be + called enc28j60. + +config ENC28J60_DBGLEVEL + int Debugging verbosity (0 = quiet, 3 = noisy) + depends on ENC28J60 + default 0 + ---help--- + Determines the verbosity level of the enc28j60 driver debugging messages. + +config ENC28J60_WRITEVERIFY + bool Enable write verify + depends on ENC28J60 + ---help--- + Enable the verify after the buffer write useful for debugging purpose. + If unsure, say N. + config SMC911X tristate SMSC LAN911[5678] support select CRC32 diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 0e5fde4..e91e3a3 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -216,6 +216,7 @@ obj-$(CONFIG_DM9000) += dm9000.o obj-$(CONFIG_FEC_8XX) += fec_8xx/ obj-$(CONFIG_PASEMI_MAC) += pasemi_mac.o obj-$(CONFIG_MLX4_CORE) += mlx4/ +obj-$(CONFIG_ENC28J60) += enc28j60.o obj-$(CONFIG_MACB) += macb.o
