Well, I think I managed to figure it out.
First, an apology - I didn't read the patch in #16457 properly and
thought it just moves the _eronly assignment while it, in fact,
changes
ABSOLUTE to LOADADDR too. Sorry for that.
After spotting this, I went and tried to find out how the change makes
things different because from my understanding, it should do the same
thing - assignment of current address where .data section begins
should
be the same as assignment of address of the .data section.
Turns out there is a difference - the linker also generated a
.trampoline section and placed it between _eronly assignment and
beginning of .data section. My guess is that when this section is not
mentioned in the linker script, the linker has free reign as to where
to
put it...? Anyway, that made _eronly value incorrect by .trampoline
section size and things went badly from there.
The issue can therefore also be fixed by this patch:
diff --git a/boards/avr/atmega/arduino-mega2560/scripts/flash.ld
b/boards/avr/atmega/arduino-mega2560/scripts/flash.ld
index f365646dfa..f9ceeb3a08 100644
--- a/boards/avr/atmega/arduino-mega2560/scripts/flash.ld
+++ b/boards/avr/atmega/arduino-mega2560/scripts/flash.ld
@@ -122,6 +122,12 @@ SECTIONS
_etext = . ;
} > flash
+ .trampolines :
+ {
+ *(.trampolines)
+ *(.trampolines.*)
+ } > flash
+
_eronly = ABSOLUTE(.);
.data :
The reason why I wasn't able to reproduce this on mega1284p-xplained
is
then simple - there is no .trampolines section for that chip in the
compiled binary.
In my opinion, the proposed patch in #16457 is a better solution.
While
it does not preserve what the code comments say about the _eronly
value
("this is where .text + .rodata ends"), it is a better match to what
the
_eronly value is actually used for ("this is start of .data section")
On 2025-05-28 11:59, kr....@kerogit.eu wrote:
> Alright, I looked into the possible bug mentioned in the last paragraph
> of my previous message and it seems the code is actually correct. What
> I thought was that if register pair Z overflows, subsequent reads would
> read from incorrect memory locations. However, that does not seem to be
> the case - according to AVR Instruction Set Manual DS40002198A, ELPM
> instruction post-increment "applies to the entire 24-bit concatenation
> of the RAMPZ and Z-Pointer Registers"
>
> As for the change of _eronly however, I still think that is incorrect.
> Will try to look into that more during the evening unless someone
> figures something else out.
>
> What compiler are you using anyway? From the kernel version mentioned
> in GitHub I assume gcc 14.2? Might be the reason why everything works
> fine for me and not for anyone else, I am still on 5.4 from Debian
> stable. (Can try that too, Debian testing also has 14.2.)
>
> On 2025-05-28 09:16, kr....@kerogit.eu wrote:
>> Hello,
>>
>> I don't that is a correct fix and in fact, from a quick look at things
>> it seems to me that it actually introduces another bug. As described
>> in src/atmega/atmega_head.S, that value is supposed to be "Start of
>> .data section in FLASH". You changed it to end of .data section in
>> FLASH. It is used after __do_copy_data label as an address where
>> initialized data start in flash memory. So if I read this correctly,
>> you are copying incorrect data to your initialized variables.
>>
>> If the program starts working this way, that might suggest that the
>> initialization data are placed incorrectly in the image? Anyway, the
>> linker script is pretty much identical to linker script of
>> mega1284p-xplained and that one works out of the box without need to
>> move _eronly.
>>
>> Hm, looking at this, I think I just spotted a bug in __do_copy_data
>> but that would only hit if your initialized data region crossed a 64kB
>> boundary. Is that the case in any chance? It would cause _some_
>> initialized variables to contain random garbage...
>>
>> On 2025-05-28 07:17, Matteo Golin wrote:
>>> Managed to find a fix here:
>>> https://github.com/apache/nuttx/pull/16457
>>>
>>> On Tue, May 27, 2025 at 11:24 PM Matteo Golin
>>> <matteo.go...@gmail.com>
>>> wrote:
>>>
>>>> Thank you for your help KR!
>>>>
>>>> I also noticed this CONFIG_RAM_START=0x800100. Unfortunately,
>>>> changing
>>>> that value to the proper 0x800200 did not really help the issue.
>>>>
>>>> I went ahead and generated the disassembly like you suggested, and
>>>> it does
>>>> appear that the .text section starts with jump instructions for all
>>>> the
>>>> interrupt vectors:
>>>>
>>>> 00000000 <_stext>:
>>>> 0: 0c 94 72 00 jmp 0xe4 ; 0xe4 <__start>
>>>> 4: 0c 94 95 00 jmp 0x12a ; 0x12a
>>>> <atmega_int0>
>>>> 8: 0c 94 98 00 jmp 0x130 ; 0x130
>>>> <atmega_int1>
>>>> c: 0c 94 9b 00 jmp 0x136 ; 0x136
>>>> <atmega_int2>
>>>> 10: 0c 94 9e 00 jmp 0x13c ; 0x13c
>>>> <atmega_int3>
>>>> 14: 0c 94 a1 00 jmp 0x142 ; 0x142
>>>> <atmega_int4>
>>>> 18: 0c 94 a4 00 jmp 0x148 ; 0x148
>>>> <atmega_int5>
>>>> 1c: 0c 94 a7 00 jmp 0x14e ; 0x14e
>>>> <atmega_int6>
>>>> 20: 0c 94 aa 00 jmp 0x154 ; 0x154
>>>> <atmega_int7>
>>>> 24: 0c 94 ad 00 jmp 0x15a ; 0x15a
>>>> <atmega_pcint0>
>>>> 28: 0c 94 b0 00 jmp 0x160 ; 0x160
>>>> <atmega_pcint1>
>>>> 2c: 0c 94 b3 00 jmp 0x166 ; 0x166
>>>> <atmega_pcint2>
>>>> 30: 0c 94 b6 00 jmp 0x16c ; 0x16c <atmega_wdt>
>>>> 34: 0c 94 b9 00 jmp 0x172 ; 0x172
>>>> <atmega_tim2_compa>
>>>> 38: 0c 94 bc 00 jmp 0x178 ; 0x178
>>>> <atmega_tim2_compb>
>>>> 3c: 0c 94 bf 00 jmp 0x17e ; 0x17e
>>>> <atmega_tim2_ovf>
>>>> 40: 0c 94 c2 00 jmp 0x184 ; 0x184
>>>> <atmega_tim1_capt>
>>>> 44: 0c 94 c5 00 jmp 0x18a ; 0x18a
>>>> <atmega_tim1_compa>
>>>> 48: 0c 94 c8 00 jmp 0x190 ; 0x190
>>>> <atmega_tim1_compb>
>>>> ...and so on...
>>>>
>>>> `g_idle_topstack` is 0x800e2b in the disassembly. With RAM_START
>>>> being
>>>> 0x800200, and a size of 8192 bytes, that gives a `heap_size` of 5076
>>>> bytes.
>>>> That all seems fine.
>>>>
>>>> However, printing out the value of `g_idle_topstack` (in binary
>>>> using
>>>> `up_putc`) I can see that the value does not match. The printed
>>>> value is
>>>> "1000011001110110" while the binary representation of 0x800e2b is
>>>> "100000000000111000101011".
>>>> Printing out the `heap_size` tells me it's "1111100111001101", which
>>>> is
>>>> not 5076.
>>>>
>>>> It seems your suspicion about a mismatch is probably correct,
>>>> although
>>>> it's not due to a misplaced vector table it seems.
>>>>
>>>> I did, for fun, try enabling `CONFIG_DEBUG_OPT_UNUSED_SECTIONS` and
>>>> it did
>>>> remove the vectors at the start of the assembly file. That image
>>>> didn't
>>>> work.
>>>> Then I kept `CONFIG_DEBUG_OPT_UNUSED_SECTIONS` enabled and put the
>>>> `KEEP(*(.vectors))` suggestion in the linker script. Vectors are
>>>> kept in
>>>> the image, and `g_idle_topstack` changes to 0x800be4. This image
>>>> didn't
>>>> work either.
>>>>
>>>> That got me thinking if there was another section getting
>>>> unexpectedly
>>>> tossed, so I put `KEEP(...)` around basically everything in the
>>>> linker
>>>> script. That didn't produce a different result, and still had the
>>>> `g_idle_topstack` of 0x800e2b.
>>>>
>>>> I will continue to debug this, but if you have any other suggestions
>>>> please let me know. Thank you very much for all your help so far!
>>>>
>>>> Matteo
>>>>
>>>> On Tue, May 27, 2025 at 7:33 PM <kr....@kerogit.eu> wrote:
>>>>
>>>>> Thanks to you as well.
>>>>>
>>>>> I spotted an error in the config posted both in the issue #16444
>>>>> and PR
>>>>> #16443. It is the
>>>>>
>>>>> CONFIG_RAM_START=0x800100
>>>>>
>>>>> Unless I overlooked something, the RAM starts at address 0x200
>>>>> (0x800200) for AtMega 2560.
>>>>>
>>>>> Not sure if that's what causing the problem though - as far as I
>>>>> can
>>>>> see, that value is actually only used as an input to CONFIG_RAM_END
>>>>> calculation and incorrect value in this macro simply reduces heap
>>>>> size.
>>>>>
>>>>> The symptoms of the bug would match though - the program would be
>>>>> trying
>>>>> to use I/O registers as RAM.
>>>>>
>>>>> Thing is - seeing the bug was narrowed down to heap initialization
>>>>> in
>>>>> nx_start - I remember that I ran into pretty much the same issue
>>>>> when
>>>>> adding support for AVR DA/DB family... and I can't remember what
>>>>> was the
>>>>> problem.
>>>>>
>>>>> Maybe check the disassembly file? (RAW_DISASSEMBLY needs to be set
>>>>> for
>>>>> the file to be generated.) Verify that the .text section starts
>>>>> with jmp
>>>>> instructions for interrupt vectors. This is only a vague memory but
>>>>> I
>>>>> think that I ran into the issue because the linker removed .vectors
>>>>> section, thinking it was unreferenced. (But that should not happen
>>>>> here,
>>>>> the linked config shows CONFIG_DEBUG_OPT_UNUSED_SECTIONS is not
>>>>> set.)
>>>>>
>>>>> Anyway, when that happened, the whole program got "shifted" -
>>>>> except for
>>>>> program memory that stored values of initialized variables. The
>>>>> data
>>>>> copy loop in initialization code (atmega_head.S) then read the
>>>>> values
>>>>> from the shifted location leading to a mismatch - variables were
>>>>> populated with wrong data. One of those variables was
>>>>> g_idle_topstack
>>>>> which directly sets start of heap. I think it initialized it to
>>>>> zero...?
>>>>>
>>>>> Again, all of this is based on vague memory. TLDR version is: if
>>>>> able,
>>>>> check g_idle_topstack in up_allocate_heap and verify that it is
>>>>> correct.
>>>>>
>>>>>
>>>>> On 2025-05-27 15:28, Alan C. Assis wrote:
>>>>> > Thank you KR,
>>>>> >
>>>>> > I posted your message in that issue for reference.
>>>>> >
>>>>> > I have a BK-AVR128 board:
>>>>> > https://aliexpress.com/item/1005006234334573.html
>>>>> > and will test NuttX on it to confirm.
>>>>> >
>>>>> > If everything is working as expected I will submit the board
support to
>>>>> > the
>>>>> > mainline.
>>>>> >
>>>>> > BR,
>>>>> >
>>>>> > Alan
>>>>> >
>>>>> > On Tue, May 27, 2025 at 6:39 AM <kr....@kerogit.eu> wrote:
>>>>> >
>>>>> >> Hello,
>>>>> >>
>>>>> >> as mentioned in one of my previous e-mails to this list (RFC:
>>>>> >> decoupling
>>>>> >> ability to always panic from board_reset), I noticed #16444 and
#16443
>>>>> >> on GitHub.
>>>>> >>
>>>>> >> I found some time to look into this and I believe I found the
reason
>>>>> >> for
>>>>> >> "If you add avr_lowputc calls in the board initialization code,
you'll
>>>>> >> see that the TX LED stays stuck on indefinitely."
>>>>> >>
>>>>> >> Provided the board initialization code that sentence is talking
about
>>>>> >> is
>>>>> >> atmega_boardinitialize() in avr_boot.c, then avr_lowputc() likely
does
>>>>> >> not work because of the configuration used. According to #16444,
the
>>>>> >> configuration has:
>>>>> >>
>>>>> >> CONFIG_DEV_CONSOLE=y
>>>>> >> CONFIG_CONSOLE_SYSLOG=y
>>>>> >>
>>>>> >> These two are processed in src/atmega/atmega_config.h
>>>>> >>
>>>>> >> #ifndef CONFIG_DEV_CONSOLE
>>>>> >> # undef USE_SERIALDRIVER
>>>>> >> # undef USE_EARLYSERIALINIT
>>>>> >> #else
>>>>> >> # if defined(CONFIG_CONSOLE_SYSLOG)
>>>>> >> # undef USE_SERIALDRIVER
>>>>> >> # undef USE_EARLYSERIALINIT
>>>>> >> # elif defined(HAVE_USART_DEVICE)
>>>>> >> # define USE_SERIALDRIVER 1
>>>>> >> # define USE_EARLYSERIALINIT 1
>>>>> >> # else
>>>>> >> # undef USE_SERIALDRIVER
>>>>> >> # undef USE_EARLYSERIALINIT
>>>>> >> # endif
>>>>> >> #endif
>>>>> >>
>>>>> >> With the configuration above, the outer ifndef is not true and
first
>>>>> >> if
>>>>> >> defined in else block is true, which results into:
>>>>> >>
>>>>> >> undef USE_SERIALDRIVER
>>>>> >> undef USE_EARLYSERIALINIT
>>>>> >>
>>>>> >> Since CONFIG_STANDARD_SERIAL is also set, the undef of
>>>>> >> USE_SERIALDRIVER
>>>>> >> is reverted by define USE_SERIALDRIVER 1 later. However,
>>>>> >> USE_EARLYSERIALINIT remains unset.
>>>>> >>
>>>>> >> This causes avr_earlyserialinit() to not be built nor called from
>>>>> >> avr_lowinit(). Serial port peripheral is therefore not
initialized yet
>>>>> >> when atmega_boardinitialize() is called. I don't know what exactly
>>>>> >> happens when you attempt to transmit data with the port not
enabled
>>>>> >> but
>>>>> >> my guess would be that "transmit data register empty" status flag
is
>>>>> >> just never cleared and the program ends up in a loop waiting for
that
>>>>> >> to
>>>>> >> happen.
>>>>> >>
>>>>> >>
>>>>> >> Other than that - I recently tested NSH on mega1284p-xplained
(well, a
>>>>> >> breadboard with the chip stuck in it actually) and it worked for
me.
>>>>> >> As
>>>>> >> far as I can see, all AtMega devices use the same code for
managing
>>>>> >> serial ports so it should work out of the box.
>>>>> >>
>>>>> >> Someone somewhere in some forum on the net has or had a footer in
his
>>>>> >> posts saying something along the lines of that non-functional
serial
>>>>> >> port is 99% mismatching baud rates, might be worth a re-check.
>>>>> >>
>>>>> >> As for the PR itself (copying from e-mail mentioned at the
beginning)
>>>>> >> -
>>>>> >> I would recommend trying to use KEEP(*(.vectors)) as seen in
>>>>> >> boards/avr/avrdx/breadxavr/scripts/breadxavr.ld - the default
config
>>>>> >> should then not need the "# CONFIG_DEBUG_OPT_UNUSED_SECTIONS is
not
>>>>> >> set"
>>>>> >> line.
>>>>> >>
>>>>>
>>>>
