• State Resolved
  • Locked Locked
  • Replies 7 replies
  • Answers 1 answer
  • Subscribers 30 subscribers
  • Views 418 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

MSP432P401R: Bug in SimpleLink SDK Button driver?

Nathan Vander Wilt
Intellectual 340 points

Part Number: MSP432P401R

While trying to figure out how the SysConfig UI settings actually got handled by the SimpleLink "Button" driver logic, I found in  "C:\ti\simplelink_msp432p4_sdk_3_40_01_02\source\ti\drivers\apps\Button.c" the following code:

GPIO_getConfig(hw->gpioIndex, &pinConfig);
if (pinConfig & GPIO_CFG_IN_NOPULL)
{
  /*
   * Must infer pull from trigger edge. There is likely an external pull
   * up/down attached to the button
   */
[…snip, code looks at INT_FALLING/INT_RISING config…]
}
else
{
  /* Using an internal pull up/down */
[…snip, code looks at PU/PD config…]
}

This code is probably not working as expected. The problem is that `GPIO_CFG_IN_NOPULL == GPIO_CFG_INPUT` so that any pin used with a button that is an input (which of course they all should be!) will take the first path for "no pull-up, configure based on interrupt edge" instead of the second path for "has pull-up, configure based on that".

The root cause of this is probably the questionable way the SimpleLink SDK uses "bitmask" constants (some are just "0" outright, others like GPIO_CFG_IN_NOPULL here are essentially "SOMETHING_RELATED | 0") which imo is sloppy but admittedly does read nicely in some code. The problem is that it also encourages code like this, which reads nicely but doesn't actually work as intended!

To fix this I think the conditions would need to be switched, i.e. start by checking `(pinConfig & (GPIO_CFG_IN_PU | GPIO_CFG_IN_PD))` and handle that case, else handle the NOPULL case as a fallback. (Or just actually record the SysConfig UI choice for e.g. "Active Low No Pull" in the generated hwAttrs in the first place...?)

  • 0
    TI__Mastermind 39775 points

    Hi 

    Can you tell me more details of "This code is probably not working as expected"?

    Are you referring the example code like "gpiointerrupt_MSP_EXP432P401R_tirtos_ccs"?

    After I better understand your problem, I will transmit it to our software team.

    Eason

  • 0 in reply to Eason Zhou
    Intellectual 340 points

    If a GPIO pin is configured as input it will have a config like `(GPIO_CFG_INPUT | CPIO_CFG_IN_PU)` — yes? The value of this combination would be 0x00030000.

    So `(pinConfig & GPIO_CFG_IN_NOPULL)` will evaluate to `(1 << GPIO_CFG_IO_LSB)` which is true — yes? The result is (0x00030000 & 0x00010000) which is 0x00010000.

    So if a GPIO pin is configure as input the code in Button_open at line 464 will take the first branch. This branch is "must infer pull from trigger edge" — yes? Even though the pin is configured to pull up.

    In order to take the "else" branch which starts at line 479 of Button.c (intended for if pull up or pull down) the GPIO will have to be configured as an output — yes?

  • 0 in reply to Nathan Vander Wilt
    TI__Mastermind 39775 points

    Here is the code to sent a GPIO to be input with pull up:

         GPIO_setConfig(CONFIG_GPIO_BUTTON_1, GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_FALLING);

    You can see that it doesn't use (GPIO_CFG_INPUT | GPIO_CFG_IN_PU), it only use GPIO_CFG_IN_PU

  • 0 in reply to Eason Zhou
    Intellectual 340 points

    Doesn't matter. The value of GPIO_CFG_INPUT is already set in the GPIO_CFG_IN_PU! And GPIO_CFG_IN_NOPULL is the exact same as GPIO_CFG_INPUT! Look:

    #define GPIO_CFG_IO_LSB            16
#define GPIO_CFG_INT_LSB           24
#define GPIO_CFG_INPUT             (((uint32_t) 1) << GPIO_CFG_IO_LSB)
#define GPIO_CFG_IN_NOPULL         (((uint32_t) 1) << GPIO_CFG_IO_LSB)
#define GPIO_CFG_IN_PU             (((uint32_t) 3) << GPIO_CFG_IO_LSB) 
#define GPIO_CFG_IN_INT_FALLING    (((uint32_t) 1) << GPIO_CFG_INT_LSB)

    So the value of your setConfig is 0x01030000. You can test that `(0x01030000 & GPIO_CFG_IN_NOPULL)` is true.

  • 0 in reply to Nathan Vander Wilt
    TI__Mastermind 39775 points

    Yes, you are right.

    I finally understand,  pinConfig & GPIO_CFG_IN_NOPULL will always be true. Because BIT16 of GPIO_CFG_IN_PU and GPIO_CFG_IN_NOPULL is all be 1.

    Is it right?

  • 0 in reply to Eason Zhou
    Intellectual 340 points

    Yes, that's correct. Thanks!

  • +1 in reply to Nathan Vander Wilt
    TI__Mastermind 39775 points

    I have report the problem to our related team. Thank you for your great help!