Skip to content

Conversation

RudolphRiedel
Copy link

As I added here: adafruit/Adafruit-GFX-Library#349
I ran into an issue when compiling my own library using PlatformIO which depends on ZeroDMA for the ATSAMD51.

I further debugged the issue today and neither my breakpoint for the DMAC_0_Handler() or the IRQHandler() function was triggered.

Sandwiched in between I found this block:
#ifdef SAMD51
void DMAC_1_Handler(void) attribute((weak, alias("DMAC_0_Handler")));
void DMAC_2_Handler(void) attribute((weak, alias("DMAC_0_Handler")));
void DMAC_3_Handler(void) attribute((weak, alias("DMAC_0_Handler")));
void DMAC_4_Handler(void) attribute((weak, alias("DMAC_0_Handler")));
#endif

And as stepped thru Adafruit_ZeroDMA::startJob I noticed that it configured an interrupt for DMA channel 2.
And still the interrupt triggered ended up in the default handler.

So I duplicated void DMAC_0_Handler(void), renamed it DMAC_2_Handler() and removed the alias line.
And it worked just fine.
Somehow the alias functions are not used when using ZeroDMA from another library.

Next I removed the "weak" attributes and now my code is running just fine.

@RudolphRiedel
Copy link
Author

Hmm, the zerodma_spi2 example builds just fine for the Metro M4 and Zero for me, what the build script here is tripping over does not seem to have any relation to my changes.

@OpusElectronics
Copy link

Hi, I just ran into this problem as well. It made me scratch my head for a good while and finally figured it out thanks to this PR.

Please note that it's not a matter of things getting archived in libraries or not. I compile my projects with just makefiles and each file gets compiled as an object file, no library, same problem.

Linking order explains it (which explains why some people have run into it, some others not, some have noticed it happened with using library archives, but it's mostly in how it influences linking order, and that behavior cannot even be relied upon.)

The reason is quite simple. Once you understand it all. Most Cortex-M interrupts handlers (including of course the DMAC_ handlers here) are defined as weak symbols already by the low-level startup code. Then the Adafruit_ZeroDMA also defines the DMAC_x_Handler functions (x >= 1) as weak symbols. You end up with two weak symbols for each of these, and no strong symbol anywhere. So there is no definite rule here (the linker will do as it pleases pretty much): whichever weak symbol will get linked in probably depends on linking order, but for all practical purposes, it should be considered random.

That's not good. One should never define several weak symbols for the same symbol, because you can't really predict which one will be picked by the linker. That's why the only reliable fix here is to remove the "weak" attribute in Adafruit_ZeroDMA.cpp, as you did here.

Maybe the idea was to still allow users to override these handlers with their own, outside of Adafruit_ZeroDMA, but that would be a really bad idea anyway, as mixing Adafruit_ZeroDMA and custom DMA handling in the same project is a recipe for disasters.

I see that your PR is still open and the merge doesn't seem to have ended up in the official branch, that apparently everyone still pulls. Adafruit_ZeroDMA has been made a submodule of Adafruit's ArduinoCore-samd fork, but it still updates to the original code, without your changes. As is, it's IMHO not correct and will give headaches to other people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants