| Message ID | 20110425175156.2ADE2F88CF@sepang.rtg.net |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 25, 2011 at 11:51:56AM -0600, Tim Gardner wrote: > From 2b3494e8471c105708f0b69ae7ec2a97bafa6697 Mon Sep 17 00:00:00 2001 > From: Felix Fietkau <nbd@openwrt.org> > Date: Fri, 14 Jan 2011 00:06:27 +0100 > Subject: [PATCH] (pre stable) ath9k_hw: partially revert "fix dma descriptor rx error bit parsing" > > BugLink: http://bugs.launchpad.net/bugs/735171 > > The rx error bit parsing was changed to consider PHY errors and various > decryption errors separately. While correct according to the documentation, > this is causing spurious decryption error reports in some situations. > > Fix this by restoring the original order of the checks in those places, > where the errors are meant to be mutually exclusive. > > If a CRC error is reported, then MIC failure and decryption errors > are irrelevant, and a PHY error is unlikely. > > Signed-off-by: Felix Fietkau <nbd@openwrt.org> > Signed-off-by: John W. Linville <linville@tuxdriver.com> > (cherry picked from commit 115dad7a7f42e68840392767323ceb9306dbdb36) > > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Acked-by: Seth Forshee <seth.forshee@canonical.com> > --- > drivers/net/wireless/ath/ath9k/ar9003_mac.c | 8 ++++---- > drivers/net/wireless/ath/ath9k/mac.c | 14 ++++++++++---- > 2 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c > index 4ceddbb..038a0cb 100644 > --- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c > +++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c > @@ -615,7 +615,7 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, > */ > if (rxsp->status11 & AR_CRCErr) > rxs->rs_status |= ATH9K_RXERR_CRC; > - if (rxsp->status11 & AR_PHYErr) { > + else if (rxsp->status11 & AR_PHYErr) { > phyerr = MS(rxsp->status11, AR_PHYErrCode); > /* > * If we reach a point here where AR_PostDelimCRCErr is > @@ -638,11 +638,11 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, > rxs->rs_phyerr = phyerr; > } > > - } > - if (rxsp->status11 & AR_DecryptCRCErr) > + } else if (rxsp->status11 & AR_DecryptCRCErr) > rxs->rs_status |= ATH9K_RXERR_DECRYPT; > - if (rxsp->status11 & AR_MichaelErr) > + else if (rxsp->status11 & AR_MichaelErr) > rxs->rs_status |= ATH9K_RXERR_MIC; > + > if (rxsp->status11 & AR_KeyMiss) > rxs->rs_status |= ATH9K_RXERR_DECRYPT; > } > diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c > index 2915b11..5efc869 100644 > --- a/drivers/net/wireless/ath/ath9k/mac.c > +++ b/drivers/net/wireless/ath/ath9k/mac.c > @@ -690,17 +690,23 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds, > rs->rs_flags |= ATH9K_RX_DECRYPT_BUSY; > > if ((ads.ds_rxstatus8 & AR_RxFrameOK) == 0) { > + /* > + * Treat these errors as mutually exclusive to avoid spurious > + * extra error reports from the hardware. If a CRC error is > + * reported, then decryption and MIC errors are irrelevant, > + * the frame is going to be dropped either way > + */ > if (ads.ds_rxstatus8 & AR_CRCErr) > rs->rs_status |= ATH9K_RXERR_CRC; > - if (ads.ds_rxstatus8 & AR_PHYErr) { > + else if (ads.ds_rxstatus8 & AR_PHYErr) { > rs->rs_status |= ATH9K_RXERR_PHY; > phyerr = MS(ads.ds_rxstatus8, AR_PHYErrCode); > rs->rs_phyerr = phyerr; > - } > - if (ads.ds_rxstatus8 & AR_DecryptCRCErr) > + } else if (ads.ds_rxstatus8 & AR_DecryptCRCErr) > rs->rs_status |= ATH9K_RXERR_DECRYPT; > - if (ads.ds_rxstatus8 & AR_MichaelErr) > + else if (ads.ds_rxstatus8 & AR_MichaelErr) > rs->rs_status |= ATH9K_RXERR_MIC; > + > if (ads.ds_rxstatus8 & AR_KeyMiss) > rs->rs_status |= ATH9K_RXERR_DECRYPT; > } > -- > 1.7.0.4 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team >
On 04/25/2011 10:51 AM, Tim Gardner wrote: > From 2b3494e8471c105708f0b69ae7ec2a97bafa6697 Mon Sep 17 00:00:00 2001 > From: Felix Fietkau <nbd@openwrt.org> > Date: Fri, 14 Jan 2011 00:06:27 +0100 > Subject: [PATCH] (pre stable) ath9k_hw: partially revert "fix dma descriptor rx error bit parsing" > > BugLink: http://bugs.launchpad.net/bugs/735171 > > The rx error bit parsing was changed to consider PHY errors and various > decryption errors separately. While correct according to the documentation, > this is causing spurious decryption error reports in some situations. > > Fix this by restoring the original order of the checks in those places, > where the errors are meant to be mutually exclusive. > > If a CRC error is reported, then MIC failure and decryption errors > are irrelevant, and a PHY error is unlikely. > > Signed-off-by: Felix Fietkau <nbd@openwrt.org> > Signed-off-by: John W. Linville <linville@tuxdriver.com> > (cherry picked from commit 115dad7a7f42e68840392767323ceb9306dbdb36) > > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> well it doesn't seem to hurt my ath9k and it looks good so Acked-by: John Johansen <john.johansen@canonical.com> > --- > drivers/net/wireless/ath/ath9k/ar9003_mac.c | 8 ++++---- > drivers/net/wireless/ath/ath9k/mac.c | 14 ++++++++++---- > 2 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c > index 4ceddbb..038a0cb 100644 > --- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c > +++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c > @@ -615,7 +615,7 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, > */ > if (rxsp->status11 & AR_CRCErr) > rxs->rs_status |= ATH9K_RXERR_CRC; > - if (rxsp->status11 & AR_PHYErr) { > + else if (rxsp->status11 & AR_PHYErr) { > phyerr = MS(rxsp->status11, AR_PHYErrCode); > /* > * If we reach a point here where AR_PostDelimCRCErr is > @@ -638,11 +638,11 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, > rxs->rs_phyerr = phyerr; > } > > - } > - if (rxsp->status11 & AR_DecryptCRCErr) > + } else if (rxsp->status11 & AR_DecryptCRCErr) > rxs->rs_status |= ATH9K_RXERR_DECRYPT; > - if (rxsp->status11 & AR_MichaelErr) > + else if (rxsp->status11 & AR_MichaelErr) > rxs->rs_status |= ATH9K_RXERR_MIC; > + > if (rxsp->status11 & AR_KeyMiss) > rxs->rs_status |= ATH9K_RXERR_DECRYPT; > } > diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c > index 2915b11..5efc869 100644 > --- a/drivers/net/wireless/ath/ath9k/mac.c > +++ b/drivers/net/wireless/ath/ath9k/mac.c > @@ -690,17 +690,23 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds, > rs->rs_flags |= ATH9K_RX_DECRYPT_BUSY; > > if ((ads.ds_rxstatus8 & AR_RxFrameOK) == 0) { > + /* > + * Treat these errors as mutually exclusive to avoid spurious > + * extra error reports from the hardware. If a CRC error is > + * reported, then decryption and MIC errors are irrelevant, > + * the frame is going to be dropped either way > + */ > if (ads.ds_rxstatus8 & AR_CRCErr) > rs->rs_status |= ATH9K_RXERR_CRC; > - if (ads.ds_rxstatus8 & AR_PHYErr) { > + else if (ads.ds_rxstatus8 & AR_PHYErr) { > rs->rs_status |= ATH9K_RXERR_PHY; > phyerr = MS(ads.ds_rxstatus8, AR_PHYErrCode); > rs->rs_phyerr = phyerr; > - } > - if (ads.ds_rxstatus8 & AR_DecryptCRCErr) > + } else if (ads.ds_rxstatus8 & AR_DecryptCRCErr) > rs->rs_status |= ATH9K_RXERR_DECRYPT; > - if (ads.ds_rxstatus8 & AR_MichaelErr) > + else if (ads.ds_rxstatus8 & AR_MichaelErr) > rs->rs_status |= ATH9K_RXERR_MIC; > + > if (ads.ds_rxstatus8 & AR_KeyMiss) > rs->rs_status |= ATH9K_RXERR_DECRYPT; > }
On 04/25/2011 11:51 AM, Tim Gardner wrote:
> BugLink:http://bugs.launchpad.net/bugs/735171
applied
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c index 4ceddbb..038a0cb 100644 --- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c +++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c @@ -615,7 +615,7 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, */ if (rxsp->status11 & AR_CRCErr) rxs->rs_status |= ATH9K_RXERR_CRC; - if (rxsp->status11 & AR_PHYErr) { + else if (rxsp->status11 & AR_PHYErr) { phyerr = MS(rxsp->status11, AR_PHYErrCode); /* * If we reach a point here where AR_PostDelimCRCErr is @@ -638,11 +638,11 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs, rxs->rs_phyerr = phyerr; } - } - if (rxsp->status11 & AR_DecryptCRCErr) + } else if (rxsp->status11 & AR_DecryptCRCErr) rxs->rs_status |= ATH9K_RXERR_DECRYPT; - if (rxsp->status11 & AR_MichaelErr) + else if (rxsp->status11 & AR_MichaelErr) rxs->rs_status |= ATH9K_RXERR_MIC; + if (rxsp->status11 & AR_KeyMiss) rxs->rs_status |= ATH9K_RXERR_DECRYPT; } diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c index 2915b11..5efc869 100644 --- a/drivers/net/wireless/ath/ath9k/mac.c +++ b/drivers/net/wireless/ath/ath9k/mac.c @@ -690,17 +690,23 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds, rs->rs_flags |= ATH9K_RX_DECRYPT_BUSY; if ((ads.ds_rxstatus8 & AR_RxFrameOK) == 0) { + /* + * Treat these errors as mutually exclusive to avoid spurious + * extra error reports from the hardware. If a CRC error is + * reported, then decryption and MIC errors are irrelevant, + * the frame is going to be dropped either way + */ if (ads.ds_rxstatus8 & AR_CRCErr) rs->rs_status |= ATH9K_RXERR_CRC; - if (ads.ds_rxstatus8 & AR_PHYErr) { + else if (ads.ds_rxstatus8 & AR_PHYErr) { rs->rs_status |= ATH9K_RXERR_PHY; phyerr = MS(ads.ds_rxstatus8, AR_PHYErrCode); rs->rs_phyerr = phyerr; - } - if (ads.ds_rxstatus8 & AR_DecryptCRCErr) + } else if (ads.ds_rxstatus8 & AR_DecryptCRCErr) rs->rs_status |= ATH9K_RXERR_DECRYPT; - if (ads.ds_rxstatus8 & AR_MichaelErr) + else if (ads.ds_rxstatus8 & AR_MichaelErr) rs->rs_status |= ATH9K_RXERR_MIC; + if (ads.ds_rxstatus8 & AR_KeyMiss) rs->rs_status |= ATH9K_RXERR_DECRYPT; }