Fix various issues with I/O expander and GPIO lower half drivers from testing with simulated I/O expander

arch/sim/src/up_internal.h

@@ -300,7 +300,7 @@ int sim_ajoy_initialize(void);
 
 #ifdef CONFIG_SIM_IOEXPANDER
 struct ioexpander_dev_s;
-FAR struct ioexpander_dev_s *sim_initialize(void);
+FAR struct ioexpander_dev_s *sim_ioexpander_initialize(void);
 #endif
 
 /* up_tapdev.c ************************************************************/

arch/sim/src/up_ioexpander.c

@@ -343,7 +343,7 @@ static int sim_writepin(FAR struct ioexpander_dev_s *dev, uint8_t pin,
 
   DEBUGASSERT(priv != NULL && pin < CONFIG_IOEXPANDER_NPINS);
 
-  gpioinfo("pin=%u value=%u\n", pin, value);
+  gpioinfo("pin=%u value=%u\n", pin, (unsigned int)value);
 
   /* Set output pins default value (before configuring it as output) The
    * Output Port Register shows the outgoing logic levels of the pins
@@ -405,7 +405,8 @@ static int sim_readpin(FAR struct ioexpander_dev_s *dev, uint8_t pin,
   /* Return 0 or 1 to indicate the state of pin */
 
   retval = (((inval >> pin) & 1) != 0);
-  return ((priv->invert & (1 << pin)) != 0) ? !retval : retval;
+  *value = ((priv->invert & (1 << pin)) != 0) ? !retval : retval;
+  return OK;
 }
 
 /****************************************************************************
@@ -434,6 +435,9 @@ static int sim_multiwritepin(FAR struct ioexpander_dev_s *dev,
   uint8_t pin;
   int i;
 
+  gpioinfo("count=%d\n", count);
+  DEBUGASSERT(priv != NULL && pins != NULL && values != NULL && count > 0);
+
   /* Apply the user defined changes */
 
   for (i = 0; i < count; i++)
