PHY_HALTED and PHY_READY both are non-started states and quite similar.
Major difference is that phy_start() changes from PHY_HALTED to
PHY_RESUMING which doesn't reconfigure aneg (what PHY_UP does).
There's no guarantee that PHY registers are completely untouched when
waking up from power-down, e.g. after system suspend. Therefore it's
safer to reconfigure aneg also when starting from PHY_HALTED. This can
be achieved and state machine made simpler by making PHY_HALTED going
to PHY_READY after having stopped everything. Then the only way up is
over PHY_UP. Also let's warn in phy_start() if it's called from a
state other than PHY_READY.
As part of the change PHY_HALTED is renamed to PHY_HALT to reflect that
it is a transition state.
Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
---
drivers/net/phy/phy.c | 41 +++++++++++++++++------------------------
include/linux/phy.h | 16 ++++++++--------
2 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d33e7b3ca..2a69d947e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -47,12 +47,12 @@ static const char *phy_state_to_str(enum phy_state st)
switch (st) {
PHY_STATE_STR(DOWN)
PHY_STATE_STR(READY)
+ PHY_STATE_STR(HALT)
PHY_STATE_STR(UP)
PHY_STATE_STR(RUNNING)
PHY_STATE_STR(NOLINK)
PHY_STATE_STR(FORCING)
PHY_STATE_STR(CHANGELINK)
- PHY_STATE_STR(HALTED)
PHY_STATE_STR(RESUMING)
}
@@ -733,7 +733,7 @@ static void phy_error(struct phy_device *phydev)
WARN_ON(1);
mutex_lock(&phydev->lock);
- phydev->state = PHY_HALTED;
+ phydev->state = PHY_HALT;
mutex_unlock(&phydev->lock);
phy_trigger_machine(phydev);
@@ -859,16 +859,11 @@ void phy_stop(struct phy_device *phydev)
if (phy_interrupt_is_valid(phydev))
phy_disable_interrupts(phydev);
- phydev->state = PHY_HALTED;
+ phydev->state = PHY_HALT;
mutex_unlock(&phydev->lock);
phy_state_machine(&phydev->state_queue.work);
-
- /* Cannot call flush_scheduled_work() here as desired because
- * of rtnl_lock(), but PHY_HALTED shall guarantee irq handler
- * will not reenable interrupts.
- */
}
EXPORT_SYMBOL(phy_stop);
@@ -888,29 +883,26 @@ void phy_start(struct phy_device *phydev)
mutex_lock(&phydev->lock);
- switch (phydev->state) {
- case PHY_READY:
- phydev->state = PHY_UP;
- break;
- case PHY_HALTED:
+ if (phydev->state == PHY_READY) {
/* if phy was suspended, bring the physical link up again */
__phy_resume(phydev);
/* make sure interrupts are re-enabled for the PHY */
if (phy_interrupt_is_valid(phydev)) {
err = phy_enable_interrupts(phydev);
- if (err < 0)
- break;
+ if (err < 0) {
+ WARN_ON(1);
+ goto out;
+ }
}
-
- phydev->state = PHY_RESUMING;
- break;
- default:
- break;
+ phydev->state = PHY_UP;
+ phy_trigger_machine(phydev);
+ } else {
+ WARN(1, "called from state %s\n",
+ phy_state_to_str(phydev->state));
}
+out:
mutex_unlock(&phydev->lock);
-
- phy_trigger_machine(phydev);
}
EXPORT_SYMBOL(phy_start);
@@ -962,12 +954,13 @@ void phy_state_machine(struct work_struct *work)
phy_link_down(phydev, false);
}
break;
- case PHY_HALTED:
+ case PHY_HALT:
if (phydev->link) {
phydev->link = 0;
phy_link_down(phydev, true);
do_suspend = true;
}
+ phydev->state = PHY_READY;
break;
}
@@ -990,7 +983,7 @@ void phy_state_machine(struct work_struct *work)
* PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
* between states from phy_mac_interrupt().
*
- * In state PHY_HALTED the PHY gets suspended, so rescheduling the
+ * In state PHY_HALT the PHY gets suspended, so rescheduling the
* state machine would be pointless and possibly error prone when
* called from phy_disconnect() synchronously.
*/
diff --git a/include/linux/phy.h b/include/linux/phy.h
index da039f211..21e553f51 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -288,38 +288,38 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int
addr);
*
* NOLINK: PHY is up, but not currently plugged in.
* - irq or timer will set RUNNING if link comes back
- * - phy_stop moves to HALTED
+ * - phy_stop moves to HALT
*
* FORCING: PHY is being configured with forced settings
* - if link is up, move to RUNNING
* - If link is down, we drop to the next highest setting, and
* retry (FORCING) after a timeout
- * - phy_stop moves to HALTED
+ * - phy_stop moves to HALT
*
* RUNNING: PHY is currently up, running, and possibly sending
* and/or receiving packets
* - irq or timer will set NOLINK if link goes down
- * - phy_stop moves to HALTED
+ * - phy_stop moves to HALT
*
* CHANGELINK: PHY experienced a change in link state
* - timer moves to RUNNING if link
* - timer moves to NOLINK if the link is down
- * - phy_stop moves to HALTED
+ * - phy_stop moves to HALT
*
- * HALTED: PHY is up, but no polling or interrupts are done. Or
+ * HALT: PHY is up, but no polling or interrupts are done. Or
* PHY is in an error state.
*
- * - phy_start moves to RESUMING
+ * - moves to READY
*
* RESUMING: PHY was halted, but now wants to run again.
* - If we are forcing, or aneg is done, timer moves to RUNNING
* - If aneg is not done, timer moves to AN
- * - phy_stop moves to HALTED
+ * - phy_stop moves to HALT
*/
enum phy_state {
PHY_DOWN = 0,
PHY_READY,
- PHY_HALTED,
+ PHY_HALT,
PHY_UP,
PHY_RUNNING,
PHY_NOLINK,
--
2.20.1
