-
Notifications
You must be signed in to change notification settings - Fork 1.4k
mpsl: fem: nrf21540: add MPSL_FEM_NRF21540_PA_LEAD_TIME_ADDITIONAL_US #21842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The SoC output power starts to rise earlier than at the EVENTS_READY of the RADIO. To compensate this phenomenon and avoid spurious emission issue additional Kconfig option for the nRF21540 FEM MPSL_FEM_NRF21540_PA_LEAD_TIME_ADDITIONAL_US is added. The default value is selected so that the TX_EN pin of the nrf21540 is enabled 26us before EVENTS_READY. Ref: KRKNWK-17752 Signed-off-by: Andrzej Kuros <[email protected]>
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 341b4af43d5c187a299f63b898a28f215bb852b7 more detailssdk-nrf:
Github labels
List of changed files detected by CI (5)Outputs:ToolchainVersion: 7cbc0036f4 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
You can find the documentation preview for this PR here. Preview links for modified nRF Connect SDK documents: https://ncsdoc.z6.web.core.windows.net/PR-21842/nrf/app_dev/device_guides/fem/fem_nRF21540_optional_properties.html |
The MPSL_FEM_NRF21540_PA_LEAD_TIME_ADDITIONAL_US Kconfig option is added. This commit adjusts doc on where it is used. Signed-off-by: Andrzej Kuros <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understandable that you need a different value for softdevice, but where has the thought been to how customers will use this? This is a devicetree property, it is configured in devicetree, now suddenly it's configured in devicetree and Kconfig - how does that make sense? @carlescufi
|
@nordicjm , regarding #21842 (review) . The ncs-radio-sw is code owner of the FEM in sdk-nrf. Otherwise what's the point of ownership. We prefer this way of fix. Possibly everything can be do better, but we have limited time/human and resources. Our goal is to fix customers' issue, and have it fixed by default, so that the customers don't need to tweak these values on their own while providing them ability to do so if they need. We believe that customers will use NCS "as is". They will tweak parameters if and only if they have to. Approaches considered so far:
Reminder on the "why is this even needed": The need to increase the lead time when TX_EN pin is activated comes from the way the SoC is constructed AND the way the software in NCS (not in upstream Zephyr) handles it. The default value of 11 for The alternatives: Please pick one of proposed alternative, propose better one or approve this PR as it is. We have limited time. |
|
Fix it in boards that need it, this is literally what a board dts file is for |
What I asked for was "Please pick one of proposed alternative, propose better one or approve this PR as it is." What you are proposing is one of the discarded solutions. It is worse because:
Please pick different one, from "the alternatives" that I enumerated. |
|
It seems we are stuck in a clinch, @bpienk @carlescufi , mediation needed. |
|
Replaced by upstream modification approach and with [nrf fromlist] cherry-picked commit nrfconnect/sdk-zephyr#2833 |
The SoC output power starts to rise earlier than at the EVENTS_READY of the RADIO. To compensate this phenomenon and avoid spurious emission issue additional Kconfig option for the nRF21540 FEM MPSL_FEM_NRF21540_PA_LEAD_TIME_ADDITIONAL_US is added.
The default value is selected so that the TX_EN pin of the nrf21540 is enabled 26us before EVENTS_READY.
In the NCS 3.0 release the same spurious emission issue has been fixed by simply setting default value of
tx-en-settle-time-usto 26. https://github.com/nrfconnect/sdk-zephyr/blob/v4.0.99-ncs1-branch/dts/bindings/net/wireless/nordic%2Cnrf21540-fem.yaml#L98 . However, to provide the fix without [nrf noup] commit (unwanted, see here ) the additional parameter on the nRF Connect SDK level is proposed.Ref: KRKNWK-17752