Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> writes: > On 8/12/2022 9:10 AM, Kalle Valo wrote: >> From: Kalle Valo <quic_kvalo@xxxxxxxxxxx> >> >> (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@xxxxxxxxxxx> >> --- >> 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! -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches