b43: implement PPR (Power Per Rate) management/API

Signed-off-by Rafa? Mi?ecki <[email protected]>

---
This is based on top of
[PATCH 7/7] b43: enable radio 0x2057 rev 14 support (AKA BCM43217) (not sure if applies cleanly otherwise)

drivers/net/wireless/b43/phy_n.c +10 -0

+	b43_nphy_tx_power_ctrl(dev, nphy->txpwrctrl);
+	if (dev->dev->core_rev == 11 || dev->dev->core_rev == 12)
+		b43_maskset32(dev, B43_MMIO_MACCTL, ~B43_MACCTL_PHY_LOCK, 0);
+	b43_mac_enable(dev);
+
+	nphy->tx_pwr_last_recalc_freq = channel->center_freq;
+	nphy->tx_pwr_last_recalc_limit = phy->desired_txpower;
+
+	return B43_TXPWR_RES_DONE;
+}

drivers/net/wireless/b43/phy_n.c const struct b43_phy_operations b43_phyops_n = { · line 6659

Quoted 5 lines · On Sun, 20 Jul 2014 13:00:22 +0200 Rafa? Mi?ecki <[email protected]> wrote:

drivers/net/wireless/b43/phy_n.c +0 -1

@@ -6656,5 +6721,4 @@ const struct b43_phy_operations b43_phyops_n = {
      .switch_channel         = b43_nphy_op_switch_channel,
      .get_default_chan       = b43_nphy_op_get_default_chan,
      .recalc_txpower         = b43_nphy_op_recalc_txpower,
-     .adjust_txpower         = b43_nphy_op_adjust_txpower,

recalc_txpower once was designed to just recalculate the txpower and not write it to hardware. adjust_txpower was supposed to write it to hardware afterwards. That had to do with some locking foo and stuff I forgot (AFAIR these callbacks are called in different contexts, but I may be wrong).
But I don't think it really matters. Just as a general hint here.

Quoted 13 lines · Michael B?sch

drivers/net/wireless/b43/phy_n.c const struct b43_phy_operations b43_phyops_n = { · line 6659

Quoted 5 lines · On Sun, 20 Jul 2014 13:00:22 +0200 Rafa? Mi?ecki <[email protected]> wrote:

drivers/net/wireless/b43/phy_n.c +0 -1

@@ -6656,5 +6721,4 @@ const struct b43_phy_operations b43_phyops_n = {
      .switch_channel         = b43_nphy_op_switch_channel,
      .get_default_chan       = b43_nphy_op_get_default_chan,
      .recalc_txpower         = b43_nphy_op_recalc_txpower,
-     .adjust_txpower         = b43_nphy_op_adjust_txpower,

recalc_txpower once was designed to just recalculate the txpower and not write it to hardware. adjust_txpower was supposed to write it to hardware afterwards. That had to do with some locking foo and stuff I forgot (AFAIR these callbacks are called in different contexts, but I may be wrong).
But I don't think it really matters. Just as a general hint here.

Yeah, I noticed that and was starring at it for a long time.

First of all I verified if @config is allowed to sleep. It is, so our "recalc_txpower" can sleep as well if it needs to.

I think that using adjust_txpower on G-PHY could have something to do with the way TX power is controlled on these devices. They don't have

In N-PHY world there are two registers for setting TX power: 0x1E7 and 0x222 (one per core) and two for reading current state: 0x1ED and 0x1EE. However you don't really touch them unless you're doing some initialization/calibration. For most of the time hardware is supposed to adjust TX power, you simply tell the firmware what should it be (this is what 0x1EA register is for).

Signed-off-by Rafa? Mi?ecki <[email protected]>

---
This is based on top of
[PATCH 7/7] b43: enable radio 0x2057 rev 14 support (AKA BCM43217) (not sure if applies cleanly otherwise)

---
 drivers/net/wireless/b43/b43.h   |  1 +
 drivers/net/wireless/b43/phy_n.c | 88 ++++++++++++++++++++++++++++++++++------
 drivers/net/wireless/b43/phy_n.h |  3 ++
 3 files changed, 80 insertions(+), 12 deletions(-)

drivers/net/wireless/b43/phy_n.h +2 -1

+       u16 tx_pwr_last_recalc_freq;
+       int tx_pwr_last_recalc_limit;
 
        u8 txrx_chain;
        u16 tx_rx_cal_phy_saveregs[11];
--

1.8.4.5

drivers/net/wireless/b43/phy_n.h struct b43_phy_n { · line 970

Quoted 9 lines · Rafa? Mi?ecki

drivers/net/wireless/b43/phy_n.h +1 -0

index 30bec81..252d843 100644
@@ -967,6 +967,9 @@ struct b43_phy_n {
        struct b43_phy_n_txpwrindex txpwrindex[2];
        struct b43_phy_n_pwr_ctl_info pwr_ctl_info[2];
        struct b43_chanspec txiqlocal_chanspec;
+       struct b43_ppr *tx_pwr_max_ppr;

Why not just make this a struct member? As far as I can tell, it will always be allocated, and you would lose one alloc/free call, and probably one pointer dereference.

Quoted 14 lines · Jonas Gorski

drivers/net/wireless/b43/phy_n.h struct b43_phy_n { · line 970

Quoted 9 lines · Rafa? Mi?ecki

drivers/net/wireless/b43/phy_n.h +1 -0

index 30bec81..252d843 100644
@@ -967,6 +967,9 @@ struct b43_phy_n {
        struct b43_phy_n_txpwrindex txpwrindex[2];
        struct b43_phy_n_pwr_ctl_info pwr_ctl_info[2];
        struct b43_chanspec txiqlocal_chanspec;
+       struct b43_ppr *tx_pwr_max_ppr;

Why not just make this a struct member? As far as I can tell, it will always be allocated, and you would lose one alloc/free call, and probably one pointer dereference.

My idea was to prevent driver parts from knowing PPR implementation details. Just to don't mess with its internals and allow redesigning in the future. That is why I put "struct b43_ppr_rates" in .c file and all other driver parts use a pointer only.

Seriously, I doubt in the sense of posting RFCs :P

drivers/net/wireless/b43/ppr.c new file +14 -0

+#define B43_PPR_CCK_RATES_NUM		4
+#define B43_PPR_OFDM_RATES_NUM		8
+#define B43_PPR_MCS_RATES_NUM		8
+
+#define B43_PPR_RATES_NUM	(B43_PPR_CCK_RATES_NUM +	\
+				 B43_PPR_OFDM_RATES_NUM * 2 +	\
+				 B43_PPR_MCS_RATES_NUM * 4)
+
+struct b43_ppr_rates {
+	u8 cck[B43_PPR_CCK_RATES_NUM];
+	u8 ofdm[B43_PPR_OFDM_RATES_NUM];
+	u8 ofdm_20_cdd[B43_PPR_OFDM_RATES_NUM];
+	u8 mcs_20[B43_PPR_MCS_RATES_NUM]; /* SISO */
+	u8 mcs_20_cdd[B43_PPR_MCS_RATES_NUM];

Quoted 14 lines · On Sun, 20 Jul 2014 13:00:21 +0200 Rafa? Mi?ecki <[email protected]> wrote:

drivers/net/wireless/b43/ppr.c new file +14 -0

+#define B43_PPR_CCK_RATES_NUM                4
+#define B43_PPR_OFDM_RATES_NUM               8
+#define B43_PPR_MCS_RATES_NUM                8
+
+#define B43_PPR_RATES_NUM    (B43_PPR_CCK_RATES_NUM +        \
+                              B43_PPR_OFDM_RATES_NUM * 2 +   \
+                              B43_PPR_MCS_RATES_NUM * 4)
+
+struct b43_ppr_rates {
+     u8 cck[B43_PPR_CCK_RATES_NUM];
+     u8 ofdm[B43_PPR_OFDM_RATES_NUM];
+     u8 ofdm_20_cdd[B43_PPR_OFDM_RATES_NUM];
+     u8 mcs_20[B43_PPR_MCS_RATES_NUM]; /* SISO */
+     u8 mcs_20_cdd[B43_PPR_MCS_RATES_NUM];

drivers/net/wireless/b43/ppr.c line 27

Quoted 10 lines

drivers/net/wireless/b43/ppr.c new file +10 -0

+     u8 mcs_20_stbc[B43_PPR_MCS_RATES_NUM];
+     u8 mcs_20_sdm[B43_PPR_MCS_RATES_NUM];
+} __packed;
+
+struct b43_ppr {
+     /* All powers are in qdbm (Q5.2) */
+     union {
+             u8 __all_rates[B43_PPR_RATES_NUM];
+             struct b43_ppr_rates rates;
+     } __packed;

I don't think this union has to be packed.
And most likely struct b43_ppr_rates is ok without packing, too.
You could probably remove it there and add a BUILD_BUG which checks something like (pseudocode):
BUILD_BUG_ON(offsetof(struct b43_ppr_rates.mcs_20_sdm) != B43_PPR_CCK_RATES_NUM + B43_PPR_OFDM_RATES_NUM * 2 + B43_PPR_MCS_RATES_NUM * 3);

Removing packed will produce much nicer code.

Broadcom hardware supports auto-adjustment of TX power depending on the currently used rate. So far all calculations were handled without any helpers (API) using big arrays and magic offsets.
It seems Broadcom recently decided to clean this up by developing PPR. Their wlc_ppr.h can be found in open parts of the SDK.
As we plan to implement support for rate-based TX power it makes sense to also implement our version of PPR as well.

Signed-off-by Rafa? Mi?ecki <[email protected]>

---
 drivers/net/wireless/b43/Makefile |   1 +
 drivers/net/wireless/b43/b43.h    |   7 ++
 drivers/net/wireless/b43/ppr.c    | 210 ++++++++++++++++++++++++++++++++++++++
 drivers/net/wireless/b43/ppr.h    |  21 ++++
 4 files changed, 239 insertions(+)
 create mode 100644 drivers/net/wireless/b43/ppr.c
 create mode 100644 drivers/net/wireless/b43/ppr.h

drivers/net/wireless/b43/Makefile +0 -0

index 6e00b88..9f7965a 100644

drivers/net/wireless/b43/Makefile +0 -0

 b43-$(CONFIG_B43_LEDS)         += leds.o
 b43-$(CONFIG_B43_PCMCIA)       += pcmcia.o
 b43-$(CONFIG_B43_SDIO)         += sdio.o

drivers/net/wireless/b43/b43.h +7 -0

index 4113b69..6cfd86d 100644
@@ -791,6 +791,13 @@ struct b43_firmware {
        bool pcm_request_failed;
 };
 
+enum b43_band {
+       B43_BAND_2G = 0,
+       B43_BAND_5G_LO = 1,
+       B43_BAND_5G_MI = 2,
+       B43_BAND_5G_HI = 3,
+};
+
 /* Device (802.11 core) initialization status. */
 enum {
        B43_STAT_UNINIT = 0,    /* Uninitialized. */

drivers/net/wireless/b43/ppr.c new file +0 -0

new file mode 100644
index 0000000..36c4b419

drivers/net/wireless/b43/ppr.c new file +8 -0

+
+#define B43_PPR_CCK_RATES_NUM          4
+#define B43_PPR_OFDM_RATES_NUM         8
+#define B43_PPR_MCS_RATES_NUM          8
+
+#define B43_PPR_RATES_NUM      (B43_PPR_CCK_RATES_NUM +        \
+                                B43_PPR_OFDM_RATES_NUM * 2 +   \
+                                B43_PPR_MCS_RATES_NUM * 4)

drivers/net/wireless/b43/ppr.c new file +58 -0

+};
+
+#define ppr_for_each_entry(ppr, i, entry)                              \
+       for (i = 0, entry = &(ppr)->__all_rates[i];                     \
+            i < B43_PPR_RATES_NUM;                                     \
+            i++, entry++)
+
+struct b43_ppr *b43_ppr_alloc(struct b43_wldev *dev)
+{
+       return kzalloc(sizeof(struct b43_ppr), GFP_KERNEL);
+}
+
+void b43_ppr_clear(struct b43_wldev *dev, struct b43_ppr *ppr)
+{
+       memset(ppr, 0, sizeof(*ppr));
+}
+
+void b43_ppr_add(struct b43_wldev *dev, struct b43_ppr *ppr, int diff)
+{
+       int i;
+       u8 *rate;
+
+       ppr_for_each_entry(ppr, i, rate) {
+               *rate = clamp_val(*rate + diff, 0, 127);
+       }
+}
+
+void b43_ppr_apply_max(struct b43_wldev *dev, struct b43_ppr *ppr, u8 max)
+{
+       int i;
+       u8 *rate;
+
+       ppr_for_each_entry(ppr, i, rate) {
+               *rate = min(*rate, max);
+       }
+}
+
+void b43_ppr_apply_min(struct b43_wldev *dev, struct b43_ppr *ppr, u8 min)
+{
+       int i;
+       u8 *rate;
+
+       ppr_for_each_entry(ppr, i, rate) {
+               *rate = max(*rate, min);
+       }
+}
+
+u8 b43_ppr_get_max(struct b43_wldev *dev, struct b43_ppr *ppr)
+{
+       u8 res = 0;
+       int i;
+       u8 *rate;
+
+       ppr_for_each_entry(ppr, i, rate) {
+               res = max(*rate, res);
+       }
+
+       return res;

drivers/net/wireless/b43/ppr.c new file +29 -0

+       u16 *sprom_mcs_po;
+       u8 extra_cdd_po, extra_stbc_po;
+       int i;
+
+       switch (band) {
+       case B43_BAND_2G:
+               maxpwr = min(sprom->core_pwr_info[0].maxpwr_2g,
+                            sprom->core_pwr_info[1].maxpwr_2g);
+               sprom_ofdm_po = &sprom->ofdm2gpo;
+               sprom_mcs_po = sprom->mcs2gpo;
+               extra_cdd_po = (sprom->cddpo >> 0) & 0xf;
+               extra_stbc_po = (sprom->stbcpo >> 0) & 0xf;
+               break;
+       case B43_BAND_5G_LO:
+               maxpwr = min(sprom->core_pwr_info[0].maxpwr_5gl,
+                            sprom->core_pwr_info[1].maxpwr_5gl);
+               sprom_ofdm_po = &sprom->ofdm5glpo;
+               sprom_mcs_po = sprom->mcs5glpo;
+               extra_cdd_po = (sprom->cddpo >> 8) & 0xf;
+               extra_stbc_po = (sprom->stbcpo >> 8) & 0xf;
+               break;
+       case B43_BAND_5G_MI:
+               maxpwr = min(sprom->core_pwr_info[0].maxpwr_5g,
+                            sprom->core_pwr_info[1].maxpwr_5g);
+               sprom_ofdm_po = &sprom->ofdm5gpo;
+               sprom_mcs_po = sprom->mcs5gpo;
+               extra_cdd_po = (sprom->cddpo >> 4) & 0xf;
+               extra_stbc_po = (sprom->stbcpo >> 4) & 0xf;
+               break;

drivers/net/wireless/b43/ppr.c new file +19 -0

+               return false;
+       }
+
+       if (band == B43_BAND_2G) {
+               for (i = 0; i < 4; i++) {
+                       off = ((sprom->cck2gpo >> (i * 4)) & 0xF) * 2;
+                       rates->cck[i] = maxpwr - off;
+               }
+       }
+
+       /* OFDM */
+       for (i = 0; i < 8; i++) {
+               off = ((*sprom_ofdm_po >> (i * 4)) & 0xF) * 2;
+               rates->ofdm[i] = maxpwr - off;
+       }
+
+       /* MCS 20 SISO */
+       rates->mcs_20[0] = rates->ofdm[0];
+       rates->mcs_20[1] = rates->ofdm[2];

drivers/net/wireless/b43/ppr.c new file +14 -0

+       }
+
+       /* OFDM 20 CDD */
+       rates->ofdm_20_cdd[0] = rates->mcs_20_cdd[0];
+       rates->ofdm_20_cdd[1] = rates->mcs_20_cdd[0];
+       rates->ofdm_20_cdd[2] = rates->mcs_20_cdd[1];
+       rates->ofdm_20_cdd[3] = rates->mcs_20_cdd[2];
+       rates->ofdm_20_cdd[4] = rates->mcs_20_cdd[3];
+       rates->ofdm_20_cdd[5] = rates->mcs_20_cdd[4];
+       rates->ofdm_20_cdd[6] = rates->mcs_20_cdd[5];
+       rates->ofdm_20_cdd[7] = rates->mcs_20_cdd[6];
+
+       /* MCS 20 STBC */
+       for (i = 0; i < 4; i++) {