diff mbox series

[02/13] wifi: ath12k: indicate NON MBSSID vdev by default during vdev start

Message ID 20231030222700.18914-3-quic_rajkbhag@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: ath12k: QCN9274 dualmac bring up | expand

Commit Message

Raj Kumar Bhagat Oct. 30, 2023, 10:26 p.m. UTC
From: Sriram R <quic_srirrama@quicinc.com>

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@quicinc.com>
Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 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(+)

Comments

Jeff Johnson Oct. 30, 2023, 11:12 p.m. UTC | #1
On 10/30/2023 3:26 PM, Raj Kumar Bhagat wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> 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@quicinc.com>
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>   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?

> +	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
Raj Kumar Bhagat Jan. 5, 2024, 6:35 p.m. UTC | #2
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@quicinc.com>
>>
>> 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@quicinc.com>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>   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.
diff mbox series

Patch

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),
+	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),
+};
+
 #define WMI_HOST_MAX_PDEV 3
 
 struct ath12k_wmi_host_mem_chunk_params {