lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 07 Feb 2008 11:49:01 +0100 From: Claudio Lanconelli <lanconelli.claudio@...ar.com> To: netdev@...r.kernel.org CC: David Brownell <david-b@...bell.net> Subject: 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 majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists