[ath9k-devel] Might the the macVersion be wrongly assigned sometimes ?

[Xavi Drudis Ferran <xdrudis@tinet.cat>]

Thank you for ath9k. Sorry if this is silly, but I had some problems
with 9271 and looked at the code, but then I saw a suspicious line. I
don't really understand the code or whether this is a big problem or
just dead code or maybe it is ok after all, just thought I'd better
tell in case someone here thinks it needs correction. I haven't found
(yet) the register docs that may tell which bits are the mac version.
I doubt it's important, because it seems it's been like this since
2009, so it may just be me not understanding, but anyway:

drivers/net/wireless/ath/ath9k/hw.c:289
         ah->hw_version.macVersion =
                 (val & AR_SREV_VERSION2) >> AR_SREV_TYPE2_S;

shouldn't it be 

         ah->hw_version.macVersion =
                 (val & AR_SREV_VERSION2) >> AR_SREV_VERSION2_S;

or 

         ah->hw_version.macVersion = MS(val, AR_SREV_VERSION2);

?

It seems it uses the mask for the version but the shift for the type ?
That should give the macVersion a value 16 times its correct value when 
REG_READ(ah, AR_SREV) & AR_SREV_ID == 0xFF (I don't know when that is). 

The context is 

static void ath9k_hw_read_revisions(struct ath_hw *ah)
 {
[...]
 val = REG_READ(ah, AR_SREV) & AR_SREV_ID;

 if (val == 0xFF) {
         val = REG_READ(ah, AR_SREV);
         ah->hw_version.macVersion =
                 (val & AR_SREV_VERSION2) >> AR_SREV_TYPE2_S;
         ah->hw_version.macRev = MS(val, AR_SREV_REVISION2);

         if (AR_SREV_9462(ah) || AR_SREV_9565(ah))
                 ah->is_pciexpress = true;
         else
                 ah->is_pciexpress = (val &
                                      AR_SREV_TYPE2_HOST_MODE) ? 0 : 1;
 }
[...]


I can see this macVersion is used later so I guess if it needs to be
changed it might change some behaviour but I still don't understand
for which models and what should be tested exactly. It might break
something, but it still looks like a typo, doesn't it ? At least it's 
confusing.

Thank you for reading. 

-- 
Xavi Drudis Ferran