Re: [PATCH 4/4] ath5k: add hardware CCMP encyption support

[Bob Copeland <me@bobcopeland.com>]

Subject: [PATCH] ath5k: clarify srev comparison for CCMP check

As Pavel Roskin noted, the check for mac version as copied from
legacy_hal made no sense.  This replaces it with the equivalent
and makes up a suitable #define for the mac version legacy_hal
checked.

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---

On Wed, Aug 26, 2009 at 10:09:31PM -0400, Pavel Roskin wrote:
> On Mon, 2009-08-24 at 23:00 -0400, Bob Copeland wrote:
> > +	ah->ah_aes_support =
> > +		(ee->ee_version >= AR5K_EEPROM_VERSION_5_0 &&
> > +		 !AR5K_EEPROM_AES_DIS(ee->ee_misc5) &&
> > +		 (ah->ah_mac_version > (AR5K_SREV_AR5212 >> 4) ||
> > +		  (ah->ah_mac_version == (AR5K_SREV_AR5212 >> 4) &&
> > +		   ah->ah_mac_revision >= (AR5K_SREV_AR5211 >> 4))));
> 
> The above use of ah->ah_mac_revision is clearly incorrect.  You are
> comparing a revision with a symbol for a version.
> 
> I suggest that you use ah_mac_srev instead.  Before this patch,
> ah_mac_revision was a write-only variable and was a good candidate for
> removal.
> 
> The last three quoted lines are equivalent to (ah->ah_mac_srev >= 0x54)

Ok, here's a fixup.  I'm not sure if 0x54 was really intended since we
don't know what srev that is, but this assumes legacy hal is correct.

 drivers/net/wireless/ath/ath5k/ath5k.h  |    1 +
 drivers/net/wireless/ath/ath5k/attach.c |    7 ++-----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index cdc79cd..1275ba0 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -306,6 +306,7 @@ struct ath5k_srev_name {
 #define AR5K_SREV_AR5311B	0x30 /* Spirit */
 #define AR5K_SREV_AR5211	0x40 /* Oahu */
 #define AR5K_SREV_AR5212	0x50 /* Venice */
+#define AR5K_SREV_AR5212_V4	0x54 /* ??? */
 #define AR5K_SREV_AR5213	0x55 /* ??? */
 #define AR5K_SREV_AR5213A	0x59 /* Hainan */
 #define AR5K_SREV_AR2413	0x78 /* Griffin lite */
diff --git a/drivers/net/wireless/ath/ath5k/attach.c b/drivers/net/wireless/ath/ath5k/attach.c
index 109ab7b..4819f39 100644
--- a/drivers/net/wireless/ath/ath5k/attach.c
+++ b/drivers/net/wireless/ath/ath5k/attach.c
@@ -318,12 +318,9 @@ struct ath5k_hw *ath5k_hw_attach(struct ath5k_softc *sc, u8 mac_version)
 
 	/* Crypto settings */
 	ee = &ah->ah_capabilities.cap_eeprom;
-	ah->ah_aes_support =
+	ah->ah_aes_support = srev >= AR5K_SREV_AR5212_V4 &&
 		(ee->ee_version >= AR5K_EEPROM_VERSION_5_0 &&
-		 !AR5K_EEPROM_AES_DIS(ee->ee_misc5) &&
-		 (ah->ah_mac_version > (AR5K_SREV_AR5212 >> 4) ||
-		  (ah->ah_mac_version == (AR5K_SREV_AR5212 >> 4) &&
-		   ah->ah_mac_revision >= (AR5K_SREV_AR5211 >> 4))));
+		 !AR5K_EEPROM_AES_DIS(ee->ee_misc5));
 
 	if (srev >= AR5K_SREV_AR2414) {
 		ah->ah_combined_mic = true;
-- 
1.6.2.5



-- 
Bob Copeland %% www.bobcopeland.com