Pavel Roskin wrote:
> This piece of code is almost certainly wrong (it addition to being
> longer than 80 characters:
>
> if (AR_SREV_9271(ah) && (ah->eep_ops->get_eeprom(ah, EEP_TXGAIN_TYPE) == 1)) {
> REG_WRITE_ARRAY(&ah->iniModes_high_power_tx_gain_9271,
> modesIndex, regWrites);
> } else {
> REG_WRITE_ARRAY(&ah->iniModes_normal_power_tx_gain_9271,
> modesIndex, regWrites);
> }
>
> I think the right code would be
>
> if (AR_SREV_9271(ah)) {
> if (ah->eep_ops->get_eeprom(ah, EEP_TXGAIN_TYPE) == 1))
> REG_WRITE_ARRAY(&ah->iniModes_high_power_tx_gain_9271,
> modesIndex, regWrites);
> else
> REG_WRITE_ARRAY(&ah->iniModes_normal_power_tx_gain_9271,
> modesIndex, regWrites);
> }
>
> Better yet, I would use a variable and refactor REG_WRITE_ARRAY.
Indeed. Good catch !
> This code also looks wrong:
>
> val = REG_READ(ah, AR_PCU_MISC_MODE2) &
> (~AR_PCU_MISC_MODE2_HWWAR1);
>
> if (!AR_SREV_9271(ah))
> val &= ~AR_PCU_MISC_MODE2_HWWAR1;
>
> I think it should be:
>
> val = REG_READ(ah, AR_PCU_MISC_MODE2);
>
> if (!AR_SREV_9271(ah))
> val &= ~AR_PCU_MISC_MODE2_HWWAR1;
Again, great catch. ;)
> The change involving ATH_HW_INITIALIZED appears to be a serious bugfix,
> perhaps even suitable for stable kernels.
Right, I'll do that.
> The message "Running PA Calibration" would be more suitable in
> ath9k_hw_init_cal().
>
> The original patch should be run through checkpatch.pl. In addition to
> overly long lines, there are some other formatting issues.
I thought that the default was 106 (or 132 ?) these days ..
Sujith
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
