Running on the Atmega2560

[mikenick42]

Hi,
I'm trying to run either ChibiOS or NilOS on a knockoff Arduino Mega. The blink and fade demos from the Arduino IDE work, so I'm pretty sure that at least the AVR chip is real.
Starting from the demos in ChibiOS/demos/AVR I can compile the NIL demo and load it using avrdude but then the LED just stays fully illuminated and never blinks.
The ChibiOS demo won't compile because of some issues with testthd.c. Once that is removed the LED is off and never blinks.
Since NIL compiles with no modifications, that's what I've been focusing on primarily. If I add a palClearPad in main between halInit() and chSysInit() the LED is very dimly illuminated as though it's being run with a low PWM setting.

Any help or advice is appreciated.

Thanks,
Mike

[Giovanni]

Hi,

Demos are done to work with a specific board, you need to create new "board files" if you want to execute them on a different board. Board files contain the initial I/O settings.

Giovanni

[utzig]

Hi,

I tested that demo on the ATmega1280, so you need to check if the same PIN is used for the LED on the 2560 (but I assume it is). Tonight I'll look into what error is happening with testthd.c.

Also, check this thread: viewtopic.php?f=2&t=197

There may be issues with the ATmega2560 because it uses a three byte PC. The modifications to make it work are described on the thread linked above and were also applied to the Chibios-Arduino repo: https://github.com/greiman/ChibiOS-Arduino (__AVR_3_BYTE_PC__ on /libraries/ChibiOS_AVR/utility/chcore.h). If that solves your problem please mention it here and I will apply the modifications.

Cheers,
Fabio Utzig

[utzig]

Btw, also change the MCU in the demo Makefile from:

Code: Select all

MCU = atmega1280

to:

Code: Select all

MCU = atmega2560

[mikenick42]

It took me way too long to realize that I needed to edit nilcore.h, but adding pcx to the struct and the setup works with the demo.

The error I'm seeing in Chibi, after some notes and warnings about default return types, is:

Code: Select all

../../../test/rt/testthd.c: In function ‘thd1_execute’:
../../../test/rt/testthd.c:67:73: error: ‘thread’ undeclared (first use in this function)
   threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriorityX()-5, thread, "E");

[utzig]

Hi,

Would you mind providing a patch so that I can apply it later? (a gist on github would also be ok!)

Your error is interesting considering that thread is declared just above the line that generated the error. I'll try harder to not forget to look into this tonight! Also, did you have to change the MCU type on the Makefile?

[mikenick42]

Here's the patch to nilconf.h and chconf.h that is working for me with a 2560. I don't have anything with a 1280 to test with.
The makefile does need to be changed, the 2560 and the 1280 have different device signatures.
NIL seems to be working fine. ChibiOS will load and can run a while loop in main to blink the LED, but anything thread related is causing issues.

Starting when things break, here is the full log of running make on the RT-ARDUINOMEGA project:

Code: Select all

Compiling C: ../../../test/rt/testthd.c
avr-gcc -c -mmcu=atmega2560 -I. -gdwarf-2 -DF_CPU=16000000UL -O2 -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -Wall -Wstrict-prototypes -Wa,-adhlns=./../../../test/rt/testthd.lst -I../../../os/rt/ports/AVR -I../../../os/rt/ports/AVR/compilers/GCC -I../../../os/rt/include -I../../../test/rt -I../../../os/hal/include -I../../../os/hal/osal/rt -I../../../os/hal/ports/AVR -I../../../os/hal/boards/ARDUINO_MEGA -I../../../os/various -std=gnu11 -Wundef -MMD -MP -MF .dep/testthd.o.d ../../../test/rt/testthd.c -o ../../../test/rt/testthd.o
In file included from ../../../os/rt/include/ch.h:78:0,
                 from ../../../test/rt/testthd.c:17:
../../../os/rt/include/chschd.h:158:34: warning: return type defaults to ‘int’ [enabled by default]
 #define THD_FUNCTION(tname, arg) PORT_THD_FUNCTION(tname, arg)
                                  ^
