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