On Thursday 18 March 2010 09:05:43 Pavel Roskin wrote: > On Thu, 2010-03-18 at 09:53 +1300, Derek Smithies wrote: > > By stating all the flags > > > > >>> ~(AR_MIBC_COW | AR_MIBC_FMC | AR_MIBC_CMC | AR_MIBC_MCS) > > > > we are clearly saying, set all these things to off. > > But we are not saying what is on. It's a simple fact of life that the > code cannot say everything. Sometimes comments are needed, sometimes > finding the full meaning is left to the reader. > > > using > > REG_WRITE(ah, AR_MIBC, 0); > > is bad - the 0 is a magic number. > > 0 is acceptable for registers like AR_MIBC where bits are ORed - it > shows that no bits are on. 0 is the baseline for OR operation, so it's > special rather than magic. > > > My view is that if anything is to be changed, the 0x0F should be replaced > > with something else - it is a magic number and should be avoided. > > 0x0F is much worse than 0. It gives preference to some bits without > making it clear their meaning. It could be written as (AR_MIBC_COW | > AR_MIBC_FMC | AR_MIBC_CMC | AR_MIBC_MCS), but then the whole expression > would look even more ridiculous. > > I won't bother asking Sam Leffler to patch HAL, but Linux code should be > cleaned up of magic numbers such as 0x0F. > > Sorry for participating in this bikeshed party, but your comments could > be taken by some as an encouragement for writing new code in such > convoluted way.
thanks, pavel! :) bruno _______________________________________________ ath5k-devel mailing list ath5k-devel@lists.ath5k.org https://lists.ath5k.org/mailman/listinfo/ath5k-devel