@@ -483,9 +487,8 @@ static int sim_multireadpin(FAR struct ioexpander_dev_s *dev,
   bool pinval;
   int i;
 
-  DEBUGASSERT(priv != NULL && pins != NULL && values != NULL && count > 0);
-
   gpioinfo("count=%d\n", count);
+  DEBUGASSERT(priv != NULL && pins != NULL && values != NULL && count > 0);
 
   /* Update the input status with the 8 bits read from the expander */
 
@@ -540,6 +543,9 @@ static FAR void *sim_attach(FAR struct ioexpander_dev_s *dev,
   FAR void *handle = NULL;
   int i;
 
+  gpioinfo("pinset=%lx callback=%p arg=%p\n",
+           (unsigned long)pinset, callback, arg);
+
   /* Find and available in entry in the callback table */
 
   for (i = 0; i < CONFIG_SIM_INT_NCALLBACKS; i++)
@@ -581,6 +587,8 @@ static int sim_detach(FAR struct ioexpander_dev_s *dev, FAR void *handle)
   FAR struct sim_dev_s *priv = (FAR struct sim_dev_s *)dev;
   FAR struct sim_callback_s *cb = (FAR struct sim_callback_s *)handle;
 
+  gpioinfo("handle=%p\n", handle);
+
   DEBUGASSERT(priv != NULL && cb != NULL);
   DEBUGASSERT((uintptr_t)cb >= (uintptr_t)&priv->cb[0] &&
               (uintptr_t)cb <= (uintptr_t)&priv->cb[CONFIG_SIM_INT_NCALLBACKS-1]);
@@ -602,11 +610,32 @@ static int sim_detach(FAR struct ioexpander_dev_s *dev, FAR void *handle)
 
 static ioe_pinset_t sim_int_update(FAR struct sim_dev_s *priv)
 {
+  ioe_pinset_t toggles;
   ioe_pinset_t diff;
   ioe_pinset_t input;
   ioe_pinset_t intstat;
   bool pinval;
   int pin;
+  int i;
+
+  /* First, toggle all input bits that have associated, attached interrupt
+   * handler.  This is a crude simulation for toggle interrupt inputs.
+   */
+
+  toggles = 0;
+  for (i = 0; i < CONFIG_SIM_INT_NCALLBACKS; i++)
+    {
+      /* Is there a callback attached?  */
+
+      if (priv->cb[i].cbfunc != NULL)
+        {
+          /* Yes, add the input pins to set of pins to toggle */
+
+          toggles |= (priv->cb[i].pinset & priv->inpins);
+        }
+    }
+
+  priv->inval = (priv->inval & ~toggles) | (~priv->inval & toggles);
 
   /* Check the changed bits from last read (Only applies to input pins) */
 
@@ -619,6 +648,10 @@ static ioe_pinset_t sim_int_update(FAR struct sim_dev_s *priv)
       return 0;
     }
 
+  gpioinfo("toggles=%lx inval=%lx last=%lx diff=%lx\n",
+           (unsigned long)toggles, (unsigned long)priv->inval,
+           (unsigned long)priv->last, (unsigned long)diff);
+
   priv->last = input;
   intstat    = 0;
 
@@ -626,20 +659,27 @@ static ioe_pinset_t sim_int_update(FAR struct sim_dev_s *priv)
 
   for (pin = 0; pin < CONFIG_IOEXPANDER_NPINS; pin++)
     {
-      if (SIM_EDGE_SENSITIVE(priv, pin) && (diff & 1))
+      if (SIM_EDGE_SENSITIVE(priv, pin))
         {
-          pinval = ((input & 1) != 0);
+          /* Edge triggered. Was there a change in the level? */
-          if ((priv->invert & (1 << pin)) != 0)
-            {
-              pinval = !pinval;
-            }
 
-          /* Edge triggered. Set interrupt in function of edge type */
+          if ((diff & 1) != 0)
-
-          if ((!pinval && SIM_EDGE_FALLING(priv, pin)) ||
-              ( pinval && SIM_EDGE_RISING(priv, pin)))
             {
-              intstat |= 1 << pin;
+              /* Get the value of the pin (accounting for inversion) */
+
+              pinval = ((input & 1) != 0);
+              if ((priv->invert & (1 << pin)) != 0)
+                {
+                  pinval = !pinval;
+                }
+
+              /* Set interrupt as a function of edge type */
+
+              if ((!pinval && SIM_EDGE_FALLING(priv, pin)) ||
+                  ( pinval && SIM_EDGE_RISING(priv, pin)))
+                {
+                  intstat |= 1 << pin;
+                }
             }
         }
       else /* if (SIM_LEVEL_SENSITIVE(priv, pin)) */
@@ -681,24 +721,28 @@ static void sim_interrupt_work(void *arg)
   /* Update the input status with the 32 bits read from the expander */
 
   intstat = sim_int_update(priv);
-
+  if (intstat != 0)
-  /* Perform pin interrupt callbacks */
-
-  for (i = 0; i < CONFIG_SIM_INT_NCALLBACKS; i++)
     {
-      /* Is this entry valid (i.e., callback attached)?  */
+      gpioinfo("intstat=%lx\n", (unsigned long)intstat);
 
-      if (priv->cb[i].cbfunc != NULL)
+      /* Perform pin interrupt callbacks */
+
+      for (i = 0; i < CONFIG_SIM_INT_NCALLBACKS; i++)
         {
-          /* Did any of the requested pin interrupts occur? */
+          /* Is this entry valid (i.e., callback attached)?  */
 
-          ioe_pinset_t match = intstat & priv->cb[i].pinset;
+          if (priv->cb[i].cbfunc != NULL)
-          if (match != 0)
             {
-              /* Yes.. perform the callback */
+              /* Did any of the requested pin interrupts occur? */
 
-              (void)priv->cb[i].cbfunc(&priv->dev, match,
+              ioe_pinset_t match = intstat & priv->cb[i].pinset;
-                                       priv->cb[i].cbarg);
+              if (match != 0)
+                {
+                  /* Yes.. perform the callback */
+
+                  (void)priv->cb[i].cbfunc(&priv->dev, match,
+                                           priv->cb[i].cbarg);
+                }
             }
         }
     }
@@ -756,7 +800,7 @@ static void sim_interrupt(int argc, wdparm_t arg1, ...)
  ****************************************************************************/
 
 /****************************************************************************
- * Name: sim_initialize
+ * Name: sim_ioexpander_initialize
  *
  * Description:
  *   Instantiate and configure the I/O Expander device driver to use the provided
@@ -772,7 +816,7 @@ static void sim_interrupt(int argc, wdparm_t arg1, ...)
  *
  ****************************************************************************/
 
-FAR struct ioexpander_dev_s *sim_initialize(void)
+FAR struct ioexpander_dev_s *sim_ioexpander_initialize(void)
 {
   FAR struct sim_dev_s *priv = &g_ioexpander;
   int ret;

drivers/ioexpander/gpio_lower_half.c

@@ -151,7 +151,7 @@ static int gplh_read(FAR struct gpio_dev_s *gpio, FAR bool *value)
 {
   FAR struct gplh_dev_s *priv = (FAR struct gplh_dev_s *)gpio;
 
-  DEBUGASSERT(priv != NULL && priv->ioe != NULL);
+  DEBUGASSERT(priv != NULL && priv->ioe != NULL && value != NULL);
 
   gpioinfo("pin%u: value=%p\n", priv->pin, value);
 
@@ -355,6 +355,7 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
   /* Initialize the non-zero elements of the newly allocated instance */
 
   priv->pin        = (uint8_t)pin;
+  priv->ioe        = ioe;
   gpio             = &priv->gpio;
   gpio->gp_pintype = (uint8_t)pintype;
   gpio->gp_ops     = &g_gplh_ops;

drivers/ioexpander/pcf8574.c

@@ -515,7 +515,8 @@ static int pcf8574_readpin(FAR struct ioexpander_dev_s *dev, uint8_t pin,
 
   /* Return 0 or 1 to indicate the state of pin */
 
-  ret = (regval >> (pin & 7)) & 1;
+  *value = (bool)((regval >> (pin & 7)) & 1);
+  ret = OK;
 
 errout_with_lock:
   pcf8574_unlock(priv);
@@ -811,14 +812,19 @@ static void pcf8574_int_update(void *handle, uint8_t input)
 
   for (pin = 0; pin < 8; pin++)
     {
-      if (PCF8574_EDGE_SENSITIVE(priv, pin) && (diff & 1))
+      if (PCF8574_EDGE_SENSITIVE(priv, pin))
         {
-          /* Edge triggered. Set interrupt in function of edge type */
+          /* Edge triggered. Was there a change in the level? */
 
-          if (((input & 1) == 0 && PCF8574_EDGE_FALLING(priv, pin)) ||
+          if ((diff & 1) != 0)
-              ((input & 1) != 0 && PCF8574_EDGE_RISING(priv, pin)))
             {
-              priv->intstat |= 1 << pin;
+              /* Set interrupt as a function of edge type */
+
+              if (((input & 1) == 0 && PCF8574_EDGE_FALLING(priv, pin)) ||
+                  ((input & 1) != 0 && PCF8574_EDGE_RISING(priv, pin)))
+                {
+                  priv->intstat |= 1 << pin;
+                }
             }
         }
       else /* if (PCF8574_LEVEL_SENSITIVE(priv, pin)) */

drivers/ioexpander/skeleton.c

@@ -337,6 +337,9 @@ static int skel_readpin(FAR struct ioexpander_dev_s *dev, uint8_t pin,
   /* Read the pin value */
 #warning Missing logic
 
+  /* Return the pin value via the value pointer */
+#warning Missing logic
+
   skel_unlock(priv);
   return ret;
 }

drivers/ioexpander/tca64xx.c

@@ -757,7 +757,8 @@ static int tca64_readpin(FAR struct ioexpander_dev_s *dev, uint8_t pin,
 
   /* Return 0 or 1 to indicate the state of pin */
 
-  ret = (regval >> (pin & 7)) & 1;
+  *value = (bool)((regval >> (pin & 7)) & 1);
+  ret = OK;
 
 errout_with_lock:
   tca64_unlock(priv);
@@ -1054,14 +1055,19 @@ static void tca64_int_update(FAR struct tca64_dev_s *priv, ioe_pinset_t input,
 
   for (pin = 0; pin < ngios; pin++)
     {
-      if (TCA64_EDGE_SENSITIVE(priv, pin) && (diff & 1))
+      if (TCA64_EDGE_SENSITIVE(priv, pin))
         {
-          /* Edge triggered. Set interrupt in function of edge type */
+          /* Edge triggered. Was there a change in the level? */
 
-          if (((input & 1) == 0 && TCA64_EDGE_FALLING(priv, pin)) ||
+          if ((diff & 1) != 0)
-              ((input & 1) != 0 && TCA64_EDGE_RISING(priv, pin)))
             {
-              priv->intstat |= 1 << pin;
+              /* Set interrupt as a function of edge type */
+
+              if (((input & 1) == 0 && TCA64_EDGE_FALLING(priv, pin)) ||
+                  ((input & 1) != 0 && TCA64_EDGE_RISING(priv, pin)))
+                {
+                  priv->intstat |= 1 << pin;
+                }
             }
         }
       else /* if (TCA64_LEVEL_SENSITIVE(priv, pin)) */

include/nuttx/ioexpander/gpio.h

@@ -42,6 +42,7 @@
 
 #include <nuttx/config.h>
 
+#include <sys/types.h>
 #include <stdint.h>
 #include <stdbool.h>