Message ID | 87ppclq4mu.fsf@kamboji.qca.qualcomm.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> This creates a single, common path for MSDU, >> A-MSDU and fragmented Rx. >> >> Hopefully this will make it easier to understand >> Rx path and make it easier to work with. >> >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> > > This patch had few checkpatch warnings. I fixed them with the folded > patch and full patch here: > > https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63 Thanks! [...] > @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, > bool has_fcs_err; > bool has_crypto_err; > bool has_tkip_err; > - bool has_peer_idx_invalid; > + bool has_idx_invalid; > bool is_decrypted; I don't really like the has_idx_invalid. Perhaps has_peer_err conveys a bit more of the original meaning? As much as I'd like to leave the original var name I'd like to be checkpatch warning free. Sigh.. Micha? -- 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
Michal Kazior <michal.kazior@tieto.com> writes: > On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Michal Kazior <michal.kazior@tieto.com> writes: >> >>> This creates a single, common path for MSDU, >>> A-MSDU and fragmented Rx. >>> >>> Hopefully this will make it easier to understand >>> Rx path and make it easier to work with. >>> >>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >> >> This patch had few checkpatch warnings. I fixed them with the folded >> patch and full patch here: >> >> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63 > > Thanks! > > [...] >> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, >> bool has_fcs_err; >> bool has_crypto_err; >> bool has_tkip_err; >> - bool has_peer_idx_invalid; >> + bool has_idx_invalid; >> bool is_decrypted; > > I don't really like the has_idx_invalid. Perhaps has_peer_err conveys > a bit more of the original meaning? What about just peer_idx_invalid? IMHO we really don't need the has_ prefix in that relatively small function. > As much as I'd like to leave the original var name I'd like to be > checkpatch warning free. Sigh.. Same here. The checkpatch is just so useful tool to keep the style unified.
On 17 November 2014 16:11, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: >> On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> wrote: [...] >>> This patch had few checkpatch warnings. I fixed them with the folded >>> patch and full patch here: >>> >>> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63 [...] >>> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, >>> bool has_fcs_err; >>> bool has_crypto_err; >>> bool has_tkip_err; >>> - bool has_peer_idx_invalid; >>> + bool has_idx_invalid; >>> bool is_decrypted; >> >> I don't really like the has_idx_invalid. Perhaps has_peer_err conveys >> a bit more of the original meaning? > > What about just peer_idx_invalid? IMHO we really don't need the has_ > prefix in that relatively small function. Good point but the assignment line with `peer_idx_invalid` will still be over 80 chars long, no? So perhaps instead of doing `rxd->attention.flags & __cpu_to_le32(FLAG)` it could be `attention = __le32_to_cpu(rxd->attention.flags)` and then `attention & FLAG` ? This way it shouldn't exceed the 80 char limit and var names won't need to be changed. Hopefully compiler will optimize it away. Micha? -- 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/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index 6abfea768173..08963439891b 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -1066,7 +1066,8 @@ static void ath10k_htt_rx_h_undecap(struct ath10k *ar, switch (decap) { case RX_MSDU_DECAP_RAW: - ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype, is_decrypted); + ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype, + is_decrypted); break; case RX_MSDU_DECAP_NATIVE_WIFI: ath10k_htt_rx_h_undecap_nwifi(ar, msdu, status, first_hdr); @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, bool has_fcs_err; bool has_crypto_err; bool has_tkip_err; - bool has_peer_idx_invalid; + bool has_idx_invalid; bool is_decrypted; if (skb_queue_empty(amsdu)) @@ -1167,8 +1168,8 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, __cpu_to_le32(RX_ATTENTION_FLAGS_DECRYPT_ERR)); has_tkip_err = !!(rxd->attention.flags & __cpu_to_le32(RX_ATTENTION_FLAGS_TKIP_MIC_ERR)); - has_peer_idx_invalid = !!(rxd->attention.flags & - __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID)); + has_idx_invalid = !!(rxd->attention.flags & + __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID)); /* Note: If hardware captures an encrypted frame that it can't decrypt, * e.g. due to fcs error, missing peer or invalid key data it will @@ -1177,7 +1178,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar, is_decrypted = (enctype != HTT_RX_MPDU_ENCRYPT_NONE && !has_fcs_err && !has_crypto_err && - !has_peer_idx_invalid); + !has_idx_invalid); /* Clear per-MPDU flags while leaving per-PPDU flags intact. */ status->flag &= ~(RX_FLAG_FAILED_FCS_CRC |