Skip to contentSkip to conversation

Might the the macVersion be wrongly assigned sometimes ?

ath9k-devel Xavi Drudis Ferran 1 participant 1 message Nov 19, 2016

lore
Share & tools
lore
#0 lore raw

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.

Signature · 1 line
Xavi Drudis Ferran