net: phy: consider PHY_IGNORE_INTERRUPT in state machine PHY_NOLINK handling
Subsystems (3)
Heiner Kallweit
maintainerPATCH 1/1 net: phy: consider PHY_IGNORE_INTERRUPT in state machine PHY_NOLINK handling
We can bail out immediately also in case of PHY_IGNORE_INTERRUPT because phy_mac_interupt() informs us once the link is up.
--- drivers/net/phy/phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/phy/phy.c +1 -1
@@ -894,7 +894,7 @@ void phy_state_machine(struct work_struct *work) needs_aneg = true;break ;case PHY_NOLINK : -if (phy_interrupt_is_valid (phydev)) +if (phydev->irq !=PHY_POLL )break ; err =phy_read_status (phydev);
Signature · 1 line
2.17.1
David Miller
maintainerFrom: Heiner Kallweit <[email protected]>
Date: Wed, 30 May 2018 22:13:20 +0200
Quoted 4 lines
We can bail out immediately also in case of
PHY_IGNORE_INTERRUPTbecausephy_mac_interupt()informs us once the link is up.Signed-off-by Heiner Kallweit <[email protected]>
When state is PHY_NOLINK, the phy_mac_interrupt() code paths will change the state to PHY_CHANGELINK before queueing up the state machine invocation.
So I can't even see how we can enter phy_state_machine with ->state == PHY_NOLINK is the mac interrupt paths are being used properly.
Therefore it looks like the code as written is harmless.
Did you actually hit a problem with this test or is this a change based purely upon code inspection?
Heiner Kallweit
maintainerQuoted 8 lines · David Miller
From: Heiner Kallweit <[email protected]>
Date: Wed, 30 May 2018 22:13:20 +0200Quoted 4 lines
We can bail out immediately also in case of
PHY_IGNORE_INTERRUPTbecausephy_mac_interupt()informs us once the link is up.Signed-off-by Heiner Kallweit <[email protected]>
Quoted 8 lines
When state is
PHY_NOLINK, thephy_mac_interrupt()code paths will change the state toPHY_CHANGELINKbefore queueing up the state machine invocation.So I can't even see how we can enter
phy_state_machinewith ->state ==PHY_NOLINKis the mac interrupt paths are being used properly.
We could enter the state machine with PHY_NOLINK in case any other activity triggers a state machine run whilst the link is down. But I'm not sure whether such a scenario exists.
Quoted 5 lines
Therefore it looks like the code as written is harmless.
Did you actually hit a problem with this test or is this a change based purely upon code inspection?
Right, there is no actual problem, the existing code is harmless and the change is just based on code inspection. Small benefit is that it makes clearer that this code path is applicable in polling mode only.
From: Heiner Kallweit <[email protected]>
Date: Wed, 30 May 2018 22:13:20 +0200
Quoted 4 lines
We can bail out immediately also in case of
PHY_IGNORE_INTERRUPTbecausephy_mac_interupt()informs us once the link is up.Signed-off-by Heiner Kallweit <[email protected]>
Applied, thanks.