netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits
@ 2022-07-19 14:32 Bryan O'Donoghue
  2022-07-19 14:32 ` [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum Bryan O'Donoghue
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-19 14:32 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev,
	bryan.odonoghue

V2:
Drops "FW Cap = " prefix from each feature item - Loic

cat /sys/kernel/debug/ieee80211/phy0/wcn36xx/firmware_feat_caps 
MCC
P2P
DOT11AC
SLM_SESSIONIZATION
DOT11AC_OPMODE
SAP32STA
TDLS
P2P_GO_NOA_DECOUPLE_INIT_SCAN
WLANACTIVE_OFFLOAD
BEACON_OFFLOAD
SCAN_OFFLOAD
BCN_MISS_OFFLOAD
STA_POWERSAVE
STA_ADVANCED_PWRSAVE
BCN_FILTER
RTT
RATECTRL
WOW
WLAN_ROAM_SCAN_OFFLOAD
SPECULATIVE_PS_POLL
IBSS_HEARTBEAT_OFFLOAD
WLAN_SCAN_OFFLOAD
WLAN_PERIODIC_TX_PTRN
ADVANCE_TDLS
BATCH_SCAN
FW_IN_TX_PATH
EXTENDED_NSOFFLOAD_SLOT
CH_SWITCH_V1
HT40_OBSS_SCAN
UPDATE_CHANNEL_LIST
WLAN_MCADDR_FLT
WLAN_CH144
TDLS_SCAN_COEXISTENCE
LINK_LAYER_STATS_MEAS
MU_MIMO
EXTENDED_SCAN
DYNAMIC_WMM_PS
MAC_SPOOFED_SCAN
FW_STATS
WPS_PRBRSP_TMPL
BCN_IE_FLT_DELTA

V1:
This series tidies up the code to get/set/clear discovered firmware feature
bits and adds a new debugfs entry to read the feature bits as strings.

cat /sys/kernel/debug/ieee80211/phy0/wcn36xx/firmware_feat_caps

wcn3680b:
FW Cap = MCC
FW Cap = P2P
FW Cap = DOT11AC
FW Cap = SLM_SESSIONIZATION
FW Cap = DOT11AC_OPMODE
FW Cap = SAP32STA
FW Cap = TDLS
FW Cap = P2P_GO_NOA_DECOUPLE_INIT_SCAN
FW Cap = WLANACTIVE_OFFLOAD
FW Cap = BEACON_OFFLOAD
FW Cap = SCAN_OFFLOAD
FW Cap = BCN_MISS_OFFLOAD
FW Cap = STA_POWERSAVE
FW Cap = STA_ADVANCED_PWRSAVE
FW Cap = BCN_FILTER
FW Cap = RTT
FW Cap = RATECTRL
FW Cap = WOW
FW Cap = WLAN_ROAM_SCAN_OFFLOAD
FW Cap = SPECULATIVE_PS_POLL
FW Cap = IBSS_HEARTBEAT_OFFLOAD
FW Cap = WLAN_SCAN_OFFLOAD
FW Cap = WLAN_PERIODIC_TX_PTRN
FW Cap = ADVANCE_TDLS
FW Cap = BATCH_SCAN
FW Cap = FW_IN_TX_PATH
FW Cap = EXTENDED_NSOFFLOAD_SLOT
FW Cap = CH_SWITCH_V1
FW Cap = HT40_OBSS_SCAN
FW Cap = UPDATE_CHANNEL_LIST
FW Cap = WLAN_MCADDR_FLT
FW Cap = WLAN_CH144
FW Cap = TDLS_SCAN_COEXISTENCE
FW Cap = LINK_LAYER_STATS_MEAS
FW Cap = MU_MIMO
FW Cap = EXTENDED_SCAN
FW Cap = DYNAMIC_WMM_PS
FW Cap = MAC_SPOOFED_SCAN
FW Cap = FW_STATS
FW Cap = WPS_PRBRSP_TMPL
FW Cap = BCN_IE_FLT_DELTA

wcn3620:
FW Cap = MCC
FW Cap = P2P
FW Cap = SLM_SESSIONIZATION
FW Cap = DOT11AC_OPMODE
FW Cap = SAP32STA
FW Cap = TDLS
FW Cap = P2P_GO_NOA_DECOUPLE_INIT_SCAN
FW Cap = WLANACTIVE_OFFLOAD
FW Cap = BEACON_OFFLOAD
FW Cap = SCAN_OFFLOAD
FW Cap = BCN_MISS_OFFLOAD
FW Cap = STA_POWERSAVE
FW Cap = STA_ADVANCED_PWRSAVE
FW Cap = BCN_FILTER
FW Cap = RTT
FW Cap = RATECTRL
FW Cap = WOW
FW Cap = WLAN_ROAM_SCAN_OFFLOAD
FW Cap = SPECULATIVE_PS_POLL
FW Cap = IBSS_HEARTBEAT_OFFLOAD
FW Cap = WLAN_SCAN_OFFLOAD
FW Cap = WLAN_PERIODIC_TX_PTRN
FW Cap = ADVANCE_TDLS
FW Cap = BATCH_SCAN
FW Cap = FW_IN_TX_PATH
FW Cap = EXTENDED_NSOFFLOAD_SLOT
FW Cap = CH_SWITCH_V1
FW Cap = HT40_OBSS_SCAN
FW Cap = UPDATE_CHANNEL_LIST
FW Cap = WLAN_MCADDR_FLT
FW Cap = WLAN_CH144
FW Cap = TDLS_SCAN_COEXISTENCE
FW Cap = LINK_LAYER_STATS_MEAS
FW Cap = EXTENDED_SCAN
FW Cap = DYNAMIC_WMM_PS
FW Cap = MAC_SPOOFED_SCAN
FW Cap = FW_STATS
FW Cap = WPS_PRBRSP_TMPL
FW Cap = BCN_IE_FLT_DELTA

This is a handy way of debugging WiFi on different platforms without
necessarily having to recompile to see the debug printout on firmware boot.

Bryan O'Donoghue (4):
  wcn36xx: Rename clunky firmware feature bit enum
  wcn36xx: Move firmware feature bit storage to dedicated firmware.c
    file
  wcn36xx: Move capability bitmap to string translation function to
    firmware.c
  wcn36xx: Add debugfs entry to read firmware feature strings

 drivers/net/wireless/ath/wcn36xx/Makefile   |   3 +-
 drivers/net/wireless/ath/wcn36xx/debug.c    |  37 ++++++
 drivers/net/wireless/ath/wcn36xx/debug.h    |   1 +
 drivers/net/wireless/ath/wcn36xx/firmware.c | 125 ++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/firmware.h |  84 +++++++++++++
 drivers/net/wireless/ath/wcn36xx/hal.h      |  68 -----------
 drivers/net/wireless/ath/wcn36xx/main.c     |  86 ++------------
 drivers/net/wireless/ath/wcn36xx/smd.c      |  57 ++-------
 drivers/net/wireless/ath/wcn36xx/smd.h      |   3 -
 9 files changed, 264 insertions(+), 200 deletions(-)
 create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.c
 create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.h

-- 
2.36.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum
  2022-07-19 14:32 [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits Bryan O'Donoghue
@ 2022-07-19 14:32 ` Bryan O'Donoghue
  2022-07-20  7:14   ` Loic Poulain
  2022-07-19 14:33 ` [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file Bryan O'Donoghue
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-19 14:32 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev,
	bryan.odonoghue

The enum name "place_holder_in_cap_bitmap" is self descriptively asking to
be changed to something else.

Rename place_holder_in_cap_bitmap to wcn36xx_firmware_feat_caps so that the
contents and intent of the enum is obvious.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  | 2 +-
 drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
 drivers/net/wireless/ath/wcn36xx/smd.c  | 6 +++---
 drivers/net/wireless/ath/wcn36xx/smd.h  | 6 +++---
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 46a49f0a51b3..5e48c97682c2 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -4760,7 +4760,7 @@ struct wcn36xx_hal_set_power_params_resp {
 
 /* Capability bitmap exchange definitions and macros starts */
 
-enum place_holder_in_cap_bitmap {
+enum wcn36xx_firmware_feat_caps {
 	MCC = 0,
 	P2P = 1,
 	DOT11AC = 2,
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index e34d3d0b7082..efd776b20e60 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -260,7 +260,7 @@ static const char * const wcn36xx_caps_names[] = {
 
 #undef DEFINE
 
-static const char *wcn36xx_get_cap_name(enum place_holder_in_cap_bitmap x)
+static const char *wcn36xx_get_cap_name(enum wcn36xx_firmware_feat_caps x)
 {
 	if (x >= ARRAY_SIZE(wcn36xx_caps_names))
 		return "UNKNOWN";
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 7ac9a1e6f768..88ee92be8562 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2431,7 +2431,7 @@ int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
 	return ret;
 }
 
-void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
+void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
 {
 	int arr_idx, bit_idx;
 
@@ -2445,7 +2445,7 @@ void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
 	bitmap[arr_idx] |= (1 << bit_idx);
 }
 
-int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
+int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
 {
 	int arr_idx, bit_idx;
 
@@ -2460,7 +2460,7 @@ int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
 	return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
 }
 
-void clear_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
+void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
 {
 	int arr_idx, bit_idx;
 
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 3fd598ac2a27..186dad4fe80e 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -125,9 +125,9 @@ int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn,
 int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
 			     u32 arg3, u32 arg4, u32 arg5);
 int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn);
-void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
-int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
-void clear_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
+void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
+int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
+void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
 
 int wcn36xx_smd_add_ba_session(struct wcn36xx *wcn,
 		struct ieee80211_sta *sta,
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file
  2022-07-19 14:32 [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits Bryan O'Donoghue
  2022-07-19 14:32 ` [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum Bryan O'Donoghue
@ 2022-07-19 14:33 ` Bryan O'Donoghue
  2022-07-20  7:16   ` Loic Poulain
  2022-07-19 14:33 ` [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c Bryan O'Donoghue
  2022-07-19 14:33 ` [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings Bryan O'Donoghue
  3 siblings, 1 reply; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-19 14:33 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev,
	bryan.odonoghue

The naming of the get/set/clear firmware feature capability bits doesn't
really follow the established namespace pattern of
wcn36xx_logicalblock_do_something();

The feature bits are accessed by smd.c and main.c. It would be nice to
display the found feature bits in debugfs. To do so though we should tidy
up the namespace a bit.

Move the firmware feature exchange API to its own file - firmware.c giving
us the opportunity to functionally decompose other firmware related
accessors as appropriate in future.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/Makefile   |  3 +-
 drivers/net/wireless/ath/wcn36xx/firmware.c | 50 +++++++++++++
 drivers/net/wireless/ath/wcn36xx/firmware.h | 82 +++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/hal.h      | 68 -----------------
 drivers/net/wireless/ath/wcn36xx/main.c     |  7 +-
 drivers/net/wireless/ath/wcn36xx/smd.c      | 57 ++------------
 drivers/net/wireless/ath/wcn36xx/smd.h      |  3 -
 7 files changed, 146 insertions(+), 124 deletions(-)
 create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.c
 create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.h

diff --git a/drivers/net/wireless/ath/wcn36xx/Makefile b/drivers/net/wireless/ath/wcn36xx/Makefile
index 27413703ad69..26bec795b372 100644
--- a/drivers/net/wireless/ath/wcn36xx/Makefile
+++ b/drivers/net/wireless/ath/wcn36xx/Makefile
@@ -5,6 +5,7 @@ wcn36xx-y +=   main.o \
                txrx.o \
                smd.o \
                pmc.o \
-               debug.o
+               debug.o \
+               firmware.o
 
 wcn36xx-$(CONFIG_NL80211_TESTMODE) += testmode.o
diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.c b/drivers/net/wireless/ath/wcn36xx/firmware.c
new file mode 100644
index 000000000000..03b93d2bdcf9
--- /dev/null
+++ b/drivers/net/wireless/ath/wcn36xx/firmware.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "wcn36xx.h"
+#include "firmware.h"
+
+void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
+				    enum wcn36xx_firmware_feat_caps cap)
+{
+	int arr_idx, bit_idx;
+
+	if (cap < 0 || cap > 127) {
+		wcn36xx_warn("error cap idx %d\n", cap);
+		return;
+	}
+
+	arr_idx = cap / 32;
+	bit_idx = cap % 32;
+	bitmap[arr_idx] |= (1 << bit_idx);
+}
+
+int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
+				   enum wcn36xx_firmware_feat_caps cap)
+{
+	int arr_idx, bit_idx;
+
+	if (cap < 0 || cap > 127) {
+		wcn36xx_warn("error cap idx %d\n", cap);
+		return -EINVAL;
+	}
+
+	arr_idx = cap / 32;
+	bit_idx = cap % 32;
+
+	return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
+}
+
+void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
+				      enum wcn36xx_firmware_feat_caps cap)
+{
+	int arr_idx, bit_idx;
+
+	if (cap < 0 || cap > 127) {
+		wcn36xx_warn("error cap idx %d\n", cap);
+		return;
+	}
+
+	arr_idx = cap / 32;
+	bit_idx = cap % 32;
+	bitmap[arr_idx] &= ~(1 << bit_idx);
+}
diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.h b/drivers/net/wireless/ath/wcn36xx/firmware.h
new file mode 100644
index 000000000000..552c0e9325e1
--- /dev/null
+++ b/drivers/net/wireless/ath/wcn36xx/firmware.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _FIRMWARE_H_
+#define _FIRMWARE_H_
+
+/* Capability bitmap exchange definitions and macros starts */
+
+enum wcn36xx_firmware_feat_caps {
+	MCC = 0,
+	P2P = 1,
+	DOT11AC = 2,
+	SLM_SESSIONIZATION = 3,
+	DOT11AC_OPMODE = 4,
+	SAP32STA = 5,
+	TDLS = 6,
+	P2P_GO_NOA_DECOUPLE_INIT_SCAN = 7,
+	WLANACTIVE_OFFLOAD = 8,
+	BEACON_OFFLOAD = 9,
+	SCAN_OFFLOAD = 10,
+	ROAM_OFFLOAD = 11,
+	BCN_MISS_OFFLOAD = 12,
+	STA_POWERSAVE = 13,
+	STA_ADVANCED_PWRSAVE = 14,
+	AP_UAPSD = 15,
+	AP_DFS = 16,
+	BLOCKACK = 17,
+	PHY_ERR = 18,
+	BCN_FILTER = 19,
+	RTT = 20,
+	RATECTRL = 21,
+	WOW = 22,
+	WLAN_ROAM_SCAN_OFFLOAD = 23,
+	SPECULATIVE_PS_POLL = 24,
+	SCAN_SCH = 25,
+	IBSS_HEARTBEAT_OFFLOAD = 26,
+	WLAN_SCAN_OFFLOAD = 27,
+	WLAN_PERIODIC_TX_PTRN = 28,
+	ADVANCE_TDLS = 29,
+	BATCH_SCAN = 30,
+	FW_IN_TX_PATH = 31,
+	EXTENDED_NSOFFLOAD_SLOT = 32,
+	CH_SWITCH_V1 = 33,
+	HT40_OBSS_SCAN = 34,
+	UPDATE_CHANNEL_LIST = 35,
+	WLAN_MCADDR_FLT = 36,
+	WLAN_CH144 = 37,
+	NAN = 38,
+	TDLS_SCAN_COEXISTENCE = 39,
+	LINK_LAYER_STATS_MEAS = 40,
+	MU_MIMO = 41,
+	EXTENDED_SCAN = 42,
+	DYNAMIC_WMM_PS = 43,
+	MAC_SPOOFED_SCAN = 44,
+	BMU_ERROR_GENERIC_RECOVERY = 45,
+	DISA = 46,
+	FW_STATS = 47,
+	WPS_PRBRSP_TMPL = 48,
+	BCN_IE_FLT_DELTA = 49,
+	TDLS_OFF_CHANNEL = 51,
+	RTT3 = 52,
+	MGMT_FRAME_LOGGING = 53,
+	ENHANCED_TXBD_COMPLETION = 54,
+	LOGGING_ENHANCEMENT = 55,
+	EXT_SCAN_ENHANCED = 56,
+	MEMORY_DUMP_SUPPORTED = 57,
+	PER_PKT_STATS_SUPPORTED = 58,
+	EXT_LL_STAT = 60,
+	WIFI_CONFIG = 61,
+	ANTENNA_DIVERSITY_SELECTION = 62,
+
+	MAX_FEATURE_SUPPORTED = 128,
+};
+
+void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
+				    enum wcn36xx_firmware_feat_caps cap);
+int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
+				   enum wcn36xx_firmware_feat_caps cap);
+void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
+				      enum wcn36xx_firmware_feat_caps cap);
+
+#endif /* _FIRMWARE_H_ */
+
diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 5e48c97682c2..5c45b23c7880 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -4758,74 +4758,6 @@ struct wcn36xx_hal_set_power_params_resp {
 	u32 status;
 } __packed;
 
-/* Capability bitmap exchange definitions and macros starts */
-
-enum wcn36xx_firmware_feat_caps {
-	MCC = 0,
-	P2P = 1,
-	DOT11AC = 2,
-	SLM_SESSIONIZATION = 3,
-	DOT11AC_OPMODE = 4,
-	SAP32STA = 5,
-	TDLS = 6,
-	P2P_GO_NOA_DECOUPLE_INIT_SCAN = 7,
-	WLANACTIVE_OFFLOAD = 8,
-	BEACON_OFFLOAD = 9,
-	SCAN_OFFLOAD = 10,
-	ROAM_OFFLOAD = 11,
-	BCN_MISS_OFFLOAD = 12,
-	STA_POWERSAVE = 13,
-	STA_ADVANCED_PWRSAVE = 14,
-	AP_UAPSD = 15,
-	AP_DFS = 16,
-	BLOCKACK = 17,
-	PHY_ERR = 18,
-	BCN_FILTER = 19,
-	RTT = 20,
-	RATECTRL = 21,
-	WOW = 22,
-	WLAN_ROAM_SCAN_OFFLOAD = 23,
-	SPECULATIVE_PS_POLL = 24,
-	SCAN_SCH = 25,
-	IBSS_HEARTBEAT_OFFLOAD = 26,
-	WLAN_SCAN_OFFLOAD = 27,
-	WLAN_PERIODIC_TX_PTRN = 28,
-	ADVANCE_TDLS = 29,
-	BATCH_SCAN = 30,
-	FW_IN_TX_PATH = 31,
-	EXTENDED_NSOFFLOAD_SLOT = 32,
-	CH_SWITCH_V1 = 33,
-	HT40_OBSS_SCAN = 34,
-	UPDATE_CHANNEL_LIST = 35,
-	WLAN_MCADDR_FLT = 36,
-	WLAN_CH144 = 37,
-	NAN = 38,
-	TDLS_SCAN_COEXISTENCE = 39,
-	LINK_LAYER_STATS_MEAS = 40,
-	MU_MIMO = 41,
-	EXTENDED_SCAN = 42,
-	DYNAMIC_WMM_PS = 43,
-	MAC_SPOOFED_SCAN = 44,
-	BMU_ERROR_GENERIC_RECOVERY = 45,
-	DISA = 46,
-	FW_STATS = 47,
-	WPS_PRBRSP_TMPL = 48,
-	BCN_IE_FLT_DELTA = 49,
-	TDLS_OFF_CHANNEL = 51,
-	RTT3 = 52,
-	MGMT_FRAME_LOGGING = 53,
-	ENHANCED_TXBD_COMPLETION = 54,
-	LOGGING_ENHANCEMENT = 55,
-	EXT_SCAN_ENHANCED = 56,
-	MEMORY_DUMP_SUPPORTED = 57,
-	PER_PKT_STATS_SUPPORTED = 58,
-	EXT_LL_STAT = 60,
-	WIFI_CONFIG = 61,
-	ANTENNA_DIVERSITY_SELECTION = 62,
-
-	MAX_FEATURE_SUPPORTED = 128,
-};
-
 #define WCN36XX_HAL_CAPS_SIZE 4
 
 struct wcn36xx_hal_feat_caps_msg {
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index efd776b20e60..af62911a4659 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -28,6 +28,7 @@
 #include <net/ipv6.h>
 #include "wcn36xx.h"
 #include "testmode.h"
+#include "firmware.h"
 
 unsigned int wcn36xx_dbg_mask;
 module_param_named(debug_mask, wcn36xx_dbg_mask, uint, 0644);
@@ -272,7 +273,7 @@ static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
 	int i;
 
 	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
-		if (get_feat_caps(wcn->fw_feat_caps, i))
+		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i))
 			wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n", wcn36xx_get_cap_name(i));
 	}
 }
@@ -705,7 +706,7 @@ static int wcn36xx_hw_scan(struct ieee80211_hw *hw,
 {
 	struct wcn36xx *wcn = hw->priv;
 
-	if (!get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
+	if (!wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
 		/* fallback to mac80211 software scan */
 		return 1;
 	}
@@ -743,7 +744,7 @@ static void wcn36xx_cancel_hw_scan(struct ieee80211_hw *hw,
 	wcn->scan_aborted = true;
 	mutex_unlock(&wcn->scan_lock);
 
-	if (get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
+	if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
 		/* ieee80211_scan_completed will be called on FW scan
 		 * indication */
 		wcn36xx_smd_stop_hw_scan(wcn);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 88ee92be8562..d2a994fee812 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -22,6 +22,7 @@
 #include <linux/bitops.h>
 #include <linux/rpmsg.h>
 #include "smd.h"
+#include "firmware.h"
 
 struct wcn36xx_cfg_val {
 	u32 cfg_id;
@@ -295,7 +296,7 @@ static void wcn36xx_smd_set_sta_vht_params(struct wcn36xx *wcn,
 		sta_params->vht_capable = sta->deflink.vht_cap.vht_supported;
 		sta_params->vht_ldpc_enabled =
 			is_cap_supported(caps, IEEE80211_VHT_CAP_RXLDPC);
-		if (get_feat_caps(wcn->fw_feat_caps, MU_MIMO)) {
+		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, MU_MIMO)) {
 			sta_params->vht_tx_mu_beamformee_capable =
 				is_cap_supported(caps, IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE);
 			if (sta_params->vht_tx_mu_beamformee_capable)
@@ -2431,49 +2432,6 @@ int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
 	return ret;
 }
 
-void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
-{
-	int arr_idx, bit_idx;
-
-	if (cap < 0 || cap > 127) {
-		wcn36xx_warn("error cap idx %d\n", cap);
-		return;
-	}
-
-	arr_idx = cap / 32;
-	bit_idx = cap % 32;
-	bitmap[arr_idx] |= (1 << bit_idx);
-}
-
-int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
-{
-	int arr_idx, bit_idx;
-
-	if (cap < 0 || cap > 127) {
-		wcn36xx_warn("error cap idx %d\n", cap);
-		return -EINVAL;
-	}
-
-	arr_idx = cap / 32;
-	bit_idx = cap % 32;
-
-	return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
-}
-
-void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
-{
-	int arr_idx, bit_idx;
-
-	if (cap < 0 || cap > 127) {
-		wcn36xx_warn("error cap idx %d\n", cap);
-		return;
-	}
-
-	arr_idx = cap / 32;
-	bit_idx = cap % 32;
-	bitmap[arr_idx] &= ~(1 << bit_idx);
-}
-
 int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn)
 {
 	struct wcn36xx_hal_feat_caps_msg msg_body, *rsp;
@@ -2482,11 +2440,12 @@ int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn)
 	mutex_lock(&wcn->hal_mutex);
 	INIT_HAL_MSG(msg_body, WCN36XX_HAL_FEATURE_CAPS_EXCHANGE_REQ);
 
-	set_feat_caps(msg_body.feat_caps, STA_POWERSAVE);
+	wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, STA_POWERSAVE);
 	if (wcn->rf_id == RF_IRIS_WCN3680) {
-		set_feat_caps(msg_body.feat_caps, DOT11AC);
-		set_feat_caps(msg_body.feat_caps, WLAN_CH144);
-		set_feat_caps(msg_body.feat_caps, ANTENNA_DIVERSITY_SELECTION);
+		wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, DOT11AC);
+		wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, WLAN_CH144);
+		wcn36xx_firmware_set_feat_caps(msg_body.feat_caps,
+					       ANTENNA_DIVERSITY_SELECTION);
 	}
 
 	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
@@ -3300,7 +3259,7 @@ int wcn36xx_smd_add_beacon_filter(struct wcn36xx *wcn,
 	size_t payload_size;
 	int ret;
 
-	if (!get_feat_caps(wcn->fw_feat_caps, BCN_FILTER))
+	if (!wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, BCN_FILTER))
 		return -EOPNOTSUPP;
 
 	mutex_lock(&wcn->hal_mutex);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 186dad4fe80e..cf15cde2a364 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -125,9 +125,6 @@ int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn,
 int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
 			     u32 arg3, u32 arg4, u32 arg5);
 int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn);
-void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
-int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
-void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
 
 int wcn36xx_smd_add_ba_session(struct wcn36xx *wcn,
 		struct ieee80211_sta *sta,
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c
  2022-07-19 14:32 [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits Bryan O'Donoghue
  2022-07-19 14:32 ` [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum Bryan O'Donoghue
  2022-07-19 14:33 ` [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file Bryan O'Donoghue
@ 2022-07-19 14:33 ` Bryan O'Donoghue
  2022-07-20  7:21   ` Loic Poulain
  2022-07-19 14:33 ` [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings Bryan O'Donoghue
  3 siblings, 1 reply; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-19 14:33 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev,
	bryan.odonoghue

Move wcn36xx_get_cap_name() function in main.c into firmware.c as
wcn36xx_firmware_get_cap_name().

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/firmware.c | 75 +++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/firmware.h |  2 +
 drivers/net/wireless/ath/wcn36xx/main.c     | 81 +--------------------
 3 files changed, 81 insertions(+), 77 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.c b/drivers/net/wireless/ath/wcn36xx/firmware.c
index 03b93d2bdcf9..4b7f439e4db5 100644
--- a/drivers/net/wireless/ath/wcn36xx/firmware.c
+++ b/drivers/net/wireless/ath/wcn36xx/firmware.c
@@ -3,6 +3,81 @@
 #include "wcn36xx.h"
 #include "firmware.h"
 
+#define DEFINE(s)[s] = #s
+
+static const char * const wcn36xx_firmware_caps_names[] = {
+	DEFINE(MCC),
+	DEFINE(P2P),
+	DEFINE(DOT11AC),
+	DEFINE(SLM_SESSIONIZATION),
+	DEFINE(DOT11AC_OPMODE),
+	DEFINE(SAP32STA),
+	DEFINE(TDLS),
+	DEFINE(P2P_GO_NOA_DECOUPLE_INIT_SCAN),
+	DEFINE(WLANACTIVE_OFFLOAD),
+	DEFINE(BEACON_OFFLOAD),
+	DEFINE(SCAN_OFFLOAD),
+	DEFINE(ROAM_OFFLOAD),
+	DEFINE(BCN_MISS_OFFLOAD),
+	DEFINE(STA_POWERSAVE),
+	DEFINE(STA_ADVANCED_PWRSAVE),
+	DEFINE(AP_UAPSD),
+	DEFINE(AP_DFS),
+	DEFINE(BLOCKACK),
+	DEFINE(PHY_ERR),
+	DEFINE(BCN_FILTER),
+	DEFINE(RTT),
+	DEFINE(RATECTRL),
+	DEFINE(WOW),
+	DEFINE(WLAN_ROAM_SCAN_OFFLOAD),
+	DEFINE(SPECULATIVE_PS_POLL),
+	DEFINE(SCAN_SCH),
+	DEFINE(IBSS_HEARTBEAT_OFFLOAD),
+	DEFINE(WLAN_SCAN_OFFLOAD),
+	DEFINE(WLAN_PERIODIC_TX_PTRN),
+	DEFINE(ADVANCE_TDLS),
+	DEFINE(BATCH_SCAN),
+	DEFINE(FW_IN_TX_PATH),
+	DEFINE(EXTENDED_NSOFFLOAD_SLOT),
+	DEFINE(CH_SWITCH_V1),
+	DEFINE(HT40_OBSS_SCAN),
+	DEFINE(UPDATE_CHANNEL_LIST),
+	DEFINE(WLAN_MCADDR_FLT),
+	DEFINE(WLAN_CH144),
+	DEFINE(NAN),
+	DEFINE(TDLS_SCAN_COEXISTENCE),
+	DEFINE(LINK_LAYER_STATS_MEAS),
+	DEFINE(MU_MIMO),
+	DEFINE(EXTENDED_SCAN),
+	DEFINE(DYNAMIC_WMM_PS),
+	DEFINE(MAC_SPOOFED_SCAN),
+	DEFINE(BMU_ERROR_GENERIC_RECOVERY),
+	DEFINE(DISA),
+	DEFINE(FW_STATS),
+	DEFINE(WPS_PRBRSP_TMPL),
+	DEFINE(BCN_IE_FLT_DELTA),
+	DEFINE(TDLS_OFF_CHANNEL),
+	DEFINE(RTT3),
+	DEFINE(MGMT_FRAME_LOGGING),
+	DEFINE(ENHANCED_TXBD_COMPLETION),
+	DEFINE(LOGGING_ENHANCEMENT),
+	DEFINE(EXT_SCAN_ENHANCED),
+	DEFINE(MEMORY_DUMP_SUPPORTED),
+	DEFINE(PER_PKT_STATS_SUPPORTED),
+	DEFINE(EXT_LL_STAT),
+	DEFINE(WIFI_CONFIG),
+	DEFINE(ANTENNA_DIVERSITY_SELECTION),
+};
+
+#undef DEFINE
+
+const char *wcn36xx_firmware_get_cap_name(enum wcn36xx_firmware_feat_caps x)
+{
+	if (x >= ARRAY_SIZE(wcn36xx_firmware_caps_names))
+		return "UNKNOWN";
+	return wcn36xx_firmware_caps_names[x];
+}
+
 void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
 				    enum wcn36xx_firmware_feat_caps cap)
 {
diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.h b/drivers/net/wireless/ath/wcn36xx/firmware.h
index 552c0e9325e1..f991cf959f82 100644
--- a/drivers/net/wireless/ath/wcn36xx/firmware.h
+++ b/drivers/net/wireless/ath/wcn36xx/firmware.h
@@ -78,5 +78,7 @@ int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
 void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
 				      enum wcn36xx_firmware_feat_caps cap);
 
+const char *wcn36xx_firmware_get_cap_name(enum wcn36xx_firmware_feat_caps x);
+
 #endif /* _FIRMWARE_H_ */
 
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index af62911a4659..fec85e89a02f 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -193,88 +193,15 @@ static inline u8 get_sta_index(struct ieee80211_vif *vif,
 	       sta_priv->sta_index;
 }
 
-#define DEFINE(s) [s] = #s
-
-static const char * const wcn36xx_caps_names[] = {
-	DEFINE(MCC),
-	DEFINE(P2P),
-	DEFINE(DOT11AC),
-	DEFINE(SLM_SESSIONIZATION),
-	DEFINE(DOT11AC_OPMODE),
-	DEFINE(SAP32STA),
-	DEFINE(TDLS),
-	DEFINE(P2P_GO_NOA_DECOUPLE_INIT_SCAN),
-	DEFINE(WLANACTIVE_OFFLOAD),
-	DEFINE(BEACON_OFFLOAD),
-	DEFINE(SCAN_OFFLOAD),
-	DEFINE(ROAM_OFFLOAD),
-	DEFINE(BCN_MISS_OFFLOAD),
-	DEFINE(STA_POWERSAVE),
-	DEFINE(STA_ADVANCED_PWRSAVE),
-	DEFINE(AP_UAPSD),
-	DEFINE(AP_DFS),
-	DEFINE(BLOCKACK),
-	DEFINE(PHY_ERR),
-	DEFINE(BCN_FILTER),
-	DEFINE(RTT),
-	DEFINE(RATECTRL),
-	DEFINE(WOW),
-	DEFINE(WLAN_ROAM_SCAN_OFFLOAD),
-	DEFINE(SPECULATIVE_PS_POLL),
-	DEFINE(SCAN_SCH),
-	DEFINE(IBSS_HEARTBEAT_OFFLOAD),
-	DEFINE(WLAN_SCAN_OFFLOAD),
-	DEFINE(WLAN_PERIODIC_TX_PTRN),
-	DEFINE(ADVANCE_TDLS),
-	DEFINE(BATCH_SCAN),
-	DEFINE(FW_IN_TX_PATH),
-	DEFINE(EXTENDED_NSOFFLOAD_SLOT),
-	DEFINE(CH_SWITCH_V1),
-	DEFINE(HT40_OBSS_SCAN),
-	DEFINE(UPDATE_CHANNEL_LIST),
-	DEFINE(WLAN_MCADDR_FLT),
-	DEFINE(WLAN_CH144),
-	DEFINE(NAN),
-	DEFINE(TDLS_SCAN_COEXISTENCE),
-	DEFINE(LINK_LAYER_STATS_MEAS),
-	DEFINE(MU_MIMO),
-	DEFINE(EXTENDED_SCAN),
-	DEFINE(DYNAMIC_WMM_PS),
-	DEFINE(MAC_SPOOFED_SCAN),
-	DEFINE(BMU_ERROR_GENERIC_RECOVERY),
-	DEFINE(DISA),
-	DEFINE(FW_STATS),
-	DEFINE(WPS_PRBRSP_TMPL),
-	DEFINE(BCN_IE_FLT_DELTA),
-	DEFINE(TDLS_OFF_CHANNEL),
-	DEFINE(RTT3),
-	DEFINE(MGMT_FRAME_LOGGING),
-	DEFINE(ENHANCED_TXBD_COMPLETION),
-	DEFINE(LOGGING_ENHANCEMENT),
-	DEFINE(EXT_SCAN_ENHANCED),
-	DEFINE(MEMORY_DUMP_SUPPORTED),
-	DEFINE(PER_PKT_STATS_SUPPORTED),
-	DEFINE(EXT_LL_STAT),
-	DEFINE(WIFI_CONFIG),
-	DEFINE(ANTENNA_DIVERSITY_SELECTION),
-};
-
-#undef DEFINE
-
-static const char *wcn36xx_get_cap_name(enum wcn36xx_firmware_feat_caps x)
-{
-	if (x >= ARRAY_SIZE(wcn36xx_caps_names))
-		return "UNKNOWN";
-	return wcn36xx_caps_names[x];
-}
-
 static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
 {
 	int i;
 
 	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
-		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i))
-			wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n", wcn36xx_get_cap_name(i));
+		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
+			wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n",
+				    wcn36xx_firmware_get_cap_name(i));
+		}
 	}
 }
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings
  2022-07-19 14:32 [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits Bryan O'Donoghue
                   ` (2 preceding siblings ...)
  2022-07-19 14:33 ` [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c Bryan O'Donoghue
@ 2022-07-19 14:33 ` Bryan O'Donoghue
  2022-07-20  7:24   ` Loic Poulain
  2022-07-27 10:31   ` Kalle Valo
  3 siblings, 2 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-19 14:33 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev,
	bryan.odonoghue

Add in the ability to easily find the firmware feature bits reported in the
get feature exchange without having to compile-in debug prints.

root@linaro-alip:~# cat /sys/kernel/debug/ieee80211/phy0/wcn36xx/firmware_feat_caps
MCC
P2P
DOT11AC
SLM_SESSIONIZATION
DOT11AC_OPMODE
SAP32STA
TDLS
P2P_GO_NOA_DECOUPLE_INIT_SCAN
WLANACTIVE_OFFLOAD
BEACON_OFFLOAD
SCAN_OFFLOAD
BCN_MISS_OFFLOAD
STA_POWERSAVE
STA_ADVANCED_PWRSAVE
BCN_FILTER
RTT
RATECTRL
WOW
WLAN_ROAM_SCAN_OFFLOAD
SPECULATIVE_PS_POLL
IBSS_HEARTBEAT_OFFLOAD
WLAN_SCAN_OFFLOAD
WLAN_PERIODIC_TX_PTRN
ADVANCE_TDLS
BATCH_SCAN
FW_IN_TX_PATH
EXTENDED_NSOFFLOAD_SLOT
CH_SWITCH_V1
HT40_OBSS_SCAN
UPDATE_CHANNEL_LIST
WLAN_MCADDR_FLT
WLAN_CH144
TDLS_SCAN_COEXISTENCE
LINK_LAYER_STATS_MEAS
MU_MIMO
EXTENDED_SCAN
DYNAMIC_WMM_PS
MAC_SPOOFED_SCAN
FW_STATS
WPS_PRBRSP_TMPL
BCN_IE_FLT_DELTA

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/debug.c | 37 ++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/debug.h |  1 +
 2 files changed, 38 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/debug.c b/drivers/net/wireless/ath/wcn36xx/debug.c
index 6af306ae41ad..220f338045bd 100644
--- a/drivers/net/wireless/ath/wcn36xx/debug.c
+++ b/drivers/net/wireless/ath/wcn36xx/debug.c
@@ -21,6 +21,7 @@
 #include "wcn36xx.h"
 #include "debug.h"
 #include "pmc.h"
+#include "firmware.h"
 
 #ifdef CONFIG_WCN36XX_DEBUGFS
 
@@ -136,6 +137,40 @@ static const struct file_operations fops_wcn36xx_dump = {
 	.write =       write_file_dump,
 };
 
+static ssize_t read_file_firmware_feature_caps(struct file *file,
+					       char __user *user_buf,
+					       size_t count, loff_t *ppos)
+{
+	struct wcn36xx *wcn = file->private_data;
+	unsigned long page = get_zeroed_page(GFP_KERNEL);
+	char *p = (char *)page;
+	int i;
+	int ret;
+
+	if (!p)
+		return -ENOMEM;
+
+	mutex_lock(&wcn->hal_mutex);
+	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
+		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
+			p += sprintf(p, "%s\n",
+				     wcn36xx_firmware_get_cap_name(i));
+		}
+	}
+	mutex_unlock(&wcn->hal_mutex);
+
+	ret = simple_read_from_buffer(user_buf, count, ppos, (char *)page,
+				      (unsigned long)p - page);
+
+	free_page(page);
+	return ret;
+}
+
+static const struct file_operations fops_wcn36xx_firmware_feat_caps = {
+	.open = simple_open,
+	.read = read_file_firmware_feature_caps,
+};
+
 #define ADD_FILE(name, mode, fop, priv_data)		\
 	do {							\
 		struct dentry *d;				\
@@ -163,6 +198,8 @@ void wcn36xx_debugfs_init(struct wcn36xx *wcn)
 
 	ADD_FILE(bmps_switcher, 0600, &fops_wcn36xx_bmps, wcn);
 	ADD_FILE(dump, 0200, &fops_wcn36xx_dump, wcn);
+	ADD_FILE(firmware_feat_caps, 0200,
+		 &fops_wcn36xx_firmware_feat_caps, wcn);
 }
 
 void wcn36xx_debugfs_exit(struct wcn36xx *wcn)
diff --git a/drivers/net/wireless/ath/wcn36xx/debug.h b/drivers/net/wireless/ath/wcn36xx/debug.h
index 46307aa562d3..7116d96e0543 100644
--- a/drivers/net/wireless/ath/wcn36xx/debug.h
+++ b/drivers/net/wireless/ath/wcn36xx/debug.h
@@ -31,6 +31,7 @@ struct wcn36xx_dfs_entry {
 	struct dentry *rootdir;
 	struct wcn36xx_dfs_file file_bmps_switcher;
 	struct wcn36xx_dfs_file file_dump;
+	struct wcn36xx_dfs_file file_firmware_feat_caps;
 };
 
 void wcn36xx_debugfs_init(struct wcn36xx *wcn);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum
  2022-07-19 14:32 ` [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum Bryan O'Donoghue
@ 2022-07-20  7:14   ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2022-07-20  7:14 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: kvalo, davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev

On Tue, 19 Jul 2022 at 16:33, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> The enum name "place_holder_in_cap_bitmap" is self descriptively asking to
> be changed to something else.
>
> Rename place_holder_in_cap_bitmap to wcn36xx_firmware_feat_caps so that the
> contents and intent of the enum is obvious.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
>  drivers/net/wireless/ath/wcn36xx/hal.h  | 2 +-
>  drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>  drivers/net/wireless/ath/wcn36xx/smd.c  | 6 +++---
>  drivers/net/wireless/ath/wcn36xx/smd.h  | 6 +++---
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index 46a49f0a51b3..5e48c97682c2 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -4760,7 +4760,7 @@ struct wcn36xx_hal_set_power_params_resp {
>
>  /* Capability bitmap exchange definitions and macros starts */
>
> -enum place_holder_in_cap_bitmap {
> +enum wcn36xx_firmware_feat_caps {
>         MCC = 0,
>         P2P = 1,
>         DOT11AC = 2,
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index e34d3d0b7082..efd776b20e60 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -260,7 +260,7 @@ static const char * const wcn36xx_caps_names[] = {
>
>  #undef DEFINE
>
> -static const char *wcn36xx_get_cap_name(enum place_holder_in_cap_bitmap x)
> +static const char *wcn36xx_get_cap_name(enum wcn36xx_firmware_feat_caps x)
>  {
>         if (x >= ARRAY_SIZE(wcn36xx_caps_names))
>                 return "UNKNOWN";
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 7ac9a1e6f768..88ee92be8562 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -2431,7 +2431,7 @@ int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
>         return ret;
>  }
>
> -void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
> +void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
>  {
>         int arr_idx, bit_idx;
>
> @@ -2445,7 +2445,7 @@ void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
>         bitmap[arr_idx] |= (1 << bit_idx);
>  }
>
> -int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
> +int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
>  {
>         int arr_idx, bit_idx;
>
> @@ -2460,7 +2460,7 @@ int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
>         return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
>  }
>
> -void clear_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap)
> +void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
>  {
>         int arr_idx, bit_idx;
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 3fd598ac2a27..186dad4fe80e 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -125,9 +125,9 @@ int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn,
>  int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
>                              u32 arg3, u32 arg4, u32 arg5);
>  int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn);
> -void set_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
> -int get_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
> -void clear_feat_caps(u32 *bitmap, enum place_holder_in_cap_bitmap cap);
> +void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
> +int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
> +void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
>
>  int wcn36xx_smd_add_ba_session(struct wcn36xx *wcn,
>                 struct ieee80211_sta *sta,
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file
  2022-07-19 14:33 ` [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file Bryan O'Donoghue
@ 2022-07-20  7:16   ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2022-07-20  7:16 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: kvalo, davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev

On Tue, 19 Jul 2022 at 16:33, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> The naming of the get/set/clear firmware feature capability bits doesn't
> really follow the established namespace pattern of
> wcn36xx_logicalblock_do_something();
>
> The feature bits are accessed by smd.c and main.c. It would be nice to
> display the found feature bits in debugfs. To do so though we should tidy
> up the namespace a bit.
>
> Move the firmware feature exchange API to its own file - firmware.c giving
> us the opportunity to functionally decompose other firmware related
> accessors as appropriate in future.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
>  drivers/net/wireless/ath/wcn36xx/Makefile   |  3 +-
>  drivers/net/wireless/ath/wcn36xx/firmware.c | 50 +++++++++++++
>  drivers/net/wireless/ath/wcn36xx/firmware.h | 82 +++++++++++++++++++++
>  drivers/net/wireless/ath/wcn36xx/hal.h      | 68 -----------------
>  drivers/net/wireless/ath/wcn36xx/main.c     |  7 +-
>  drivers/net/wireless/ath/wcn36xx/smd.c      | 57 ++------------
>  drivers/net/wireless/ath/wcn36xx/smd.h      |  3 -
>  7 files changed, 146 insertions(+), 124 deletions(-)
>  create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.c
>  create mode 100644 drivers/net/wireless/ath/wcn36xx/firmware.h
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/Makefile b/drivers/net/wireless/ath/wcn36xx/Makefile
> index 27413703ad69..26bec795b372 100644
> --- a/drivers/net/wireless/ath/wcn36xx/Makefile
> +++ b/drivers/net/wireless/ath/wcn36xx/Makefile
> @@ -5,6 +5,7 @@ wcn36xx-y +=   main.o \
>                 txrx.o \
>                 smd.o \
>                 pmc.o \
> -               debug.o
> +               debug.o \
> +               firmware.o
>
>  wcn36xx-$(CONFIG_NL80211_TESTMODE) += testmode.o
> diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.c b/drivers/net/wireless/ath/wcn36xx/firmware.c
> new file mode 100644
> index 000000000000..03b93d2bdcf9
> --- /dev/null
> +++ b/drivers/net/wireless/ath/wcn36xx/firmware.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "wcn36xx.h"
> +#include "firmware.h"
> +
> +void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
> +                                   enum wcn36xx_firmware_feat_caps cap)
> +{
> +       int arr_idx, bit_idx;
> +
> +       if (cap < 0 || cap > 127) {
> +               wcn36xx_warn("error cap idx %d\n", cap);
> +               return;
> +       }
> +
> +       arr_idx = cap / 32;
> +       bit_idx = cap % 32;
> +       bitmap[arr_idx] |= (1 << bit_idx);
> +}
> +
> +int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
> +                                  enum wcn36xx_firmware_feat_caps cap)
> +{
> +       int arr_idx, bit_idx;
> +
> +       if (cap < 0 || cap > 127) {
> +               wcn36xx_warn("error cap idx %d\n", cap);
> +               return -EINVAL;
> +       }
> +
> +       arr_idx = cap / 32;
> +       bit_idx = cap % 32;
> +
> +       return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
> +}
> +
> +void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
> +                                     enum wcn36xx_firmware_feat_caps cap)
> +{
> +       int arr_idx, bit_idx;
> +
> +       if (cap < 0 || cap > 127) {
> +               wcn36xx_warn("error cap idx %d\n", cap);
> +               return;
> +       }
> +
> +       arr_idx = cap / 32;
> +       bit_idx = cap % 32;
> +       bitmap[arr_idx] &= ~(1 << bit_idx);
> +}
> diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.h b/drivers/net/wireless/ath/wcn36xx/firmware.h
> new file mode 100644
> index 000000000000..552c0e9325e1
> --- /dev/null
> +++ b/drivers/net/wireless/ath/wcn36xx/firmware.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef _FIRMWARE_H_
> +#define _FIRMWARE_H_
> +
> +/* Capability bitmap exchange definitions and macros starts */
> +
> +enum wcn36xx_firmware_feat_caps {
> +       MCC = 0,
> +       P2P = 1,
> +       DOT11AC = 2,
> +       SLM_SESSIONIZATION = 3,
> +       DOT11AC_OPMODE = 4,
> +       SAP32STA = 5,
> +       TDLS = 6,
> +       P2P_GO_NOA_DECOUPLE_INIT_SCAN = 7,
> +       WLANACTIVE_OFFLOAD = 8,
> +       BEACON_OFFLOAD = 9,
> +       SCAN_OFFLOAD = 10,
> +       ROAM_OFFLOAD = 11,
> +       BCN_MISS_OFFLOAD = 12,
> +       STA_POWERSAVE = 13,
> +       STA_ADVANCED_PWRSAVE = 14,
> +       AP_UAPSD = 15,
> +       AP_DFS = 16,
> +       BLOCKACK = 17,
> +       PHY_ERR = 18,
> +       BCN_FILTER = 19,
> +       RTT = 20,
> +       RATECTRL = 21,
> +       WOW = 22,
> +       WLAN_ROAM_SCAN_OFFLOAD = 23,
> +       SPECULATIVE_PS_POLL = 24,
> +       SCAN_SCH = 25,
> +       IBSS_HEARTBEAT_OFFLOAD = 26,
> +       WLAN_SCAN_OFFLOAD = 27,
> +       WLAN_PERIODIC_TX_PTRN = 28,
> +       ADVANCE_TDLS = 29,
> +       BATCH_SCAN = 30,
> +       FW_IN_TX_PATH = 31,
> +       EXTENDED_NSOFFLOAD_SLOT = 32,
> +       CH_SWITCH_V1 = 33,
> +       HT40_OBSS_SCAN = 34,
> +       UPDATE_CHANNEL_LIST = 35,
> +       WLAN_MCADDR_FLT = 36,
> +       WLAN_CH144 = 37,
> +       NAN = 38,
> +       TDLS_SCAN_COEXISTENCE = 39,
> +       LINK_LAYER_STATS_MEAS = 40,
> +       MU_MIMO = 41,
> +       EXTENDED_SCAN = 42,
> +       DYNAMIC_WMM_PS = 43,
> +       MAC_SPOOFED_SCAN = 44,
> +       BMU_ERROR_GENERIC_RECOVERY = 45,
> +       DISA = 46,
> +       FW_STATS = 47,
> +       WPS_PRBRSP_TMPL = 48,
> +       BCN_IE_FLT_DELTA = 49,
> +       TDLS_OFF_CHANNEL = 51,
> +       RTT3 = 52,
> +       MGMT_FRAME_LOGGING = 53,
> +       ENHANCED_TXBD_COMPLETION = 54,
> +       LOGGING_ENHANCEMENT = 55,
> +       EXT_SCAN_ENHANCED = 56,
> +       MEMORY_DUMP_SUPPORTED = 57,
> +       PER_PKT_STATS_SUPPORTED = 58,
> +       EXT_LL_STAT = 60,
> +       WIFI_CONFIG = 61,
> +       ANTENNA_DIVERSITY_SELECTION = 62,
> +
> +       MAX_FEATURE_SUPPORTED = 128,
> +};
> +
> +void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
> +                                   enum wcn36xx_firmware_feat_caps cap);
> +int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
> +                                  enum wcn36xx_firmware_feat_caps cap);
> +void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
> +                                     enum wcn36xx_firmware_feat_caps cap);
> +
> +#endif /* _FIRMWARE_H_ */
> +
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index 5e48c97682c2..5c45b23c7880 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -4758,74 +4758,6 @@ struct wcn36xx_hal_set_power_params_resp {
>         u32 status;
>  } __packed;
>
> -/* Capability bitmap exchange definitions and macros starts */
> -
> -enum wcn36xx_firmware_feat_caps {
> -       MCC = 0,
> -       P2P = 1,
> -       DOT11AC = 2,
> -       SLM_SESSIONIZATION = 3,
> -       DOT11AC_OPMODE = 4,
> -       SAP32STA = 5,
> -       TDLS = 6,
> -       P2P_GO_NOA_DECOUPLE_INIT_SCAN = 7,
> -       WLANACTIVE_OFFLOAD = 8,
> -       BEACON_OFFLOAD = 9,
> -       SCAN_OFFLOAD = 10,
> -       ROAM_OFFLOAD = 11,
> -       BCN_MISS_OFFLOAD = 12,
> -       STA_POWERSAVE = 13,
> -       STA_ADVANCED_PWRSAVE = 14,
> -       AP_UAPSD = 15,
> -       AP_DFS = 16,
> -       BLOCKACK = 17,
> -       PHY_ERR = 18,
> -       BCN_FILTER = 19,
> -       RTT = 20,
> -       RATECTRL = 21,
> -       WOW = 22,
> -       WLAN_ROAM_SCAN_OFFLOAD = 23,
> -       SPECULATIVE_PS_POLL = 24,
> -       SCAN_SCH = 25,
> -       IBSS_HEARTBEAT_OFFLOAD = 26,
> -       WLAN_SCAN_OFFLOAD = 27,
> -       WLAN_PERIODIC_TX_PTRN = 28,
> -       ADVANCE_TDLS = 29,
> -       BATCH_SCAN = 30,
> -       FW_IN_TX_PATH = 31,
> -       EXTENDED_NSOFFLOAD_SLOT = 32,
> -       CH_SWITCH_V1 = 33,
> -       HT40_OBSS_SCAN = 34,
> -       UPDATE_CHANNEL_LIST = 35,
> -       WLAN_MCADDR_FLT = 36,
> -       WLAN_CH144 = 37,
> -       NAN = 38,
> -       TDLS_SCAN_COEXISTENCE = 39,
> -       LINK_LAYER_STATS_MEAS = 40,
> -       MU_MIMO = 41,
> -       EXTENDED_SCAN = 42,
> -       DYNAMIC_WMM_PS = 43,
> -       MAC_SPOOFED_SCAN = 44,
> -       BMU_ERROR_GENERIC_RECOVERY = 45,
> -       DISA = 46,
> -       FW_STATS = 47,
> -       WPS_PRBRSP_TMPL = 48,
> -       BCN_IE_FLT_DELTA = 49,
> -       TDLS_OFF_CHANNEL = 51,
> -       RTT3 = 52,
> -       MGMT_FRAME_LOGGING = 53,
> -       ENHANCED_TXBD_COMPLETION = 54,
> -       LOGGING_ENHANCEMENT = 55,
> -       EXT_SCAN_ENHANCED = 56,
> -       MEMORY_DUMP_SUPPORTED = 57,
> -       PER_PKT_STATS_SUPPORTED = 58,
> -       EXT_LL_STAT = 60,
> -       WIFI_CONFIG = 61,
> -       ANTENNA_DIVERSITY_SELECTION = 62,
> -
> -       MAX_FEATURE_SUPPORTED = 128,
> -};
> -
>  #define WCN36XX_HAL_CAPS_SIZE 4
>
>  struct wcn36xx_hal_feat_caps_msg {
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index efd776b20e60..af62911a4659 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -28,6 +28,7 @@
>  #include <net/ipv6.h>
>  #include "wcn36xx.h"
>  #include "testmode.h"
> +#include "firmware.h"
>
>  unsigned int wcn36xx_dbg_mask;
>  module_param_named(debug_mask, wcn36xx_dbg_mask, uint, 0644);
> @@ -272,7 +273,7 @@ static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
>         int i;
>
>         for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
> -               if (get_feat_caps(wcn->fw_feat_caps, i))
> +               if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i))
>                         wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n", wcn36xx_get_cap_name(i));
>         }
>  }
> @@ -705,7 +706,7 @@ static int wcn36xx_hw_scan(struct ieee80211_hw *hw,
>  {
>         struct wcn36xx *wcn = hw->priv;
>
> -       if (!get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
> +       if (!wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
>                 /* fallback to mac80211 software scan */
>                 return 1;
>         }
> @@ -743,7 +744,7 @@ static void wcn36xx_cancel_hw_scan(struct ieee80211_hw *hw,
>         wcn->scan_aborted = true;
>         mutex_unlock(&wcn->scan_lock);
>
> -       if (get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
> +       if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
>                 /* ieee80211_scan_completed will be called on FW scan
>                  * indication */
>                 wcn36xx_smd_stop_hw_scan(wcn);
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 88ee92be8562..d2a994fee812 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -22,6 +22,7 @@
>  #include <linux/bitops.h>
>  #include <linux/rpmsg.h>
>  #include "smd.h"
> +#include "firmware.h"
>
>  struct wcn36xx_cfg_val {
>         u32 cfg_id;
> @@ -295,7 +296,7 @@ static void wcn36xx_smd_set_sta_vht_params(struct wcn36xx *wcn,
>                 sta_params->vht_capable = sta->deflink.vht_cap.vht_supported;
>                 sta_params->vht_ldpc_enabled =
>                         is_cap_supported(caps, IEEE80211_VHT_CAP_RXLDPC);
> -               if (get_feat_caps(wcn->fw_feat_caps, MU_MIMO)) {
> +               if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, MU_MIMO)) {
>                         sta_params->vht_tx_mu_beamformee_capable =
>                                 is_cap_supported(caps, IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE);
>                         if (sta_params->vht_tx_mu_beamformee_capable)
> @@ -2431,49 +2432,6 @@ int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
>         return ret;
>  }
>
> -void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
> -{
> -       int arr_idx, bit_idx;
> -
> -       if (cap < 0 || cap > 127) {
> -               wcn36xx_warn("error cap idx %d\n", cap);
> -               return;
> -       }
> -
> -       arr_idx = cap / 32;
> -       bit_idx = cap % 32;
> -       bitmap[arr_idx] |= (1 << bit_idx);
> -}
> -
> -int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
> -{
> -       int arr_idx, bit_idx;
> -
> -       if (cap < 0 || cap > 127) {
> -               wcn36xx_warn("error cap idx %d\n", cap);
> -               return -EINVAL;
> -       }
> -
> -       arr_idx = cap / 32;
> -       bit_idx = cap % 32;
> -
> -       return (bitmap[arr_idx] & (1 << bit_idx)) ? 1 : 0;
> -}
> -
> -void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap)
> -{
> -       int arr_idx, bit_idx;
> -
> -       if (cap < 0 || cap > 127) {
> -               wcn36xx_warn("error cap idx %d\n", cap);
> -               return;
> -       }
> -
> -       arr_idx = cap / 32;
> -       bit_idx = cap % 32;
> -       bitmap[arr_idx] &= ~(1 << bit_idx);
> -}
> -
>  int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn)
>  {
>         struct wcn36xx_hal_feat_caps_msg msg_body, *rsp;
> @@ -2482,11 +2440,12 @@ int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn)
>         mutex_lock(&wcn->hal_mutex);
>         INIT_HAL_MSG(msg_body, WCN36XX_HAL_FEATURE_CAPS_EXCHANGE_REQ);
>
> -       set_feat_caps(msg_body.feat_caps, STA_POWERSAVE);
> +       wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, STA_POWERSAVE);
>         if (wcn->rf_id == RF_IRIS_WCN3680) {
> -               set_feat_caps(msg_body.feat_caps, DOT11AC);
> -               set_feat_caps(msg_body.feat_caps, WLAN_CH144);
> -               set_feat_caps(msg_body.feat_caps, ANTENNA_DIVERSITY_SELECTION);
> +               wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, DOT11AC);
> +               wcn36xx_firmware_set_feat_caps(msg_body.feat_caps, WLAN_CH144);
> +               wcn36xx_firmware_set_feat_caps(msg_body.feat_caps,
> +                                              ANTENNA_DIVERSITY_SELECTION);
>         }
>
>         PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
> @@ -3300,7 +3259,7 @@ int wcn36xx_smd_add_beacon_filter(struct wcn36xx *wcn,
>         size_t payload_size;
>         int ret;
>
> -       if (!get_feat_caps(wcn->fw_feat_caps, BCN_FILTER))
> +       if (!wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, BCN_FILTER))
>                 return -EOPNOTSUPP;
>
>         mutex_lock(&wcn->hal_mutex);
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 186dad4fe80e..cf15cde2a364 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -125,9 +125,6 @@ int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn,
>  int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
>                              u32 arg3, u32 arg4, u32 arg5);
>  int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn);
> -void set_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
> -int get_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
> -void clear_feat_caps(u32 *bitmap, enum wcn36xx_firmware_feat_caps cap);
>
>  int wcn36xx_smd_add_ba_session(struct wcn36xx *wcn,
>                 struct ieee80211_sta *sta,
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c
  2022-07-19 14:33 ` [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c Bryan O'Donoghue
@ 2022-07-20  7:21   ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2022-07-20  7:21 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: kvalo, davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev

On Tue, 19 Jul 2022 at 16:33, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Move wcn36xx_get_cap_name() function in main.c into firmware.c as
> wcn36xx_firmware_get_cap_name().
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
>  drivers/net/wireless/ath/wcn36xx/firmware.c | 75 +++++++++++++++++++
>  drivers/net/wireless/ath/wcn36xx/firmware.h |  2 +
>  drivers/net/wireless/ath/wcn36xx/main.c     | 81 +--------------------
>  3 files changed, 81 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.c b/drivers/net/wireless/ath/wcn36xx/firmware.c
> index 03b93d2bdcf9..4b7f439e4db5 100644
> --- a/drivers/net/wireless/ath/wcn36xx/firmware.c
> +++ b/drivers/net/wireless/ath/wcn36xx/firmware.c
> @@ -3,6 +3,81 @@
>  #include "wcn36xx.h"
>  #include "firmware.h"
>
> +#define DEFINE(s)[s] = #s
> +
> +static const char * const wcn36xx_firmware_caps_names[] = {
> +       DEFINE(MCC),
> +       DEFINE(P2P),
> +       DEFINE(DOT11AC),
> +       DEFINE(SLM_SESSIONIZATION),
> +       DEFINE(DOT11AC_OPMODE),
> +       DEFINE(SAP32STA),
> +       DEFINE(TDLS),
> +       DEFINE(P2P_GO_NOA_DECOUPLE_INIT_SCAN),
> +       DEFINE(WLANACTIVE_OFFLOAD),
> +       DEFINE(BEACON_OFFLOAD),
> +       DEFINE(SCAN_OFFLOAD),
> +       DEFINE(ROAM_OFFLOAD),
> +       DEFINE(BCN_MISS_OFFLOAD),
> +       DEFINE(STA_POWERSAVE),
> +       DEFINE(STA_ADVANCED_PWRSAVE),
> +       DEFINE(AP_UAPSD),
> +       DEFINE(AP_DFS),
> +       DEFINE(BLOCKACK),
> +       DEFINE(PHY_ERR),
> +       DEFINE(BCN_FILTER),
> +       DEFINE(RTT),
> +       DEFINE(RATECTRL),
> +       DEFINE(WOW),
> +       DEFINE(WLAN_ROAM_SCAN_OFFLOAD),
> +       DEFINE(SPECULATIVE_PS_POLL),
> +       DEFINE(SCAN_SCH),
> +       DEFINE(IBSS_HEARTBEAT_OFFLOAD),
> +       DEFINE(WLAN_SCAN_OFFLOAD),
> +       DEFINE(WLAN_PERIODIC_TX_PTRN),
> +       DEFINE(ADVANCE_TDLS),
> +       DEFINE(BATCH_SCAN),
> +       DEFINE(FW_IN_TX_PATH),
> +       DEFINE(EXTENDED_NSOFFLOAD_SLOT),
> +       DEFINE(CH_SWITCH_V1),
> +       DEFINE(HT40_OBSS_SCAN),
> +       DEFINE(UPDATE_CHANNEL_LIST),
> +       DEFINE(WLAN_MCADDR_FLT),
> +       DEFINE(WLAN_CH144),
> +       DEFINE(NAN),
> +       DEFINE(TDLS_SCAN_COEXISTENCE),
> +       DEFINE(LINK_LAYER_STATS_MEAS),
> +       DEFINE(MU_MIMO),
> +       DEFINE(EXTENDED_SCAN),
> +       DEFINE(DYNAMIC_WMM_PS),
> +       DEFINE(MAC_SPOOFED_SCAN),
> +       DEFINE(BMU_ERROR_GENERIC_RECOVERY),
> +       DEFINE(DISA),
> +       DEFINE(FW_STATS),
> +       DEFINE(WPS_PRBRSP_TMPL),
> +       DEFINE(BCN_IE_FLT_DELTA),
> +       DEFINE(TDLS_OFF_CHANNEL),
> +       DEFINE(RTT3),
> +       DEFINE(MGMT_FRAME_LOGGING),
> +       DEFINE(ENHANCED_TXBD_COMPLETION),
> +       DEFINE(LOGGING_ENHANCEMENT),
> +       DEFINE(EXT_SCAN_ENHANCED),
> +       DEFINE(MEMORY_DUMP_SUPPORTED),
> +       DEFINE(PER_PKT_STATS_SUPPORTED),
> +       DEFINE(EXT_LL_STAT),
> +       DEFINE(WIFI_CONFIG),
> +       DEFINE(ANTENNA_DIVERSITY_SELECTION),
> +};
> +
> +#undef DEFINE
> +
> +const char *wcn36xx_firmware_get_cap_name(enum wcn36xx_firmware_feat_caps x)
> +{
> +       if (x >= ARRAY_SIZE(wcn36xx_firmware_caps_names))
> +               return "UNKNOWN";
> +       return wcn36xx_firmware_caps_names[x];
> +}
> +
>  void wcn36xx_firmware_set_feat_caps(u32 *bitmap,
>                                     enum wcn36xx_firmware_feat_caps cap)
>  {
> diff --git a/drivers/net/wireless/ath/wcn36xx/firmware.h b/drivers/net/wireless/ath/wcn36xx/firmware.h
> index 552c0e9325e1..f991cf959f82 100644
> --- a/drivers/net/wireless/ath/wcn36xx/firmware.h
> +++ b/drivers/net/wireless/ath/wcn36xx/firmware.h
> @@ -78,5 +78,7 @@ int wcn36xx_firmware_get_feat_caps(u32 *bitmap,
>  void wcn36xx_firmware_clear_feat_caps(u32 *bitmap,
>                                       enum wcn36xx_firmware_feat_caps cap);
>
> +const char *wcn36xx_firmware_get_cap_name(enum wcn36xx_firmware_feat_caps x);
> +
>  #endif /* _FIRMWARE_H_ */
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index af62911a4659..fec85e89a02f 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -193,88 +193,15 @@ static inline u8 get_sta_index(struct ieee80211_vif *vif,
>                sta_priv->sta_index;
>  }
>
> -#define DEFINE(s) [s] = #s
> -
> -static const char * const wcn36xx_caps_names[] = {
> -       DEFINE(MCC),
> -       DEFINE(P2P),
> -       DEFINE(DOT11AC),
> -       DEFINE(SLM_SESSIONIZATION),
> -       DEFINE(DOT11AC_OPMODE),
> -       DEFINE(SAP32STA),
> -       DEFINE(TDLS),
> -       DEFINE(P2P_GO_NOA_DECOUPLE_INIT_SCAN),
> -       DEFINE(WLANACTIVE_OFFLOAD),
> -       DEFINE(BEACON_OFFLOAD),
> -       DEFINE(SCAN_OFFLOAD),
> -       DEFINE(ROAM_OFFLOAD),
> -       DEFINE(BCN_MISS_OFFLOAD),
> -       DEFINE(STA_POWERSAVE),
> -       DEFINE(STA_ADVANCED_PWRSAVE),
> -       DEFINE(AP_UAPSD),
> -       DEFINE(AP_DFS),
> -       DEFINE(BLOCKACK),
> -       DEFINE(PHY_ERR),
> -       DEFINE(BCN_FILTER),
> -       DEFINE(RTT),
> -       DEFINE(RATECTRL),
> -       DEFINE(WOW),
> -       DEFINE(WLAN_ROAM_SCAN_OFFLOAD),
> -       DEFINE(SPECULATIVE_PS_POLL),
> -       DEFINE(SCAN_SCH),
> -       DEFINE(IBSS_HEARTBEAT_OFFLOAD),
> -       DEFINE(WLAN_SCAN_OFFLOAD),
> -       DEFINE(WLAN_PERIODIC_TX_PTRN),
> -       DEFINE(ADVANCE_TDLS),
> -       DEFINE(BATCH_SCAN),
> -       DEFINE(FW_IN_TX_PATH),
> -       DEFINE(EXTENDED_NSOFFLOAD_SLOT),
> -       DEFINE(CH_SWITCH_V1),
> -       DEFINE(HT40_OBSS_SCAN),
> -       DEFINE(UPDATE_CHANNEL_LIST),
> -       DEFINE(WLAN_MCADDR_FLT),
> -       DEFINE(WLAN_CH144),
> -       DEFINE(NAN),
> -       DEFINE(TDLS_SCAN_COEXISTENCE),
> -       DEFINE(LINK_LAYER_STATS_MEAS),
> -       DEFINE(MU_MIMO),
> -       DEFINE(EXTENDED_SCAN),
> -       DEFINE(DYNAMIC_WMM_PS),
> -       DEFINE(MAC_SPOOFED_SCAN),
> -       DEFINE(BMU_ERROR_GENERIC_RECOVERY),
> -       DEFINE(DISA),
> -       DEFINE(FW_STATS),
> -       DEFINE(WPS_PRBRSP_TMPL),
> -       DEFINE(BCN_IE_FLT_DELTA),
> -       DEFINE(TDLS_OFF_CHANNEL),
> -       DEFINE(RTT3),
> -       DEFINE(MGMT_FRAME_LOGGING),
> -       DEFINE(ENHANCED_TXBD_COMPLETION),
> -       DEFINE(LOGGING_ENHANCEMENT),
> -       DEFINE(EXT_SCAN_ENHANCED),
> -       DEFINE(MEMORY_DUMP_SUPPORTED),
> -       DEFINE(PER_PKT_STATS_SUPPORTED),
> -       DEFINE(EXT_LL_STAT),
> -       DEFINE(WIFI_CONFIG),
> -       DEFINE(ANTENNA_DIVERSITY_SELECTION),
> -};
> -
> -#undef DEFINE
> -
> -static const char *wcn36xx_get_cap_name(enum wcn36xx_firmware_feat_caps x)
> -{
> -       if (x >= ARRAY_SIZE(wcn36xx_caps_names))
> -               return "UNKNOWN";
> -       return wcn36xx_caps_names[x];
> -}
> -
>  static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
>  {
>         int i;
>
>         for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
> -               if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i))
> -                       wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n", wcn36xx_get_cap_name(i));
> +               if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
> +                       wcn36xx_dbg(WCN36XX_DBG_MAC, "FW Cap %s\n",
> +                                   wcn36xx_firmware_get_cap_name(i));
> +               }
>         }
>  }
>
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings
  2022-07-19 14:33 ` [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings Bryan O'Donoghue
@ 2022-07-20  7:24   ` Loic Poulain
  2022-07-27 10:31   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2022-07-20  7:24 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: kvalo, davem, edumazet, kuba, pabeni, wcn36xx, linux-wireless, netdev

On Tue, 19 Jul 2022 at 16:33, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Add in the ability to easily find the firmware feature bits reported in the
> get feature exchange without having to compile-in debug prints.
>
> root@linaro-alip:~# cat /sys/kernel/debug/ieee80211/phy0/wcn36xx/firmware_feat_caps
> MCC
> P2P
> DOT11AC
> SLM_SESSIONIZATION
> DOT11AC_OPMODE
> SAP32STA
> TDLS
> P2P_GO_NOA_DECOUPLE_INIT_SCAN
> WLANACTIVE_OFFLOAD
> BEACON_OFFLOAD
> SCAN_OFFLOAD
> BCN_MISS_OFFLOAD
> STA_POWERSAVE
> STA_ADVANCED_PWRSAVE
> BCN_FILTER
> RTT
> RATECTRL
> WOW
> WLAN_ROAM_SCAN_OFFLOAD
> SPECULATIVE_PS_POLL
> IBSS_HEARTBEAT_OFFLOAD
> WLAN_SCAN_OFFLOAD
> WLAN_PERIODIC_TX_PTRN
> ADVANCE_TDLS
> BATCH_SCAN
> FW_IN_TX_PATH
> EXTENDED_NSOFFLOAD_SLOT
> CH_SWITCH_V1
> HT40_OBSS_SCAN
> UPDATE_CHANNEL_LIST
> WLAN_MCADDR_FLT
> WLAN_CH144
> TDLS_SCAN_COEXISTENCE
> LINK_LAYER_STATS_MEAS
> MU_MIMO
> EXTENDED_SCAN
> DYNAMIC_WMM_PS
> MAC_SPOOFED_SCAN
> FW_STATS
> WPS_PRBRSP_TMPL
> BCN_IE_FLT_DELTA
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>


Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
>  drivers/net/wireless/ath/wcn36xx/debug.c | 37 ++++++++++++++++++++++++
>  drivers/net/wireless/ath/wcn36xx/debug.h |  1 +
>  2 files changed, 38 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/debug.c b/drivers/net/wireless/ath/wcn36xx/debug.c
> index 6af306ae41ad..220f338045bd 100644
> --- a/drivers/net/wireless/ath/wcn36xx/debug.c
> +++ b/drivers/net/wireless/ath/wcn36xx/debug.c
> @@ -21,6 +21,7 @@
>  #include "wcn36xx.h"
>  #include "debug.h"
>  #include "pmc.h"
> +#include "firmware.h"
>
>  #ifdef CONFIG_WCN36XX_DEBUGFS
>
> @@ -136,6 +137,40 @@ static const struct file_operations fops_wcn36xx_dump = {
>         .write =       write_file_dump,
>  };
>
> +static ssize_t read_file_firmware_feature_caps(struct file *file,
> +                                              char __user *user_buf,
> +                                              size_t count, loff_t *ppos)
> +{
> +       struct wcn36xx *wcn = file->private_data;
> +       unsigned long page = get_zeroed_page(GFP_KERNEL);
> +       char *p = (char *)page;
> +       int i;
> +       int ret;
> +
> +       if (!p)
> +               return -ENOMEM;
> +
> +       mutex_lock(&wcn->hal_mutex);
> +       for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
> +               if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
> +                       p += sprintf(p, "%s\n",
> +                                    wcn36xx_firmware_get_cap_name(i));
> +               }
> +       }
> +       mutex_unlock(&wcn->hal_mutex);
> +
> +       ret = simple_read_from_buffer(user_buf, count, ppos, (char *)page,
> +                                     (unsigned long)p - page);
> +
> +       free_page(page);
> +       return ret;
> +}
> +
> +static const struct file_operations fops_wcn36xx_firmware_feat_caps = {
> +       .open = simple_open,
> +       .read = read_file_firmware_feature_caps,
> +};
> +
>  #define ADD_FILE(name, mode, fop, priv_data)           \
>         do {                                                    \
>                 struct dentry *d;                               \
> @@ -163,6 +198,8 @@ void wcn36xx_debugfs_init(struct wcn36xx *wcn)
>
>         ADD_FILE(bmps_switcher, 0600, &fops_wcn36xx_bmps, wcn);
>         ADD_FILE(dump, 0200, &fops_wcn36xx_dump, wcn);
> +       ADD_FILE(firmware_feat_caps, 0200,
> +                &fops_wcn36xx_firmware_feat_caps, wcn);
>  }
>
>  void wcn36xx_debugfs_exit(struct wcn36xx *wcn)
> diff --git a/drivers/net/wireless/ath/wcn36xx/debug.h b/drivers/net/wireless/ath/wcn36xx/debug.h
> index 46307aa562d3..7116d96e0543 100644
> --- a/drivers/net/wireless/ath/wcn36xx/debug.h
> +++ b/drivers/net/wireless/ath/wcn36xx/debug.h
> @@ -31,6 +31,7 @@ struct wcn36xx_dfs_entry {
>         struct dentry *rootdir;
>         struct wcn36xx_dfs_file file_bmps_switcher;
>         struct wcn36xx_dfs_file file_dump;
> +       struct wcn36xx_dfs_file file_firmware_feat_caps;
>  };
>
>  void wcn36xx_debugfs_init(struct wcn36xx *wcn);
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings
  2022-07-19 14:33 ` [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings Bryan O'Donoghue
  2022-07-20  7:24   ` Loic Poulain
@ 2022-07-27 10:31   ` Kalle Valo
  2022-07-27 10:41     ` Bryan O'Donoghue
  1 sibling, 1 reply; 12+ messages in thread
From: Kalle Valo @ 2022-07-27 10:31 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: loic.poulain, davem, edumazet, kuba, pabeni, wcn36xx,
	linux-wireless, netdev

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Add in the ability to easily find the firmware feature bits reported in the
> get feature exchange without having to compile-in debug prints.
>
> root@linaro-alip:~# cat /sys/kernel/debug/ieee80211/phy0/wcn36xx/firmware_feat_caps
> MCC
> P2P
> DOT11AC
> SLM_SESSIONIZATION
> DOT11AC_OPMODE
> SAP32STA
> TDLS
> P2P_GO_NOA_DECOUPLE_INIT_SCAN
> WLANACTIVE_OFFLOAD
> BEACON_OFFLOAD
> SCAN_OFFLOAD
> BCN_MISS_OFFLOAD
> STA_POWERSAVE
> STA_ADVANCED_PWRSAVE
> BCN_FILTER
> RTT
> RATECTRL
> WOW
> WLAN_ROAM_SCAN_OFFLOAD
> SPECULATIVE_PS_POLL
> IBSS_HEARTBEAT_OFFLOAD
> WLAN_SCAN_OFFLOAD
> WLAN_PERIODIC_TX_PTRN
> ADVANCE_TDLS
> BATCH_SCAN
> FW_IN_TX_PATH
> EXTENDED_NSOFFLOAD_SLOT
> CH_SWITCH_V1
> HT40_OBSS_SCAN
> UPDATE_CHANNEL_LIST
> WLAN_MCADDR_FLT
> WLAN_CH144
> TDLS_SCAN_COEXISTENCE
> LINK_LAYER_STATS_MEAS
> MU_MIMO
> EXTENDED_SCAN
> DYNAMIC_WMM_PS
> MAC_SPOOFED_SCAN
> FW_STATS
> WPS_PRBRSP_TMPL
> BCN_IE_FLT_DELTA
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[...]

> +static ssize_t read_file_firmware_feature_caps(struct file *file,
> +					       char __user *user_buf,
> +					       size_t count, loff_t *ppos)
> +{
> +	struct wcn36xx *wcn = file->private_data;
> +	unsigned long page = get_zeroed_page(GFP_KERNEL);
> +	char *p = (char *)page;
> +	int i;
> +	int ret;
> +
> +	if (!p)
> +		return -ENOMEM;
> +
> +	mutex_lock(&wcn->hal_mutex);
> +	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
> +		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
> +			p += sprintf(p, "%s\n",
> +				     wcn36xx_firmware_get_cap_name(i));
> +		}
> +	}
> +	mutex_unlock(&wcn->hal_mutex);
> +
> +	ret = simple_read_from_buffer(user_buf, count, ppos, (char *)page,
> +				      (unsigned long)p - page);
> +
> +	free_page(page);
> +	return ret;
> +}

Why not use the normal use kzalloc() and kfree()? That way you would not
need a separate page variable. What's the benefit from
get_zeroed_page()?

Also I don't see any checks for a memory allocation failure.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings
  2022-07-27 10:31   ` Kalle Valo
@ 2022-07-27 10:41     ` Bryan O'Donoghue
  2022-07-27 11:08       ` Kalle Valo
  0 siblings, 1 reply; 12+ messages in thread
From: Bryan O'Donoghue @ 2022-07-27 10:41 UTC (permalink / raw)
  To: Kalle Valo
  Cc: loic.poulain, davem, edumazet, kuba, pabeni, wcn36xx,
	linux-wireless, netdev

On 27/07/2022 11:31, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

>> +static ssize_t read_file_firmware_feature_caps(struct file *file,
>> +					       char __user *user_buf,
>> +					       size_t count, loff_t *ppos)
>> +{
>> +	struct wcn36xx *wcn = file->private_data;
>> +	unsigned long page = get_zeroed_page(GFP_KERNEL);
>> +	char *p = (char *)page;
>> +	int i;
>> +	int ret;
>> +
>> +	if (!p)
>> +		return -ENOMEM;
>> +
>> +	mutex_lock(&wcn->hal_mutex);
>> +	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
>> +		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
>> +			p += sprintf(p, "%s\n",
>> +				     wcn36xx_firmware_get_cap_name(i));
>> +		}
>> +	}
>> +	mutex_unlock(&wcn->hal_mutex);
>> +
>> +	ret = simple_read_from_buffer(user_buf, count, ppos, (char *)page,
>> +				      (unsigned long)p - page);
>> +
>> +	free_page(page);
>> +	return ret;
>> +}
> 
> Why not use the normal use kzalloc() and kfree()? That way you would not
> need a separate page variable. What's the benefit from
> get_zeroed_page()?


TBH I did a copy/paste here from another driver... I forget which
> 
> Also I don't see any checks for a memory allocation failure.
> 

its there

char *p = (char*) page;

if (!p)
     return -ENOMEM;

I can V2 this for kzalloc and kfree if you prefer though

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings
  2022-07-27 10:41     ` Bryan O'Donoghue
@ 2022-07-27 11:08       ` Kalle Valo
  0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2022-07-27 11:08 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: loic.poulain, davem, edumazet, kuba, pabeni, wcn36xx,
	linux-wireless, netdev

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> On 27/07/2022 11:31, Kalle Valo wrote:
>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>
>>> +static ssize_t read_file_firmware_feature_caps(struct file *file,
>>> +					       char __user *user_buf,
>>> +					       size_t count, loff_t *ppos)
>>> +{
>>> +	struct wcn36xx *wcn = file->private_data;
>>> +	unsigned long page = get_zeroed_page(GFP_KERNEL);
>>> +	char *p = (char *)page;
>>> +	int i;
>>> +	int ret;
>>> +
>>> +	if (!p)
>>> +		return -ENOMEM;
>>> +
>>> +	mutex_lock(&wcn->hal_mutex);
>>> +	for (i = 0; i < MAX_FEATURE_SUPPORTED; i++) {
>>> +		if (wcn36xx_firmware_get_feat_caps(wcn->fw_feat_caps, i)) {
>>> +			p += sprintf(p, "%s\n",
>>> +				     wcn36xx_firmware_get_cap_name(i));
>>> +		}
>>> +	}
>>> +	mutex_unlock(&wcn->hal_mutex);
>>> +
>>> +	ret = simple_read_from_buffer(user_buf, count, ppos, (char *)page,
>>> +				      (unsigned long)p - page);
>>> +
>>> +	free_page(page);
>>> +	return ret;
>>> +}
>>
>> Why not use the normal use kzalloc() and kfree()? That way you would not
>> need a separate page variable. What's the benefit from
>> get_zeroed_page()?
>
>
> TBH I did a copy/paste here from another driver... I forget which
>>
>> Also I don't see any checks for a memory allocation failure.
>>
>
> its there
>
> char *p = (char*) page;
>
> if (!p)
>     return -ENOMEM;

Ah, it's pretty evil to have the error handling so far away from the
actual call :)

> I can V2 this for kzalloc and kfree if you prefer though

Yes, please do that. We should use standard infrastructure as much as
possible.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-07-27 11:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 14:32 [PATCH v2 0/4] wcn36xx: Add in debugfs export of firmware feature bits Bryan O'Donoghue
2022-07-19 14:32 ` [PATCH v2 1/4] wcn36xx: Rename clunky firmware feature bit enum Bryan O'Donoghue
2022-07-20  7:14   ` Loic Poulain
2022-07-19 14:33 ` [PATCH v2 2/4] wcn36xx: Move firmware feature bit storage to dedicated firmware.c file Bryan O'Donoghue
2022-07-20  7:16   ` Loic Poulain
2022-07-19 14:33 ` [PATCH v2 3/4] wcn36xx: Move capability bitmap to string translation function to firmware.c Bryan O'Donoghue
2022-07-20  7:21   ` Loic Poulain
2022-07-19 14:33 ` [PATCH v2 4/4] wcn36xx: Add debugfs entry to read firmware feature strings Bryan O'Donoghue
2022-07-20  7:24   ` Loic Poulain
2022-07-27 10:31   ` Kalle Valo
2022-07-27 10:41     ` Bryan O'Donoghue
2022-07-27 11:08       ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).