| Message ID | 1458318955-32070-1-git-send-email-luisbg@osg.samsung.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: > Fix order of mac80211_rx_flags description to match the enum. > > Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> > --- > Hi, > > I want ahead and fixed the order of the descriptions. checkpatch.pl > was giving > a warning to my previous patch and I had a hunch it was because the > wrong order > breaks the parser. Indeed it does and with this patch below > checkpatch.pl does > not complain about this flag descriptions anymore. > Huh. I think we should fix checkpatch.pl instead. While the current order isn't likely really good, I believe kernel-doc will output the documentation in the order it's listed, and that can be useful for the documentation output to group related things, even if their bits may be further apart. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: > Fix order of mac80211_rx_flags description to match the enum. > > Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> > --- > Hi, > > I want ahead and fixed the order of the descriptions. checkpatch.pl was giving > a warning to my previous patch and I had a hunch it was because the wrong order > breaks the parser. Indeed it does and with this patch below checkpatch.pl does > not complain about this flag descriptions anymore. checkpatch complains? About what? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 17:29, Joe Perches wrote: > On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: >> Fix order of mac80211_rx_flags description to match the enum. >> >> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> >> --- >> Hi, >> >> I want ahead and fixed the order of the descriptions. checkpatch.pl was giving >> a warning to my previous patch and I had a hunch it was because the wrong order >> breaks the parser. Indeed it does and with this patch below checkpatch.pl does >> not complain about this flag descriptions anymore. > > checkpatch complains? About what? > warning: Enum value 'RX_FLAG_DUP_VALIDATED' not described in enum 'mac80211_rx_flags' Is the warning checkpatch.pl gives against my first patch (add doc for RX_FLAG_DUP_VALIDATED flag). This is silenced by reordering the descriptions to match the order in the enum. Thanks, Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-03-18 at 17:40 +0000, Luis de Bethencourt wrote: > On 18/03/16 17:29, Joe Perches wrote: > > On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: > > > Fix order of mac80211_rx_flags description to match the enum. [] > > > I want ahead and fixed the order of the descriptions. checkpatch.pl was giving > > > a warning to my previous patch and I had a hunch it was because the wrong order > > > breaks the parser. Indeed it does and with this patch below checkpatch.pl does > > > not complain about this flag descriptions anymore. > > checkpatch complains? About what? > warning: Enum value 'RX_FLAG_DUP_VALIDATED' not described in enum 'mac80211_rx_flags' That's not a checkpatch warning. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 16:49, Johannes Berg wrote: > On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: >> Fix order of mac80211_rx_flags description to match the enum. >> >> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> >> --- >> Hi, >> >> I want ahead and fixed the order of the descriptions. checkpatch.pl >> was giving >> a warning to my previous patch and I had a hunch it was because the >> wrong order >> breaks the parser. Indeed it does and with this patch below >> checkpatch.pl does >> not complain about this flag descriptions anymore. >> > Huh. I think we should fix checkpatch.pl instead. While the current > order isn't likely really good, I believe kernel-doc will output the > documentation in the order it's listed, and that can be useful for the > documentation output to group related things, even if their bits may be > further apart. > > johannes > I agree checkpatch.pl should be fixed. I can look into it. No promises though, it has been a long time since I look at Perl code. I understand the logic of grouping the documentation in logical blocks. It is unfortunate though that this means the enum and documentation won't match, which makes reading the code harder. In this particular case I don't see the order of the documentation broken because flags are being grouped. Thanks for the review :) Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 17:46, Joe Perches wrote: > On Fri, 2016-03-18 at 17:40 +0000, Luis de Bethencourt wrote: >> On 18/03/16 17:29, Joe Perches wrote: >>> On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: >>>> Fix order of mac80211_rx_flags description to match the enum. > [] >>>> I want ahead and fixed the order of the descriptions. checkpatch.pl was giving >>>> a warning to my previous patch and I had a hunch it was because the wrong order >>>> breaks the parser. Indeed it does and with this patch below checkpatch.pl does >>>> not complain about this flag descriptions anymore. >>> checkpatch complains? About what? >> warning: Enum value 'RX_FLAG_DUP_VALIDATED' not described in enum 'mac80211_rx_flags' > > That's not a checkpatch warning. > > cheers, Joe > Oh no! Joe is right. That isn't a checkpatch warning, but checkpatch just printing the line in my commit message that goes over 75 characters. Which I kept that big to keep the format from make htmldocs. I still think the order of the documentation should match the enum regardless, it is nicer for developers reading the code. Sorry :( my bad for misinterpreting checkpatch's output. Thanks you Joe for correcting me, Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 17:46, Luis de Bethencourt wrote: > On 18/03/16 16:49, Johannes Berg wrote: >> On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote: >>> Fix order of mac80211_rx_flags description to match the enum. >>> >>> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> >>> --- >>> Hi, >>> >>> I want ahead and fixed the order of the descriptions. checkpatch.pl >>> was giving >>> a warning to my previous patch and I had a hunch it was because the >>> wrong order >>> breaks the parser. Indeed it does and with this patch below >>> checkpatch.pl does >>> not complain about this flag descriptions anymore. >>> >> Huh. I think we should fix checkpatch.pl instead. While the current >> order isn't likely really good, I believe kernel-doc will output the >> documentation in the order it's listed, and that can be useful for the >> documentation output to group related things, even if their bits may be >> further apart. >> >> johannes >> > > I agree checkpatch.pl should be fixed. I can look into it. No promises > though, it has been a long time since I look at Perl code. > > I understand the logic of grouping the documentation in logical blocks. > It is unfortunate though that this means the enum and documentation won't > match, which makes reading the code harder. > > In this particular case I don't see the order of the documentation broken > because flags are being grouped. > > Thanks for the review :) > Luis > Sorry Johannes, Update. checkpatch doesn't need fixing and it does see the documentation independently of the order. It was my mistake. Joe Perches pointed it out in this email branch: https://lkml.org/lkml/2016/3/18/532 Thanks for looking into this, sorry for my blunder. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 42c8cde..4b215a3 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -992,17 +992,13 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * @RX_FLAG_MMIC_ERROR: Michael MIC error was reported on this frame. * Use together with %RX_FLAG_MMIC_STRIPPED. * @RX_FLAG_DECRYPTED: This frame was decrypted in hardware. + * @RX_FLAG_MACTIME_PLCP_START: The timestamp passed in the RX status (@mactime + * field) is valid and contains the time the SYNC preamble was received. * @RX_FLAG_MMIC_STRIPPED: the Michael MIC is stripped off this frame, * verification has been done by the hardware. * @RX_FLAG_IV_STRIPPED: The IV/ICV are stripped from this frame. * If this flag is set, the stack cannot do any replay detection * hence the driver or hardware will have to do that. - * @RX_FLAG_PN_VALIDATED: Currently only valid for CCMP/GCMP frames, this - * flag indicates that the PN was verified for replay protection. - * Note that this flag is also currently only supported when a frame - * is also decrypted (ie. @RX_FLAG_DECRYPTED must be set) - * @RX_FLAG_DUP_VALIDATED: Set this flag for the driver to check for packet - * duplication by itself. * @RX_FLAG_FAILED_FCS_CRC: Set this flag if the FCS check failed on * the frame. * @RX_FLAG_FAILED_PLCP_CRC: Set this flag if the PCLP check failed on @@ -1011,14 +1007,8 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * field) is valid and contains the time the first symbol of the MPDU * was received. This is useful in monitor mode and for proper IBSS * merging. - * @RX_FLAG_MACTIME_END: The timestamp passed in the RX status (@mactime - * field) is valid and contains the time the last symbol of the MPDU - * (including FCS) was received. - * @RX_FLAG_MACTIME_PLCP_START: The timestamp passed in the RX status (@mactime - * field) is valid and contains the time the SYNC preamble was received. * @RX_FLAG_SHORTPRE: Short preamble was used for this frame * @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index - * @RX_FLAG_VHT: VHT MCS was used and rate_index is MCS index * @RX_FLAG_40MHZ: HT40 (40 MHz) was used * @RX_FLAG_SHORT_GI: Short guard interval was used * @RX_FLAG_NO_SIGNAL_VAL: The signal strength value is not present. @@ -1029,6 +1019,12 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * @RX_FLAG_AMPDU_DETAILS: A-MPDU details are known, in particular the reference * number (@ampdu_reference) must be populated and be a distinct number for * each A-MPDU + * @RX_FLAG_PN_VALIDATED: Currently only valid for CCMP/GCMP frames, this + * flag indicates that the PN was verified for replay protection. + * Note that this flag is also currently only supported when a frame + * is also decrypted (ie. @RX_FLAG_DECRYPTED must be set) + * @RX_FLAG_DUP_VALIDATED: Set this flag for the driver to check for packet + * duplication by itself. * @RX_FLAG_AMPDU_LAST_KNOWN: last subframe is known, should be set on all * subframes of a single A-MPDU * @RX_FLAG_AMPDU_IS_LAST: this subframe is the last subframe of the A-MPDU @@ -1036,8 +1032,10 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * on this subframe * @RX_FLAG_AMPDU_DELIM_CRC_KNOWN: The delimiter CRC field is known (the CRC * is stored in the @ampdu_delimiter_crc field) - * @RX_FLAG_MIC_STRIPPED: The mic was stripped of this packet. Decryption was - * done by the hardware + * @RX_FLAG_MACTIME_END: The timestamp passed in the RX status (@mactime + * field) is valid and contains the time the last symbol of the MPDU + * (including FCS) was received. + * @RX_FLAG_VHT: VHT MCS was used and rate_index is MCS index * @RX_FLAG_LDPC: LDPC was used * @RX_FLAG_ONLY_MONITOR: Report frame only to monitor interfaces without * processing it in any regular way. @@ -1062,6 +1060,8 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * @RX_FLAG_RADIOTAP_VENDOR_DATA: This frame contains vendor-specific * radiotap data in the skb->data (before the frame) as described by * the &struct ieee80211_vendor_radiotap. + * @RX_FLAG_MIC_STRIPPED: The mic was stripped of this packet. Decryption was + * done by the hardware */ enum mac80211_rx_flags { RX_FLAG_MMIC_ERROR = BIT(0),
Fix order of mac80211_rx_flags description to match the enum. Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com> --- Hi, I want ahead and fixed the order of the descriptions. checkpatch.pl was giving a warning to my previous patch and I had a hunch it was because the wrong order breaks the parser. Indeed it does and with this patch below checkpatch.pl does not complain about this flag descriptions anymore. Thanks, Luis include/net/mac80211.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)