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

us_ticker_read() return a 0xFFFF less value for STM32F103RB. #816

Closed
leibin2014 opened this issue Dec 26, 2014 · 12 comments
Closed

us_ticker_read() return a 0xFFFF less value for STM32F103RB. #816

leibin2014 opened this issue Dec 26, 2014 · 12 comments

Comments

@leibin2014
Copy link
Contributor

I use the following code to simulate a 500us/2000us software PWM on PB_0 :

 #include "mbed.h"

DigitalOut out0(PB_0);

Timeout t;

extern void pin_off_callback();
void pin_on_callback()
{
    out0 = 1;
    t.attach_us(&pin_off_callback, 500);
}
void pin_off_callback()
{
    out0 = 0;
    t.attach_us(&pin_on_callback, 2000-500);
}

int main()
{
    out0 = 1;
    t.attach_us(&pin_off_callback, 500);
        while(1)
      {

      }
}

No problem is found when it run on STM32F100RB, but found a problem when run it on STM32F103RB. I found some 65ms continuous high(1) or low(0) level voltage on PB_0 each about 10 seconds.

After digging into thIS problem, I found this is due to in us_ticker.c of STM32F103RB it uses the same IRQ function for update and compare interrupts. So if us_ticker_read() is called in us_ticker_irq_handler(), and at the same time if a update interrupt happens, the variable SlaveCounter has no chance to increase 1. SlaveCounter will be just updated in the next calling of IRQ function. So the function us_ticker_read() will return a 0xFFFF less value in current IRQ function, then cause the 65ms blank area problem.

If running the previous code is easily to reproduce this problem on STM32F103RB. This problem can't be reproduced on STM32F100R since STM32F100RB uses different IRQ fucntions for update and compare interrupts with different priorities.

I know the cause, but don't have good idea about how to fix it. Could any one help to have a check and help to fix it?

@bcostm
Copy link
Contributor

bcostm commented Jun 10, 2015

This issue is related to the 16-bit timer used for the Ticker.
It is also present on STM32F0xx, STM32F1xx and STM32L0xx targets. Other targets using a 32-bit timer has not this problem (STM32F3xx, STM32F4xx, STM32L1xx).
No satisfying solution found yet.

@JojoS62
Copy link
Contributor

JojoS62 commented Jan 13, 2016

I see the same problem. The 65 ms burst looks like a 16 bit overflow: it is something like 65535 microseconds. Then after this burst, the new timestanps are calculated. As they are in the past, the INT is called immediately. This results in calls with high frequency until the missing calls are worked up and the normal cycle is running. Until the next overflow.
Screenshots show Ticker with 5 kHz toggling a portPin.
tickerburst1
tickerburst

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 14, 2016

@bcostm What's the status?

@bcostm
Copy link
Contributor

bcostm commented Jan 14, 2016

Same as in June 2015. No solution found on this problem.

@JojoS62
Copy link
Contributor

JojoS62 commented Jan 14, 2016

I think leibin2014 found already the reason. The problem is that a 32 bit counter is needed and the implementation to extend the 16 bit counter is wrong. The 16 bit overflow is not handled correctly, it does not increment the high word of the counter. My suggestion:

in: https://github.com/mbedmicro/mbed/blob/master/libraries/mbed/targets/hal/TARGET_STM/TARGET_STM32F1/us_ticker.c

// remove oc_int_part, oc_rem_part declararations

volatile uint32_t SlaveCounter = 0;
volatile uint16_t counterOld = 0;

uint32_t us_ticker_read()
{
    uint16_t counter;
    if (!us_ticker_inited) us_ticker_init();

    counter  = TIM_MST->CNT;
    if (counter < counterOld) {
        SlaveCounter += 0x10000;    // timer overflow, increment high word 
    }

    SlaveCounter += (SlaveCounter & 0xFFFF) + counter;
    counterOld = counter;

    return SlaveCounter;
}

I don't have the ARM/Keil toolchain for testing, maybe someone else can try this?

@bcostm
Copy link
Contributor

bcostm commented Jan 14, 2016

ok thanks. I have Keil and IAR. I will check this.

@JojoS62
Copy link
Contributor

JojoS62 commented Jan 14, 2016

I'm trying also with gcc-arm. And I see already that us_ticker_set_interrupt() needs to be modifyed also.

@JojoS62
Copy link
Contributor

JojoS62 commented Jan 14, 2016

sorry, that is too complicated for me at the moment. There is also a dependency on hal_tick.c, function timer_irq_handler(). This tries to handle time spans >65536 µs.

erwango pushed a commit to erwango/mbed that referenced this issue May 11, 2016
NUCLEO_F103RB is using a 16-bit timer as a internal ticker but the
mBed ticker needs a 32-bit timer implementation, so the upper part
of that 32-bit timer is being calculated in software.

Software bug has been fixed - Continous HIGH/LOW voltage levels
could be observerd for 65ms due to 16-bit timer overflow.

Now current value of TIM_MST->CNT is stored in cnt_val and is
updated in interrupt context only. This avoids master timer
overflow without SlaveCounter update.

