[PATCH 0/2 net-next v2] phy: PHY_IGNORE_INTERRUPT fixes and improvements

STALE4892d

4 messages, 2 authors, 2013-05-20 · open the first message on its own page

[PATCH 0/2 net-next v2] phy: PHY_IGNORE_INTERRUPT fixes and improvements

From: "Florian Fainelli" <f.fainelli@gmail.com>
Date: 2013-05-20 08:54:14

David, Andy,

These two patches contain fixes and improvements for using PHY_IGNORE_INTERRUPT
in network drivers. Since the second patch is an addition and not a bug fix
per-se, these patches are based on "net-next". The first patch applies cleanly
to "net" too.

Florian Fainelli (2):
  phy: fix the use of PHY_IGNORE_INTERRUPT
  phy: add phy_mac_interrupt() to use with PHY_IGNORE_INTERRUPT

 drivers/net/phy/phy.c        | 23 ++++++++++++++---------
 drivers/net/phy/phy_device.c |  8 ++++++--
 include/linux/phy.h          | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 11 deletions(-)

-- 
1.8.1.2

[PATCH 1/2 v2 net-next] phy: fix the use of PHY_IGNORE_INTERRUPT

From: "Florian Fainelli" <f.fainelli@gmail.com>
Date: 2013-05-20 08:54:14

When a PHY device is registered with the special IRQ value
PHY_IGNORE_INTERRUPT (-2) it will not properly be handled by the PHY
library:

- it continues to poll its register, while we do not want this
  because such PHY link events or register changes are serviced by an
  Ethernet MAC
- it will still try to configure PHY interrupts at the PHY level, such
  interrupts do not exist at the PHY but at the MAC level
- the state machine only handles PHY_POLL, but should also handle
  PHY_IGNORE_INTERRUPT similarly

This patch updates the PHY state machine and initialization paths to
account for the specific PHY_IGNORE_INTERRUPT. Based on an earlier patch
by Thomas Petazzoni, and reworked to add the missing bits. Add a helper
phy_interrupt_is_valid() which specifically tests for a PHY interrupt
not to be PHY_POLL or PHY_IGNORE_INTERRUPT and use it throughout the
code.

Signed-off-by: Thomas Petazzoni <redacted>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes since v1:
- add helper function phy_interrupt_is_valid() which tests for
  PHY_POLL and PHY_IGNORE_INTERRUPT as suggested by David

 drivers/net/phy/phy.c        |  9 +++++----
 drivers/net/phy/phy_device.c |  7 +++++--
 include/linux/phy.h          | 12 ++++++++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c14f147..3bcf099 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c

@@ -682,7 +682,7 @@ void phy_stop(struct phy_device *phydev)
 	if (PHY_HALTED == phydev->state)
 		goto out_unlock;
 
-	if (phydev->irq != PHY_POLL) {
+	if (phy_interrupt_is_valid(phydev)) {
 		/* Disable PHY Interrupts */
 		phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);

@@ -828,8 +828,9 @@ void phy_state_machine(struct work_struct *work)
 			break;
 		case PHY_RUNNING:
 			/* Only register a CHANGE if we are
-			 * polling */
-			if (PHY_POLL == phydev->irq)
+			 * polling or ignoring interrupts
+			 */
+			if (!phy_interrupt_is_valid(phydev))
 				phydev->state = PHY_CHANGELINK;
 			break;
 		case PHY_CHANGELINK:

@@ -848,7 +849,7 @@ void phy_state_machine(struct work_struct *work)
 
 			phydev->adjust_link(phydev->attached_dev);
 
-			if (PHY_POLL != phydev->irq)
+			if (phy_interrupt_is_valid(phydev))
 				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 3657b4a..8e29d22 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c

@@ -1009,8 +1009,11 @@ static int phy_probe(struct device *dev)
 	phydrv = to_phy_driver(drv);
 	phydev->drv = phydrv;
 
-	/* Disable the interrupt if the PHY doesn't support it */
-	if (!(phydrv->flags & PHY_HAS_INTERRUPT))
+	/* Disable the interrupt if the PHY doesn't support it
+	 * but the interrupt is still a valid one
+	 */
+	if (!(phydrv->flags & PHY_HAS_INTERRUPT) &&
+			phy_interrupt_is_valid(phydev))
 		phydev->irq = PHY_POLL;
 
 	mutex_lock(&phydev->lock);

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9e11039..8e4bc8a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -508,6 +508,18 @@ static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
 	return mdiobus_write(phydev->bus, phydev->addr, regnum, val);
 }
 
