Re: RFC: PHY Abstraction Layer II

[Stephen Hemminger <shemminger@osdl.org>]

Andy Fleming wrote:

>
> On May 31, 2005, at 12:59, Stephen Hemminger wrote:
>
>> 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.
>
>>     * 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).