Skip to content
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

nimble/ll: Add support for custom TX power configuration #1359

Closed
wants to merge 1 commit into from

Conversation

sjanc
Copy link
Contributor

@sjanc sjanc commented Sep 15, 2022

This allows to provide custom TX power setting and rounding functions which can take into account FEM configuration. Since this is highly dependend on HW configuration (FEM type and mode, BPFs, PA power etc) there is no point in providing "default" implementation as this should be typically provided by BSP or application.

This allows to provide custom TX power setting and rounding
functions which can take into account FEM configuration. Since
this is highly dependend on HW configuration (FEM type and mode,
BPFs, PA power etc) there is no point in providing "default"
implementation as this should be typically provided by BSP or
application.
@apache-mynewt-bot
Copy link

Style check summary

Our coding style is here!

nimble/controller/src/ble_ll_dtm.c

@@ -34,16 +34,16 @@
 #include "ble_ll_priv.h"
 
 STATS_SECT_START(ble_ll_dtm_stats)
-    STATS_SECT_ENTRY(rx_count)
-    STATS_SECT_ENTRY(tx_failed)
-    STATS_SECT_ENTRY(rx_failed)
+STATS_SECT_ENTRY(rx_count)
+STATS_SECT_ENTRY(tx_failed)
+STATS_SECT_ENTRY(rx_failed)
 STATS_SECT_END
 STATS_SECT_DECL(ble_ll_dtm_stats) ble_ll_dtm_stats;
 
 STATS_NAME_START(ble_ll_dtm_stats)
-    STATS_NAME(ble_ll_dtm_stats, rx_count)
-    STATS_NAME(ble_ll_dtm_stats, tx_failed)
-    STATS_NAME(ble_ll_dtm_stats, rx_failed)
+STATS_NAME(ble_ll_dtm_stats, rx_count)
+STATS_NAME(ble_ll_dtm_stats, tx_failed)
+STATS_NAME(ble_ll_dtm_stats, rx_failed)
 STATS_NAME_END(ble_phy_stats)
 
 struct dtm_ctx {

@andrzej-kaczmarek
Copy link
Contributor

andrzej-kaczmarek commented Sep 20, 2022

so I thought it over and I think it should work a bit different:

  • LL should always set TX power instead of relying on FEM to do this
  • we should have API to get PA/LNA gain, LL should call it and then calculate actual TX power or RSSI using that value and also RF path compensation, if set by host
  • optionally we can have syscfg to define constant gain for PA/LNA to have some build time optimizations possible

proposed names for APIs would be ble_ll_fem_(pa|lna)_gain_get()
syscfg BLE_LL_FEM_(PA|LNA)_GAIN - if set to 0 means LL has to call API to get actual gain, otherwise use that value

@sjanc
Copy link
Contributor Author

sjanc commented Sep 26, 2022

The thing is that FEM might not have fixed gain, ie in SKY66112 TX gain depends on SKY configuration and input power, so LL will have to iterate (?) to match expected output power with whatever can be set by phy, alternatively we can add some output parameters where FEM returns also expected PHY tx power

@andrzej-kaczmarek
Copy link
Contributor

The "current" solution will call FEM API which will set tx power via phy. Proposed solution will call FEM API to obtain tx power and set it via phy - I suspect this will be the same value, so what is the problem here?

@sjanc
Copy link
Contributor Author

sjanc commented Sep 26, 2022

"LL should call it and then calculate actual TX power or RSSI using that value and also RF path compensation, if set by host"

this is other way around, only FEM can calculate actual TX power since it (may) depends on input power and FEM (possibly runtime) configuration.

Also to cover all scenarios, like rounding, reading supported power ranges etc we need ble_ll_fem_pa_gain_set() which will configure FEM as needed (as oppose to get() which would just return values for rounding)

Than, ble_ll_fem_pa_gain_get() would return actually configured gain (compared to PHY tx power that was last "set")

@sjanc
Copy link
Contributor Author

sjanc commented Oct 3, 2022

this will be handled in #1374

@sjanc sjanc closed this Oct 3, 2022
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.

None yet

3 participants