Page 1 of 1

I2C in a thread causes a chSysHalt (on every board I try)

Posted: Sun Nov 29, 2015 10:01 pm
by mikemoretti
Hi,

I'm currently using a Nucleo F411RE with a MCP23017. I have i2c on GPIOB 6,7 (SCL, SDA), a gpio interrupt off its INTA pin attached to GPIOB 4, and GPIOB 5 on its reset pin. I can do as many i2c write/reads as I want from the main() function and everything works, I press a button, it generates an interrupt, I read the MCP23017 registers and I get proper values. However, as soon as I try to do ANYTHING with i2c in a thread, it halts in i2c_lld_serve_rx_end_irq / chThdResumeI. This happens no matter what nucleo I try, I've tried a F401, a L152 and some others just to make sure it's not the F4 that's the issue. I'm using ChibiStudio, chibios 3, and I copied the Nucleo F411RE project (and edited it). I disabled all the other peripherals I don't need except I2C in both the mcuconf and halconf. I left everything else alone (and I turned on debug stuff in chconf). I also tried both STD_DUTY_CYCLE and FAST_DUTY_CYCLE_2. I have 1.5k pullups on the SDA/SCL lines.

Does it look like there's anything I'm doing wrong in my code? Anyone have any ideas?

Thanks!

Here's my stack trace:
chibii2cbug.png
chibii2cbug.png (12.93 KiB) Viewed 3177 times


Here's my code (I've included the entire example in case someone wants to build it with their own confs and so you can see everything I've tried to make this work):

Code: Select all

#include <string.h>
#include "ch.h"
#include "hal.h"
#include "test.h"

static const I2CConfig i2cfg1  = {
        .op_mode = OPMODE_I2C,
        .clock_speed = 100000,
        .duty_cycle = STD_DUTY_CYCLE,
};

typedef enum {
    MCP23017_PIN0 = 0,
    MCP23017_PIN1 = 2,
    MCP23017_PIN2 = 4,
    MCP23017_PIN3 = 8,
    MCP23017_PIN4 = 16,
    MCP23017_PIN5 = 32,
    MCP23017_PIN6 = 64,
    MCP23017_PIN7 = 128,
} MCP23017_Pin_t;

typedef struct {
    GPIO_TypeDef *port;
    uint32_t pin;
} GPIOPin_t;

typedef struct {
    uint8_t address;
    GPIOPin_t  intPin;
    uint16_t intEnabledPins; // Internal, DO NOT MESS WITH THIS!
} MCP23017_t;

typedef enum {
    REG_IODIRA = 0,
    REG_IODIRB,
    REG_IPOLA,
    REG_IPOLB,
    REG_GPINTENA,
    REG_GPINTENB,
    REG_DEFVALA,
    REG_DEFVALB,
    REG_INTCONA,
    REG_INTCONB,
    REG_IOCONA,
    REG_IOCONB,
    REG_GPPUA,
    REG_GPPUB,
    REG_INTFA,
    REG_INTFB,
    REG_INTCAPA,
    REG_INTCAPB,
    REG_GPIOA,
    REG_GPIOB,
    REG_OLATA,
    REG_OLATB,
} MCP23017_Registers_t;

typedef enum {
    IOCON_UNIMPLEMENTED = 0,
    IOCON_INTPOL = 2,
    IOCON_ODR = 4,
    IOCON_HAEN = 8,
    IOCON_DISSLW = 16,
    IOCON_SEQOP = 32,
    IOCON_MIRROR = 64,
    IOCON_BANK = 128,
} MCP23017_IOCON_Bits_t;

#define MCP23017_ADDRESS(chip) (0x20 | chip->address)

#define PortAByte(x) (x & 0xFF)
#define PortBByte(x) (x >> 8)
#define PortABWord(buff) (buff[0] | (buff[1] << 8))

void MCP23017_Init(MCP23017_t *chip) {
    chip->intEnabledPins = 0;
}

void MCP23017_SetInterruptPins(MCP23017_t* chip, uint16_t pins) {
    // This will write GPINTENA, B, DEFVALA, B, and INTCONA, B all at the same time because we're in SEQ mode
    // We just give it the start address and it bumps the address for each successive byte
    // (all of those registers are in successive address order)
    // Interrupt compares on previous pin value
    uint8_t data[] = { REG_GPINTENA, PortAByte(pins), PortBByte(pins), 0, 0, 0, 0 };
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), data, sizeof(data), NULL, 0, MS2ST(1000));
    chip->intEnabledPins = pins;
}

