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
diff --git a/drivers/net/wireless/b43/Makefile b/drivers/net/wireless/b43/Makefile
index 6e00b88..9f7965a 100644
--- a/drivers/net/wireless/b43/Makefile
+++ b/drivers/net/wireless/b43/Makefile
@@ -18,6 +18,7 @@ b43-y += xmit.o
b43-y += dma.o
b43-y += pio.o
b43-y += rfkill.o
+b43-y += ppr.o
b43-$(CONFIG_B43_LEDS) += leds.o
b43-$(CONFIG_B43_PCMCIA) += pcmcia.o
b43-$(CONFIG_B43_SDIO) += sdio.o
diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
index 4113b69..6cfd86d 100644
--- a/drivers/net/wireless/b43/b43.h
+++ b/drivers/net/wireless/b43/b43.h
@@ -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. */
diff --git a/drivers/net/wireless/b43/ppr.c b/drivers/net/wireless/b43/ppr.c
new file mode 100644
index 0000000..36c4b419
--- /dev/null
+++ b/drivers/net/wireless/b43/ppr.c
@@ -0,0 +1,210 @@
+#include "ppr.h"
+#include "b43.h"
+
+#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];
+ 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;
+};
+
+#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;
+}
+
+bool b43_ppr_load_max_from_sprom(struct b43_wldev *dev, struct b43_ppr *ppr,
+ enum b43_band band)
+{
+ struct b43_ppr_rates *rates = &ppr->rates;
+ struct ssb_sprom *sprom = dev->dev->bus_sprom;
+ struct b43_phy *phy = &dev->phy;
+ u8 maxpwr, off;
+ u32 *sprom_ofdm_po;
+ 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;
+ case B43_BAND_5G_HI:
+ maxpwr = min(sprom->core_pwr_info[0].maxpwr_5gh,
+ sprom->core_pwr_info[1].maxpwr_5gh);
+ sprom_ofdm_po = &sprom->ofdm5ghpo;
+ sprom_mcs_po = sprom->mcs5ghpo;
+ extra_cdd_po = (sprom->cddpo >> 12) & 0xf;
+ extra_stbc_po = (sprom->stbcpo >> 12) & 0xf;
+ break;
+ default:
+ BUG_ON(1);
+ 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];
+ rates->mcs_20[2] = rates->ofdm[3];
+ rates->mcs_20[3] = rates->ofdm[4];
+ rates->mcs_20[4] = rates->ofdm[5];
+ rates->mcs_20[5] = rates->ofdm[6];
+ rates->mcs_20[6] = rates->ofdm[7];
+ rates->mcs_20[7] = rates->ofdm[7];
+
+ /* MCS 20 CDD */
+ for (i = 0; i < 4; i++) {
+ off = ((sprom_mcs_po[0] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_cdd[i] = maxpwr - off;
+ if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
+ rates->mcs_20_cdd[i] -= extra_cdd_po;
+ }
+ for (i = 0; i < 4; i++) {
+ off = ((sprom_mcs_po[1] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_cdd[4 + i] = maxpwr - off;
+ if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
+ rates->mcs_20_cdd[i] -= extra_cdd_po;
+ }
+
+ /* 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++) {
+ off = ((sprom_mcs_po[0] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_stbc[i] = maxpwr - off;
+ if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
+ rates->mcs_20_stbc[i] -= extra_stbc_po;
+ }
+ for (i = 0; i < 4; i++) {
+ off = ((sprom_mcs_po[1] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_stbc[4 + i] = maxpwr - off;
+ if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
+ rates->mcs_20_stbc[i] -= extra_stbc_po;
+ }
+
+ /* MCS 20 SDM */
+ for (i = 0; i < 4; i++) {
+ off = ((sprom_mcs_po[2] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_sdm[i] = maxpwr - off;
+ }
+ for (i = 0; i < 4; i++) {
+ off = ((sprom_mcs_po[3] >> (i * 4)) & 0xF) * 2;
+ rates->mcs_20_sdm[4 + i] = maxpwr - off;
+ }
+
+ return true;
+}
diff --git a/drivers/net/wireless/b43/ppr.h b/drivers/net/wireless/b43/ppr.h
new file mode 100644
index 0000000..5a96c31
--- /dev/null
+++ b/drivers/net/wireless/b43/ppr.h
@@ -0,0 +1,21 @@
+#ifndef LINUX_B43_PPR_H_
+#define LINUX_B43_PPR_H_
+
+#include <linux/types.h>
+
+struct b43_ppr;
+struct b43_wldev;
+enum b43_band;
+
+struct b43_ppr *b43_ppr_alloc(struct b43_wldev *dev);
+void b43_ppr_clear(struct b43_wldev *dev, struct b43_ppr *ppr);
+
+void b43_ppr_add(struct b43_wldev *dev, struct b43_ppr *ppr, int diff);
+void b43_ppr_apply_max(struct b43_wldev *dev, struct b43_ppr *ppr, u8 max);
+void b43_ppr_apply_min(struct b43_wldev *dev, struct b43_ppr *ppr, u8 min);
+u8 b43_ppr_get_max(struct b43_wldev *dev, struct b43_ppr *ppr);
+
+bool b43_ppr_load_max_from_sprom(struct b43_wldev *dev, struct b43_ppr *ppr,
+ enum b43_band band);
+
+#endif /* LINUX_B43_PPR_H_ */
--
1.8.4.5
On Sun, Jul 20, 2014 at 1:00 PM, Rafał Miłecki <[email protected]> wrote:
> 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.
Nice one.
> 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
>
> diff --git a/drivers/net/wireless/b43/Makefile b/drivers/net/wireless/b43/Makefile
> index 6e00b88..9f7965a 100644
> --- a/drivers/net/wireless/b43/Makefile
> +++ b/drivers/net/wireless/b43/Makefile
> @@ -18,6 +18,7 @@ b43-y += xmit.o
> b43-y += dma.o
> b43-y += pio.o
> b43-y += rfkill.o
> +b43-y += ppr.o
Is this only used for N-PHY, or also for others? If not, it might make
sense to only include it if N-PHY support is selected. Not sure if
it's large enough to matter much.
> b43-$(CONFIG_B43_LEDS) += leds.o
> b43-$(CONFIG_B43_PCMCIA) += pcmcia.o
> b43-$(CONFIG_B43_SDIO) += sdio.o
> diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
> index 4113b69..6cfd86d 100644
> --- a/drivers/net/wireless/b43/b43.h
> +++ b/drivers/net/wireless/b43/b43.h
> @@ -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. */
> diff --git a/drivers/net/wireless/b43/ppr.c b/drivers/net/wireless/b43/ppr.c
> new file mode 100644
> index 0000000..36c4b419
> --- /dev/null
> +++ b/drivers/net/wireless/b43/ppr.c
> @@ -0,0 +1,210 @@
> +#include "ppr.h"
> +#include "b43.h"
This file could use a license/copyright header. It looks a bit odd without one.
> +
> +#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];
> + u8 mcs_20_stbc[B43_PPR_MCS_RATES_NUM];
> + u8 mcs_20_sdm[B43_PPR_MCS_RATES_NUM];
> +} __packed;
not sure if __packed makes a difference if *all* members are u8.
> +
> +struct b43_ppr {
> + /* All powers are in qdbm (Q5.2) */
> + union {
> + u8 __all_rates[B43_PPR_RATES_NUM];
> + struct b43_ppr_rates rates;
> + } __packed;
Same here.
> +};
> +
> +#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;
> +}
> +
> +bool b43_ppr_load_max_from_sprom(struct b43_wldev *dev, struct b43_ppr *ppr,
> + enum b43_band band)
> +{
> + struct b43_ppr_rates *rates = &ppr->rates;
> + struct ssb_sprom *sprom = dev->dev->bus_sprom;
> + struct b43_phy *phy = &dev->phy;
> + u8 maxpwr, off;
> + u32 *sprom_ofdm_po;
sprom->ofdm2gpo is a single u32, you gain nothing by using a pointer
and dereferencing it (actually you will double your stack usage on 64
bit system).
> + 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;
> + case B43_BAND_5G_HI:
> + maxpwr = min(sprom->core_pwr_info[0].maxpwr_5gh,
> + sprom->core_pwr_info[1].maxpwr_5gh);
> + sprom_ofdm_po = &sprom->ofdm5ghpo;
> + sprom_mcs_po = sprom->mcs5ghpo;
> + extra_cdd_po = (sprom->cddpo >> 12) & 0xf;
> + extra_stbc_po = (sprom->stbcpo >> 12) & 0xf;
> + break;
> + default:
> + BUG_ON(1);
Is it really that critical that it should halt the system? Maybe a
WARN_ON[_ONCE](1); is enough.
> + 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];
> + rates->mcs_20[2] = rates->ofdm[3];
> + rates->mcs_20[3] = rates->ofdm[4];
> + rates->mcs_20[4] = rates->ofdm[5];
> + rates->mcs_20[5] = rates->ofdm[6];
> + rates->mcs_20[6] = rates->ofdm[7];
> + rates->mcs_20[7] = rates->ofdm[7];
> +
> + /* MCS 20 CDD */
> + for (i = 0; i < 4; i++) {
> + off = ((sprom_mcs_po[0] >> (i * 4)) & 0xF) * 2;
You should decide whether to use upper case or lower case hex (above
is all lower).
> + rates->mcs_20_cdd[i] = maxpwr - off;
> + if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
> + rates->mcs_20_cdd[i] -= extra_cdd_po;
> + }
> + for (i = 0; i < 4; i++) {
> + off = ((sprom_mcs_po[1] >> (i * 4)) & 0xF) * 2;
> + rates->mcs_20_cdd[4 + i] = maxpwr - off;
> + if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
> + rates->mcs_20_cdd[i] -= extra_cdd_po;
Did you intend to reduce rates->mcs_20_cdd[4 + i] here?
> + }
> +
> + /* 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++) {
> + off = ((sprom_mcs_po[0] >> (i * 4)) & 0xF) * 2;
> + rates->mcs_20_stbc[i] = maxpwr - off;
> + if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
> + rates->mcs_20_stbc[i] -= extra_stbc_po;
> + }
> + for (i = 0; i < 4; i++) {
> + off = ((sprom_mcs_po[1] >> (i * 4)) & 0xF) * 2;
> + rates->mcs_20_stbc[4 + i] = maxpwr - off;
> + if (phy->type == B43_PHYTYPE_N && phy->rev >= 3)
> + rates->mcs_20_stbc[i] -= extra_stbc_po;
Same question here, did you intend rates->mcs_20_stbc[4 + i]?
> + }
> +
> + /* MCS 20 SDM */
> + for (i = 0; i < 4; i++) {
> + off = ((sprom_mcs_po[2] >> (i * 4)) & 0xF) * 2;
> + rates->mcs_20_sdm[i] = maxpwr - off;
> + }
> + for (i = 0; i < 4; i++) {
> + off = ((sprom_mcs_po[3] >> (i * 4)) & 0xF) * 2;
> + rates->mcs_20_sdm[4 + i] = maxpwr - off;
I guess you did ;-)
> + }
> +
> + return true;
> +}
Rest looks fine.
Jonas
On Sun, Jul 20, 2014 at 1:53 PM, Rafał Miłecki <[email protected]> wrote:
> On 20 July 2014 13:49, Jonas Gorski <[email protected]> wrote:
>> On Sun, Jul 20, 2014 at 1:00 PM, Rafał Miłecki <[email protected]> wrote:
>>> diff --git a/drivers/net/wireless/b43/phy_n.h b/drivers/net/wireless/b43/phy_n.h
>>> index 30bec81..252d843 100644
>>> --- a/drivers/net/wireless/b43/phy_n.h
>>> +++ b/drivers/net/wireless/b43/phy_n.h
>>> @@ -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.
If this were a library used by several drivers I could understand. But
hiding things from yourself? Seriously? ;P
>
> Seriously, I doubt in the sense of posting RFCs :P
I know that feel :p. I usually try to do a thorough code review if I
review, and was missing the time when you posted the RFC. Also I'm
usually a bit more lenient with RFCs as I assume this is usually a
request for commenting on the intend of the code, and less for finding
nitpicks/bugs.
Jonas
On 20 July 2014 13:00, Rafał Miłecki <[email protected]> wrote:
> 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)
Forgot to say. I've tested this on BCM4321, BCM4322, BCM43224,
BCM43217 and BCM43228. No performance regressions observed, so I guess
my code doesn't do anything stupid ;)
On Sun, 20 Jul 2014 13:50:39 +0200
Rafał Miłecki <[email protected]> wrote:
> On 20 July 2014 13:26, Michael Büsch <[email protected]> wrote:
> > On Sun, 20 Jul 2014 13:00:22 +0200
> > Rafał Miłecki <[email protected]> wrote:
> >> @@ -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
adjustment of txpower may be very expensive there. So I think this was
the reason I split it in half and let the heavier part run in workqueue context or
something. Also recalc used to be called from interrupt tasklet. Not sure if
that's still the case anymore, though.
> 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).
Yeah looks fine.
--
Michael
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(-)
diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
index 6cfd86d..9b8feb9 100644
--- a/drivers/net/wireless/b43/b43.h
+++ b/drivers/net/wireless/b43/b43.h
@@ -457,6 +457,7 @@ enum {
#define B43_MACCTL_RADIOLOCK 0x00080000 /* Radio lock */
#define B43_MACCTL_BEACPROMISC 0x00100000 /* Beacon Promiscuous */
#define B43_MACCTL_KEEP_BADPLCP 0x00200000 /* Keep frames with bad PLCP */
+#define B43_MACCTL_PHY_LOCK 0x00200000
#define B43_MACCTL_KEEP_CTL 0x00400000 /* Keep control frames */
#define B43_MACCTL_KEEP_BAD 0x00800000 /* Keep bad frames (FCS) */
#define B43_MACCTL_PROMISC 0x01000000 /* Promiscuous mode */
diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
index 11d7543..e984516 100644
--- a/drivers/net/wireless/b43/phy_n.c
+++ b/drivers/net/wireless/b43/phy_n.c
@@ -34,6 +34,7 @@
#include "radio_2056.h"
#include "radio_2057.h"
#include "main.h"
+#include "ppr.h"
struct nphy_txgains {
u16 tx_lpf[2];
@@ -3605,16 +3606,6 @@ static void b43_nphy_iq_cal_gain_params(struct b43_wldev *dev, u16 core,
* Tx and Rx
**************************************************/
-static void b43_nphy_op_adjust_txpower(struct b43_wldev *dev)
-{//TODO
-}
-
-static enum b43_txpwr_result b43_nphy_op_recalc_txpower(struct b43_wldev *dev,
- bool ignore_tssi)
-{//TODO
- return B43_TXPWR_RES_DONE;
-}
-
/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/TxPwrCtrlEnable */
static void b43_nphy_tx_power_ctrl(struct b43_wldev *dev, bool enable)
{
@@ -4145,7 +4136,11 @@ static void b43_nphy_tx_power_ctl_setup(struct b43_wldev *dev)
b1[0] = b1[1] = -1393;
}
}
- /* target[0] = target[1] = nphy->tx_power_max; */
+
+ if (nphy->tx_pwr_max_ppr) {
+ target[0] = b43_ppr_get_max(dev, nphy->tx_pwr_max_ppr);
+ target[1] = target[0];
+ }
if (dev->phy.rev >= 3) {
if (sprom->fem.ghz2.tssipos)
@@ -5871,6 +5866,74 @@ static void b43_nphy_set_rx_core_state(struct b43_wldev *dev, u8 mask)
b43_mac_enable(dev);
}
+static enum b43_txpwr_result b43_nphy_op_recalc_txpower(struct b43_wldev *dev,
+ bool ignore_tssi)
+{
+ struct b43_phy *phy = &dev->phy;
+ struct b43_phy_n *nphy = dev->phy.n;
+ struct ieee80211_channel *channel = dev->wl->hw->conf.chandef.chan;
+ u8 max; /* qdBm */
+ bool tx_pwr_state;
+
+ if (nphy->tx_pwr_last_recalc_freq == channel->center_freq &&
+ nphy->tx_pwr_last_recalc_limit == phy->desired_txpower)
+ return B43_TXPWR_RES_DONE;
+
+ /* Make sure we have a clean PPR */
+ if (!nphy->tx_pwr_max_ppr) {
+ nphy->tx_pwr_max_ppr = b43_ppr_alloc(dev);
+ if (WARN_ON(!nphy->tx_pwr_max_ppr))
+ return B43_TXPWR_RES_DONE;
+ } else {
+ b43_ppr_clear(dev, nphy->tx_pwr_max_ppr);
+ }
+
+ /* HW limitations */
+ b43_ppr_load_max_from_sprom(dev, nphy->tx_pwr_max_ppr, B43_BAND_2G);
+
+ /* Regulatory & user settings */
+ max = INT_TO_Q52(phy->chandef->chan->max_power);
+ if (phy->desired_txpower)
+ max = min_t(u8, max, INT_TO_Q52(phy->desired_txpower));
+ b43_ppr_apply_max(dev, nphy->tx_pwr_max_ppr, max);
+ if (b43_debug(dev, B43_DBG_XMITPOWER))
+ b43dbg(dev->wl, "Calculated TX power: " Q52_FMT "\n",
+ Q52_ARG(b43_ppr_get_max(dev, nphy->tx_pwr_max_ppr)));
+
+ /* TODO: Enable this once we get gains working */
+#if 0
+ /* Some extra gains */
+ hw_gain = 6; /* N-PHY specific */
+ if (b43_current_band(dev->wl) == IEEE80211_BAND_2GHZ)
+ hw_gain += sprom->antenna_gain.a0;
+ else
+ hw_gain += sprom->antenna_gain.a1;
+ b43_ppr_add(dev, nphy->tx_pwr_max_ppr, -hw_gain);
+#endif
+
+ /* Make sure we didn't go too low */
+ b43_ppr_apply_min(dev, nphy->tx_pwr_max_ppr, INT_TO_Q52(8));
+
+ /* Apply */
+ tx_pwr_state = nphy->txpwrctrl;
+ b43_mac_suspend(dev);
+ b43_nphy_tx_power_ctl_setup(dev);
+ if (dev->dev->core_rev == 11 || dev->dev->core_rev == 12) {
+ b43_maskset32(dev, B43_MMIO_MACCTL, ~0, B43_MACCTL_PHY_LOCK);
+ b43_read32(dev, B43_MMIO_MACCTL);
+ udelay(1);
+ }
+ 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;
+}
+
/**************************************************
* N-PHY init
**************************************************/
@@ -6401,6 +6464,7 @@ static int b43_nphy_op_allocate(struct b43_wldev *dev)
nphy = kzalloc(sizeof(*nphy), GFP_KERNEL);
if (!nphy)
return -ENOMEM;
+
dev->phy.n = nphy;
return 0;
@@ -6464,6 +6528,7 @@ static void b43_nphy_op_free(struct b43_wldev *dev)
struct b43_phy *phy = &dev->phy;
struct b43_phy_n *nphy = phy->n;
+ kfree(nphy->tx_pwr_max_ppr);
kfree(nphy);
phy->n = NULL;
}
@@ -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,
};
diff --git a/drivers/net/wireless/b43/phy_n.h b/drivers/net/wireless/b43/phy_n.h
index 30bec81..252d843 100644
--- a/drivers/net/wireless/b43/phy_n.h
+++ b/drivers/net/wireless/b43/phy_n.h
@@ -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;
+ 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
On 20 July 2014 13:26, Michael Büsch <[email protected]> wrote:
> On Sun, 20 Jul 2014 13:00:22 +0200
> Rafał Miłecki <[email protected]> wrote:
>> @@ -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
hardware (firmware?) power control, so much more has to be done in the
driver. It's probably what Broadcom calls a "software" TX power
recalc. If you take a look at
[PATCH] Staging: Add initial release of brcm80211 - Broadcom 802.11n
wireless LAN driver.
you will see there was a wlc_phy_txpower_target_set with following calls:
wlc_phy_txpower_recalc_target(pi);
wlc_phy_cal_txpower_recalc_sw(pi);
The second was an empty function, most probably because Broadcom
stripped it our of G-PHY code before releasing.
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).
In G-PHY you were setting that manually, so you could indeed need some
hacks to avoid some conflict or sth. This should not be needed for
N-PHY.
--
Rafał
On 20 July 2014 13:49, Jonas Gorski <[email protected]> wrote:
> On Sun, Jul 20, 2014 at 1:00 PM, Rafał Miłecki <[email protected]> wrote:
>> diff --git a/drivers/net/wireless/b43/phy_n.h b/drivers/net/wireless/b43/phy_n.h
>> index 30bec81..252d843 100644
>> --- a/drivers/net/wireless/b43/phy_n.h
>> +++ b/drivers/net/wireless/b43/phy_n.h
>> @@ -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
--
Rafał
On Sun, Jul 20, 2014 at 1:00 PM, Rafał Miłecki <[email protected]> wrote:
> 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(-)
>
(snip)
> diff --git a/drivers/net/wireless/b43/phy_n.h b/drivers/net/wireless/b43/phy_n.h
> index 30bec81..252d843 100644
> --- a/drivers/net/wireless/b43/phy_n.h
> +++ b/drivers/net/wireless/b43/phy_n.h
> @@ -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.
> + 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
Jonas
On Sun, 20 Jul 2014 13:00:22 +0200
Rafał Miłecki <[email protected]> wrote:
> + /* Apply */
> + tx_pwr_state = nphy->txpwrctrl;
> + b43_mac_suspend(dev);
> + b43_nphy_tx_power_ctl_setup(dev);
> + if (dev->dev->core_rev == 11 || dev->dev->core_rev == 12) {
> + b43_maskset32(dev, B43_MMIO_MACCTL, ~0, B43_MACCTL_PHY_LOCK);
> + b43_read32(dev, B43_MMIO_MACCTL);
> + udelay(1);
> + }
You could probably write a function b43_phy_lock/unlock() similar to b43_mac_suspend/enable
to make this more readable.
The PHY_LOCK bit is related to the mac-suspend bit, after all. These are both used to lock
the firmware in one way or the other.
> + 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;
> +}
> @@ -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.
--
Michael
On Sun, 20 Jul 2014 13:00:21 +0200
Rafał Miłecki <[email protected]> wrote:
> +#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];
> + 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.
--
Michael
On 20 July 2014 13:19, Michael Büsch <[email protected]> wrote:
> On Sun, 20 Jul 2014 13:00:21 +0200
> Rafał Miłecki <[email protected]> wrote:
>
>> +#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];
>> + 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.
Thanks for your comments. I'll rework this.
John: please wait for V2
Btw. I was waiting for comments since Monday you slug (just kidding!) ;)
--
Rafał