WM8904 interface enable method now returns the previous interrupt state. Correct ordering of some WM8904: Need to provide MCLK before initializing the WM8904, not after

2 files changed, 104 insertions, 66 deletions

diff --git a/nuttx/configs/sama5d3x-ek/src/sam_wm8904.c b/nuttx/configs/sama5d3x-ek/src/sam_wm8904.c
index d44f9523c..919e96089 100644
--- a/nuttx/configs/sama5d3x-ek/src/sam_wm8904.c
+++ b/nuttx/configs/sama5d3x-ek/src/sam_wm8904.c
@@ -99,7 +99,7 @@ struct sama5d3ek_mwinfo_s
 
 static int  wm8904_attach(FAR const struct wm8904_lower_s *lower,
                           wm8904_handler_t isr, FAR void *arg);
-static void wm8904_enable(FAR const struct wm8904_lower_s *lower,
+static bool wm8904_enable(FAR const struct wm8904_lower_s *lower,
                           bool enable);
 
 /****************************************************************************
@@ -114,7 +114,7 @@ static void wm8904_enable(FAR const struct wm8904_lower_s *lower,
  * by the driver and is presumed to persist while the driver is active.
  */
 
-static struct sama5d3ek_mwinfo_s g_mxtinfo =
+static struct sama5d3ek_mwinfo_s g_wm8904info =
 {
   .lower =
   {
@@ -158,41 +158,60 @@ static int wm8904_attach(FAR const struct wm8904_lower_s *lower,
        */
 
       audvdbg("Attaching %p\n", isr);
-      g_mxtinfo.handler = isr;
-      g_mxtinfo.arg = arg;
+      g_wm8904info.handler = isr;
+      g_wm8904info.arg = arg;
     }
   else
     {
-      audvdbg("Detaching %p\n", g_mxtinfo.handler);
-      wm8904_enable(lower, false);
-      g_mxtinfo.handler = NULL;
-      g_mxtinfo.arg = NULL;
+      audvdbg("Detaching %p\n", g_wm8904info.handler);
+      (void)wm8904_enable(lower, false);
+      g_wm8904info.handler = NULL;
+      g_wm8904info.arg = NULL;
     }
 
   return OK;
 }
 
-static void wm8904_enable(FAR const struct wm8904_lower_s *lower, bool enable)
+static bool wm8904_enable(FAR const struct wm8904_lower_s *lower, bool enable)
 {
-  /* Enable or disable interrupts */
+  static bool enabled;
+  irqstate_t flags;
+  bool ret;
 
-  if (enable && g_mxtinfo.handler)
-    {
-      sam_pioirqenable(IRQ_INT_WM8904);
-    }
-  else
+  /* Has the interrupt state changed */
+
+  flags = irqsave();
+  if (enable != enabled)
     {
-      sam_pioirqdisable(IRQ_INT_WM8904);
+      /* Enable or disable interrupts */
+
+      if (enable && g_wm8904info.handler)
+        {
+          audvdbg("Enabling\n");
+          sam_pioirqenable(IRQ_INT_WM8904);
+          enabled = true;
+        }
+      else
+        {
+          audvdbg("Disabling\n");
+          sam_pioirqdisable(IRQ_INT_WM8904);
+          enabled = false;
+        }
     }
+
+  ret = enabled;
+  irqrestore(flags);
+  return ret;
 }
 
 static int wm8904_interrupt(int irq, FAR void *context)
 {
   /* Just forward the interrupt to the WM8904 driver */
 
-  if (g_mxtinfo.handler)
+  audvdbg("handler %p\n", g_wm8904info.handler);
+  if (g_wm8904info.handler)
     {
-      return g_mxtinfo.handler(&g_mxtinfo.lower, g_mxtinfo.arg);
+      return g_wm8904info.handler(&g_wm8904info.lower, g_wm8904info.arg);
     }
 
   /* We got an interrupt with no handler.  This should not
@@ -269,18 +288,6 @@ int sam_wm8904_initialize(int minor)
           goto errout_with_i2c;
         }
 
-      /* Now we can use these I2C and I2S interfaces to initialize the
-       * MW8904 which will return an audio interface.
-       */
-
-      wm8904 = wm8904_initialize(i2c, i2s, &g_mxtinfo.lower);
-      if (!wm8904)
-        {
-          auddbg("Failed to initialize the WM8904\n");
-          ret = -ENODEV;
-          goto errout_with_i2s;
-        }
-
       /* Configure the DAC master clock.  This clock is provided by PCK0 (PD30)
        * that is connected to the WM8904 MCLK.
        */
@@ -307,7 +314,19 @@ int sam_wm8904_initialize(int minor)
       if (ret < 0)
         {
           auddbg("ERROR: Failed to attach WM8904 interrupt: %d\n", ret);
-          goto errout_with_audio;
+          goto errout_with_i2s;
+        }
+
+      /* Now we can use these I2C and I2S interfaces to initialize the
+       * MW8904 which will return an audio interface.
+       */
+
+      wm8904 = wm8904_initialize(i2c, i2s, &g_wm8904info.lower);
+      if (!wm8904)
+        {
+          auddbg("Failed to initialize the WM8904\n");
+          ret = -ENODEV;
+          goto errout_with_irq;
         }
 
       /* No we can embed the WM8904/I2C/I2S conglomerate into a PCM decoder
@@ -320,7 +339,7 @@ int sam_wm8904_initialize(int minor)
         {
           auddbg("ERROR: Failed create the PCM decoder\n");
           ret = -ENODEV;
-          goto errout_with_irq;
+          goto errout_with_wm8904;
         }
 
       /* Create a device name */
@@ -351,9 +370,9 @@ int sam_wm8904_initialize(int minor)
    */
 
 errout_with_pcm:
+errout_with_wm8904:
 errout_with_irq:
   irq_detach(IRQ_INT_WM8904);
-errout_with_audio:
 errout_with_i2s:
 errout_with_i2c:
 errout:
diff --git a/nuttx/configs/sama5d4-ek/src/sam_wm8904.c b/nuttx/configs/sama5d4-ek/src/sam_wm8904.c
index 69f4cd557..c11eb1457 100644
--- a/nuttx/configs/sama5d4-ek/src/sam_wm8904.c
+++ b/nuttx/configs/sama5d4-ek/src/sam_wm8904.c
@@ -99,7 +99,7 @@ struct sama5d4ek_mwinfo_s
 
 static int  wm8904_attach(FAR const struct wm8904_lower_s *lower,
                           wm8904_handler_t isr, FAR void *arg);
-static void wm8904_enable(FAR const struct wm8904_lower_s *lower,
+static bool wm8904_enable(FAR const struct wm8904_lower_s *lower,
                           bool enable);
 
 /****************************************************************************
@@ -114,7 +114,7 @@ static void wm8904_enable(FAR const struct wm8904_lower_s *lower,
  * by the driver and is presumed to persist while the driver is active.
  */
 
-static struct sama5d4ek_mwinfo_s g_mxtinfo =
+static struct sama5d4ek_mwinfo_s g_wm8904info =
 {
   .lower =
   {
@@ -158,41 +158,60 @@ static int wm8904_attach(FAR const struct wm8904_lower_s *lower,
        */
 
       audvdbg("Attaching %p\n", isr);
-      g_mxtinfo.handler = isr;
-      g_mxtinfo.arg = arg;
+      g_wm8904info.handler = isr;
+      g_wm8904info.arg = arg;
     }
   else
     {
-      audvdbg("Detaching %p\n", g_mxtinfo.handler);
-      wm8904_enable(lower, false);
-      g_mxtinfo.handler = NULL;
-      g_mxtinfo.arg = NULL;
+      audvdbg("Detaching %p\n", g_wm8904info.handler);
+      (void)wm8904_enable(lower, false);
+      g_wm8904info.handler = NULL;
+      g_wm8904info.arg = NULL;
     }
 
   return OK;
 }
 
-static void wm8904_enable(FAR const struct wm8904_lower_s *lower, bool enable)
+static bool wm8904_enable(FAR const struct wm8904_lower_s *lower, bool enable)
 {
-  /* Enable or disable interrupts */
+  static bool enabled;
+  irqstate_t flags;
+  bool ret;
 
-  if (enable && g_mxtinfo.handler)
-    {
-      sam_pioirqenable(IRQ_INT_WM8904);
-    }
-  else
+  /* Has the interrupt state changed */
+
+  flags = irqsave();
+  if (enable != enabled)
     {
-      sam_pioirqdisable(IRQ_INT_WM8904);
+      /* Enable or disable interrupts */
+
+      if (enable && g_wm8904info.handler)
+        {
+          audvdbg("Enabling\n");
+          sam_pioirqenable(IRQ_INT_WM8904);
+          enabled = true;
+        }
+      else
+        {
+          audvdbg("Disabling\n");
+          sam_pioirqdisable(IRQ_INT_WM8904);
+          enabled = false;
+        }
     }
+
+  ret = enabled;
+  irqrestore(flags);
+  return ret;
 }
 
 static int wm8904_interrupt(int irq, FAR void *context)
 {
   /* Just forward the interrupt to the WM8904 driver */
 
-  if (g_mxtinfo.handler)
+  audvdbg("handler %p\n", g_wm8904info.handler);
+  if (g_wm8904info.handler)
     {
-      return g_mxtinfo.handler(&g_mxtinfo.lower, g_mxtinfo.arg);
+      return g_wm8904info.handler(&g_wm8904info.lower, g_wm8904info.arg);
     }
 
   /* We got an interrupt with no handler.  This should not
@@ -269,18 +288,6 @@ int sam_wm8904_initialize(int minor)
           goto errout_with_i2c;
         }
 
-      /* Now we can use these I2C and I2S interfaces to initialize the
-       * MW8904 which will return an audio interface.
-       */
-
-      wm8904 = wm8904_initialize(i2c, i2s, &g_mxtinfo.lower);
-      if (!wm8904)
-        {
-          auddbg("Failed to initialize the WM8904\n");
-          ret = -ENODEV;
-          goto errout_with_i2s;
-        }
-
       /* Configure the DAC master clock.  This clock is provided by PCK2 (PB10)
        * that is connected to the WM8904 MCLK.
        */
@@ -307,7 +314,19 @@ int sam_wm8904_initialize(int minor)
       if (ret < 0)
         {
           auddbg("ERROR: Failed to attach WM8904 interrupt: %d\n", ret);
-          goto errout_with_audio;
+          goto errout_with_i2s;
+        }
+
+      /* Now we can use these I2C and I2S interfaces to initialize the
+       * MW8904 which will return an audio interface.
+       */
+
+      wm8904 = wm8904_initialize(i2c, i2s, &g_wm8904info.lower);
+      if (!wm8904)
+        {
+          auddbg("Failed to initialize the WM8904\n");
+          ret = -ENODEV;
+          goto errout_with_irq;
         }
 
       /* No we can embed the WM8904/I2C/I2S conglomerate into a PCM decoder
@@ -320,7 +339,7 @@ int sam_wm8904_initialize(int minor)
         {
           auddbg("ERROR: Failed create the PCM decoder\n");
           ret = -ENODEV;
-          goto errout_with_irq;
+          goto errout_with_wm8904;
         }
 
       /* Create a device name */
@@ -351,9 +370,9 @@ int sam_wm8904_initialize(int minor)
    */
 
 errout_with_pcm:
+errout_with_wm8904:
 errout_with_irq:
   irq_detach(IRQ_INT_WM8904);
-errout_with_audio:
 errout_with_i2s:
 errout_with_i2c:
 errout: