From e8cfd9d6c7727a067b38dbe7655ca02377fdb301 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Tue, 18 Sep 2018 21:56:32 +0200 Subject: net: phy: call state machine synchronously in phy_stop phy_stop() may be called e.g. when suspending, therefore all needed actions should be performed synchronously. Therefore add a synchronous call to the state machine. Signed-off-by: Heiner Kallweit Tested-by: Geert Uytterhoeven Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 1ee2587..a5e0f07 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -861,6 +861,8 @@ void phy_stop(struct phy_device *phydev) out_unlock: 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 phy_change() * will not reenable interrupts. -- cgit v1.1 From 075ddebc3283e83ac56fcc8f4bb44c15cef0d7ce Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Thu, 20 Sep 2018 22:34:25 +0200 Subject: net: phy: don't reschedule state machine when PHY is halted When being in state PHY_HALTED we don't have to reschedule the state machine, phy_start() will start it again. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index a5e0f07..a1f8e48 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1123,9 +1123,13 @@ void phy_state_machine(struct work_struct *work) /* Only re-schedule a PHY state machine change if we are polling the * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving - * between states from phy_mac_interrupt() + * between states from phy_mac_interrupt(). + * + * In state PHY_HALTED the PHY gets suspended, so rescheduling the + * state machine would be pointless and possibly error prone when + * called from phy_disconnect() synchronously. */ - if (phy_polling_mode(phydev)) + if (phy_polling_mode(phydev) && old_state != PHY_HALTED) queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, PHY_STATE_TIME * HZ); } -- cgit v1.1 From 9f2959b6b52d43326b2f6a0e0d7ffe6f4fc3b5ca Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 28 Sep 2018 08:51:09 +0200 Subject: net: phy: improve handling delayed work Using mod_delayed_work() allows to simplify handling delayed work and removes the need for the sync parameter in phy_trigger_machine(). Also introduce a helper phy_queue_state_machine() to encapsulate the low-level delayed work calls. No functional change intended. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index a1f8e48..14509a8 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -537,7 +537,7 @@ out_unlock: mutex_unlock(&phydev->lock); if (trigger) - phy_trigger_machine(phydev, sync); + phy_trigger_machine(phydev); return err; } @@ -635,6 +635,13 @@ int phy_speed_up(struct phy_device *phydev) } EXPORT_SYMBOL_GPL(phy_speed_up); +static void phy_queue_state_machine(struct phy_device *phydev, + unsigned int secs) +{ + mod_delayed_work(system_power_efficient_wq, &phydev->state_queue, + secs * HZ); +} + /** * phy_start_machine - start PHY state machine tracking * @phydev: the phy_device struct @@ -647,7 +654,7 @@ EXPORT_SYMBOL_GPL(phy_speed_up); */ void phy_start_machine(struct phy_device *phydev) { - queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, HZ); + phy_queue_state_machine(phydev, 1); } EXPORT_SYMBOL_GPL(phy_start_machine); @@ -655,19 +662,14 @@ EXPORT_SYMBOL_GPL(phy_start_machine); * phy_trigger_machine - trigger the state machine to run * * @phydev: the phy_device struct - * @sync: indicate whether we should wait for the workqueue cancelation * * Description: There has been a change in state which requires that the * state machine runs. */ -void phy_trigger_machine(struct phy_device *phydev, bool sync) +void phy_trigger_machine(struct phy_device *phydev) { - if (sync) - cancel_delayed_work_sync(&phydev->state_queue); - else - cancel_delayed_work(&phydev->state_queue); - queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0); + phy_queue_state_machine(phydev, 0); } /** @@ -703,7 +705,7 @@ static void phy_error(struct phy_device *phydev) phydev->state = PHY_HALTED; mutex_unlock(&phydev->lock); - phy_trigger_machine(phydev, false); + phy_trigger_machine(phydev); } /** @@ -745,7 +747,7 @@ static irqreturn_t phy_change(struct phy_device *phydev) mutex_unlock(&phydev->lock); /* reschedule state queue work to run as soon as possible */ - phy_trigger_machine(phydev, true); + phy_trigger_machine(phydev); if (phy_interrupt_is_valid(phydev) && phy_clear_interrupt(phydev)) goto phy_err; @@ -911,7 +913,7 @@ void phy_start(struct phy_device *phydev) } mutex_unlock(&phydev->lock); - phy_trigger_machine(phydev, true); + phy_trigger_machine(phydev); } EXPORT_SYMBOL(phy_start); @@ -1130,8 +1132,7 @@ void phy_state_machine(struct work_struct *work) * called from phy_disconnect() synchronously. */ if (phy_polling_mode(phydev) && old_state != PHY_HALTED) - queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, - PHY_STATE_TIME * HZ); + phy_queue_state_machine(phydev, PHY_STATE_TIME); } /** -- cgit v1.1 From 6384e483239fd07a2d4393f888027118fecd4c6e Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Thu, 11 Oct 2018 19:31:47 +0200 Subject: net: phy: trigger state machine immediately in phy_start_machine When starting the state machine there may be work to be done immediately, e.g. if the initial state is PHY_UP then the state machine may trigger an autonegotiation. Having said that I see no need to wait a second until the state machine is run first time. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 14509a8..7044282 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -654,7 +654,7 @@ static void phy_queue_state_machine(struct phy_device *phydev, */ void phy_start_machine(struct phy_device *phydev) { - phy_queue_state_machine(phydev, 1); + phy_trigger_machine(phydev); } EXPORT_SYMBOL_GPL(phy_start_machine); -- cgit v1.1 From 74fb5e25a3e925286e80eb2e0253f88f2b44ec96 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Thu, 11 Oct 2018 22:36:56 +0200 Subject: net: phy: improve handling of PHY_RUNNING in state machine Handling of state PHY_RUNNING seems to be more complex than it needs to be. If not polling, then we don't have to do anything, we'll receive an interrupt and go to state PHY_CHANGELINK once the link goes down. If polling and link is down, we don't have to go the extra mile over PHY_CHANGELINK and call phy_read_status() again but can set status PHY_NOLINK directly. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 7044282..696955d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -941,7 +941,6 @@ void phy_state_machine(struct work_struct *work) bool needs_aneg = false, do_suspend = false; enum phy_state old_state; int err = 0; - int old_link; mutex_lock(&phydev->lock); @@ -1025,26 +1024,16 @@ void phy_state_machine(struct work_struct *work) } break; case PHY_RUNNING: - /* Only register a CHANGE if we are polling and link changed - * since latest checking. - */ - if (phy_polling_mode(phydev)) { - old_link = phydev->link; - err = phy_read_status(phydev); - if (err) - break; + if (!phy_polling_mode(phydev)) + break; - if (old_link != phydev->link) - phydev->state = PHY_CHANGELINK; - } - /* - * Failsafe: check that nobody set phydev->link=0 between two - * poll cycles, otherwise we won't leave RUNNING state as long - * as link remains down. - */ - if (!phydev->link && phydev->state == PHY_RUNNING) { - phydev->state = PHY_CHANGELINK; - phydev_err(phydev, "no link in PHY_RUNNING\n"); + err = phy_read_status(phydev); + if (err) + break; + + if (!phydev->link) { + phydev->state = PHY_NOLINK; + phy_link_down(phydev, true); } break; case PHY_CHANGELINK: -- cgit v1.1 From eb4c470a154d2d3176d08155c3136973661f69ad Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Thu, 11 Oct 2018 22:37:38 +0200 Subject: net: phy: simplify handling of PHY_RESUMING in state machine Simplify code for handling state PHY_RESUMING, no functional change intended. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 43 ++++++++++++++----------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 696955d..d03bdbb 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1059,41 +1059,26 @@ void phy_state_machine(struct work_struct *work) case PHY_RESUMING: if (AUTONEG_ENABLE == phydev->autoneg) { err = phy_aneg_done(phydev); - if (err < 0) + if (err < 0) { break; - - /* err > 0 if AN is done. - * Otherwise, it's 0, and we're still waiting for AN - */ - if (err > 0) { - err = phy_read_status(phydev); - if (err) - break; - - if (phydev->link) { - phydev->state = PHY_RUNNING; - phy_link_up(phydev); - } else { - phydev->state = PHY_NOLINK; - phy_link_down(phydev, false); - } - } else { + } else if (!err) { phydev->state = PHY_AN; phydev->link_timeout = PHY_AN_TIMEOUT; - } - } else { - err = phy_read_status(phydev); - if (err) break; - - if (phydev->link) { - phydev->state = PHY_RUNNING; - phy_link_up(phydev); - } else { - phydev->state = PHY_NOLINK; - phy_link_down(phydev, false); } } + + err = phy_read_status(phydev); + if (err) + break; + + if (phydev->link) { + phydev->state = PHY_RUNNING; + phy_link_up(phydev); + } else { + phydev->state = PHY_NOLINK; + phy_link_down(phydev, false); + } break; } -- cgit v1.1 From c45d7150656fc33181af5806c94bfe0e8f90c1a6 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Mon, 15 Oct 2018 21:25:13 +0200 Subject: net: phy: merge phy_start_aneg and phy_start_aneg_priv After commit 9f2959b6b52d ("net: phy: improve handling delayed work") the sync parameter isn't needed any longer in phy_start_aneg_priv(). This allows to merge phy_start_aneg() and phy_start_aneg_priv(). Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) (limited to 'drivers/net/phy/phy.c') diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d03bdbb..1d73ac3 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -482,16 +482,15 @@ static int phy_config_aneg(struct phy_device *phydev) } /** - * phy_start_aneg_priv - start auto-negotiation for this PHY device + * phy_start_aneg - start auto-negotiation for this PHY device * @phydev: the phy_device struct - * @sync: indicate whether we should wait for the workqueue cancelation * * Description: Sanitizes the settings (if we're not autonegotiating * them), and then calls the driver's config_aneg function. * If the PHYCONTROL Layer is operating, we change the state to * reflect the beginning of Auto-negotiation or forcing. */ -static int phy_start_aneg_priv(struct phy_device *phydev, bool sync) +int phy_start_aneg(struct phy_device *phydev) { bool trigger = 0; int err; @@ -541,20 +540,6 @@ out_unlock: return err; } - -/** - * phy_start_aneg - start auto-negotiation for this PHY device - * @phydev: the phy_device struct - * - * Description: Sanitizes the settings (if we're not autonegotiating - * them), and then calls the driver's config_aneg function. - * If the PHYCONTROL Layer is operating, we change the state to - * reflect the beginning of Auto-negotiation or forcing. - */ -int phy_start_aneg(struct phy_device *phydev) -{ - return phy_start_aneg_priv(phydev, true); -} EXPORT_SYMBOL(phy_start_aneg); static int phy_poll_aneg_done(struct phy_device *phydev) @@ -1085,7 +1070,7 @@ void phy_state_machine(struct work_struct *work) mutex_unlock(&phydev->lock); if (needs_aneg) - err = phy_start_aneg_priv(phydev, false); + err = phy_start_aneg(phydev); else if (do_suspend) phy_suspend(phydev); -- cgit v1.1