[PATCH net-next 0/2] net: phy: improve phy_driver callback handle_interrupt

[PATCH net-next 0/2] net: phy: improve phy_driver callback handle_interrupt

[Heiner Kallweit]

did_interrupt() clears the interrupt, therefore handle_interrupt() can
not check which event triggered the interrupt. To overcome this
constraint and allow more flexibility for customer interrupt handlers,
let's decouple handle_interrupt() from parts of the phylib interrupt
handling. Custom interrupt handlers now have to implement the
did_interrupt() functionality in handle_interrupt() if needed.

Fortunately we have just one custom interrupt handler so far (in the
mscc PHY driver), convert it to the changed API and make use of the
benefits.

Heiner Kallweit (2):
  net: phy: improve phy_driver callback handle_interrupt
  net: phy: mscc: consider interrupt source in interrupt handler

 drivers/net/phy/mscc/mscc_main.c | 18 ++++++++++++++----
 drivers/net/phy/phy.c            | 26 ++++++++++++--------------
 include/linux/phy.h              |  3 ++-
 3 files changed, 28 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH net-next 1/2] net: phy: improve phy_driver callback handle_interrupt

[Heiner Kallweit]

did_interrupt() clears the interrupt, therefore handle_interrupt() can
not check which event triggered the interrupt. To overcome this
constraint and allow more flexibility for customer interrupt handlers,
let's decouple handle_interrupt() from parts of the phylib interrupt
handling. Custom interrupt handlers now have to implement the
did_interrupt() functionality in handle_interrupt() if needed.

Fortunately we have just one custom interrupt handler so far (in the
mscc PHY driver), convert it to the changed API.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/mscc/mscc_main.c | 11 +++++++++--
 drivers/net/phy/phy.c            | 26 ++++++++++++--------------
 include/linux/phy.h              |  3 ++-
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index cb4d65f81..4727aba8e 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1429,11 +1429,18 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	return ret;
 }
 
-static int vsc8584_handle_interrupt(struct phy_device *phydev)
+static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev)
 {
+	int irq_status;
+
+	irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS);
+	if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK))
+		return IRQ_NONE;
+
 	vsc8584_handle_macsec_interrupt(phydev);
 	phy_mac_interrupt(phydev);
-	return 0;
+
+	return IRQ_HANDLED;
 }
 
 static int vsc85xx_config_init(struct phy_device *phydev)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 355bfdef4..d71212a41 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -715,26 +715,24 @@ static int phy_disable_interrupts(struct phy_device *phydev)
 static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 {
 	struct phy_device *phydev = phy_dat;
+	struct phy_driver *drv = phydev->drv;
 
-	if (phydev->drv->did_interrupt && !phydev->drv->did_interrupt(phydev))
+	if (drv->handle_interrupt)
+		return drv->handle_interrupt(phydev);
+
+	if (drv->did_interrupt && !drv->did_interrupt(phydev))
 		return IRQ_NONE;
 
-	if (phydev->drv->handle_interrupt) {
-		if (phydev->drv->handle_interrupt(phydev))
-			goto phy_err;
-	} else {
-		/* reschedule state queue work to run as soon as possible */
-		phy_trigger_machine(phydev);
-	}
+	/* reschedule state queue work to run as soon as possible */
+	phy_trigger_machine(phydev);
 
 	/* did_interrupt() may have cleared the interrupt already */
-	if (!phydev->drv->did_interrupt && phy_clear_interrupt(phydev))
-		goto phy_err;
-	return IRQ_HANDLED;
+	if (!drv->did_interrupt && phy_clear_interrupt(phydev)) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
 
-phy_err:
-	phy_error(phydev);
-	return IRQ_NONE;
+	return IRQ_HANDLED;
 }
 
 /**
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6b872aed8..cb5a2182b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -23,6 +23,7 @@
 #include <linux/workqueue.h>
 #include <linux/mod_devicetable.h>
 #include <linux/u64_stats_sync.h>
+#include <linux/irqreturn.h>
 
 #include <linux/atomic.h>
 
@@ -568,7 +569,7 @@ struct phy_driver {
 	int (*did_interrupt)(struct phy_device *phydev);
 
 	/* Override default interrupt handling */
-	int (*handle_interrupt)(struct phy_device *phydev);
+	irqreturn_t (*handle_interrupt)(struct phy_device *phydev);
 
 	/* Clears up any memory if needed */
 	void (*remove)(struct phy_device *phydev);
-- 
2.25.1

[PATCH net-next 2/2] net: phy: mscc: consider interrupt source in interrupt handler

[Heiner Kallweit]

Trigger the respective interrupt handler functionality only if the
related interrupt source bit is set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/mscc/mscc_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 4727aba8e..2f6229a70 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1437,8 +1437,11 @@ static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev)
 	if (irq_status < 0 || !(irq_status & MII_VSC85XX_INT_MASK_MASK))
 		return IRQ_NONE;
 
-	vsc8584_handle_macsec_interrupt(phydev);
-	phy_mac_interrupt(phydev);
+	if (irq_status & MII_VSC85XX_INT_MASK_EXT)
+		vsc8584_handle_macsec_interrupt(phydev);
+
+	if (irq_status & MII_VSC85XX_INT_MASK_LINK_CHG)
+		phy_mac_interrupt(phydev);
 
 	return IRQ_HANDLED;
 }
-- 
2.25.1

Re: [PATCH net-next 0/2] net: phy: improve phy_driver callback handle_interrupt

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 16 Mar 2020 22:31:48 +0100

> did_interrupt() clears the interrupt, therefore handle_interrupt() can
> not check which event triggered the interrupt. To overcome this
> constraint and allow more flexibility for customer interrupt handlers,
> let's decouple handle_interrupt() from parts of the phylib interrupt
> handling. Custom interrupt handlers now have to implement the
> did_interrupt() functionality in handle_interrupt() if needed.
> 
> Fortunately we have just one custom interrupt handler so far (in the
> mscc PHY driver), convert it to the changed API and make use of the
> benefits.

Series applied, thanks Heiner.