Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* attribute types

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2018-06-14 at 15:55 +0000, Omer Efrat wrote:
> 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.

I disagree, they aren't a bugfix.

> This enum already has different numbering in different versions (attributes removed from the middle,
> i.e. NL80211_STA_INFO_MAX_RSSI).

This must be in some non-upstream tree, because it certainly never
happened in upstream, nor did that attribute (MAX_RSSI) ever exist
there.

> 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.

I don't think it is.

The bugfix is certainly legitimate, but I don't want to claim such a
long patch as the bugfix, with a single compiler warning to show for.

If you prefer, I can do the bugfix separately myself, and then you can
focus on the remaining patches as cleanups for -next.

johannes