ATMega328 / Arduino

[Vik]

Yes, did that initially but I figured that I would find the root cause of the overflow.

The more I think about this, the more I think that this should go in. The result of multiplying two 16 bit numbers would definitely not always fit in a 16 bit number. And then you are taking that result and converting it to 32 bit for the remaining calculations (the subtraction and division) anyway before truncating it at the end.

[tinito]

As Vik sent me a PM about the AVR port, I noticed this thread.

A student here already wrote some LLDs for the ATMega (and the Arduino board files), which are available here:
https://github.com/matteoserva/ChibiOS-AVR/

He got ADC GPT, PWM, SPI, and Serial driver working.

I'm not an AVR user so I cannot help too much with the port, but I can contact the student, maybe he can contribute in some way.

[Vik]

Hi,

I have started my own tree on GitHub that has all my changes: https://github.com/viktorradnai/ChibiOS

I also started pulling in Matteo's work, so far I only got the ADC but will go through the rest as well.

Cheers,
Vik

[utzig]

Hi,

I was working on a driver for a nRF24L01+ module now, which I'm using mostly to re-test the SPI driver. I had this issue of overflows too and it took me a long time to find out. I changed CH_FREQUENCY from 100 to 100L as suggested and it solved the issue for me. I remember using the trunk sources 3 months ago when I wrote the initial SPI driver and I had no issues with overflows.

Giovanni, any idea what new code was commited in the last 3 months that may be the root cause of this issue?

Fabio Utzig

[utzig]

I found an old Chibios tree here and tested it and everything works. It is SVN revision 5548 (2013-04-06).

Tomorrow I'll try to find where exactly from there till now it broke.

Cheers

[Giovanni]

Yes, those macros received a fix 3 weeks ago in order to fix a problem, probably this overflow is a side effect of the fix.

Giovanni

[Vik]

Yes, those macros received a fix 3 weeks ago in order to fix a problem, probably this overflow is a side effect of the fix.

Yes, I can see the problem. It is a side effect.

Code: Select all

Index: chibios-svn/os/kernel/include/chvt.h
===================================================================
--- chibios-svn/os/kernel/include/chvt.h   (revision 5583)
+++ chibios-svn/os/kernel/include/chvt.h   (working copy)
@@ -55,8 +56,8 @@
  *
  * @api
  */
-#define MS2ST(msec) ((systime_t)(((((msec) - 1L) * CH_FREQUENCY) /          \
-                                   1000L) + 1L))
+#define MS2ST(msec)                                                         \
+  ((systime_t)((((msec) * CH_FREQUENCY - 1L) / 1000L) + 1L))
 
 /**
  * @brief   Microseconds to system ticks.
@@ -68,8 +69,8 @@
  *
  * @api
  */
-#define US2ST(usec) ((systime_t)(((((usec) - 1L) * CH_FREQUENCY) /          \
-                                  1000000L) + 1L))
+#define US2ST(usec)                                                         \
+  ((systime_t)((((usec) * CH_FREQUENCY - 1L) / 1000000L) + 1L))
 /** @} */
 
 /**

Note how in the previous version msec (or usec) was subtracted from 1L, which is a long value, before multiplying. This means that the result of that operation was stored in a long, then subsequently multiplied by CH_FREQUENCY (an int), with the result stored in a long again, because the intermediate value is a long.

In the new version two integer values are multiplied and the result stored as an int which overflows. This is then converted to long when 1L is substracted, but that makes no difference to the already truncted value.

Giovanni, I have a patch for this as discussed earlier.

Cheers,
Vik

[Giovanni]

I'll add a cast to uint32_t to both the parameter and CH_FREQUENCY in next version.

Giovanni

[utzig]

Hi,

I rolled back theses changes here and they fixed the issues with chThdSleep*. But I still have issues with some other functions. Right now the chSequentialStreamPut and chprintf functions just hang the system. They also do work in Revision 5548 (everything does actually!). So, there must be something else.

Now for the strange part... when I checkout the tree as "anonymous" from

https://chibios.svn.sourceforge.net/svn ... bios/trunk

I get Revision 5548.

When I checkout from

svn+ssh://[email protected]/p/chibios/svn/trunk

I get Revision 5890 (at least yesterday).

I see little differences in the URL but are not "trunk" trees supposed to be the same thing? What am I doing wrong here?

[Giovanni]

The first URL is the old repository on SF pre-update, it is frozen. You need to use the new URL.

Giovanni