On Fri, Mar 22, 2013 at 10:08:19PM +0100, Johannes Berg wrote:
> On Fri, 2013-03-22 at 16:48 +0100, Karl Beldan wrote:
>
> > + if (chandef.chan == local->_oper_chandef.chan)
> > + chandef = local->_oper_chandef;
> > + else {
> > + chandef.width = NL80211_CHAN_WIDTH_20_NOHT;
> > + chandef.center_freq1 = chandef.chan->center_freq;
> > + }
>
> I'll agree with checkpatch that you should probably put braces against
> both arms of the if :)
>
> > - if (chan != local->_oper_channel ||
> > - channel_type != local->_oper_channel_type)
> > + chandef.chan = local->tmp_channel;
> > + chandef.width = NL80211_CHAN_WIDTH_20_NOHT;
> > + chandef.center_freq1 = chandef.chan->center_freq;
> > + } else
> > + chandef = local->_oper_chandef;
>
> same here I guess
>
> > + /*
> > + * FIXME: Here we are downgrading to NL80211_CHAN_WIDTH_20_NOHT
> > + * and don't adjust our ht/vht settings
> > + * This is wrong - we should behave according to the CSA params
> > + */
>
> I can live with this in this patch. I know we need to fix it, but I
> don't want to do it in this patch nor do I really want to require it for
> this patch. If you feel it's a requirement for this feel free to pick up
> my other patches, but then I'd prefer you did it in a separate patch.
>
Then I'll remove the FIXME
> > - out:
> > +out:
>
> that seems a little unnecessary :)
>
> > #define CHANDEF_ASSIGN(c) \
> > - __entry->chan_width = (c)->width; \
> > - __entry->center_freq1 = (c)->center_freq1; \
> > + __entry->chan_width = (c)->width; \
> > + __entry->center_freq1 = (c)->center_freq1; \
>
> hmm, why did these lines change?
>
To align the ending backslashes with that of the control_freq line in :
(your quote is missing the line that induced the shifts)
#define CHANDEF_ASSIGN(c) \
- __entry->control_freq = (c)->chan->center_freq; \
- __entry->chan_width = (c)->width; \
- __entry->center_freq1 = (c)->center_freq1; \
+ __entry->control_freq = (c)->chan ? (c)->chan->center_freq : 0; \
+ __entry->chan_width = (c)->width; \
+ __entry->center_freq1 = (c)->center_freq1; \
Now, looking at this it seems I forgot to align the CHANDEF_ASSIGN
ending backslash .. and to be perfectly in line I'd add that it asks to
shift every CHANDEF_* ending backslashes which, coincidentally, would
get aligned with the VIF_* ending backslashes.
Karl
--
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
