SAMA5 PHY: Add more debug instrumentation; Fix logic that would be used to disable PHY interrupts

configs/sama5d3-xplained/src/sam_ethernet.c

@@ -39,6 +39,15 @@
 
 #include <nuttx/config.h>
 
+/* Force verbose debug on in this file only to support unit-level testing. */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  undef  CONFIG_DEBUG_VERBOSE
+#  define CONFIG_DEBUG_VERBOSE 1
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <assert.h>
 #include <debug.h>
@@ -69,6 +78,19 @@
 #  define SAMA5_EMAC_DEVNAME "eth1"
 #endif
 
+/* Debug ********************************************************************/
+/* Extra, in-depth debug output that is only available if
+ * CONFIG_NETDEV_PHY_DEBUG us defined.
+ */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  define phydbg    dbg
+#  define phylldbg  lldbg
+#else
+#  define phydbg(x...)
+#  define phylldbg(x...)
+#endif
+
 /************************************************************************************
  * Private Data
  ************************************************************************************/
@@ -117,9 +139,10 @@ void weak_function sam_netinitialize(void)
    *   Speed Mode:100Mbps
    *   Nway Auto-Negotiation:Enable
    *
-   * The KSZ8051 PHY interrtup is available on PE30 INT_ETH1
+   * The KSZ8051 PHY interrupt is available on PE30 INT_ETH1
   */
 
+  phydbg("Configuring %08x\n", PIO_INT_ETH1);
   sam_configpio(PIO_INT_ETH1);
 #endif
 
@@ -136,6 +159,7 @@ void weak_function sam_netinitialize(void)
    * The KSZ9021/31 interrupt is available on PB35 INT_GETH0
    */
 
+  phydbg("Configuring %08x\n", PIO_INT_ETH0);
   sam_configpio(PIO_INT_ETH0);
 #endif
 }
@@ -204,9 +228,18 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   DEBUGASSERT(intf);
 
