[PATCH] wifi: ath12k: Fix pdev id sent to firmware for single phy devices
Kalle Valo
kvalo at kernel.org
Wed Jun 19 10:35:25 PDT 2024
Ramya Gnanasekar <quic_rgnanase at quicinc.com> writes:
> From: Lingbo Kong <quic_lingbok at quicinc.com>
>
> Pdev id from mac phy capabilities will be sent as a part of
> HTT/WMI command to firmware. This causes issue with single pdev
> devices where firmware does not respond to the WMI/HTT request
> sent from host.
>
> For single pdev devices firmware expects pdev id as 1 for 5 GHz/6 GHz
> phy and 2 for 2 GHz band. Add wrapper ath12k_mac_get_target_pdev_id()
> to help fetch right pdev for single pdev devices.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Signed-off-by: Lingbo Kong <quic_lingbok at quicinc.com>
> Signed-off-by: Ramya Gnanasekar <quic_rgnanase at quicinc.com>
[...]
> +static bool ath12k_mac_band_match(enum nl80211_band band1, enum WMI_HOST_WLAN_BAND band2)
> +{
> + return (((band1 == NL80211_BAND_2GHZ) && (band2 & WMI_HOST_WLAN_2G_CAP)) ||
> + (((band1 == NL80211_BAND_5GHZ) || (band1 == NL80211_BAND_6GHZ)) &&
> + (band2 & WMI_HOST_WLAN_5G_CAP)));
> +}
This is not really pleasent to read. What about something like this:
switch (band1) {
case NL80211_BAND_2GHZ:
if (band2 & WMI_HOST_WLAN_2G_CAP)
return true;
break;
case NL80211_BAND_5GHZ:
case NL80211_BAND_6GHZ:
if (band2 & WMI_HOST_WLAN_5G_CAP)
return true;
break;
}
return false;
Or any other ideas?
> +u8 ath12k_mac_get_target_pdev_id(struct ath12k *ar)
> +{
> + struct ath12k_vif *arvif;
> + struct ath12k_base *ab = ar->ab;
> +
> + if (!ab->hw_params->single_pdev_only)
> + return ar->pdev->pdev_id;
> +
> + arvif = ath12k_mac_get_vif_up(ar);
> +
> + if (arvif)
> + return ath12k_mac_get_target_pdev_id_from_vif(arvif);
> + else
> + return ar->ab->fw_pdev[0].pdev_id;
> +}
I find this easier to read:
arvif = ath12k_mac_get_vif_up(ar);
if (!arvif)
return ar->ab->fw_pdev[0].pdev_id;
return ath12k_mac_get_target_pdev_id_from_vif(arvif);
But I still would prefer to have some a code comment explaining the idea
behind here, especially why it's safe to use fw_pdev[0].pdev_id directly
and what scenario that is.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
More information about the ath12k
mailing list