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

MSP430: periph_timer clock config wrong #8251

Open
roberthartung opened this issue Dec 13, 2017 · 15 comments · May be fixed by #20582
Open

MSP430: periph_timer clock config wrong #8251

roberthartung opened this issue Dec 13, 2017 · 15 comments · May be fixed by #20582
Assignees
Labels
Area: timers Area: timer subsystems Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@roberthartung
Copy link
Member

roberthartung commented Dec 13, 2017

The TelosB node and Tmote Sky node should be identical (hardware wise). However, using xtimer_sleep(2) is not sleeping two seconds, but more like 7 seconds. Will investigate.

@PeterKietzmann PeterKietzmann added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: MSP Platform: This PR/issue effects MSP-based platforms Area: timers Area: timer subsystems labels Dec 13, 2017
@PeterKietzmann
Copy link
Member

@roberthartung on my TelosB xtimer_sleep(1) takes about three seconds, just FYI

@roberthartung
Copy link
Member Author

roberthartung commented Dec 13, 2017

@PeterKietzmann Nevermind, it was 1 second, not two. But still, we're off the same factor!

@PeterKietzmann
Copy link
Member

Whoops, yes. It helps if you can read...

@roberthartung
Copy link
Member Author

Whoops, yes. It helps if you can read...

nono, it was wrong in the issue/my code. You didnt do any mistake :D I edited the issue sneaky beaky like ;)

@kYc0o kYc0o added this to To Do - Timer Issues in Bug tracker 2018.04 Jan 16, 2018
@kYc0o
Copy link
Contributor

kYc0o commented Jul 19, 2018

@roberthartung if the hardware is exactly the same, then the behaviour should be the same. AFAIK we don't support TmoteSky, so I guess there should be at least one difference. As you may know, xtimer timings rely strongly on the initialisation of the hardware timer, which must be multiple of 1MHz. Thus, for me it seems that the problem might be periph_timer related.

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@PeterKietzmann
Copy link
Member

@roberthartung does the issue still exist? I'll set the "don't stale" label so it's not automatically being closed

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 19, 2019
@roberthartung
Copy link
Member Author

@PeterKietzmann Good question. Have to test again!

@roberthartung
Copy link
Member Author

roberthartung commented Jan 31, 2020

@MichelRottleuthner Tested again with a simple xtimer_sleep(n), n \in {1,2,4}:

#include <stdio.h>
#include "xtimer.h"

int main(void)
{
    while (1) {
	printf("sleeping 1\r\n");
        xtimer_sleep(1);
	printf("done.\r\n");

	printf("sleeping 2\r\n");
        xtimer_sleep(2);
	printf("done.\r\n");

	printf("sleeping 4\r\n");
        xtimer_sleep(4);
	printf("done.\r\n");
    }
}

The configuration still seems off:

2020-01-31 08:56:19,779 # sleeping 1
2020-01-31 08:56:22,333 # done.
2020-01-31 08:56:22,348 # sleeping 2
2020-01-31 08:56:28,976 # done.
2020-01-31 08:56:28,991 # sleeping 4
2020-01-31 08:56:42,306 # done.

Tested on a MAXFOR MTM-CM5000MSP.

@MichelRottleuthner
Copy link
Contributor

I think this is not related to xtimer itself. Changing this in cpu/msp430fxyz/periph/timer.c:

---TIMER_BASE->CTL = (TIMER_CTL_TASSEL_SMCLK | TIMER_CTL_ID_DIV8);
+++TIMER_BASE->CTL = (TIMER_CTL_TASSEL_SMCLK | TIMER_CTL_ID_DIV2);

gives me something at least close to the expected speed.

Note: changing only the above line was just meant as a quick test and will probably break configuration of other peripherals driven by SMCLK (if not already broken as is).
The overall clock configuration needs to be considered for a proper fix.
Looking at the clock configuration and the periph_timer implementation it looks like the code could also use some love ;)
Even with a correction to the divider it may not be the best choice to drive the timer by the inaccurate DCO clock.
A quick look at the datasheet indicates that using X1 or X2 as clock sources might be better alternatives.

@maribu aren't you also using MSP430s? What do you think about this?

@maribu
Copy link
Member

maribu commented Jan 31, 2020

@maribu aren't you also using MSP430s?

Not really. I worked once on some code cleanup, but I even can remember what exactly I did. But I can recommend Inrias IoT-Lab for testing, if you (like me) have no MSP430 boards to test your changes on.

What do you think about this?

I have no idea about the hardware, but if I guess correctly currently an internal clock source (DSO) is used and you suggest to use an external crystal for higher precision. This sounds like a good idea, but maybe external crystals are not available on all boards. But in any case, I don't really have the expertise in the MSP430 area to be of much help.

@MichelRottleuthner
Copy link
Contributor

Ok, thanks for the quick response.

(...) but if I guess correctly (...)

Yep, that's pretty much what I wanted to say. I'm also no active user of the MSPs but from the datasheet it looks rather straight forward.

@roberthartung do you want to put some effort into fixing this? I'd be happy to assist.

@MichelRottleuthner MichelRottleuthner changed the title telosb: xtimer config wrong when running on a tmote sky MSP430: periph_timer clock config wrong Jan 31, 2020
@roberthartung
Copy link
Member Author

I don't think there is an external crystal available on these boards.

@MichelRottleuthner
Copy link
Contributor

Yep it seems like there is indeed no HF crystal available. But for the Tmote Sky it looks like it has at least some LF crystal X0.
They also say "though the DCO frequency changes with voltage and temperature, it may be calibrated by using the 32kHz oscillator".
Also it might be worth to check if there is some external resistor connected to the R_OSC to increase accuracy a bit.

@miri64 miri64 added this to the Release 2020.07 milestone Jul 1, 2020
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@maribu maribu linked a pull request Apr 30, 2024 that will close this issue
1 task
@maribu
Copy link
Member

maribu commented Apr 30, 2024

I just rediscovered this issue. #20582 fixes it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: timers Area: timer subsystems Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
No open projects
Bug tracker 2018.04
  
To Do - Timer Issues
Development

Successfully merging a pull request may close this issue.

7 participants