| Message ID | 91377107-d931-55a4-b231-0c530aa9359b@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: improve stopping PHY | expand |
On Wed, Jan 16, 2019 at 09:25:54PM +0100, Heiner Kallweit wrote: > The call to the phylib state machine in phy_stop() just ensures that > the state machine isn't re-triggered, but a state machine call may > be scheduled already. So lets's call phy_stop_machine(). > This also allows to get rid of the call to phy_stop_machine() in > phy_disconnect(). > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> At some point it would be good to audit the code and see if anything is still used in interrupt context. PHY interrupt handling is now done in a threaded interrupt handler. So i think it is just callers to phy_mac_interrupt() that need to be checked. It could be the work queue can be removed and everything done synchronous. Andrew
On 16.01.2019 22:59, Andrew Lunn wrote: > On Wed, Jan 16, 2019 at 09:25:54PM +0100, Heiner Kallweit wrote: >> The call to the phylib state machine in phy_stop() just ensures that >> the state machine isn't re-triggered, but a state machine call may >> be scheduled already. So lets's call phy_stop_machine(). >> This also allows to get rid of the call to phy_stop_machine() in >> phy_disconnect(). >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > At some point it would be good to audit the code and see if anything > is still used in interrupt context. PHY interrupt handling is now done > in a threaded interrupt handler. So i think it is just callers to > phy_mac_interrupt() that need to be checked. It could be the work > queue can be removed and everything done synchronous. > Yes, I think few calls to phy_trigger_machine() could be changed to be synchronous. But I think the workqueue will still be needed: - for phy_mac_interrupt() which is typically called from hard irq context - for polling link status (delayed work) > Andrew > Heiner
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 2ffe08537..96e9ec252 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -853,6 +853,7 @@ void phy_stop(struct phy_device *phydev) mutex_unlock(&phydev->lock); phy_state_machine(&phydev->state_queue.work); + phy_stop_machine(phydev); /* Cannot call flush_scheduled_work() here as desired because * of rtnl_lock(), but PHY_HALTED shall guarantee irq handler diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 46ec71021..60cd976f4 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1007,8 +1007,6 @@ void phy_disconnect(struct phy_device *phydev) if (phydev->irq > 0) phy_stop_interrupts(phydev); - phy_stop_machine(phydev); - phydev->adjust_link = NULL; phy_detach(phydev);
The call to the phylib state machine in phy_stop() just ensures that the state machine isn't re-triggered, but a state machine call may be scheduled already. So lets's call phy_stop_machine(). This also allows to get rid of the call to phy_stop_machine() in phy_disconnect(). Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy.c | 1 + drivers/net/phy/phy_device.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)