On Wed, 2008-05-14 at 01:24 +0200, Michael Buesch wrote: > On Tuesday 13 May 2008 21:55:18 Harvey Harrison wrote: > > The 4-bit reversal flip_4bit is replaced with the bitrev helper > > bitrev8 and a 4-bit shift. The B43_WARN is moved to the location > > where a register is read from for checking there. The other caller > > explicitly passes an array index which is guaranteed to be within range > > and so a B43_WARN is not added there. > > > > Signed-off-by: Harvey Harrison <harvey.harrison@xxxxxxxxx> > > ACK > > But I'd prefer if we had something like the following > and use that: > > #define bitrev4(x) (bitrev8(x) >> 4) > > This way the confusing (confusing to me :) ) shifts in the code would go away. > I have a hard time realizing that > bitrev8(x) >> 3 > does actually mean > bitrev4(x) << 1 > Maybe I'm just stupid, though. :) > You're not, this was only valid because x < 16....if bit 4 could be set this doesn't work anymore as bitrev8(x) >> 4 would truncate bit 4 before shifting left. So there was some subtlety here that makes >> 3 ok. Do you want an incremental patch adding a bitrev4 to phy.c or are you going to add it? Harvey -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