void MCP23017_MirrorInterrupt(MCP23017_t* chip, uint8_t mirror) {
    // First, read IOCON
    uint8_t getIOCON[] = { REG_IOCONA };
    uint8_t ioCONValue[] = { 0, 0 }; // Fucking lame i2c bug requires minimum 2 byte read
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), getIOCON, sizeof(getIOCON), ioCONValue, sizeof(ioCONValue), MS2ST(1000));
    // Now munge the MIRROR bit and write IOCON back out
    uint8_t setIOCON[] = { REG_IOCONA, 0 };
    if (mirror)
        setIOCON[1] = *ioCONValue | IOCON_MIRROR;
    else
        setIOCON[1] = *ioCONValue & ~IOCON_MIRROR;
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), setIOCON, sizeof(setIOCON), NULL, 0, MS2ST(1000));
}

void MCP23017_SetPullups(MCP23017_t* chip, uint16_t pins) {
    uint8_t data[] = { REG_GPPUA, PortAByte(pins), PortBByte(pins) };
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), data, sizeof(data), NULL, 0, MS2ST(1000));
}

/// @brief gets the interrupt capture values for pins with interrupts enabled, and gpio values for the ones that aren't
uint16_t MCP23017_GetPinValues(MCP23017_t* chip) {
    uint16_t pinValues = 0;

    if (chip->intEnabledPins != 0) {
        uint8_t getIntPins[] = { REG_INTCAPA };
        uint8_t intPins[2];
        i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), getIntPins, sizeof(getIntPins), intPins, sizeof(intPins), MS2ST(1000));
        pinValues = PortABWord(intPins);
    }

    uint8_t getGPIOPins[] = { REG_GPIOA };
    uint8_t gpioPins[2];
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), getGPIOPins, sizeof(getGPIOPins), gpioPins, sizeof(gpioPins), MS2ST(1000));
    uint16_t gpioValue = PortABWord(gpioPins);
    pinValues |= (gpioValue & ~chip->intEnabledPins); // Don't let current gpio values overwrite the intcap values

    return pinValues;
}

void MCP23017_GetRegisters(MCP23017_t* chip, uint8_t *registers) {
    uint8_t getRegisters[] = { REG_IODIRA };
    i2cMasterTransmitTimeout(&I2CD1, MCP23017_ADDRESS(chip), getRegisters, sizeof(getRegisters), registers, 22, MS2ST(1000));
}

static uint8_t pinInterrupted = 0;

// Tried this too with no luck
//CH_IRQ_HANDLER(EXTI4_Handler) {
//  CH_IRQ_PROLOGUE();
////  chSysLockFromISR();
//  pinInterrupted = 1;
////  chSysUnlockFromISR();
//  CH_IRQ_EPILOGUE();
//}

static thread_reference_t thread2Wakeup = NULL;


static void keypadInterrupt(EXTDriver *extp, expchannel_t channel)
{
  (void)extp;
  (void)channel;
  chSysLockFromISR();
//  pinInterrupted = 1;
  chThdResumeI(&thread2Wakeup, (msg_t)0x1337);
  chSysUnlockFromISR();
}

static const EXTConfig extcfg = {
                                 {
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_FALLING_EDGE | EXT_CH_MODE_AUTOSTART | EXT_MODE_GPIOB, keypadInterrupt},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL},
                                 {EXT_CH_MODE_DISABLED, NULL}
                                 }
};

typedef struct {
    uint16_t pins;
    uint32_t time;
} Press_t;

static MCP23017_t keypadNButtons = {
        .address = 0,
        .intPin = {
                    GPIOB,
                    4 },
};

static THD_WORKING_AREA(waThread1, 128);
static THD_FUNCTION(Thread1, arg) {

  (void)arg;
  chRegSetThreadName("blinker");

  static Press_t pinValuesOverTime[16];
  memset(pinValuesOverTime, 0, sizeof(pinValuesOverTime));
  uint16_t pinValuesIndex = 0;

  while (true) {
    chSysLock();
    msg_t msg = chThdSuspendS(&thread2Wakeup);
    chSysUnlock();
//    if (pinInterrupted) {
      chThdSleepMilliseconds(20);
//        pinInterrupted = 0;
      i2cAcquireBus(&I2CD1);
//      i2cStart(&I2CD1, &i2cfg1); // this didn't help either
        pinValuesOverTime[pinValuesIndex].pins = MCP23017_GetPinValues(&keypadNButtons);
//        i2cStop(&I2CD1);
        i2cReleaseBus(&I2CD1);
        pinValuesOverTime[pinValuesIndex].time = chVTGetSystemTime();
        pinValuesIndex++;
        pinValuesIndex %= sizeof(pinValuesOverTime);
//    }
  }
}

