On 3/20/2025 5:54 AM, Ping-Ke Shih wrote:
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 9fda97667d4e..661167acaa69 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -3450,7 +3450,9 @@ static void ath12k_recalculate_mgmt_rate(struct ath12k *ar,
>> }
>>
>> sband = hw->wiphy->bands[def->chan->band];
>> - basic_rate_idx = ffs(bss_conf->basic_rates) - 1;
>> + basic_rate_idx = __ffs(bss_conf->basic_rates);
>> + if (basic_rate_idx)
>> + basic_rate_idx -= 1;
>
> It looks like you misunderstood what I meant.
>
> The difference of ffs() and __ffs():
> ffs(0x0) = 0, ffs(0x1) = 1
> __ffs(0x0) = undefined, __ffs(0x1) = 0
>
> So you need to ensure argument isn't zero before calling __ffs(), and no
> need to minus 1 after the call.
>
Noted the difference, thanks for explaining. I'll do something like:
if (bss_conf->basic_rates)
basic_rate_idx = __ffs(bss_conf->basic_rates);
else
basic_rate_idx = 0;
>> bitrate = sband->bitrates[basic_rate_idx].bitrate;
>>
>> hw_rate_code = ath12k_mac_get_rate_hw_value(bitrate);
>> @@ -3983,10 +3985,13 @@ static void ath12k_mac_bss_info_changed(struct ath12k *ar,
>> band = def.chan->band;
>> mcast_rate = info->mcast_rate[band];
>>
>> - if (mcast_rate > 0)
>> + if (mcast_rate > 0) {
>> rateidx = mcast_rate - 1;
>> - else
>> - rateidx = ffs(info->basic_rates) - 1;
>> + } else {
>> + rateidx = __ffs(info->basic_rates);
>> + if (rateidx)
>> + rateidx -= 1;
>
> Here should be:
>
> if (info->basic_rates)
> rateidx = __ffs(info->basic_rates);
> else
> rateidx = 0;
>
Sure, will change this in next version.
>> + }
>>
>> if (ar->pdev->cap.supported_bands & WMI_HOST_WLAN_5G_CAP)
>> rateidx += ATH12K_MAC_FIRST_OFDM_RATE_IDX;
>>
>> base-commit: b6f473c96421b8b451a8df8ccb620bcd71d4b3f4
>> --
>> 2.34.1
>>
>