../../../test/rt/testthd.c:60:8: note: in expansion of macro ‘THD_FUNCTION’
 static THD_FUNCTION(thread, p) {
        ^
../../../os/rt/include/chschd.h:158:34: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
 #define THD_FUNCTION(tname, arg) PORT_THD_FUNCTION(tname, arg)
                                  ^
../../../test/rt/testthd.c:60:8: note: in expansion of macro ‘THD_FUNCTION’
 static THD_FUNCTION(thread, p) {
        ^
../../../test/rt/testthd.c: In function ‘PORT_THD_FUNCTION’:
../../../os/rt/include/chschd.h:158:34: warning: type of ‘thread’ defaults to ‘int’ [enabled by default]
 #define THD_FUNCTION(tname, arg) PORT_THD_FUNCTION(tname, arg)
                                  ^
../../../test/rt/testthd.c:60:8: note: in expansion of macro ‘THD_FUNCTION’
 static THD_FUNCTION(thread, p) {
        ^
../../../os/rt/include/chschd.h:158:34: warning: type of ‘p’ defaults to ‘int’ [enabled by default]
 #define THD_FUNCTION(tname, arg) PORT_THD_FUNCTION(tname, arg)
                                  ^
../../../test/rt/testthd.c:60:8: note: in expansion of macro ‘THD_FUNCTION’
 static THD_FUNCTION(thread, p) {
        ^
../../../test/rt/testthd.c: In function ‘thd1_execute’:
../../../test/rt/testthd.c:67:73: error: ‘thread’ undeclared (first use in this function)
   threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriorityX()-5, thread, "E");
                                                                         ^
../../../test/rt/testthd.c:67:73: note: each undeclared identifier is reported only once for each function it appears in
../../../test/rt/testthd.c: In function ‘thd2_execute’:
../../../test/rt/testthd.c:95:73: error: ‘thread’ undeclared (first use in this function)
   threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriorityX()-4, thread, "D");
                                                                         ^
In file included from ../../../os/rt/include/ch.h:78:0,
                 from ../../../test/rt/testthd.c:17:
../../../test/rt/testthd.c: At top level:
../../../os/rt/include/chschd.h:158:34: warning: ‘PORT_THD_FUNCTION’ defined but not used [-Wunused-function]
 #define THD_FUNCTION(tname, arg) PORT_THD_FUNCTION(tname, arg)
                                  ^
../../../test/rt/testthd.c:60:8: note: in expansion of macro ‘THD_FUNCTION’
 static THD_FUNCTION(thread, p) {
        ^
Makefile:500: recipe for target '../../../test/rt/testthd.o' failed
make: *** [../../../test/rt/testthd.o] Error 1


[utzig]

Hey, thanks for the patch! I applied it with some minor changes. I also fixed some missing macros for RT and now it should be building/running without issues.

Cheers,
Fabio Utzig

[mikenick42]

Hi Fabio,
Thanks for applying that patch, I grabbed the latest code from git and both the NIL and RT demos are working now.

It was mentioned in the other thread that the line:

Code: Select all

tp->p_ctx.sp->pcx = 0;   

may need to change if a program gets big enough.
I've replaced the 0 in each with "(int)_port_thread_start >> 16; " as it's consistent with the existing code and it seems to work on the demos, but is getting a little beyond my comfort level with the OS code and AVR assembly.

Code: Select all

diff --git a/os/nil/ports/AVR/nilcore.h b/os/nil/ports/AVR/nilcore.h
index c8636eb..28d06ce 100644
--- a/os/nil/ports/AVR/nilcore.h
+++ b/os/nil/ports/AVR/nilcore.h
@@ -187,7 +187,7 @@ struct port_intctx {
     (tp)->ctxp->r3  = (int)pf >> 8;                                         \
     (tp)->ctxp->r4  = (int)arg;                                             \
     (tp)->ctxp->r5  = (int)arg >> 8;                                        \
-    (tp)->ctxp->pcx = 0;                                                    \
+    (tp)->ctxp->pcx = (int)_port_thread_start >> 16;                        \^M
     (tp)->ctxp->pcl = (int)_port_thread_start >> 8;                         \
     (tp)->ctxp->pch = (int)_port_thread_start;                              \
 }
diff --git a/os/rt/ports/AVR/chcore.h b/os/rt/ports/AVR/chcore.h
index b119f06..c9ba077 100644
--- a/os/rt/ports/AVR/chcore.h
+++ b/os/rt/ports/AVR/chcore.h
@@ -164,7 +164,7 @@ struct context {
   tp->p_ctx.sp->r3  = (int)pf >> 8;                                         \
   tp->p_ctx.sp->r4  = (int)arg;                                             \
   tp->p_ctx.sp->r5  = (int)arg >> 8;                                        \
-  tp->p_ctx.sp->pcx = 0;                                               \
+  tp->p_ctx.sp->pcx = (int)_port_thread_start >> 16;                        \^M
   tp->p_ctx.sp->pcl = (int)_port_thread_start >> 8;                         \
   tp->p_ctx.sp->pch = (int)_port_thread_start;                              \
 }


[utzig]

Hi,

I applied your suggested changes last week because they made sense to me. Today I took some time to go through this in a more thorough way. My understanding is that this last change is not really necessary. From my understanding, it is harmless so there's no problem in having applied it. The main issue is that pointers seem to be always 16 bit sized. So getting the 16th MSbs of _port_thread_start will always return 0. Now the interesting issue here is how to jump to any address no matter where it's located in RAM. The 'ret' instruction already pops 3 bytes when running on a 3-byte sized PC so our _port_switch function is OK. But our _port_thread_start routine uses ICALL to jump to the thread's functions, which is limited to 128KB on AVR. If we had a 32-bit function pointer, we could instead make some little changes like the patch below:

Code: Select all

Index: nilcore.c
===================================================================
--- nilcore.c   (revision 8040)
+++ nilcore.c   (working copy)
@@ -129,8 +129,14 @@

   chSysUnlock();
   asm volatile ("movw    r24, r4");
+#ifdef __AVR_3_BYTE_PC__
   asm volatile ("movw    r30, r2");
+  asm volatile ("out     0x3c, r6");
+  asm volatile ("eicall");
+#else
+  asm volatile ("movw    r30, r2");
   asm volatile ("icall");
+#endif
   chSysHalt(0);
 }

Index: nilcore.h
===================================================================
--- nilcore.h   (revision 8040)
+++ nilcore.h   (working copy)
@@ -185,6 +185,7 @@
                                          sizeof(struct port_intctx));       \
     (tp)->ctxp->r2  = (int)pf;                                              \
     (tp)->ctxp->r3  = (int)pf >> 8;                                         \
+    (tp)->ctxp->r6  = (int)pf >> 16;                                        \
     (tp)->ctxp->r4  = (int)arg;                                             \
     (tp)->ctxp->r5  = (int)arg >> 8;                                        \
     (tp)->ctxp->pcx = (int)_port_thread_start >> 16;                        \


What it does is to save the bits 8 to 15 of the thread function pointer on r6 and later move r6 to EIND and use the EICALL instruction. EICALL can jump to anywhere inside an 8MB space. The problem is there's only 16 bit for pointers!

At least that's my understanding from reading this:

https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html

It seems all pointers are 16-bit sized and jumps to places beyond 128K use special linker sections called 'trampolines'. I'm not exactly sure yet on how this works but it seems to use fake destinations that "correct" the jump to the right place.

Cheers,
Fabio Utzig