Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbed OS 5.3.5 - Blinky does not work on xdot_l151cc target #3823

Closed
janjongboom opened this issue Feb 22, 2017 · 10 comments · Fixed by #3860
Closed

mbed OS 5.3.5 - Blinky does not work on xdot_l151cc target #3823

janjongboom opened this issue Feb 22, 2017 · 10 comments · Fixed by #3860

Comments

@janjongboom
Copy link
Contributor

Note: This is just a template, so feel free to use/remove the unnecessary things

Description

  • Type: Bug

Bug

Target
xdot_l151cc

Toolchain:
GCC_ARM

Toolchain version:
4.9.3

mbed-cli version:
(mbed --version)

meed-os sha:
bcf7085

Behavior:

Run mbed-os-example-blinky. Expected: LED1 blinks.

Actual: LED1 does not blink. LED stays on.

If you switch to mbed-os-5.3.1 it works...

cc @andcor02 @mfiore02

@mfiore02
Copy link
Contributor

I hooked up a debugger: the device is not locking up, but the write(0) isn't succeeding in changing the state of the pin.

Looks like the migration to a common GPIO source for all ST parts broke the behavior of gpio_write for the xDot and other L15XXC devices. I'm a little surprised this change wasn't tested more thoroughly.

mbed-os-5.3.1 implementation:

static inline void gpio_write(gpio_t *obj, int value)
{
    MBED_ASSERT(obj->pin != (PinName)NC);
    if (value) {
        *obj->reg_set = obj->mask;
    } else {
#if defined(TARGET_STM32L152RC) || defined(TARGET_STM32L151RC) || defined (TARGET_STM32L151CC)
        *obj->reg_set = obj->mask << 16;
#else
        *obj->reg_clr = obj->mask;
#endif
    }
}

mbed-os-5.3.5 implementation:

static inline void gpio_write(gpio_t *obj, int value)
{
    MBED_ASSERT(obj->pin != (PinName)NC);
    if (value) {
        *obj->reg_set = obj->mask;
    } else {
#ifdef GPIO_IP_WITHOUT_BRR
        *obj->reg_clr = obj->mask << 16;
#else
        *obj->reg_clr = obj->mask;
#endif
    }
}

This looks like a perfect spot for a new unit test - verify that GPIO outputs & inputs are functional.

  • connect 2 GPIOs together
  • configure 1 GPIO as digital output and 1 as digital input
  • verify that change in state on the output is reflected by the input

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 23, 2017

This looks like a perfect spot for a new unit test - verify that GPIO outputs & inputs are functional.

+1

Would this be satisfactory or can be improved: https://github.com/ARMmbed/ci-test-shield/blob/master/TESTS/API/DigitalIO/DigitalIO.cpp

cc @bcostm @adustm @LMESTM @jeromecoutant

@mfiore02
Copy link
Contributor

@0xc0170 LGTM. Love the use of templates to keep it clean!

@bcostm
Copy link
Contributor

bcostm commented Feb 27, 2017

@0xc0170 please change label to devices:multitech

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 27, 2017

@mfiore02 Can you send a fix ?

@0xc0170 please change label to devices:multitech

I changed however as the above message indicates, the issue is in the common target code?

@bcostm
Copy link
Contributor

bcostm commented Feb 27, 2017

Since PR #3665, the #define GPIO_IP_WITHOUT_BRR must be set for devices which don't have the BRR register. The STM32L151 has the BRR register (following datasheet) and the #define GPIO_IP_WITHOUT_BRR is NOT defined, so everything is correct to me.

I double checked the blinky LED example on the NUCLEO_L152RE (same family) and it is OK.

I tested again the ci-test digitalIO test and also OK:

+-----------------------+---------------+---------------------+----------------------------+--------+--------+--------+--------------------+
| target                | platform_name | test suite          | test case                  | passed | failed | result | elapsed_time (sec) |
+-----------------------+---------------+---------------------+----------------------------+--------+--------+--------+--------------------+
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_2/DIO_3 | 1      | 0      | OK     | 0.06               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_3/DIO_2 | 1      | 0      | OK     | 0.04               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_4/DIO_5 | 1      | 0      | OK     | 0.05               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_5/DIO_4 | 1      | 0      | OK     | 0.05               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_6/DIO_7 | 1      | 0      | OK     | 0.05               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_7/DIO_6 | 1      | 0      | OK     | 0.07               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_8/DIO_9 | 1      | 0      | OK     | 0.05               |
| NUCLEO_L152RE-GCC_ARM | NUCLEO_L152RE | tests-api-digitalio | Digital I/O on DIO_9/DIO_8 | 1      | 0      | OK     | 0.05               |
+-----------------------+---------------+---------------------+----------------------------+--------+--------+--------+--------------------+
mbedgt: test case results: 8 OK

I don't see where the error could be on your device ???

