Re: [PATCHv5 03/18] mac80211: fix timing for 5 MHz and 10 MHz channels

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2013-06-27 at 18:58 +0200, Simon Wunderlich wrote:

>  * change divisor to shift to avoid unnecesary integer divisions

That didn't really do much ;-)

>  	d->perfect_tx_time = ieee80211_frame_duration(band, 1200,
> -			rate->bitrate, erp, 1);
> +			DIV_ROUND_UP(rate->bitrate, 1 << shift), erp, 1,

That's still the integer division, unless the compiler can do something
about it?

> +			DIV_ROUND_UP(rate->bitrate, 1 << shift), erp, 1,

ditto

> +		mr->bitrate = DIV_ROUND_UP(sband->bitrates[i].bitrate,
> +					   (1 << shift) * 5);

That looks even weirder?

> +		/* IEEE 802.11-2012 18.3.2.4: all values above are:
> +		 *  * times 4 for 5 MHz
> +		 *  * times 2 for 10 MHz
> +		 */
> +		dur *= 1 << shift;

Isn't that just "dur <<= shift"?

I think "DIV_ROUND_UP(x, 1 << shift)" can probably be written more
efficiently? Unless the compiler will do something by itself?

Something like

DIV_ROUND_UP(x, 1<<shift) == (x + (1<<shift) -1) >> shift

I think?

OK, it looks like if "x" is *unsigned* then the compiler will do this,
but with *signed* values it can't actually know because for negative
values it's obviously not actually equivalent.

johannes