drivers: ieee802154: cc13xx/cc26xx_subg radio configuration #58439
Conversation
fgrandel
requested review from
rlubos,
tbursztyka,
cfriedt and
vaishnavachath
as code owners
|
This looks great - thanks @fgrandel ! @vaishnavachath - will you verify some of the radio settings? Particularly in Overall, very nice improvements. I have 2 requests:
|
|
Thanks @cfriedt for your review, encouragement and helpful feedback! More is to follow in future PRs soon as indicated by the TODO tags in the PR - especially improvements in standard conformance (interoperability and CSMA/CA timing) and multi-channel SubG PHYs from the SUN range.
Very good idea, will do. As long as this was not successfully tested on real hardware, I'll leave this PR in the draft state.
Again - very good idea, will do. I'll report results back in this PR. The biggest stumbling block wrt testing with our dev kits (CC1352P1 Launch Pad and CC1352R SensorKit) right now is debugger support - both XDS and JLink seem to be at odds with the software interrupts caused by the radio stack - at least when run through Zephyr's debugging infrastructure (west debug). Whenever an interrupt occurs (which means almost on each step) code execution is halted somewhere in the TI IRQ handling stack and the next line of code will never be reached. Maybe @vaishnavachath can help me with that or someone else from TI who knows the proprietary ROM radio stack? Problem is that similar topics reported on TI E2E were not (yet) answered plus TI engineers on that forum seem to be a little reluctant to reproduce problems on the Zephyr stack. ;-) It would also be very helpful to get some insight from TI how the proprietary radio's Carrier Sense command parameters are to be interpreted. UPDATE: We do have a solution that seemingly works but still it would be great to verify it wrt standard conformance of the timing parameters which are unclear w/o access to ROM source code. |
|
@fgrandel - if you are open to contract work, some of the things you've mentioned would likely be of some interest to TI. At one point, I was trying to get BLE working in Zephyr (again with the ROM drivers), but TI was not open to a contract at that time. Perhaps they've changed their minds at this point. Of course, every company has policy, so it's hard to say if it's feasible. I mostly do like TI's product though, and it would be amazing to have better radio integration in Zephyr. On a side note, I have also removed the jumper connecting the XDS to the cc and have successfully used OpenOCD in the past directly. It's been a while though. |
Thanks for that kind offer! I'm currently under contract but maybe there's some room for co-operation. I'll send you a PM on discord to explain my situation.
Totally agreed. I'm willing to invest quite a bit as we'd love to base a product on top of these SoCs.
Yep - we tried all options, including that one. We spent the day looking into the debugger problem, both via OpenOCD and JLink (Segger Ozone and M-Cortex extension in VSCode). The problem could be narrowed down to radio operations triggered by software interrupts running in the background. These software interrupts seem to trigger while stepping although both OpenOCD and JLink integrations claim to mask interrupts when stepping. If we set PRIMASK=1 manually the interrupts really do not trigger. Maybe the debuggers no longer use PRIMASK nowadays but some other less intrusive masking strategy and the radio MCU (which the debugger does know nothing about) somehow re-activates interrupts by the same mechanism while we're stepping? No idea... Our workaround now is to ensure that we only set breakpoints in places where we are sure that no background operation is active. Not ideal but ok until we find a better solution. |
fgrandel
force-pushed
the
fix/cc13xx-cc26xx-configuration
branch
from
ce258ad
to
78c47d6
Compare
|
@cfriedt The CSMA/CA mechanism has now been converted to soft CSMA/CA for improved standard conformance like we discussed yesterday. I tested this approach on real hardware by interferring with a continuous carrier signal from a second device which worked as expected. This also preliminarily confirms the CSMA/CA fixes underneath this change set. The other hardware tests you proposed are outstanding and will be executed asap. |
fgrandel
force-pushed
the
fix/cc13xx-cc26xx-configuration
branch
9 times, most recently
from
114e3da
to
a363c03
Compare
|
@niflostancu Tagging you because you offered in #46050 to help with hardware exposure of changes re CC13/26x2 platforms. If you like you can have a go with this branch. Would be very appreciated. You could do testing with your own set-up or help with the test scenarios that @cfriedt proposed (see his comment above for more detailed requirements):
We'll do the same but redundancy is a good thing when it comes to testing. :-) |
1 similar comment
|
@niflostancu Tagging you because you offered in #46050 to help with hardware exposure of changes re CC13/26x2 platforms. If you like you can have a go with this branch. Would be very appreciated. You could do testing with your own set-up or help with the test scenarios that @cfriedt proposed (see his comment above for more detailed requirements):
We'll do the same but redundancy is a good thing when it comes to testing. :-) |
|
@fgrandel Thank you for your contributions, overall the PR looks great. @fgrandel On the ROM radio stack, I have requested someone from the Connectivity team to help here, since I am also from a different product line I also don't have access to the necessary information you need. |
@vaishnavachath I'm really sorry to hear that. :-( Thanks for getting someone from the connectivity team involved. |
Hello. |
|
@rlubos @cfriedt @niflostancu @vaishnavachath Now that the basic CSMA/CA fixes have been merged, this change set with CC13/26xx driver updates is open for final review and merge. |
|
@fgrandel @cfriedt I did overnight Echo Client/Server testing on 2 BeagleConnect Freedom, 0 dBm TX Pwr, 10m distance and saw a PER of 0.21% ACK disabled, did not test mainline separately. I will test wpanusb/bcfserial functionality as well today and confirm the results. |
@vaishnavachath Nice, thank you for testing that! :-)
Ok, I'll do nothing about it then. |
Switch the driver to the soft CSMA/CA algorithm as an intermediate compromise for improved standard compliance (namely expontential backoff) until true hardware support can be implemented by chaining radio commands. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The CMD_CLEAR_RX and CMD_SET_TX_POWER commands are declared and initialized but not used anywhere. They are therefore removed to reduce RAM/flash footprint. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The call to radio_api->set_channel() must not switch on RX if it was previously off. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The length field in the header refers to the size of the MAC so it shouldn't rely on constants describing PHY header length. While currently both constants have the same value this will no longer be true for enhanced PHYs and/or MAC frames as the number of FCS bytes may then be four. Also introduces an assertion that ensures that the given package buffer does not exceed the TX buffer's length. An assertion is enough as the package buffer is allocated at compile time. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The radio API should indicate errors when the interface is started. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
PHY overrides have been checked against the latest version of TI's SmartRF(TM) Studio. The result was regression tested (PER/performance) against LAUNCHXL-CC1352P1 boards. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
A known issue exists that does not allow a CC13/26x2R device to establish a link to a CC13/26x2P device unless the capacitor array is tuned to a non-default value in the P device. See SimpleLink(TM) cc13xx_cc26xx SDK 6.20+ Release Notes, Known Issues. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The antenna MUX configuration should explicitly define PULL states as giving no value will leave the GPIO register in an unspecified state. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
fgrandel
force-pushed
the
fix/cc13xx-cc26xx-configuration
branch
from
3963df6
to
0d6b734
Compare
|
@vaishnavachath Could you reset to the latest version of this branch before continuing to test? @cfriedt had some good remarks which I turned into code so it would be good to test the actual thing. It would also be good to have results with ACK + CSMA/CA switched on as these are the areas that should show most improvement in PER as compared to mainline. |
|
I did a last overnight test (265105 packets) to confirm that the latest tuning/improvements due to @cfriedt's proposals did not change anything, I'm still getting 0,77 ‰ PER with ACK and CSMA/CA enabled and a quick zperf test also confirms previous results. So IMO this branch is ready for merge now. |
|
@cfriedt @vaishnavachath @niflostancu Thanks for your support. This was a really nice community team work experience. :-) |
This change set cleans up the CC13xx/CC26xx Sub-GHz driver and introduces some fixes and preparatory changes for multi-PHY support (multiple channel pages). The driver still has some issues wrt PHY standard compliance which have been documented (see the corresponding commit comments). These cannot be removed w/o breaking backwards compatibility and will therefore need to be maintained as a "proprietary" PHY for a while.
UPDATE: This change set also works around a hardware bug that wouldn't allow any link between CC13/26xxP (e.g. CC1352P1 LaunchPad) and CC13/26xxR (e.g. LPSTK-CC1352R SensorTag) devices at all, see "Known Issues" in TI's SDK release notes. (Both bug and solution were confirmed in hardware.)
This change is part of a series of changes required to enable TSCH on Zephyr, see #50336. The relevance of supporting channel page 9 as prepared in this PR is that it provides multi-channel support in the 863 MHz band which is required for channel hopping in TSCH.
All changes have been extensively regression tested against a combination of two CC1352P1 LaunchPad dev boards (+ some additional test coverage on CC1352R). A full set of measurement results can be seen in #58439 (comment) below.