+  nvdbg("%s: handler=%p\n", intf, handler);
+#ifdef CONFIG_SAMA5_EMACA
+  phydbg("EMAC: devname=%s\n", SAMA5_EMAC_DEVNAME);
+#endif
+#ifdef CONFIG_SAMA5_GMAC
+  phydbg("GMAC: devname=%s\n", SAMA5_GMAC_DEVNAME);
+#endif
+
 #ifdef CONFIG_SAMA5_EMACA
   if (strcmp(intf, SAMA5_EMAC_DEVNAME) == 0)
     {
+      phydbg("Select EMAC\n");
       phandler = &g_emac_handler;
       pinset   = PIO_INT_ETH1;
       irq      = IRQ_INT_ETH1;
@@ -216,6 +249,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 #ifdef CONFIG_SAMA5_GMAC
   if (strcmp(intf, SAMA5_GMAC_DEVNAME) == 0)
     {
+      phydbg("Select GMAC\n");
       phandler = &g_gmac_handler;
       pinset   = PIO_INT_ETH0;
       irq      = IRQ_INT_ETH0;
@@ -240,9 +274,21 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   /* Configure the interrupt */
 
+  if (handler)
+    {
+      phydbg("Configure pin: %08x\n", pinset);
       sam_pioirq(pinset);
+
+      phydbg("Enable IRQ: %d\n", irq);
       (void)irq_attach(irq, handler);
       sam_pioirqenable(irq);
+    }
+  else
+    {
+      phydbg("Disable IRQ: %d\n", irq);
+      (void)irq_detach(irq);
+      sam_pioirqdisable(irq);
+    }
 
   /* Return the old button handler (so that it can be restored) */
 

configs/sama5d3x-ek/src/sam_ethernet.c

@@ -39,6 +39,15 @@
 
 #include <nuttx/config.h>
 
+/* Force verbose debug on in this file only to support unit-level testing. */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  undef  CONFIG_DEBUG_VERBOSE
+#  define CONFIG_DEBUG_VERBOSE 1
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <assert.h>
 #include <debug.h>
@@ -69,6 +78,19 @@
 #  define SAMA5_EMAC_DEVNAME "eth1"
 #endif
 
+/* Debug ********************************************************************/
+/* Extra, in-depth debug output that is only available if
+ * CONFIG_NETDEV_PHY_DEBUG us defined.
+ */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  define phydbg    dbg
+#  define phylldbg  lldbg
+#else
+#  define phydbg(x...)
+#  define phylldbg(x...)
+#endif
+
 /************************************************************************************
  * Private Data
  ************************************************************************************/
@@ -117,9 +139,10 @@ void weak_function sam_netinitialize(void)
    *   Speed Mode:100Mbps
    *   Nway Auto-Negotiation:Enable
    *
-   * The KSZ8051 PHY interrtup is available on PE30 INT_ETH1
+   * The KSZ8051 PHY interrupt is available on PE30 INT_ETH1
    */
 
+  phydbg("Configuring %08x\n", PIO_INT_ETH1);
   sam_configpio(PIO_INT_ETH1);
 #endif
 
@@ -136,6 +159,7 @@ void weak_function sam_netinitialize(void)
    * The KSZ9021/31 interrupt is available on PB35 INT_GETH0
    */
 
+  phydbg("Configuring %08x\n", PIO_INT_ETH0);
   sam_configpio(PIO_INT_ETH0);
 #endif
 }
@@ -204,9 +228,18 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   DEBUGASSERT(intf);
 
+  nvdbg("%s: handler=%p\n", intf, handler);
+#ifdef CONFIG_SAMA5_EMACA
+  phydbg("EMAC: devname=%s\n", SAMA5_EMAC_DEVNAME);
+#endif
+#ifdef CONFIG_SAMA5_GMAC
+  phydbg("GMAC: devname=%s\n", SAMA5_GMAC_DEVNAME);
+#endif
+
 #ifdef CONFIG_SAMA5_EMACA
   if (strcmp(intf, SAMA5_EMAC_DEVNAME) == 0)
     {
+      phydbg("Select EMAC\n");
       phandler = &g_emac_handler;
       pinset   = PIO_INT_ETH1;
       irq      = IRQ_INT_ETH1;
@@ -216,6 +249,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 #ifdef CONFIG_SAMA5_GMAC
   if (strcmp(intf, SAMA5_GMAC_DEVNAME) == 0)
     {
+      phydbg("Select GMAC\n");
       phandler = &g_gmac_handler;
       pinset   = PIO_INT_ETH0;
       irq      = IRQ_INT_ETH0;
@@ -240,9 +274,21 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   /* Configure the interrupt */
 
+  if (handler)
+    {
+      phydbg("Configure pin: %08x\n", pinset);
       sam_pioirq(pinset);
+
+      phydbg("Enable IRQ: %d\n", irq);
       (void)irq_attach(irq, handler);
       sam_pioirqenable(irq);
+    }
+  else
+    {
+      phydbg("Disable IRQ: %d\n", irq);
+      (void)irq_detach(irq);
+      sam_pioirqdisable(irq);
+    }
 
   /* Return the old button handler (so that it can be restored) */
 

configs/sama5d4-ek/src/sam_ethernet.c

@@ -39,6 +39,15 @@
 
 #include <nuttx/config.h>
 
+/* Force verbose debug on in this file only to support unit-level testing. */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  undef  CONFIG_DEBUG_VERBOSE
+#  define CONFIG_DEBUG_VERBOSE 1
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <assert.h>
 #include <debug.h>
@@ -69,6 +78,19 @@
 #  define SAMA5_EMAC1_DEVNAME "eth0"
 #endif
 
+/* Debug ********************************************************************/
+/* Extra, in-depth debug output that is only available if
+ * CONFIG_NETDEV_PHY_DEBUG us defined.
+ */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  define phydbg    dbg
+#  define phylldbg  lldbg
+#else
+#  define phydbg(x...)
+#  define phylldbg(x...)
+#endif
+
 /************************************************************************************
  * Private Data
  ************************************************************************************/
@@ -101,10 +123,12 @@ static xcpt_t g_emac1_handler;
 void weak_function sam_netinitialize(void)
 {
 #ifdef CONFIG_SAMA5_EMAC0
+  phydbg("Configuring %08x\n", PIO_INT_ETH0);
   sam_configpio(PIO_INT_ETH0);
 #endif
 
 #ifdef CONFIG_SAMA5_EMAC1
+  phydbg("Configuring %08x\n", PIO_INT_ETH1);
   sam_configpio(PIO_INT_ETH1);
 #endif
 }
@@ -173,9 +197,18 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   DEBUGASSERT(intf);
 
+  nvdbg("%s: handler=%p\n", intf, handler);
+#ifdef CONFIG_SAMA5_EMAC0
+  phydbg("EMAC0: devname=%s\n", SAMA5_EMAC0_DEVNAME);
+#endif
+#ifdef CONFIG_SAMA5_EMAC0
+  phydbg("EMAC1: devname=%s\n", SAMA5_EMAC1_DEVNAME);
+#endif
+
 #ifdef CONFIG_SAMA5_EMAC0
   if (strcmp(intf, SAMA5_EMAC0_DEVNAME) == 0)
     {
+      phydbg("Select EMAC0\n");
       phandler = &g_emac0_handler;
       pinset   = PIO_INT_ETH0;
       irq      = IRQ_INT_ETH0;
@@ -185,6 +218,7 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 #ifdef CONFIG_SAMA5_EMAC1
   if (strcmp(intf, SAMA5_EMAC1_DEVNAME) == 0)
     {
+      phydbg("Select EMAC1\n");
       phandler = &g_emac1_handler;
       pinset   = PIO_INT_ETH1;
       irq      = IRQ_INT_ETH1;
@@ -209,9 +243,21 @@ xcpt_t arch_phy_irq(FAR const char *intf, xcpt_t handler)
 
   /* Configure the interrupt */
 
+  if (handler)
+    {
+      phydbg("Configure pin: %08x\n", pinset);
       sam_pioirq(pinset);
+
+      phydbg("Enable IRQ: %d\n", irq);
       (void)irq_attach(irq, handler);
       sam_pioirqenable(irq);
+    }
+  else
+    {
+      phydbg("Disable IRQ: %d\n", irq);
+      (void)irq_detach(irq);
+      sam_pioirqdisable(irq);
+    }
 
   /* Return the old button handler (so that it can be restored) */
 

drivers/net/Kconfig

@@ -349,4 +349,17 @@ config ETH1_PHY_DM9161
 	bool "Davicom DM9161 PHY"
 
 endchoice
+
+config NETDEV_PHY_DEBUG
+	bool "PHY debug"
+	default n
+	depends on DEBUG
+	---help---
+		Normally debug output is controlled by DEBUG_NET.  However, that
+		may generate a LOT of debug output, especially if DEBUG_VERBOSE is
+		also selected.  This option is intended to force VERVOSE debug
+		output from certain PHY-related even if DEBUG_NET or DEBUG_VERBOSE
+		are not selected.  This allows for focused, unit-level debug of
+		the NSH network initialization logic.
+
 endif # ARCH_HAVE_PHY

drivers/net/phy_notify.c

@@ -39,6 +39,15 @@
 
 #include <nuttx/config.h>
 
+/* Force verbose debug on in this file only to support unit-level testing. */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  undef  CONFIG_DEBUG_VERBOSE
+#  define CONFIG_DEBUG_VERBOSE 1
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <unistd.h>
 #include <string.h>
 #include <semaphore.h>
@@ -66,6 +75,19 @@
 #  define CONFIG_PHY_NOTIFICATION_NCLIENTS 4
 #endif
 
+/* Debug ********************************************************************/
+/* Extra, in-depth debug output that is only available if
+ * CONFIG_NETDEV_PHY_DEBUG us defined.
+ */
+
+#ifdef CONFIG_NETDEV_PHY_DEBUG
+#  define phydbg    dbg
+#  define phylldbg  lldbg
+#else
+#  define phydbg(x...)
+#  define phylldbg(x...)
+#endif
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
@@ -183,6 +205,7 @@ static FAR struct phy_notify_s *phy_find_unassigned(void)
           /* Return the client entry assigned to the caller */
 
           phy_semgive();
+          phydbg("Returning client %d\n", i);
           return client;
         }
     }
@@ -217,6 +240,7 @@ static FAR struct phy_notify_s *phy_find_assigned(FAR const char *intf,
           /* Return the matching client entry to the caller */
 
           phy_semgive();
+          phydbg("Returning client %d\n", i);
           return client;
         }
     }
@@ -240,6 +264,8 @@ static int phy_handler(FAR struct phy_notify_s *client)
   int ret;
 
   DEBUGASSERT(client && client->assigned);
+  phylldbg("Entry client %d, signalling PID=%d with signal %d\n",
+           client->index, client->pid, client->signo);
 
   /* Signal the client that the PHY has something interesting to say to us */
 
@@ -328,6 +354,8 @@ int phy_notify_subscribe(FAR const char *intf, pid_t pid, int signo,
   FAR struct phy_notify_s *client;
   DEBUGASSERT(intf);
 
+  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();
@@ -342,6 +370,7 @@ int phy_notify_subscribe(FAR const char *intf, pid_t pid, int signo,
   if (pid == 0)
     {
       pid = getpid();
+      phydbg("Actual PID=%d\n", pid);
     }
 
   /* Initialize the client entry */
@@ -386,6 +415,8 @@ int phy_notify_unsubscribe(FAR const char *intf, pid_t pid)
 {
   FAR struct phy_notify_s *client;
 
+  nvdbg("%s: PID=%d\n", intf, pid);
+
   /* Find the client entry for this interface */
 
   client = phy_find_assigned(intf, pid);