On 2011-02-14 3:02 PM, Mohammed Shafi wrote: > On Monday 14 February 2011 07:18 PM, Felix Fietkau wrote: >> On 2011-02-14 5:46 AM, Mohammed Shafi wrote: >> >>> On Monday 14 February 2011 10:11 AM, Mohammed Shafi wrote: >>> >>>> On Friday 11 February 2011 09:59 PM, John W. Linville wrote: >>>> >>>>> On Fri, Feb 11, 2011 at 05:21:06PM +0100, Felix Fietkau wrote: >>>>> >>>>>> On 2011-02-11 5:15 PM, John W. Linville wrote: >>>>>> >>>>>>> On Fri, Feb 11, 2011 at 01:52:23PM +0100, Felix Fietkau wrote: >>>>>>> >>>>>>>> On 2011-02-11 8:01 AM, Mohammed Shafi Shajakhan wrote: >>>>>>>> >>>>>>>>> From: Mohammed Shafi Shajakhan<mshajakhan@xxxxxxxxxxx> >>>>>>>>> >>>>>>>>> Instead of using raw numbers to assign slot time it would be >>>>>>>>> better to >>>>>>>>> make use of predefined slot time macros >>>>>>>>> >>>>>>>> How does this make it better? >>>>>>>> >>>>>>> Maybe if it was ATH9K_SHORT_SLOT_TIME it would make more sense? >>>>>>> >>>>>> Well, neither the unit of this variable, nor the values that can be >>>>>> used >>>>>> are ath9k specific. >>>>>> >>> Felix then I don't know why these macros are used here and I followed >>> the same thing: >>> >>> htc_drv_beacon.c 242 if (ah->slottime == ATH9K_SLOT_TIME_20) >>> init.c 517 sc->beacon.slottime = ATH9K_SLOT_TIME_9; >>> >> Just because the macros are there doesn't mean that it was a good idea >> to use them. As far as I know, these were simply inherited from the >> Atheros codebase that ath9k was based on. >> I actually consider the code more readable without the redundant >> "ATH9K_SLOT_TIME_" part. >> > Felix I agree the first part, but I could still see no harm in using > these macros. > Initially we using these values 6,9,20(no other values) for the slot > time and there are macros defined for them. If we are using some other > values I would agree that its wrong. > Why not make use of it ? > IMHO if we use these macros it will at least people who are reading the > code there are three standard values 6,9 and 20 How is 6 a standard value? And why use driver specific defines when it's really an 802.11 standard thing? Using something like this would make the code more readable: #define IEEE80211_SHORT_SLOT_TIME 9 #define IEEE80211_LONG_SLOT_TIME 20 ATH9K_SLOT_TIME_9 or ATH9K_SLOT_TIME_20? Not so much... > I am sure it would help us to debug issues easily(just like Fair beacon > distribution thing). I really don't see how this is helpful in any way. The main reason why I object to stuff like this is because I think that "other code is like that" is not a good reason for repeating it, especially if what was done on the other code never made much sense to begin with. In this case I think it's more of a reason to clean up the other code first and then make things more consistent :) - Felix -- 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
