Re: [PATCH 47/50] wifi: ath12k: add wmi.h

Kalle Valo
2022-11-08T15:43:40Z

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 8/12/2022 9:10 AM, Kalle Valo wrote:
>> From: Kalle Valo <quic_kvalo@quicinc.com>
>>
>> (Patches split into one patch per file for easier review, but the final
>> commit will be one big patch. See the cover letter for more info.)
>>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>> ---
>> drivers/net/wireless/ath/ath12k/wmi.h | 4796 +++++++++++++++++++++++++++++++++
>> 1 file changed, 4796 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.h
>> b/drivers/net/wireless/ath/ath12k/wmi.h
> [...]
>> +/* defines to set Packet extension values whic can be 0 us, 8 usec or 16 usec */
>
> s/whic/which/

Fixed.

>> +enum {
>
> naming the enum allows you to reference the enum in code and comments
>
>> + WMI_HOST_WLAN_2G_CAP = 0x1,
>> + WMI_HOST_WLAN_5G_CAP = 0x2,
>> + WMI_HOST_WLAN_2G_5G_CAP = 0x3,
>> +};

Renamed to enum WMI_HOST_WLAN_BAND.

>> +#define WMI_CMD_UNSUPPORTED 0
>
> why not just add this to enum wmi_tlv_cmd_id?

Added.

>> +#define WMI_PEER_MIMO_PS_STATE 0x1
>> +#define WMI_PEER_AMPDU 0x2
>> +#define WMI_PEER_AUTHORIZE 0x3
>> +#define WMI_PEER_CHWIDTH 0x4
>> +#define WMI_PEER_NSS 0x5
>> +#define WMI_PEER_USE_4ADDR 0x6
>> +#define WMI_PEER_MEMBERSHIP 0x7
>> +#define WMI_PEER_USERPOS 0x8
>> +#define WMI_PEER_CRIT_PROTO_HINT_ENABLED 0x9
>> +#define WMI_PEER_TX_FAIL_CNT_THR 0xA
>> +#define WMI_PEER_SET_HW_RETRY_CTS2S 0xB
>> +#define WMI_PEER_IBSS_ATIM_WINDOW_LENGTH 0xC
>> +#define WMI_PEER_PHYMODE 0xD
>> +#define WMI_PEER_USE_FIXED_PWR 0xE
>> +#define WMI_PEER_PARAM_FIXED_RATE 0xF
>> +#define WMI_PEER_SET_MU_WHITELIST 0x10
>> +#define WMI_PEER_SET_MAX_TX_RATE 0x11
>> +#define WMI_PEER_SET_MIN_TX_RATE 0x12
>> +#define WMI_PEER_SET_DEFAULT_ROUTING 0x13
>
> perhaps create an enum for these?

Added enum wmi_peer_param.

>> +#define WMI_VDEV_SLOT_TIME_LONG 0x1
>> +#define WMI_VDEV_SLOT_TIME_SHORT 0x2
>
> another enum?

enum wmi_slot_time

>> +#define WMI_VDEV_PREAMBLE_LONG 0x1
>> +#define WMI_VDEV_PREAMBLE_SHORT 0x2
>
> another enum?

enum wmi_preamble

>> +/* Regaulatory Rule Flags Passed by FW */
>
> s/Regaulatory/Regulatory/

Fixed.

>> +#define WMI_KEY_PAIRWISE 0x00
>> +#define WMI_KEY_GROUP 0x01
>
> create an enum?

enum wmi_key_type

>> +#define WMI_CIPHER_NONE 0x0 /* clear key */
>> +#define WMI_CIPHER_WEP 0x1
>> +#define WMI_CIPHER_TKIP 0x2
>> +#define WMI_CIPHER_AES_OCB 0x3
>> +#define WMI_CIPHER_AES_CCM 0x4
>> +#define WMI_CIPHER_WAPI 0x5
>> +#define WMI_CIPHER_CKIP 0x6
>> +#define WMI_CIPHER_AES_CMAC 0x7
>> +#define WMI_CIPHER_ANY 0x8
>> +#define WMI_CIPHER_AES_GCM 0x9
>> +#define WMI_CIPHER_AES_GMAC 0xa
>
> create an enum?

enum wmi_cipher_type

Jeff, thank you so much for all the review comments. Very much
appreciated!

--