ErichStyger / mcuoneclipse

McuOnEclipse Processor Expert components and example projects
Other
731 stars 1.29k forks source link

Waitms() inaccurate on Kinetis #1

Closed indraastra closed 11 years ago

indraastra commented 11 years ago

Hi Erich,

I noticed that for my FRDM-KL25Z board, the Wait component is generating the following code for Waitms():

WAIT1_WaitCycles(1000);

Looking at your Processor Expert driver, I noticed that the other processor families have definitions like this:

NofCyclesMs(1, CPU_BUS_CLK_HZ);

When I put that in my code, my timing is a LOT more accurate, so why does the Kinetis require a fixed 1000 cycle delay?

Pointer to code:

https://github.com/erichstyger/mcuoneclipse/blob/master/Drivers/sw/Wait.drv#L546

Thanks! Vishal

indraastra commented 11 years ago

Ok, that was a silly replacement, since NofCyclesMS() just computes a number and doesn't actually do a wait. In any case, I'm still fairly puzzled by what exactly is going on because timings seem off when I check with a logic analyzer. It could be that 1000 cycles made sense for 8MHz and not for 24MHz? Setting the clock speed manually doesn't actually change that, so maybe I just need to do some manual tuning and then freeze the component. Also, I don't fully understand the LOW/SLOW/FAST clock speeds listed.

There was also an associated problem which I've narrowed down to the way GenericTimeDate works. I think this really is a bug in the timing, because when I changed the interrupt period from 10ms to 1 ms or 20ms, my measured time was getting scaled by those factors as well. Looking deeper at the GenericTimeDate component, I see what is probably a bug:

https://github.com/ErichStyger/mcuoneclipse/blob/master/Drivers/sw/GenericTimeDate.drv#L308 Var1 = TotalHthH; %>40/* Loading actual number of tens of ms */

It seems to be that TotalHthH is the number of ticks, not the number of tens of ms. Shouldn't it be this:

Var1 = TotalHthH * TmDt1_TICK_TIME_MS / 10; /* Loading actual number of tens of ms */

Also, you might want to change the use of "hundreds of seconds" to "hundredths of seconds".

Thanks! Vishal

ErichStyger commented 11 years ago

Hi Vishal, I appologize, I missed to turn on 'notification emails' for GitHub issues, so I did not realize that you have submitted an issue ticket :-(. I need to look into this 'wait' issue: for other processors I rely on the CPU component wait routine, but for Kinetis Processor Expert has not provided this, so I'm simply waiting about one nop per cycle. As you realized, this is not very accurate.

PS: the 'hundredths' typo has been fixed, thanks for pointing this out.

ErichStyger commented 11 years ago

Hi Vishal, ok, I worked on the Wait component for Kinetis. Now it waits the correct time. There were several issues with it, one of it that I need to use the CPU clock as base, and not the bus clock. Git has now the new committed sources, see these commits: https://github.com/ErichStyger/mcuoneclipse/commit/998e42b0b03c3a09f01df2baea7be25af1d2fd4f https://github.com/ErichStyger/mcuoneclipse/commit/952eb8ec9b3849cadeff32f245fd2f7ec5976a9e

I measured now pretty accurate waiting time, e.g. with e 48 MHz clock and expected waiting for 10 ms, it was waiting for 9.97 ms. Let me know if you see any further issues. I intensively tested it with CodeWarrior and gcc, and lightly with Keil and IAR.

Thanks for reporting! Erich

indraastra commented 11 years ago

Thank you, I definitely would not have been able to come up with that assembly code myself. I'll update components and see how it goes.

ErichStyger commented 11 years ago

You are welcome. As a heads up: for Kinetis it does not support different speed modes. For Kinetis you can define up as 9 different clock speeds. This is not supported (yet).

indraastra commented 11 years ago

Hi Erich,

I believe the tens of ms bug never got fixed:

Var1 = TmDt1_TICK_TIME_MS; /* Loading actual number of tens of ms */

This only works if your ticker increments every 10ms. I think it should be:

Var1 = (TotalHthH * TmDt1_TICK_TIME_MS) / 10; /* Loading actual number of tens of ms */