From 803b540e8a52cbd2a446dd9a682dc40aa4284865 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 3 Aug 2016 12:46:54 -0600 Subject: [PATCH] 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 | 2 +- arch/sim/src/up_ioexpander.c | 102 +++++++++++++++++++-------- drivers/ioexpander/gpio_lower_half.c | 3 +- drivers/ioexpander/pcf8574.c | 18 +++-- drivers/ioexpander/skeleton.c | 3 + drivers/ioexpander/tca64xx.c | 18 +++-- include/nuttx/ioexpander/gpio.h | 1 + 7 files changed, 104 insertions(+), 43 deletions(-) diff --git a/arch/sim/src/up_internal.h b/arch/sim/src/up_internal.h index bcda5aae657..658ec0539bc 100644 --- a/arch/sim/src/up_internal.h +++ b/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 ************************************************************/ diff --git a/arch/sim/src/up_ioexpander.c b/arch/sim/src/up_ioexpander.c index 02b1c5c738b..393fdd06e6e 100644 --- a/arch/sim/src/up_ioexpander.c +++ b/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); - if ((priv->invert & (1 << pin)) != 0) - { - pinval = !pinval; - } + /* Edge triggered. Was there a change in the level? */ - /* Edge triggered. Set interrupt in function of edge type */ - - if ((!pinval && SIM_EDGE_FALLING(priv, pin)) || - ( pinval && SIM_EDGE_RISING(priv, pin))) + if ((diff & 1) != 0) { - 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); - - /* Perform pin interrupt callbacks */ - - for (i = 0; i < CONFIG_SIM_INT_NCALLBACKS; i++) + if (intstat != 0) { - /* 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 (match != 0) + if (priv->cb[i].cbfunc != NULL) { - /* Yes.. perform the callback */ + /* Did any of the requested pin interrupts occur? */ - (void)priv->cb[i].cbfunc(&priv->dev, match, - priv->cb[i].cbarg); + ioe_pinset_t match = intstat & priv->cb[i].pinset; + 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; diff --git a/drivers/ioexpander/gpio_lower_half.c b/drivers/ioexpander/gpio_lower_half.c index 492e7933282..a18d946e853 100644 --- a/drivers/ioexpander/gpio_lower_half.c +++ b/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; diff --git a/drivers/ioexpander/pcf8574.c b/drivers/ioexpander/pcf8574.c index 6f6a7d788fd..f28c2c9fff4 100644 --- a/drivers/ioexpander/pcf8574.c +++ b/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)) || - ((input & 1) != 0 && PCF8574_EDGE_RISING(priv, pin))) + if ((diff & 1) != 0) { - 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)) */ diff --git a/drivers/ioexpander/skeleton.c b/drivers/ioexpander/skeleton.c index 4ecd68a2280..03f15f8cffb 100644 --- a/drivers/ioexpander/skeleton.c +++ b/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; } diff --git a/drivers/ioexpander/tca64xx.c b/drivers/ioexpander/tca64xx.c index 72571378951..49b57ddbb45 100644 --- a/drivers/ioexpander/tca64xx.c +++ b/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)) || - ((input & 1) != 0 && TCA64_EDGE_RISING(priv, pin))) + if ((diff & 1) != 0) { - 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)) */ diff --git a/include/nuttx/ioexpander/gpio.h b/include/nuttx/ioexpander/gpio.h index 287d0fa9d20..c0d091753c1 100644 --- a/include/nuttx/ioexpander/gpio.h +++ b/include/nuttx/ioexpander/gpio.h @@ -42,6 +42,7 @@ #include +#include #include #include