On Fri, 2006-12-22 at 10:24 -0500, David Hollis wrote: > On Fri, 2006-12-22 at 15:07 +1100, Benjamin Herrenschmidt wrote: > > Hi Andy ! > > > > I've been looking at porting various drivers (EMAC, sungem, > > spider_net, ...) to the generic PHY stuff. However, I have one > > significant problem here. > > > > > One solution would be to change it to use a mutex instead of a lock as > > well, though that would change the requirements of where phy_start/stop > > can be called, and use a delayed work queue instead of a timer. > > Wouldn't this change also allow it to be used with USB Ethernet devices? > I was looking at porting the asix.c drive to use the PHY layer but the > locking killed that effort since USB devices can't do spinlocks without > hosing things up.
Yes, possibly. At the end of the day, I get the locking in the driver down to something like: - All rx/tx operatations are done between the NAPI poll and the hard_start_xmit() routine. Locking here is done with the netif_tx_lock if needed (depends on the driver). Those can't sleep. - Everything else is task level and locks against the fast path above with stopping NAPI poll and stopping tx. I have my driver_netif_stop/start routines doing that and a bit more (see below). - There is at least one issue with set_multicast which is called with the lock held. What I do here is that my driver_netif_stop above also take the lock, set a flag tellign set-mcast to say away, release the lock. start() on the other hand take the lock, clears that flag, checks if another flag was set by set-mcast telling it was here, and does the mcast setting if that was the case. I much prefer that than having most of the driver operations locked with the tx lock or some other or run at softirq. Some of the reset/recovery operations can be slow, PHY operations too, and thus it's all better run at task level. In addition, MDIO access might be able to sleep waiting on an interrupt signaling the completion of the access. Ben. - 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