Change-Id: I09cc262083b66e16affea14c4d2126287519b477
BartSX added a commit to BartSX/mbed that referenced this issue May 16, 2016
NUCLEO_L053R8 is using a 16 bit timer as a internal ticker but the mBed ticker
needs a 32 bit timer, so the upper upart of that timer is being calculated in
software. Continous HIGH/LOW voltage levels were observerd for 65ms due to timer
overflow, so to narrow down the issue, it was decided to switch to 16 bit values
and glue them to get a 32 bit timer.

Change-Id: I54a06d5aa0f8ddabd8abc194470845a2509e0c55
0xc0170 added a commit that referenced this issue May 23, 2016
BartSX pushed a commit to BartSX/mbed that referenced this issue May 23, 2016
Both STM32F0xx and STM32F1xx are using a 16-bit timer as a internal ticker
but the mBed ticker needs a 32-bit timer implementation, so the upper part
of that 32-bit timer is being calculated in software.

Software bug has been fixed where continous HIGH/LOW voltage levels
could be observerd for 65ms due to 16-bit timer overflow.

Now current value of TIM_MST->CNT is stored in cnt_val and is
updated in interrupt context only. This avoids master timer
overflow without SlaveCounter update.

This fix is only for platforms which already implements a 16-bit timer:
F103RB, F070RB, F030R8

Change-Id: I205c70ce155b373c6593ead93ade9ec38993f7f9
BartSX pushed a commit to BartSX/mbed that referenced this issue May 23, 2016
This path fixes issue ARMmbed#816.

Current value of TIM_MST->CNT is read in interrupt context only.
This avoids master timer overflow without SlaveCounter update.

Change-Id: Ie7a9bfce76990f85caa84264450d053604af33e5
BartSX added a commit to BartSX/mbed that referenced this issue May 23, 2016
This path fixes issue ARMmbed#816.

Current value of TIM_MST->CNT is read in interrupt context only.
This avoids master timer overflow without SlaveCounter update.

Change-Id: Iaaf7b9eb33aa8d8992e9354ca5e21bf01ec2413d
BartSX added a commit to BartSX/mbed that referenced this issue May 23, 2016
This path fixes issue ARMmbed#816.

Current value of TIM_MST->CNT is read in interrupt context only.
This avoids master timer overflow without SlaveCounter update.

Change-Id: I8e2ec02ce7539a4c044c7e3dfe6bedc9fcdf7736
0xc0170 added a commit that referenced this issue May 24, 2016
Fix timer #816 issue for STM32F0 and STM32F1
@JojoS62
Copy link
Contributor

JojoS62 commented May 26, 2016

how was this fix tested? A simple ticker test is still not working:

#include <mbed.h>


Ticker flipper;
DigitalOut led1(PC_13);
DigitalOut led2(PB_12);


void flip() {
    led2 = !led2;  
}

int main() 
{
    flipper.attach_us(&flip, 200); // the address of the function to be attached (flip) and the interval (x microseconds)

    // spin in a main loop. flipper will interrupt it to call flip
    while(1) {
      led1 = !led1;
      wait(0.5);
    }
}

Also led1 does not blink, so the wait() is not working too.

ticker-mbed-v121

@riktw
Copy link

riktw commented Jun 10, 2016

The fix from BartSX ([NUCLEO_L053R8] Fix issue #816 #1743) worked for me. My current us_ticker.c and hal_tick.c for STM32F103RB are:
http://pastebin.com/xurcXy2w

http://pastebin.com/nAF9qJYM

BartSX added a commit to BartSX/mbed that referenced this issue Jun 13, 2016
This is a partial revert of 07b841b and currently we are only reverting the
STM32F0xx family because new fix will be presented that's why we want to keep,
still the original solution only F1xx family (it will be fixed in future).

Change-Id: I147065299310c9fea499bf3ced8808a44699a1a1
BartSX pushed a commit to BartSX/mbed that referenced this issue Jun 13, 2016
This fix is a solution for issue ARMmbed#816 when having two separate IRQ handlers
in Timers (UPDATE Irq and OutputCompare Irq). The update priority needs to
be higher to avoid undefined behaviours.

Change-Id: I5ef2c27926167ed22108901cd63586692a5f8f90
BartSX pushed a commit to BartSX/mbed that referenced this issue Jun 13, 2016
This fix is a solution for issue ARMmbed#816 when having two separate IRQ handlers
in Timers (UPDATE Irq and OutputCompare Irq). The update priority needs to
be higher to avoid undefined behaviours.

Change-Id: Ic143ed0f3e4e42ad5f7b95337d8c005b7ec61274
0xc0170 added a commit that referenced this issue Jun 14, 2016
@ciarmcom
Copy link
Member

ciarmcom commented Aug 1, 2016

ARM Internal Ref: IOTMORF-301

@sg- sg- removed the mirrored label Aug 12, 2016
@bcostm
Copy link
Contributor

bcostm commented Apr 7, 2017

Can we close this issue ?
ST_TO_BE_CLOSED

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

No branches or pull requests

7 participants