netdev - Using phylib with a MAC-handled interrupt

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>] [day] [month] [year] [list]

Date:	Tue, 9 Oct 2012 18:17:27 +0200
From:	Thomas Petazzoni <thomas.petazzoni@...x.org>
To:	afleming@...escale.com
Cc:	netdev@...r.kernel.org, Florian Fainelli <florian@...nwrt.org>,
	Lior Amsalem <alior@...vell.com>,
	Gregory Clément 
	<gregory.clement@...e-electrons.com>,
	Maen Suleiman <maen@...vell.com>
Subject: Using phylib with a MAC-handled interrupt

Hello Andy,

I am currently working on a network driver for the MAC present in the
latest Marvell ARM SoCs. This MAC is capable of polling the PHY in
hardware, which avoids the need for Linux to poll the PHY regularly.

Therefore, I am currently trying to do an integration of the driver with
the phylib using the third solution you listed at
http://lists.openwall.net/netdev/2008/06/06/38.

Unfortunately, with the current Linux kernel, the phylib continues to
poll the PHY registers regularly even if phydev->irq is set to
PHY_IGNORE_INTERRUPT. This is due to the fact that the phylib code
override phydev->irq to be PHY_POLL whenever the PHY driver doesn't
support interrupt, which isn't correct. This is fixed by the below
patch.

However, even with the below patch, I cannot implement your solution
three described in http://lists.openwall.net/netdev/2008/06/06/38 :
taking a mutex in an interrupt handler is not possible. Therefore, I
cannot change the phydev->state field from my MAC driver to force the
PHY state machine to re-read the status of the PHY.

I tried to force the execution of the phydev->phy_queue workqueue, but
unfortunately, simply calling schedule_work(&phydev->phy_queue)
from my interrupt handler causes problems:

WARNING: at /home/thomas/projets/linux-2.6/kernel/workqueue.c:1033 __queue_work+0x1c0/0x1d8()

This is apparently because schedule_work() doesn't like when the
workqueue is already scheduled for execution, but I am not sure how to
properly handle this case.

What I would like to do is from my MAC interrupt handler, tell the
phylib "hey, something has changed, you should re-read the PHY
registers and if needed call my ->adjust_link() callback".

How can I do that with the phylib?

Thanks,

Thomas

>From 3eae62a1adb08229ea4d20068ef868b731b020cb Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Date: Tue, 9 Oct 2012 18:10:21 +0200
Subject: [PATCH] net: phy: fix support of PHY_IGNORE_INTERRUPT

When PHY_IGNORE_INTERRUPT is set as the phydev->irq, then the phylib
shouldn't poll the PHY regularly, but instead let the MAC driver tell
the phylib when to re-read the PHY status. Unfortunately, as soon as
the PHY didn't had PHY_HAS_INTERRUPT set, the phylib was forcing
PHY_POLL as the interrupt.

Also fix two other places that test against PHY_POLL while they should
instead be testing if we have an IRQ or not.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
---
 drivers/net/phy/phy.c        |    4 ++--
 drivers/net/phy/phy_device.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7ca2ff9..a1d265f 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -709,7 +709,7 @@ void phy_stop(struct phy_device *phydev)
 	if (PHY_HALTED == phydev->state)
 		goto out_unlock;

-	if (phydev->irq != PHY_POLL) {
+	if (phydev->irq > 0) {
 		/* Disable PHY Interrupts */
 		phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);

@@ -896,7 +896,7 @@ void phy_state_machine(struct work_struct *work)

 			phydev->adjust_link(phydev->attached_dev);

-			if (PHY_POLL != phydev->irq)
+			if (phydev->irq > 0)
 				err = phy_config_interrupt(phydev,
 						PHY_INTERRUPT_ENABLED);
 			break;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8af46e8..110b3a3 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1011,7 +1011,7 @@ static int phy_probe(struct device *dev)
 	phydev->drv = phydrv;

 	/* Disable the interrupt if the PHY doesn't support it */
-	if (!(phydrv->flags & PHY_HAS_INTERRUPT))
+	if (!(phydrv->flags & PHY_HAS_INTERRUPT) && phydev->irq > 0)
 		phydev->irq = PHY_POLL;

 	mutex_lock(&phydev->lock);
-- 
1.7.9.5

-- 
Thomas Petazzoni                http://thomas.enix.org
MapOSMatic                      http://www.maposmatic.org
Logiciels Libres à Toulouse     http://www.toulibre.org
Embedded Linux                  http://www.free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html