drivers/at86rf2xx: add support for ATmegaRF MCUs #12537
Conversation
benpicco
added
Area: network
Do you intend to do this later on? I personally think that some bigger refactoring of the
If a boards boots without device connected to the SPI bus, the driver loops endlessly blocking RIOT.
This is also not fulfilled. There can be multiple instances of the driver interacting with transceivers if the are the very same model. But in the driver a lot of So with that in mind: I guess that this refactoring (that in my opinion is unavoidable anyway) could make it much easier to provide an interrupt handler optimized for the periph versions of the AT86RF2xx. (Maybe some more splitting could be required, e.g. adding additionally |
|
The cleanup of the driver you are suggesting sounds tempting. Maybe it would even be possible to integrate support for at86rf215 which has a different state machine, but similar register access functions. But that is a wholly different undertaking - maybe if the tasklet work is finalized that would be a good reason to convert the driver, but I don't want to promise anything 😉 |
Please notify me if/when you start working on this. I promise to notify you when we start doing so. It would be shame if we would duplicate the effort. |
|
btw: with some tweaks to microduino-corerfsamr21-xpro |
|
Hmm. I just tried to test it. With both
|
|
Hmm, with the latest Ubuntu I still only get gcc-avr 5.4.0 edit: I found a random binary of |
|
Oha! It seems that there is no LDO on the Microduino CoreRF. I was powering it through the 5V pin (or I believed so!), but I think I can not find and LDO on the |
|
OK. Powering with 3.3V using the 3V3 pin on the Microduino CoreRF did the trick. Two issues discovered during testing:
|
Hm, my suspicion is that cpuid is kinda bogus. Can you post the output of
This works for me: short addresssamr21-xpromicroduino-corerflong addresssamr21-xpromicroduino-corerfnew long addrsamr21-xpromicroduino-corerfnew short addesssamr21-xpromicroduino-corerf |
|
It was too late for me to think properly yesterday. I'll test again with antennas plugged into the pigtail connector. That should help... I saw btw. no code to initialise the L2 address. It seems the SPI versions initialise it implicitly on boot up. Maybe that is not true for the memory mapped version? |
Its right there. |
|
Hi, ping6 2001:db8::fec2:3d00:0:2a25 --- 2001:db8::fec2:3d00:0:2a25 PING statistics --- |
|
OK. With antennas it also works on the Microduino CoreRF (ATmega128RFA1). For the record: With two @herjulf confirmed that that the driver also works on the ATmega256RFR2. By being able to use The issue with the same L2 address on both my Microduino CoreRF boards is unrelated, as |
maribu
added
the
Reviewed: 3-testing
| #if defined(MODULE_AT86RFA1) || defined(MODULE_AT86RFR2) | ||
| static const uint8_t at86rf2xx_params[] = | ||
| { | ||
| 0 /* dummy value */ |
There was a problem hiding this comment.
Hmm, maybe this byte of ROM could be saved some other way. But that could be something to tackle when refactoring the at86rf2xx driver.
maribu
added
Reviewed: 1-fundamentals
|
Please squash! (Also squash the requested changes on indent directly.) Citing the CI: maybe this could be reworded to comply? |
|
If there is more things to test I'll help. I dunno about RIOT code style yet but with a Linux background the code does not not look alarming bad. ifdef's are hard to avoid on the MCU operating systems. I'll assume we'll all like to avoid two takes of the same radio code. IMO most important that it is not break any existing platform I see there is work to do for the driver both HW AES and RPC... |
The ATmega128RFA1 and ATmega256RFR2 contain a version of this IP on the MCU. The radio core behaves mostly like a at86rf231, but all registers are mapped to memory and radio states can directly generate interrupts on the CPU. The ATmega256RFR2 adds support for automatic retransmissions. This has not been implemented yet. Co-authored-by: Josua Arndt <jarndt@ias.rwth-aachen.de>
This warning is based on the most commonly understood Git guidelines (just google 50/72 rule + at least with |
Now that the radio is enabled, some tests do not fit onto the MCU anymore.
|
@herjulf Thank you for testing! Feel free to submit a PR to add support for the For me this was more of a curiosity - the chips are easy to support by the existing driver and the work was almost done, so I didn't want to let it wither away. Good to hear this is actually useful to someone! 😃 |
|
I'll be happy to do a PR for the board. I have get into the RIOT architecture/design and reprogram myself a bit first. Newer boards have BME280 this also supported in RIOT. Wrote that driver for Contiki as BME680. But let focus on this PR with radio support come to an end. |
|
Hmm I'm into looking how to pass an address to the radio module. I'll assume this a more general question not related to this PR. Anyway in at86rf2xx_reset we fable some address via luid_get(). In the case I have there is already an unique address (from EUI64MAC chip) to be assigned. So the luid_get() should not happen. Any input here? |
|
@herjulf: Indeed, with every reset a new L2 address is generated. To me, it is also logically a strange place to generate the L2 in the reset function rather than in init. However, this is bug is not related to this PR and should be fixed in a separate PR. If someone starts to work on a PR please notify me, otherwise I'll try to find time for it tomorrow. @benpicco did you use the code of this PR for the samr21-xpro, so that no regression for the AT86RF233 showed up during testing? That would be the last box to tick before merging. I could also test tomorrow, if not. |
|
@herjulf it would indeed make sense to have a custom interface for this. If you just want a unique address but don't care about what the address is, the simplest solution would be to add a way to seed the If you want the exact same address as you have on the EUI64MAC chip, a general @maribu yes that was the whole point of choosing that board. 😄 |
maribu
added
CI: ready for build
|
Thank you all for the review and testing! |
@maribu since the reset function also always changes the lowest two bits of the first byte to uint8_t luid[8];
for (unsigned i = 0; i < 10; ++i) {
luid_get(luid, sizeof(luid));
/* make sure we mark the address as non-multicast and not globally unique */
luid[0] &= ~(0x01);
luid[0] |= (0x02);
printf("%d: ", i);
for (unsigned j = 0; j < sizeof(luid); ++j) {
printf("%02x ", luid[j]);
}
puts("");
} |
|
So yes _init() should be a better home for address initialization rather than reset() especially as luid_get() returns different addresses. |
|
See also #9656 on that. |
Contribution description
This is based on the work by @Josar
The ATmega128RFA1 and ATmega256RFR2 MCUs contain a version of the AT86RF2xx radio directly mapped to memory. It behaves mostly like the AT86RF231.
Unlike on the SPI based radios, all radio events can generate a dedicated interrupt on the CPU.
Unfortunately, taking advantage of this would require factorizing all the interrupt handler code, so instead the interrupt state register is emulated.
The radio on the ATmega256RFR2 is almost identical to the one on the ATmega128RFA, it adds automatic re-transmissions and optional additional interrupt events.
Since I don't have a ATmega256RFR2 device, I omitted the re-transmissions.
Testing procedure
Unfortunately
examples/gnrc_networkingdoes not fit onto themicroduino-corerf, so I did all my testing withexamples/default.If you happen to have a RCB256RFR2-xpro or Jiminy-Mega256RFR2, it should fit there.
I did
txtsndbetween themicroduino-corerfand asame54-xproequipped with an at86rf233 module (samr21-xproshould work as well, but I didn't have one handy right now), so I could also confirm that the driver also still works for the SPI based radios.I changed the transmit power with
ifconfig set powerand saw the change in RSSI on the receiver.I tried
ifconfig set state sleepandifconfig set state idleto confirm no packet is being received in the sleep state. Power measurements were not done.Issues/PRs references
based on #9172