Skip to content

Enable new HAL us_ticker API on fast model MPS2 platform #7175

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

Merged
merged 5 commits into from
Jun 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 59 additions & 35 deletions targets/TARGET_ARM_FM/TARGET_FVP_MPS2/us_ticker.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,77 +16,101 @@
#include <stddef.h>
#include "us_ticker_api.h"
#include "PeripheralNames.h"

#define US_TICKER_TIMER1 CMSDK_DUALTIMER1
#define US_TICKER_TIMER2 CMSDK_DUALTIMER2
#define US_TICKER_TIMER_IRQn DUALTIMER_IRQn

int us_ticker_inited = 0;
/** mbed OS HAL API defined us_ticker as an increment ticker
* MPS2 platform provided in SSE-200 are decrement tickers
* with interrupt fired counter reaches 0.
*
* So 2 Timers are used to construct mbed OS HAL ticker.
*
* TIMER1 is for counting, and returns inverted binary when read from it
* TIMER1 will be kept in free-running mode (default, and not generate interrupts)
*
* TIMER2 is for generating interrupts
* So TIMER2 is set to periodic mode, which start decrement counting form LOADVALUE generates interrupts at 0
* and TIMER2 also set into one-shot mode, which counter halts when is reaches 0
*/

static int us_ticker_inited = 0;

void us_ticker_init(void)
{
if (us_ticker_inited) {
us_ticker_disable_interrupt();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what targets do? I dont recall seeing it before. init should disable interrupt , but in this case, why only if it was already initialized?

why us_ticker_inited variable is not static?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code implements the following ticker requirement:

* * The function ticker_init allows the ticker to keep counting and disables the ticker interrupt - Verified by test ::ticker_init_test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, calling us_ticker_disable_interrupt() on an uninitialized ticker is an undefined behavior in HAL
I re-checked, upper level dirvers are not use this variable, it can be staticed, good catch.

return;
}
us_ticker_inited = 1;

US_TICKER_TIMER1->TimerControl = 0x0; // disable timer
US_TICKER_TIMER2->TimerControl = 0x00; // disable timer
US_TICKER_TIMER1->TimerLoad = 0xFFFFFFFF;
US_TICKER_TIMER2->TimerLoad = 0xFFFFFFFF;
US_TICKER_TIMER1->TimerControl = 0x0ul; // disable TIMER1 and reset all control
US_TICKER_TIMER2->TimerControl = 0x0ul; // disable TIMER2 and reset all control

US_TICKER_TIMER1->TimerLoad = 0xFFFFFFFFul;
US_TICKER_TIMER2->TimerLoad = 0xFFFFFFFFul;

US_TICKER_TIMER1->TimerControl |= CMSDK_DUALTIMER1_CTRL_SIZE_Msk; // set TIMER1 to 32 bit counter
US_TICKER_TIMER2->TimerControl |= CMSDK_DUALTIMER2_CTRL_SIZE_Msk; // set TIMER2 to 32 bit counter

US_TICKER_TIMER1->TimerControl = 0x62; // enable interrupt and set to 32 bit counter and set to periodic mode
US_TICKER_TIMER2->TimerControl = 0x42; // enable interrupt and set to 32 bit counter
US_TICKER_TIMER1->TimerControl |= 0x1 << CMSDK_DUALTIMER1_CTRL_PRESCALE_Pos; // set TIMER1 with 4 stages prescale
US_TICKER_TIMER2->TimerControl |= 0x1 << CMSDK_DUALTIMER2_CTRL_PRESCALE_Pos; // set TIMER2 with 4 stages prescale

US_TICKER_TIMER1->TimerControl |= 0x80; // enable counter
US_TICKER_TIMER2->TimerControl |= 0x80; // enable counter
US_TICKER_TIMER2->TimerControl |= CMSDK_DUALTIMER2_CTRL_MODE_Msk; // set TIMER2 periodic mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If periodic mode means that counter after reaching 0 is reloaded and continue counting, then it looks like this mode should be used for TIMER1 which is dedicated to count elapsed time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I previously mentioned in Wilfried's comment:
Based on SSE-200 TRM
the timer can be set in 2 main modes:
Free-running mode: which not triggers interrupts, and wraps counter form the MAX.
Periodic mode: generates interrupts when it reaches 0, and reload from TIMERLOAD value.

So this case, we use TIMER1 in the free-running mode for monitoring time eclipsing, and TIME2 in periodic mode, and reset TIMERLOAD when we set the interrupt.

maybe I should be put these as comments in the code, seem just reading from the mode name, it is a bit misleading.

US_TICKER_TIMER2->TimerControl |= CMSDK_DUALTIMER2_CTRL_ONESHOOT_Msk; // set TIMER2 one-shot mode

US_TICKER_TIMER1->TimerControl |= CMSDK_DUALTIMER1_CTRL_EN_Msk; // enable TIMER1 counter

NVIC_SetVector(US_TICKER_TIMER_IRQn, (uint32_t)us_ticker_irq_handler);
NVIC_EnableIRQ(US_TICKER_TIMER_IRQn);
us_ticker_inited = 1;
}

