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).
