Andy Fleming wrote:

On May 31, 2005, at 12:59, Stephen Hemminger wrote:

quoted

Here are some patches:
    * allow phy's to be modules
    * use driver owner for ref count
    * make local functions static where ever possible

I agree with all these.

quoted

    * get rid of bus read may sleep implication in comment.
      since you are holding phy spin lock it better not!!

But not this one.  The phy_read and phy_write functions are reading  
from and writing to a bus.  It is a reasonable implementation to have  
the operation block in the bus driver, and be awoken when an  
interrupt signals the operation is done.  All of the phydev spinlocks  
have been arranged so as to prevent the lock being taken during  
interrupt time.

Unless I've misunderstood spinlocks (it wouldn't be the first time),  
as long as the lock is never taken in interrupt time, it should be ok  
to hold the lock, and wait for an interrupt before clearing the lock.

The problem is that sleeping is defined in the linux kernel as meaning 
waiting on a mutual exclusion
primitive (like semaphore) that puts the current thread to sleep. It is 
not legal to sleep with a spinlock held.
In the phy_read code you do:
   
   spin_lock_bh(&bus->mdio_lock);
    retval = bus->read(bus, phydev->addr, regnum);
    spin_unlock_bh(&bus->mdio_lock);

If the bus->read function were to do something like start a request and 
wait on a semaphore, then
you would be sleeping with a spin lock held.  So bus->read can not 
sleep! (as sleep is defined in the
linux kernel).