[PATCH net-next] net: phy: micrel: Change handler interrupt for lan8814

[PATCH net-next] net: phy: micrel: Change handler interrupt for lan8814

[Horatiu Vultur]

The lan8814 represents a package of 4 PHYs. All of them are sharing the
same interrupt line. So when a link was going down/up or a frame was
timestamped, then the interrupt handler of all the PHYs was called.
Which is all fine and expected but the problem is the way the handler
interrupt works.
Basically if one of the PHYs timestamp a frame, then all the other 3
PHYs were polling the status of the interrupt until that PHY actually
cleared the interrupt by reading the timestamp.
The reason of polling was in case another PHY was also timestamping a
frame at the same time, it could miss this interrupt. But this is not
the right approach, because it is the interrupt controller who needs to
call the interrupt handlers again if the interrupt line is still
active.
Therefore change this such when the interrupt handler is called check
only if the interrupt is for itself, otherwise just exit. In this way
save CPU usage.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 26ce0c5defcdd..2243ad1b88e70 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2794,13 +2794,11 @@ static void lan8814_get_rx_ts(struct kszphy_ptp_priv *ptp_priv)
 	} while (PTP_CAP_INFO_RX_TS_CNT_GET_(reg) > 0);
 }
 
-static void lan8814_handle_ptp_interrupt(struct phy_device *phydev)
+static void lan8814_handle_ptp_interrupt(struct phy_device *phydev, u16 status)
 {
 	struct kszphy_priv *priv = phydev->priv;
 	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
-	u16 status;
 
-	status = lanphy_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
 	if (status & PTP_TSU_INT_STS_PTP_TX_TS_EN_)
 		lan8814_get_tx_ts(ptp_priv);
 
@@ -2899,8 +2897,8 @@ static int lan8804_config_intr(struct phy_device *phydev)
 
 static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
 {
-	int irq_status, tsu_irq_status;
 	int ret = IRQ_NONE;
+	int irq_status;
 
 	irq_status = phy_read(phydev, LAN8814_INTS);
 	if (irq_status < 0) {
@@ -2913,20 +2911,13 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
 		ret = IRQ_HANDLED;
 	}
 
-	while (1) {
-		tsu_irq_status = lanphy_read_page_reg(phydev, 4,
-						      LAN8814_INTR_STS_REG);
-
-		if (tsu_irq_status > 0 &&
-		    (tsu_irq_status & (LAN8814_INTR_STS_REG_1588_TSU0_ |
-				       LAN8814_INTR_STS_REG_1588_TSU1_ |
-				       LAN8814_INTR_STS_REG_1588_TSU2_ |
-				       LAN8814_INTR_STS_REG_1588_TSU3_))) {
-			lan8814_handle_ptp_interrupt(phydev);
-			ret = IRQ_HANDLED;
-		} else {
+	while (true) {
+		irq_status = lanphy_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
+		if (!irq_status)
 			break;
-		}
+
+		lan8814_handle_ptp_interrupt(phydev, irq_status);
+		ret = IRQ_HANDLED;
 	}
 
 	return ret;
-- 
2.38.0

Re: [PATCH net-next] net: phy: micrel: Change handler interrupt for lan8814

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 4 Jan 2023 20:42:18 +0100 you wrote:
> The lan8814 represents a package of 4 PHYs. All of them are sharing the
> same interrupt line. So when a link was going down/up or a frame was
> timestamped, then the interrupt handler of all the PHYs was called.
> Which is all fine and expected but the problem is the way the handler
> interrupt works.
> Basically if one of the PHYs timestamp a frame, then all the other 3
> PHYs were polling the status of the interrupt until that PHY actually
> cleared the interrupt by reading the timestamp.
> The reason of polling was in case another PHY was also timestamping a
> frame at the same time, it could miss this interrupt. But this is not
> the right approach, because it is the interrupt controller who needs to
> call the interrupt handlers again if the interrupt line is still
> active.
> Therefore change this such when the interrupt handler is called check
> only if the interrupt is for itself, otherwise just exit. In this way
> save CPU usage.
> 
> [...]

Here is the summary with links:
  - [net-next] net: phy: micrel: Change handler interrupt for lan8814
    https://git.kernel.org/netdev/net-next/c/7abd92a5b98f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html