+/**
+ * phy_interrupt_is_valid - Convenience function for testing a given PHY irq
+ * @phydev: the phy_device struct
+ *
+ * NOTE: must be kept in sync with addition/removal of PHY_POLL and
+ * PHY_IGNORE_INTERRUPT
+ */
+static inline bool phy_interrupt_is_valid(struct phy_device *phydev)
+{
+	return phydev->irq != PHY_POLL && phydev->irq != PHY_IGNORE_INTERRUPT;
+}
+
 struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 		bool is_c45, struct phy_c45_device_ids *c45_ids);
 struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);

-- 
1.8.1.2

[PATCH 2/2 net-next v2] phy: add phy_mac_interrupt() to use with PHY_IGNORE_INTERRUPT

From: "Florian Fainelli" <f.fainelli@gmail.com>
Date: 2013-05-20 08:54:19

There is currently no way for an Ethernet MAC driver servicing PHY link
interrupts to notify this to the PHY state machine without defining its
own state machine. Since most drivers are not so special, introduce a
helper: phy_mac_interrupt() which can be called from a link up/down
interrupt routine to update the PHY state machine. To avoid code
duplication some refactoring has been done to expose the workqueue and
its corresponding callback internally.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes since v1:
- rebased on top of net-next

 drivers/net/phy/phy.c        | 14 +++++++++-----
 drivers/net/phy/phy_device.c |  1 +
 include/linux/phy.h          |  2 ++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 3bcf099..2d28a0e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c

@@ -419,8 +419,6 @@ out_unlock:
 EXPORT_SYMBOL(phy_start_aneg);
 
 
-static void phy_change(struct work_struct *work);
-
 /**
  * phy_start_machine - start PHY state machine tracking
  * @phydev: the phy_device struct

@@ -565,8 +563,6 @@ int phy_start_interrupts(struct phy_device *phydev)
 {
 	int err = 0;
 
-	INIT_WORK(&phydev->phy_queue, phy_change);
-
 	atomic_set(&phydev->irq_disable, 0);
 	if (request_irq(phydev->irq, phy_interrupt,
 				IRQF_SHARED,

@@ -623,7 +619,7 @@ EXPORT_SYMBOL(phy_stop_interrupts);
  * phy_change - Scheduled by the phy_interrupt/timer to handle PHY changes
  * @work: work_struct that describes the work to be done
  */
-static void phy_change(struct work_struct *work)
+void phy_change(struct work_struct *work)
 {
 	int err;
 	struct phy_device *phydev =

@@ -922,6 +918,14 @@ void phy_state_machine(struct work_struct *work)
 	schedule_delayed_work(&phydev->state_queue, PHY_STATE_TIME * HZ);
 }
 
+void phy_mac_interrupt(struct phy_device *phydev, int new_link)
+{
+	cancel_work_sync(&phydev->phy_queue);
+	phydev->link = new_link;
+	schedule_work(&phydev->phy_queue);
+}
+EXPORT_SYMBOL(phy_mac_interrupt);
+
 static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
 				    int addr)
 {

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8e29d22..b55aa33 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c

@@ -189,6 +189,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
+	INIT_WORK(&dev->phy_queue, phy_change);
 
 	/* Request the appropriate module unconditionally; don't
 	   bother trying to do so only if it isn't already loaded,

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8e4bc8a..fdfa115 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -557,6 +557,8 @@ void phy_drivers_unregister(struct phy_driver *drv, int n);
 int phy_driver_register(struct phy_driver *new_driver);
 int phy_drivers_register(struct phy_driver *new_driver, int n);
 void phy_state_machine(struct work_struct *work);
+void phy_change(struct work_struct *work);
+void phy_mac_interrupt(struct phy_device *phydev, int new_link);
 void phy_start_machine(struct phy_device *phydev,
 		void (*handler)(struct net_device *));
 void phy_stop_machine(struct phy_device *phydev);

-- 
1.8.1.2

Re: [PATCH 0/2 net-next v2] phy: PHY_IGNORE_INTERRUPT fixes and improvements

From: David Miller <davem@davemloft.net>
Date: 2013-05-20 21:13:36

From: "Florian Fainelli" <f.fainelli@gmail.com>
Date: Mon, 20 May 2013 08:53:41 +0000

These two patches contain fixes and improvements for using PHY_IGNORE_INTERRUPT
in network drivers. Since the second patch is an addition and not a bug fix
per-se, these patches are based on "net-next". The first patch applies cleanly
to "net" too.

Series applied to net-next, thanks.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help