void us_ticker_free(void)
{
US_TICKER_TIMER1->TimerControl &= ~CMSDK_DUALTIMER1_CTRL_EN_Msk; // disable TIMER1
US_TICKER_TIMER2->TimerControl &= ~CMSDK_DUALTIMER2_CTRL_EN_Msk; // disable TIMER2
us_ticker_disable_interrupt();
us_ticker_inited = 0;
}

uint32_t us_ticker_read()
{
uint32_t return_value = 0;
if (!us_ticker_inited) {
us_ticker_init();
}
return_value = ((~US_TICKER_TIMER2->TimerValue) / 25);
return return_value;
return ~US_TICKER_TIMER1->TimerValue;
}

void us_ticker_set_interrupt(timestamp_t timestamp)
{
if (!us_ticker_inited) {
us_ticker_init();
}

uint32_t delta = timestamp - us_ticker_read();
// enable interrupt
US_TICKER_TIMER1->TimerControl = 0x0; // disable timer
US_TICKER_TIMER1->TimerControl = 0x62; // enable interrupt and set to 32 bit counter and set to periodic mode
US_TICKER_TIMER1->TimerLoad = (delta) * 25; //initialise the timer value
US_TICKER_TIMER1->TimerControl |= 0x80; //enable timer
uint32_t delta = timestamp - us_ticker_read();
US_TICKER_TIMER2->TimerControl &= ~CMSDK_DUALTIMER2_CTRL_EN_Msk; // disable TIMER2
US_TICKER_TIMER2->TimerLoad = delta; // Set TIMER2 load value
US_TICKER_TIMER2->TimerControl |= CMSDK_DUALTIMER2_CTRL_INTEN_Msk; // enable TIMER2 interrupt
US_TICKER_TIMER2->TimerControl |= CMSDK_DUALTIMER2_CTRL_EN_Msk; // enable TIMER2 counter
NVIC_EnableIRQ(US_TICKER_TIMER_IRQn);
}

void us_ticker_fire_interrupt(void)
{
NVIC_EnableIRQ(US_TICKER_TIMER_IRQn);
NVIC_SetPendingIRQ(US_TICKER_TIMER_IRQn);
}


void us_ticker_disable_interrupt(void)
{

US_TICKER_TIMER1->TimerControl &= 0xDF;
US_TICKER_TIMER2->TimerControl &= 0xDF;

US_TICKER_TIMER2->TimerControl &= ~CMSDK_DUALTIMER2_CTRL_INTEN_Msk;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also disable ticker interrupt, not only TIMER2:
NVIC_DisableIRQ(US_TICKER_TIMER_IRQn);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVIC_DisableIRQ() added

US_TICKER_TIMER2->TimerControl &= ~CMSDK_DUALTIMER2_CTRL_EN_Msk; // disable TIMER2
NVIC_DisableIRQ(US_TICKER_TIMER_IRQn);
}

void us_ticker_clear_interrupt(void)
{
US_TICKER_TIMER2->TimerIntClr = CMSDK_DUALTIMER2_INTCLR_Msk;
}

US_TICKER_TIMER1->TimerIntClr = 0x1;
US_TICKER_TIMER2->TimerIntClr = 0x1;

const ticker_info_t *us_ticker_get_info(void)
{
static const ticker_info_t info = {
1562500, // 4 stages prescaled from 25MHz (dived by 16)
32 // 32 bit counter
};
return &info;
}
2 changes: 1 addition & 1 deletion targets/targets.json
Original file line number Diff line number Diff line change
Expand Up @@ -4128,7 +4128,7 @@
"public": false,
"supported_toolchains": ["GCC_ARM", "ARM", "IAR"],
"OUTPUT_EXT": "elf",
"device_has": ["AACI", "ANALOGIN", "CLCD", "I2C", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "SERIAL", "SERIAL_FC", "SPI", "SPISLAVE", "TSC"],
"device_has": ["AACI", "ANALOGIN", "CLCD", "I2C", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "SERIAL", "SERIAL_FC", "SPI", "SPISLAVE", "TSC", "USTICKER"],
"release_versions": ["5"]
},
"FVP_MPS2_M0": {
Expand Down