From 004788d7c00ebaf8c3e89dd039b5520d18bc3eb2 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 17 Aug 2014 16:51:56 -0600 Subject: [PATCH] Change the way PHY interrupts work: disable automatically. Then we have to re-subscribe each time after the interrupt fires --- arch/arm/src/sam34/sam_emac.c | 22 ++++-- arch/arm/src/sama5/sam_emaca.c | 22 ++++-- arch/arm/src/sama5/sam_emacb.c | 22 ++++-- arch/arm/src/sama5/sam_gmac.c | 22 ++++-- arch/arm/src/stm32/stm32_eth.c | 7 +- configs/sama5d3-xplained/src/sam_ethernet.c | 78 ++++++++++++++++++--- configs/sama5d3x-ek/src/sam_ethernet.c | 78 ++++++++++++++++++--- configs/sama5d4-ek/src/sam_ethernet.c | 78 ++++++++++++++++++--- drivers/net/phy_notify.c | 63 +++++++++++------ include/nuttx/arch.h | 20 ++++-- 10 files changed, 342 insertions(+), 70 deletions(-) diff --git a/arch/arm/src/sam34/sam_emac.c b/arch/arm/src/sam34/sam_emac.c index cbca247c60..747fd08d33 100644 --- a/arch/arm/src/sam34/sam_emac.c +++ b/arch/arm/src/sam34/sam_emac.c @@ -1937,7 +1937,12 @@ static void sam_phydump(struct sam_emac_s *priv) * Function: sam_phyintenable * * Description: - * Enable link up/down PHY interrupts + * Enable link up/down PHY interrupts. The interrupt protocol is like this: + * + * - Interrupt status is cleared when the interrupt is enabled. + * - Interrupt occurs. Interrupt is disabled (at the processor level) when + * is received. + * - Interrupt status is cleared when the interrupt is re-enabled. * * Parameters: * priv - A reference to the private driver state structure @@ -1952,6 +1957,7 @@ static int sam_phyintenable(struct sam_emac_s *priv) { #if defined(CONFIG_ETH0_PHY_KSZ8051) || defined(CONFIG_ETH0_PHY_KSZ8081) uint32_t regval; + uint16_t phyval; int ret; /* Enable management port */ @@ -1959,10 +1965,18 @@ static int sam_phyintenable(struct sam_emac_s *priv) regval = sam_getreg(priv, SAM_EMAC_NCR); sam_putreg(priv, SAM_EMAC_NCR, regval | EMAC_NCR_MPE); - /* Enable link up/down interrupts */ + /* Read the interrupt status register in order to clear any pending + * interrupts + */ - ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, - (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + ret = sam_phyread(priv, priv->phyaddr, MII_KSZ8081_INT, &phyval); + if (ret == OK) + { + /* Enable link up/down interrupts */ + + ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, + (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + } /* Disable management port (probably) */ diff --git a/arch/arm/src/sama5/sam_emaca.c b/arch/arm/src/sama5/sam_emaca.c index 2cb0400ba5..701b3464e7 100644 --- a/arch/arm/src/sama5/sam_emaca.c +++ b/arch/arm/src/sama5/sam_emaca.c @@ -1978,7 +1978,12 @@ static void sam_phydump(struct sam_emac_s *priv) * Function: sam_phyintenable * * Description: - * Enable link up/down PHY interrupts + * Enable link up/down PHY interrupts. The interrupt protocol is like this: + * + * - Interrupt status is cleared when the interrupt is enabled. + * - Interrupt occurs. Interrupt is disabled (at the processor level) when + * is received. + * - Interrupt status is cleared when the interrupt is re-enabled. * * Parameters: * priv - A reference to the private driver state structure @@ -1993,6 +1998,7 @@ static int sam_phyintenable(struct sam_emac_s *priv) { #if defined(SAMA5_EMAC_PHY_KSZ8051) || defined(SAMA5_EMAC_PHY_KSZ8081) uint32_t regval; + uint16_t phyval; int ret; /* Enable management port */ @@ -2000,10 +2006,18 @@ static int sam_phyintenable(struct sam_emac_s *priv) regval = sam_getreg(priv, SAM_EMAC_NCR); sam_putreg(priv, SAM_EMAC_NCR, regval | EMAC_NCR_MPE); - /* Enable link up/down interrupts */ + /* Read the interrupt status register in order to clear any pending + * interrupts + */ - ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, - (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + ret = sam_phyread(priv, priv->phyaddr, MII_KSZ8081_INT, &phyval); + if (ret == OK) + { + /* Enable link up/down interrupts */ + + ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, + (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + } /* Disable management port (probably) */ diff --git a/arch/arm/src/sama5/sam_emacb.c b/arch/arm/src/sama5/sam_emacb.c index 0161ccd6fa..a149ccba0f 100644 --- a/arch/arm/src/sama5/sam_emacb.c +++ b/arch/arm/src/sama5/sam_emacb.c @@ -2446,7 +2446,12 @@ static bool sam_is100fdx(struct sam_emac_s *priv, uint16_t physr) * Function: sam_phyintenable * * Description: - * Enable link up/down PHY interrupts + * Enable link up/down PHY interrupts. The interrupt protocol is like this: + * + * - Interrupt status is cleared when the interrupt is enabled. + * - Interrupt occurs. Interrupt is disabled (at the processor level) when + * is received. + * - Interrupt status is cleared when the interrupt is re-enabled. * * Parameters: * priv - A reference to the private driver state structure @@ -2462,6 +2467,7 @@ static int sam_phyintenable(struct sam_emac_s *priv) #if defined(SAMA5_EMAC0_PHY_KSZ8051) || defined(SAMA5_EMAC0_PHY_KSZ8081) || \ defined(SAMA5_EMAC1_PHY_KSZ8051) || defined(SAMA5_EMAC1_PHY_KSZ8081) uint32_t regval; + uint16_t phyval; int ret; /* Does this MAC support a KSZ80x1 PHY? */ @@ -2473,10 +2479,18 @@ static int sam_phyintenable(struct sam_emac_s *priv) regval = sam_getreg(priv, SAM_EMAC_NCR_OFFSET); sam_putreg(priv, SAM_EMAC_NCR_OFFSET, regval | EMAC_NCR_MPE); - /* Enable link up/down interrupts */ + /* Read the interrupt status register in order to clear any pending + * interrupts + */ - ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, - (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + ret = sam_phyread(priv, priv->phyaddr, MII_KSZ8081_INT, &phyval); + if (ret == OK) + { + /* Enable link up/down interrupts */ + + ret = sam_phywrite(priv, priv->phyaddr, MII_KSZ8081_INT, + (MII_KSZ80x1_INT_LDEN | MII_KSZ80x1_INT_LUEN)); + } /* Disable management port (probably) */ diff --git a/arch/arm/src/sama5/sam_gmac.c b/arch/arm/src/sama5/sam_gmac.c index 251e7e8e9e..dc78bb7c26 100644 --- a/arch/arm/src/sama5/sam_gmac.c +++ b/arch/arm/src/sama5/sam_gmac.c @@ -1923,7 +1923,12 @@ static void sam_phydump(struct sam_gmac_s *priv) * Function: sam_phyintenable * * Description: - * Enable link up/down PHY interrupts + * Enable link up/down PHY interrupts. The interrupt protocol is like this: + * + * - Interrupt status is cleared when the interrupt is enabled. + * - Interrupt occurs. Interrupt is disabled (at the processor level) when + * is received. + * - Interrupt status is cleared when the interrupt is re-enabled. * * Parameters: * priv - A reference to the private driver state structure @@ -1937,18 +1942,25 @@ static void sam_phydump(struct sam_gmac_s *priv) static int sam_phyintenable(struct sam_emac_s *priv) { #if defined(SAMA5_GMAC_PHY_KSZ90x1) + uint16_t phyval; int ret; /* Enable the management port */ sam_enablemdio(priv); - /* Write to the requested register */ + /* Read the interrupt status register in order to clear any pending + * interrupts + */ - /* Enable link up/down interrupts */ + ret = sam_phyread(priv, priv->phyaddr, GMII_KSZ90x1_ICS, &phyval); + if (ret == OK) + { + /* Enable link up/down interrupts */ - ret = sam_phywrite(priv, priv->phyaddr, GMII_KSZ90x1_ICS, - (GMII_KSZ90x1_INT_LDEN | GMII_KSZ90x1_INT_LUEN)); + ret = sam_phywrite(priv, priv->phyaddr, GMII_KSZ90x1_ICS, + (GMII_KSZ90x1_INT_LDEN | GMII_KSZ90x1_INT_LUEN)); + } /* Disable the management port */ diff --git a/arch/arm/src/stm32/stm32_eth.c b/arch/arm/src/stm32/stm32_eth.c index 8b944db77f..a4d894fc8f 100644 --- a/arch/arm/src/stm32/stm32_eth.c +++ b/arch/arm/src/stm32/stm32_eth.c @@ -2572,7 +2572,12 @@ static int stm32_ioctl(struct net_driver_s *dev, int cmd, long arg) * Function: stm32_phyintenable * * Description: - * Enable link up/down PHY interrupts + * Enable link up/down PHY interrupts. The interrupt protocol is like this: + * + * - Interrupt status is cleared when the interrupt is enabled. + * - Interrupt occurs. Interrupt is disabled (at the processor level) when + * is received. + * - Interrupt status is cleared when the interrupt is re-enabled. * * Parameters: * priv - A reference to the private driver state structure diff --git a/configs/sama5d3-xplained/src/sam_ethernet.c b/configs/sama5d3-xplained/src/sam_ethernet.c index 4cb6314953..2307410a38 100644 --- a/configs/sama5d3-xplained/src/sam_ethernet.c +++ b/configs/sama5d3-xplained/src/sam_ethernet.c @@ -108,6 +108,43 @@ static xcpt_t g_gmac_handler; * Private Functions ************************************************************************************/ +/************************************************************************************ + * Name: sam_emac_phy_enable and sam_gmac_enable + ************************************************************************************/ + +#ifdef CONFIG_SAMA5_PIOE_IRQ +#ifdef CONFIG_SAMA5_EMACA +static void sam_emac_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH1, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH1); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH1); + } +} + +#endif + +#ifdef CONFIG_SAMA5_GMAC +static void sam_gmac_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH0, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH0); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH0); + } +} +#endif +#endif + /************************************************************************************ * Public Functions ************************************************************************************/ @@ -174,6 +211,10 @@ void weak_function sam_netinitialize(void) * NULL. If handler is NULL, then the interrupt is detached and disabled * instead. * + * The PHY interrupt is always disabled upon return. The caller must + * call back through the enable function point to control the state of + * the interrupt. + * * This interrupt may or may not be available on a given platform depending * on how the network hardware architecture is implemented. In a typical * case, the PHY interrupt is provided to board-level logic as a GPIO @@ -187,16 +228,20 @@ void weak_function sam_netinitialize(void) * * Typical usage: * a. OS service logic (not application logic*) attaches to the PHY - * PHY interrupt. - * b. When the PHY interrupt occurs, work should be scheduled on the - * worker thread (or perhaps a dedicated application thread). + * PHY interrupt and enables the PHY interrupt. + * b. When the PHY interrupt occurs: (1) the interrupt should be + * disabled and () work should be scheduled on the worker thread (or + * perhaps a dedicated application thread). * c. That worker thread should use the SIOCGMIIPHY, SIOCGMIIREG, * and SIOCSMIIREG ioctl calls** to communicate with the PHY, * determine what network event took place (Link Up/Down?), and * take the appropriate actions. + * d. It should then interact the the PHY to clear any pending + * interrupts, then re-enable the PHY interrupt. * * * This is an OS internal interface and should not be used from - * application space. + * application space. Rather applications should use the SIOCMIISIG + * ioctl to receive a signal when a PHY event occurs. * ** This interrupt is really of no use if the Ethernet MAC driver * does not support these ioctl calls. * @@ -208,6 +253,8 @@ void weak_function sam_netinitialize(void) * asserts an interrupt. Must reside in OS space, but can * signal tasks in user space. A value of NULL can be passed * in order to detach and disable the PHY interrupt. + * enable - A function pointer that be unsed to enable or disable the + * PHY interrupt. * * Returned Value: * The previous PHY interrupt handler address is returned. This allows you @@ -218,12 +265,13 @@ void weak_function sam_netinitialize(void) ****************************************************************************/ #ifdef CONFIG_SAMA5_PIOE_IRQ -xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) +xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler, phy_enable_t *enable) { irqstate_t flags; xcpt_t *phandler; xcpt_t oldhandler; pio_pinset_t pinset; + phy_enable_t enabler; int irq; DEBUGASSERT(intf); @@ -243,6 +291,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_emac_handler; pinset = PIO_INT_ETH1; irq = IRQ_INT_ETH1; + enabler = sam_emac_phy_enable; } else #endif @@ -253,6 +302,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_gmac_handler; pinset = PIO_INT_ETH0; irq = IRQ_INT_ETH0; + enabler = sam_gmac_phy_enable; } else #endif @@ -279,15 +329,25 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phydbg("Configure pin: %08x\n", pinset); sam_pioirq(pinset); - phydbg("Enable IRQ: %d\n", irq); + phydbg("Attach IRQ%d\n", irq); (void)irq_attach(irq, handler); - sam_pioirqenable(irq); } else { - phydbg("Disable IRQ: %d\n", irq); + phydbg("Detach IRQ%d\n", irq); (void)irq_detach(irq); - sam_pioirqdisable(irq); + enabler = NULL; + } + + /* Return with the interrupt disabled in either case */ + + sam_pioirqdisable(irq); + + /* Return the enabling function pointer */ + + if (enable) + { + *enable = enabler; } /* Return the old button handler (so that it can be restored) */ diff --git a/configs/sama5d3x-ek/src/sam_ethernet.c b/configs/sama5d3x-ek/src/sam_ethernet.c index 6628ad1d0b..e2a252e45c 100644 --- a/configs/sama5d3x-ek/src/sam_ethernet.c +++ b/configs/sama5d3x-ek/src/sam_ethernet.c @@ -108,6 +108,43 @@ static xcpt_t g_gmac_handler; * Private Functions ************************************************************************************/ +/************************************************************************************ + * Name: sam_emac_phy_enable and sam_gmac_enable + ************************************************************************************/ + +#ifdef CONFIG_SAMA5_PIOE_IRQ +#ifdef CONFIG_SAMA5_EMACA +static void sam_emac_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH1, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH1); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH1); + } +} + +#endif + +#ifdef CONFIG_SAMA5_GMAC +static void sam_gmac_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH0, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH0); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH0); + } +} +#endif +#endif + /************************************************************************************ * Public Functions ************************************************************************************/ @@ -174,6 +211,10 @@ void weak_function sam_netinitialize(void) * NULL. If handler is NULL, then the interrupt is detached and disabled * instead. * + * The PHY interrupt is always disabled upon return. The caller must + * call back through the enable function point to control the state of + * the interrupt. + * * This interrupt may or may not be available on a given platform depending * on how the network hardware architecture is implemented. In a typical * case, the PHY interrupt is provided to board-level logic as a GPIO @@ -187,16 +228,20 @@ void weak_function sam_netinitialize(void) * * Typical usage: * a. OS service logic (not application logic*) attaches to the PHY - * PHY interrupt. - * b. When the PHY interrupt occurs, work should be scheduled on the - * worker thread (or perhaps a dedicated application thread). + * PHY interrupt and enables the PHY interrupt. + * b. When the PHY interrupt occurs: (1) the interrupt should be + * disabled and () work should be scheduled on the worker thread (or + * perhaps a dedicated application thread). * c. That worker thread should use the SIOCGMIIPHY, SIOCGMIIREG, * and SIOCSMIIREG ioctl calls** to communicate with the PHY, * determine what network event took place (Link Up/Down?), and * take the appropriate actions. + * d. It should then interact the the PHY to clear any pending + * interrupts, then re-enable the PHY interrupt. * * * This is an OS internal interface and should not be used from - * application space. + * application space. Rather applications should use the SIOCMIISIG + * ioctl to receive a signal when a PHY event occurs. * ** This interrupt is really of no use if the Ethernet MAC driver * does not support these ioctl calls. * @@ -208,6 +253,8 @@ void weak_function sam_netinitialize(void) * asserts an interrupt. Must reside in OS space, but can * signal tasks in user space. A value of NULL can be passed * in order to detach and disable the PHY interrupt. + * enable - A function pointer that be unsed to enable or disable the + * PHY interrupt. * * Returned Value: * The previous PHY interrupt handler address is returned. This allows you @@ -218,12 +265,13 @@ void weak_function sam_netinitialize(void) ****************************************************************************/ #ifdef CONFIG_SAMA5_PIOE_IRQ -xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) +xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler, phy_enable_t *enable) { irqstate_t flags; xcpt_t *phandler; xcpt_t oldhandler; pio_pinset_t pinset; + phy_enable_t enabler; int irq; DEBUGASSERT(intf); @@ -243,6 +291,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_emac_handler; pinset = PIO_INT_ETH1; irq = IRQ_INT_ETH1; + enabler = sam_emac_phy_enable; } else #endif @@ -253,6 +302,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_gmac_handler; pinset = PIO_INT_ETH0; irq = IRQ_INT_ETH0; + enabler = sam_gmac_phy_enable; } else #endif @@ -279,15 +329,25 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phydbg("Configure pin: %08x\n", pinset); sam_pioirq(pinset); - phydbg("Enable IRQ: %d\n", irq); + phydbg("Attach IRQ%d\n", irq); (void)irq_attach(irq, handler); - sam_pioirqenable(irq); } else { - phydbg("Disable IRQ: %d\n", irq); + phydbg("Detach IRQ%d\n", irq); (void)irq_detach(irq); - sam_pioirqdisable(irq); + enabler = NULL; + } + + /* Return with the interrupt disabled in either case */ + + sam_pioirqdisable(irq); + + /* Return the enabling function pointer */ + + if (enable) + { + *enable = enabler; } /* Return the old button handler (so that it can be restored) */ diff --git a/configs/sama5d4-ek/src/sam_ethernet.c b/configs/sama5d4-ek/src/sam_ethernet.c index 75118c3e3e..a2eb1b1c53 100644 --- a/configs/sama5d4-ek/src/sam_ethernet.c +++ b/configs/sama5d4-ek/src/sam_ethernet.c @@ -108,6 +108,43 @@ static xcpt_t g_emac1_handler; * Private Functions ************************************************************************************/ +/************************************************************************************ + * Name: sam_emac_phy_enable and sam_gmac_enable + ************************************************************************************/ + +#ifdef CONFIG_SAMA5_PIOE_IRQ +#ifdef CONFIG_SAMA5_EMAC0 +static void sam_emac0_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH0, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH0); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH0); + } +} + +#endif + +#ifdef CONFIG_SAMA5_EMAC1 +static void sam_emac1_phy_enable(bool enable) +{ + phydbg("IRQ%d: enable=%d\n", IRQ_INT_ETH1, enable); + if (enable) + { + sam_pioirqenable(IRQ_INT_ETH1); + } + else + { + sam_pioirqdisable(IRQ_INT_ETH1); + } +} +#endif +#endif + /************************************************************************************ * Public Functions ************************************************************************************/ @@ -143,6 +180,10 @@ void weak_function sam_netinitialize(void) * NULL. If handler is NULL, then the interrupt is detached and disabled * instead. * + * The PHY interrupt is always disabled upon return. The caller must + * call back through the enable function point to control the state of + * the interrupt. + * * This interrupt may or may not be available on a given platform depending * on how the network hardware architecture is implemented. In a typical * case, the PHY interrupt is provided to board-level logic as a GPIO @@ -156,16 +197,20 @@ void weak_function sam_netinitialize(void) * * Typical usage: * a. OS service logic (not application logic*) attaches to the PHY - * PHY interrupt. - * b. When the PHY interrupt occurs, work should be scheduled on the - * worker thread (or perhaps a dedicated application thread). + * PHY interrupt and enables the PHY interrupt. + * b. When the PHY interrupt occurs: (1) the interrupt should be + * disabled and () work should be scheduled on the worker thread (or + * perhaps a dedicated application thread). * c. That worker thread should use the SIOCGMIIPHY, SIOCGMIIREG, * and SIOCSMIIREG ioctl calls** to communicate with the PHY, * determine what network event took place (Link Up/Down?), and * take the appropriate actions. + * d. It should then interact the the PHY to clear any pending + * interrupts, then re-enable the PHY interrupt. * * * This is an OS internal interface and should not be used from - * application space. + * application space. Rather applications should use the SIOCMIISIG + * ioctl to receive a signal when a PHY event occurs. * ** This interrupt is really of no use if the Ethernet MAC driver * does not support these ioctl calls. * @@ -177,6 +222,8 @@ void weak_function sam_netinitialize(void) * asserts an interrupt. Must reside in OS space, but can * signal tasks in user space. A value of NULL can be passed * in order to detach and disable the PHY interrupt. + * enable - A function pointer that be unsed to enable or disable the + * PHY interrupt. * * Returned Value: * The previous PHY interrupt handler address is returned. This allows you @@ -187,12 +234,13 @@ void weak_function sam_netinitialize(void) ****************************************************************************/ #ifdef CONFIG_SAMA5_PIOE_IRQ -xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) +xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler, phy_enable_t *enable) { irqstate_t flags; xcpt_t *phandler; xcpt_t oldhandler; pio_pinset_t pinset; + phy_enable_t enabler; int irq; DEBUGASSERT(intf); @@ -212,6 +260,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_emac0_handler; pinset = PIO_INT_ETH0; irq = IRQ_INT_ETH0; + enabler = sam_emac0_phy_enable; } else #endif @@ -222,6 +271,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phandler = &g_emac1_handler; pinset = PIO_INT_ETH1; irq = IRQ_INT_ETH1; + enabler = sam_emac1_phy_enable; } else #endif @@ -248,15 +298,25 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler) phydbg("Configure pin: %08x\n", pinset); sam_pioirq(pinset); - phydbg("Enable IRQ: %d\n", irq); + phydbg("Attach IRQ%d\n", irq); (void)irq_attach(irq, handler); - sam_pioirqenable(irq); } else { - phydbg("Disable IRQ: %d\n", irq); + phydbg("Detach IRQ%d\n", irq); (void)irq_detach(irq); - sam_pioirqdisable(irq); + enabler = NULL; + } + + /* Return with the interrupt disabled in either case */ + + sam_pioirqdisable(irq); + + /* Return the enabling function pointer */ + + if (enable) + { + *enable = enabler; } /* Return the old button handler (so that it can be restored) */ diff --git a/drivers/net/phy_notify.c b/drivers/net/phy_notify.c index 5edd35e20b..c828a644c5 100644 --- a/drivers/net/phy_notify.c +++ b/drivers/net/phy_notify.c @@ -107,6 +107,7 @@ struct phy_notify_s #endif pid_t pid; FAR void *arg; + phy_enable_t enable; }; /**************************************************************************** @@ -201,6 +202,7 @@ static FAR struct phy_notify_s *phy_find_unassigned(void) #endif client->pid = -1; client->arg = NULL; + client->enable = NULL; /* Return the client entry assigned to the caller */ @@ -247,7 +249,6 @@ static FAR struct phy_notify_s *phy_find_assigned(FAR const char *intf, /* Ooops... not found */ - ndbg("ERROR: Client entry not found\n"); phy_semgive(); return NULL; } @@ -263,10 +264,14 @@ static int phy_handler(FAR struct phy_notify_s *client) #endif int ret; - DEBUGASSERT(client && client->assigned); + DEBUGASSERT(client && client->assigned && client->enable); phylldbg("Entry client %d, signalling PID=%d with signal %d\n", client->index, client->pid, client->signo); + /* Disable further interrupts */ + + client->enable(false); + /* Signal the client that the PHY has something interesting to say to us */ #ifdef CONFIG_CAN_PASS_STRUCTS @@ -356,15 +361,6 @@ int phy_notify_subscribe(FAR const char *intf, pid_t pid, int signo, nvdbg("%s: PID=%d signo=%d arg=%p\n", intf, pid, signo, arg); - /* Find an unused slot in the client notification table */ - - client = phy_find_unassigned(); - if (!client) - { - ndbg("ERROR: Failed to allocate a client entry\n"); - return -ENOMEM; - } - /* The special value pid == 0 means to use the pid of the current task. */ if (pid == 0) @@ -373,19 +369,46 @@ int phy_notify_subscribe(FAR const char *intf, pid_t pid, int signo, phydbg("Actual PID=%d\n", pid); } - /* Initialize the client entry */ + /* Check if this client already exists */ - client->signo = signo; - client->pid = pid; - client->arg = arg; + client = phy_find_assigned(intf, pid); + if (client) + { + /* Yes.. update the signal number and argument */ + + client->signo = signo; + client->arg = arg; + } + else + { + /* No, allocate a new slot in the client notification table */ + + client = phy_find_unassigned(); + if (!client) + { + ndbg("ERROR: Failed to allocate a client entry\n"); + return -ENOMEM; + } + + /* Initialize the new client entry */ + + client->signo = signo; + client->pid = pid; + client->arg = arg; #ifdef CONFIG_NETDEV_MULTINIC - snprintf(client->intf, CONFIG_PHY_NOTIFICATION_MAXINTFLEN+1, intf); - client->intf[CONFIG_PHY_NOTIFICATION_MAXINTFLEN] = '\0'; + snprintf(client->intf, CONFIG_PHY_NOTIFICATION_MAXINTFLEN+1, intf); + client->intf[CONFIG_PHY_NOTIFICATION_MAXINTFLEN] = '\0'; #endif - /* Attach and enable the PHY interrupt */ + /* Attach/re-attach the PHY interrupt */ - (void)arch_phy_irq(intf, g_notify_handler[client->index]); + (void)arch_phy_irq(intf, g_notify_handler[client->index], &client->enable); + } + + /* Enable/re-enable the PH interrupt */ + + DEBUGASSERT(client->enable); + client->enable(true); return OK; } @@ -429,7 +452,7 @@ int phy_notify_unsubscribe(FAR const char *intf, pid_t pid) /* Detach and disable the PHY interrupt */ phy_semtake(); - (void)arch_phy_irq(intf, NULL); + (void)arch_phy_irq(intf, NULL, NULL); /* Un-initialize the client entry */ diff --git a/include/nuttx/arch.h b/include/nuttx/arch.h index 5276d3504c..4fb3a193ad 100644 --- a/include/nuttx/arch.h +++ b/include/nuttx/arch.h @@ -105,7 +105,7 @@ #include /**************************************************************************** - * Definitions + * Pre-processor definitions ****************************************************************************/ /**************************************************************************** @@ -113,6 +113,7 @@ ****************************************************************************/ typedef CODE void (*sig_deliver_t)(FAR struct tcb_s *tcb); +typedef CODE void (*phy_enable_t)(bool enable); /**************************************************************************** * Public Variables @@ -1456,6 +1457,10 @@ xcpt_t board_button_irq(int id, xcpt_t irqhandler); * NULL. If handler is NULL, then the interrupt is detached and disabled * instead. * + * The PHY interrupt is always disabled upon return. The caller must + * call back through the enable function point to control the state of + * the interrupt. + * * This interrupt may or may not be available on a given platform depending * on how the network hardware architecture is implemented. In a typical * case, the PHY interrupt is provided to board-level logic as a GPIO @@ -1469,13 +1474,16 @@ xcpt_t board_button_irq(int id, xcpt_t irqhandler); * * Typical usage: * a. OS service logic (not application logic*) attaches to the PHY - * PHY interrupt. - * b. When the PHY interrupt occurs, work should be scheduled on the - * worker thread (or perhaps a dedicated application thread). + * PHY interrupt and enables the PHY interrupt. + * b. When the PHY interrupt occurs: (1) the interrupt should be + * disabled and () work should be scheduled on the worker thread (or + * perhaps a dedicated application thread). * c. That worker thread should use the SIOCGMIIPHY, SIOCGMIIREG, * and SIOCSMIIREG ioctl calls** to communicate with the PHY, * determine what network event took place (Link Up/Down?), and * take the appropriate actions. + * d. It should then interact the the PHY to clear any pending + * interrupts, then re-enable the PHY interrupt. * * * This is an OS internal interface and should not be used from * application space. Rather applications should use the SIOCMIISIG @@ -1491,6 +1499,8 @@ xcpt_t board_button_irq(int id, xcpt_t irqhandler); * asserts an interrupt. Must reside in OS space, but can * signal tasks in user space. A value of NULL can be passed * in order to detach and disable the PHY interrupt. + * enable - A function pointer that be unsed to enable or disable the + * PHY interrupt. * * Returned Value: * The previous PHY interrupt handler address is returned. This allows you @@ -1501,7 +1511,7 @@ xcpt_t board_button_irq(int id, xcpt_t irqhandler); ****************************************************************************/ #ifdef CONFIG_ARCH_PHY_INTERRUPT -xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler); +xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler, phy_enable_t *enable); #endif /************************************************************************************