int main(void) {

  halInit();
  chSysInit();

  palSetPadMode(GPIOB, 5, PAL_MODE_OUTPUT_PUSHPULL | PAL_STM32_OSPEED_LOWEST);
  palSetPadMode(GPIOB, 6, PAL_MODE_ALTERNATE(4) | PAL_STM32_OTYPE_OPENDRAIN);
  palSetPadMode(GPIOB, 7, PAL_MODE_ALTERNATE(4) | PAL_STM32_OTYPE_OPENDRAIN);
  i2cStart(&I2CD1, &i2cfg1);
  osalSysPolledDelayX( MS2ST(100) );
//  chThdSleepMilliseconds(100);
  palClearPad(GPIOB, 5);
  osalSysPolledDelayX( MS2ST(100) );
//  chThdSleepMilliseconds(100);
  palSetPad(GPIOB, 5);
  osalSysPolledDelayX( MS2ST(100) );
//  chThdSleepMilliseconds(100);
  uint8_t initRegisters[22];
  i2cAcquireBus(&I2CD1);
//  i2cStart(&I2CD1, &i2cfg1);
  MCP23017_GetRegisters(&keypadNButtons, initRegisters);
  MCP23017_Init(&keypadNButtons);
  MCP23017_SetPullups(&keypadNButtons, 0xFFFF);
  MCP23017_SetInterruptPins(&keypadNButtons, 0xFFFF);
  MCP23017_MirrorInterrupt(&keypadNButtons, 1);
  uint8_t registers[22];
  MCP23017_GetRegisters(&keypadNButtons, registers);
//  i2cStop(&I2CD1);
  i2cReleaseBus(&I2CD1);
  extStart(&EXTD1, &extcfg);
//  NVIC_EnableIRQ(EXTI4_IRQn);

  /*
   * Creates the blinker thread.
   */
  chThdCreateStatic(waThread1, sizeof(waThread1), NORMALPRIO, Thread1, NULL);

  /*
   * Normal main() thread activity, in this demo it does nothing except
   * sleeping in a loop and check the button state.
   */
    while (true) {
// So, if I use my variable setting method and do this NOT in a thread, it works.  Neither variable setting or suspending/resuming works in a thread
//      if (pinInterrupted) {
//        osalSysPolledDelayX( MS2ST(20) ); // Debounce?
//          pinInterrupted = 0;
//          pinValuesOverTime[pinValuesIndex].pins = MCP23017_GetPinValues(&keypadNButtons);
//          pinValuesOverTime[pinValuesIndex].time = chVTGetSystemTime();
//          pinValuesIndex++;
//      }
      chThdSleepMilliseconds(50000);
    }
}

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 10:19 pm
by Giovanni
Hi,

chSysHalt() has a pointer parameter that points to an error string, what is the message there? I assume you already enabled debug options in chconf.h.

Giovanni

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 10:39 pm
by mikemoretti
Hi,

It's ChThdResumeI, I should've added that to my original message, sorry.

Thanks!
-m

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 10:42 pm
by mikemoretti
Here's a tarball of my entire project, so you can see all my configs, etc.

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 10:56 pm
by steved
Most likely stack - 128 bytes probably isn't enough. Main thread is typically allocated 400 bytes, IIRC.

I've just looked at a project I have running two I2C ports on separate threads, and that has 350 bytes of stack per thread. 32F051, but doubt your other processors are much different in their requirements (and RAM was tight for me, so I wouldn't have over-allocated stack).
If you're using ChibiStudio, the Chibi debug plugin might help show you what's going on.

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 11:03 pm
by Giovanni
Yes, likely a stack overflow, try increasing stacks. In chibistudio you can see how much space is left in each stack, you need to enable the option for stacks filling and overflow check in chconf.h.

Giovanni

Re: I2C in a thread causes a chSysHalt (on every board I try

Posted: Sun Nov 29, 2015 11:10 pm
by mikemoretti
YAY!!! That was it! It works! The only conf thing I hadn't done was the FILL option. I originally had tried to do this project with FreeRTOS and it would assert in a specific place when I ran out of stack. I gave up on FreeRTOS because I was using STM32 HAL and I just couldn't get it to work at all. I was expecting to see something similar w.r.t. stack stuff in ChibiOS, but I'm glad you were able to help me track it down. I plan on contributing most of my modules when I'm done with the project (MCP23017, ICM7228, etc drivers).

Thanks again!
-m