cpu/samd21: Added CPU_MODEL_SAMD21G18A interrupt config#7125
cpu/samd21: Added CPU_MODEL_SAMD21G18A interrupt config#7125aabadie merged 1 commit intoRIOT-OS:masterfrom
Conversation
|
@kbumsik Thanks you ! I'm looking forward to your feedback :) |
|
Thanks for this PR, I also would have spent days trying to figure out why the EXTI was not working. Just posted on #6932 that I did not see a problem with two EXTI on my samd21g18a. |
|
@kbumsik can I suggest we add a #else clause with all entries -1 in it? or a #error clause in it? This will give more meaningful results where the CPU_MODEL is not catered for by this block. |
|
@TomKeddie That's what I was thinking at the beginning. But I ended up with this kind of bug is hard to chatch unless the compiler doesn't explicitly warn about this. Since I am a mere user of RIOT I don't know if a scenario that a matched CPU_MODEL is not listed in that block is normal or can be intentional. If this case cannot be intentional I believe we better refuse compiling so that a future developer would notice this when he/she is porting a new SAMD CPU. Or, as the middle point, we might do like this using #else
#warning CPU_MODEL is not matched here. This may lead to malfunctioning EXTI.
{-1, -1, -1, -1, ...
},
{-1, -1, -1, -1, ...
},
#endifBy the way, |
|
@kbumsik I think that's a great idea, lets push it to the reviewers and see what they think. |
aabadie
left a comment
There was a problem hiding this comment.
Changes are correct here (according to the datasheet). ACK
|
and go! |
I noticed the
exti_configtable was uninitialized while porting WINC1500 inside of Arduino MKR1000 (It took me a few days to figure out 😂 ). So I added a proper table for SAMD21G18A.I also added a guard to make sure there is a matched CPU_MODEL in the source.
Related: #7092 #6881 #6666
This change is