@janjongboom janjongboom mentioned this issue Feb 27, 2017
@mfiore02
Copy link
Contributor

mfiore02 commented Feb 27, 2017

@bcostm I agree that everything looks great on the L152RE. I added some debug logging to the app and the gpio driver:

#include "mbed.h"

DigitalOut led1(LED1);

// main() runs in its own thread in the OS
// (note the calls to wait below for delays)
int main() {
    int val;
    while (true) {
        val = led1.read();
        printf("led = %d\r\n\r\n", val);

        led1 = !led1;

        val = led1.read();
        printf("led = %d\r\n\r\n", val);

        wait(5);
    }
}
static inline void gpio_write(gpio_t *obj, int value)
{
    MBED_ASSERT(obj->pin != (PinName)NC);
    printf("writing IO\r\n");
    printf("value           %d\r\n", value);
    printf("obj->reg_set    %08X\r\n", obj->reg_set);
    printf("obj->reg_clr    %08X\r\n", obj->reg_clr);
    printf("obj->mask       %08X\r\n\r\n", obj->mask);
    if (value) {
        *obj->reg_set = obj->mask;
    } else {
#ifdef GPIO_IP_WITHOUT_BRR
        *obj->reg_clr = obj->mask << 16;
#else
        *obj->reg_clr = obj->mask;
#endif
    }
}

static inline int gpio_read(gpio_t *obj)
{
    MBED_ASSERT(obj->pin != (PinName)NC);
    printf("reading IO\r\n");
    printf("obj->reg_in     %08X\r\n", obj->reg_in);
    printf("*obj->reg_in    %08X\r\n", *obj->reg_in);
    printf("obj->mask       %08X\r\n\r\n", obj->mask);
    return ((*obj->reg_in & obj->mask) ? 1 : 0);
}

I ran this code on both the xDot and the Nucleo-L152RE. The Nucleo worked perfectly, but the xDot did not. It looks like the writes to GPIOx->BRR are not having any effect.

This snippet is from the xDot's debug output.

writing IO
value           0
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000010

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E608
obj->mask       00000010

led = 0

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E608
obj->mask       00000010

writing IO
value           1
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000010

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E618
obj->mask       00000010

led = 1

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E618
obj->mask       00000010

led = 1

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E618
obj->mask       00000010

writing IO
value           0
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000010

reading IO
obj->reg_in     40020010
*obj->reg_in    0000E618
obj->mask       00000010

led = 1

The value of the IO doesn't change with writes to GPIOx->BRR after it's been set using GPIOx->BSRR.

However, the Nucleo works perfectly.

writing IO
value           0
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000020

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B808
obj->mask       00000020

led = 0

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B808
obj->mask       00000020

writing IO
value           1
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000020

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B828
obj->mask       00000020

led = 1

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B828
obj->mask       00000020

led = 1

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B828
obj->mask       00000020

writing IO
value           0
obj->reg_set    40020018
obj->reg_clr    40020028
obj->mask       00000020

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B808
obj->mask       00000020

led = 0

reading IO
obj->reg_in     40020010
*obj->reg_in    0000B808
obj->mask       00000020

led = 0

I didn't see anything about the BRR in the STM32L151CC Errata document.

If I define GPIO_IP_WITHOUT_BRR for the L1 devices, the blinky example works properly again for the xDot.

Is there any advantage in using GPIOx->BRR instead of GPIOx->BSRR to write 0s to GPIO pins if the register is available? Nothing I've seen has given me that impression. My understanding is that both registers provide atomic GPIO operations. Some discussion here.

Why don't we toss the GPIO_IP_WITHOUT_BRR macro completely and use GPIOx->BSRR for all ST devices? It would simplify the codebase and resolve the issue for the xDot. I'd be happy to make a PR with the change. Is there a downside to that solution?

@bcostm
Copy link
Contributor

bcostm commented Feb 28, 2017

It looks like the BRR register is not present in the L151CC device despite the fact that it is written in its datasheet. Very strange, I will check internally in ST.

Concerning the usage of BSRR, the only difference I can see is the addition of a 16-bits shift. This will slower the performances.

I think the best is to add the GPIO_IP_WITHOUT_BRR only for platforms using L151xx devices. I suggest to add it in the objects.h file. What do you think ?

@mfiore02
Copy link
Contributor

I have in fact already tested that solution for the xDot and it works like a charm. The Nucleo works fine as is. I'm not sure if TARGET_MOTE_L152RC needs the fix. I'd assume TARGET_NZ32_SC151 does as it appears to be a L151CC as well.

I'm happy to include either of these boards in my PR if they need the fix, but I'm not sure if they do since I don't have them to test. Can somebody let me know?

@mfiore02
Copy link
Contributor

@0xc0170 Are the ci-test-shield unit tests you referenced above part of the validation of a mbed-os release for all platforms?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants