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
