Omer Efrat wrote: >Johannes Berg wrote: >>Perhaps, though I'm not sure I see it, there's some value in switching >>them all so that if you copy something and change it to a new value you >>don't run into this problem again, but if anything that should be (a) >>separate patch(es) since this one is a bugfix and the others aren't. > >Exactly my thoughts. I accept the need for the cleanup to be separated >to different patches as well, I will send a v3. Actually, after some more thought, I don't think changing to BIT_ULL for attribute types less than 32 should be in separated patches because of the claim they are not a bug fix. This enum already has different numbering in different versions (attributes removed from the middle, i.e. NL80211_STA_INFO_MAX_RSSI). Therefore, it's hard to mark each of them as "bug fix" or "cleanup only" change. (Some versions has NL80211_STA_INFO_TID_STATS = 32, while others has NL80211_STA_INFO_TID_STATS = 31, etc.) If that's acceptable, I will send a v3 for adding which commit is being fixed by this patch series. Best Regards, Omer Efrat. ________________________________________ From: Omer Efrat Sent: Thursday, June 14, 2018 3:30:27 PM To: Johannes Berg; linux-wireless@xxxxxxxxxxxxxxx Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* attribute types Johannes Berg wrote: >Perhaps, though I'm not sure I see it, there's some value in switching >them all so that if you copy something and change it to a new value you >don't run into this problem again, but if anything that should be (a) >separate patch(es) since this one is a bugfix and the others aren't. Exactly my thoughts. I accept the need for the cleanup to be separated to different patches as well, I will send a v3. Omer Efrat. ________________________________________ From: Johannes Berg <johannes@xxxxxxxxxxxxxxxx> Sent: Thursday, June 14, 2018 2:08:05 PM To: Omer Efrat; linux-wireless@xxxxxxxxxxxxxxx Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* attribute types On Thu, 2018-06-14 at 13:11 +0300, Omer Efrat wrote: > Since 'filled' member in station_info changed to u64, BIT_ULL macro > should be used with NL80211_STA_INFO_* attribute types instead of BIT. > > The BIT macro uses unsigned long type which some architectures handle as 32bit > and this results in compilation warnings such as: > > net/mac80211/sta_info.c:2223:2: warning: left shift count >= width of type > sinfo->filled |= BIT(NL80211_STA_INFO_TID_STATS); > ^ It seems like the only change needed is with BIT(NL80211_STA_INFO_TID_STATS), so I'd argue you should restrict the patch to that. Perhaps, though I'm not sure I see it, there's some value in switching them all so that if you copy something and change it to a new value you don't run into this problem again, but if anything that should be (a) separate patch(es) since this one is a bugfix and the others aren't. johannes
