On 10/31/2023 4:42 AM, Jeff Johnson wrote:
> On 10/30/2023 3:26 PM, Raj Kumar Bhagat wrote:
>> From: Sriram R <quic_srirrama@xxxxxxxxxxx>
>>
>> When any VDEV is started, MBSSID flags are passed to firmware to
>> indicate if its a MBSSID/EMA AP vdev. If the interface is not an AP
>> or if the AP doesn't support MBSSID, the vdev needs to be brought up
>> as a non MBSSID vdev. Set these flags as a non MBSSID AP by default
>> which can be updated as and when MBSSID support is added in ath12k.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Sriram R <quic_srirrama@xxxxxxxxxxx>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@xxxxxxxxxxx>
>> ---
>> drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
>> drivers/net/wireless/ath/ath12k/wmi.c | 1 +
>> drivers/net/wireless/ath/ath12k/wmi.h | 8 ++++++++
>> 3 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index fc0d14ea3..594aa18e7 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -5987,6 +5987,11 @@ ath12k_mac_vdev_start_restart(struct ath12k_vif *arvif,
>> arg.pref_tx_streams = ar->num_tx_chains;
>> arg.pref_rx_streams = ar->num_rx_chains;
>> + /* Fill the MBSSID flags to indicate AP is non MBSSID by default
>> + * Corresponding flags would be updated with MBSSID support.
>> + */
>> + arg.mbssid_flags = WMI_VDEV_FLAGS_NON_MBSSID_AP;
>> +
>> if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
>> arg.ssid = arvif->u.ap.ssid;
>> arg.ssid_len = arvif->u.ap.ssid_len;
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>> index 0e5bf5ce8..88ec77dee 100644
>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>> @@ -1024,6 +1024,7 @@ int ath12k_wmi_vdev_start(struct ath12k *ar, struct wmi_vdev_start_req_arg *arg,
>> cmd->regdomain = cpu_to_le32(arg->regdomain);
>> cmd->he_ops = cpu_to_le32(arg->he_ops);
>> cmd->punct_bitmap = cpu_to_le32(arg->punct_bitmap);
>> + cmd->mbssid_flags = cpu_to_le32(arg->mbssid_flags);
>> if (!restart) {
>> if (arg->ssid) {
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.h b/drivers/net/wireless/ath/ath12k/wmi.h
>> index 8e1eda7aa..dfe9eb0cb 100644
>> --- a/drivers/net/wireless/ath/ath12k/wmi.h
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.h
>> @@ -2269,6 +2269,14 @@ struct ath12k_wmi_hal_reg_capabilities_ext_arg {
>> u32 high_5ghz_chan;
>> };
>> +enum {
>> + WMI_VDEV_FLAGS_NON_MBSSID_AP = BIT(0),
>
> only the 1st one is used. do we need the rest at this point?
> perhaps add the others as they are needed?
Sure, will add only one - WMI_VDEV_FLAGS_NON_MBSSID_AP.
>
>> + WMI_VDEV_FLAGS_TRANSMIT_AP = BIT(1),
>> + WMI_VDEV_FLAGS_NON_TRANSMIT_AP = BIT(2),
>> + WMI_VDEV_FLAGS_EMA_MODE = BIT(3),
>> + WMI_VDEV_FLAGS_SCAN_MODE_VAP = BIT(4),
>
> note that "vap" is a non-standard term that ideally should not be used in ath12k (although a few references are present)
>
>> +};
>
> these seem to be added at some random place in the file. considering that these are applicable to the mbssid_flags in struct wmi_vdev_start_request_cmd, I'd suggest:
>
> 1) defining the enum just before this struct
> 2) naming the enum, i.e. wmi_vdev_mbssid_flags
> 3) using a prefix that matches the name, i.e. WMI_VDEV_MBSSID_FLAGS_NON_MBSSID_AP
> 4) in struct wmi_vdev_start_request_cmd add a comment to:
> __le32 mbssid_flags; /* uses enum wmi_vdev_mbssid_flags */
>
> /jeff
Will implement the above suggestions in next version.
