Re: [PATCH 1/2] b43: implement PPR (Power Per Rate) management/API

["Michael Büsch" <m@bues.ch>]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

On Sun, 20 Jul 2014 13:00:21 +0200
Rafał Miłecki <zajec5@gmail.com> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]