netdev - [PATCH RFC v2 5/7] net: phy: make phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Mar 2018 22:23:58 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Geert Uytterhoeven <geert+renesas@...der.be>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [PATCH RFC v2 5/7] net: phy: make phy_stop synchronous

Currently phy_stop() just sets the state to PHY_HALTED and relies on the
state machine to do the remaining work. It can take up to 1s until the
state machine runs again what causes issues in situations where e.g.
driver / device is brought down directly after executing phy_stop().

Fix this by executing all phy_stop() activities synchronously.

Add a function phy_stop_suspending() which does basically the same as
phy_stop() and just adopts the state adjustment logic from
phy_stop_machine() to inform the resume callback about the status of
the PHY before suspending.

Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
---
Changes in v2:
- Extend documenting comment for phy_stop_suspend
- Address error reported by Geert by changing call to phy_link_down()
  to not trigger a linkwatch event
---
 drivers/net/phy/phy.c | 70 ++++++++++++++++++++++++++++++++++++++-------------
 include/linux/phy.h   |  1 +
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 0ca1672a..6ffb9952 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -737,21 +737,45 @@ int phy_stop_interrupts(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_stop_interrupts);
 
-/**
- * phy_stop - Bring down the PHY link, and stop checking the status
- * @phydev: target phy_device struct
- */
-void phy_stop(struct phy_device *phydev)
+static void phy_link_up(struct phy_device *phydev)
+{
+	phydev->phy_link_change(phydev, true, true);
+	phy_led_trigger_change_speed(phydev);
+}
+
+static void phy_link_down(struct phy_device *phydev, bool do_carrier)
+{
+	phydev->phy_link_change(phydev, false, do_carrier);
+	phy_led_trigger_change_speed(phydev);
+}
+
+static void __phy_stop(struct phy_device *phydev, bool suspending)
 {
 	mutex_lock(&phydev->lock);
 
 	if (PHY_HALTED == phydev->state)
 		goto out_unlock;
 
+	/* stop state machine */
+	cancel_delayed_work_sync(&phydev->state_queue);
+
 	if (phy_interrupt_is_valid(phydev))
 		phy_disable_interrupts(phydev);
 
-	phydev->state = PHY_HALTED;
+	if (phydev->link) {
+		phydev->link = 0;
+		if (phydev->adjust_link)
+			phy_link_down(phydev, false);
+	}
+
+	phy_suspend(phydev);
+
+	if (suspending) {
+		if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
+			phydev->state = PHY_UP;
+	} else {
+		phydev->state = PHY_HALTED;
+	}
 
 out_unlock:
 	mutex_unlock(&phydev->lock);
@@ -761,8 +785,30 @@ void phy_stop(struct phy_device *phydev)
 	 * will not reenable interrupts.
 	 */
 }
+
+/**
+ * phy_stop - Bring down the PHY link, and stop checking the status
+ * @phydev: target phy_device struct
+ */
+void phy_stop(struct phy_device *phydev)
+{
+	__phy_stop(phydev, false);
+}
 EXPORT_SYMBOL(phy_stop);
 
+/**
+ * phy_stop_suspending - Bring down the PHY link, preparing for system suspend
+ * @phydev: target phy_device struct
+ *
+ * Description: Basically the same as phy_stop(), just sets the state to UP
+ * (unless it wasn't up yet)
+ */
+void phy_stop_suspending(struct phy_device *phydev)
+{
+	__phy_stop(phydev, true);
+}
+EXPORT_SYMBOL(phy_stop_suspending);
+
 /**
  * phy_start - start or restart a PHY device
  * @phydev: target phy_device struct
@@ -804,18 +850,6 @@ void phy_start(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_start);
 
-static void phy_link_up(struct phy_device *phydev)
-{
-	phydev->phy_link_change(phydev, true, true);
-	phy_led_trigger_change_speed(phydev);
-}
-
-static void phy_link_down(struct phy_device *phydev, bool do_carrier)
-{
-	phydev->phy_link_change(phydev, false, do_carrier);
-	phy_led_trigger_change_speed(phydev);
-}
-
 /**
  * phy_state_machine - Handle the state machine
  * @work: work_struct that describes the work to be done
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bc7aa93c..780c2690 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -941,6 +941,7 @@ void phy_disconnect(struct phy_device *phydev);
 void phy_detach(struct phy_device *phydev);
 void phy_start(struct phy_device *phydev);
 void phy_stop(struct phy_device *phydev);
+void phy_stop_suspending(struct phy_device *phydev);
 int phy_start_aneg(struct phy_device *phydev);
 int phy_aneg_done(struct phy_device *phydev);
 
-- 
2.16.2