Linux-Wireless Archive on lore.kernel.org
 help / Atom feed
* [PATCH 1/6] ath10k: add missing values to wmi_service_name()
@ 2019-02-11 15:09 Kalle Valo
  2019-02-11 15:09 ` [PATCH 2/6] ath10k: make wmi_service_name() warn about missing service ids Kalle Valo
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

After implementing the next patch GCC reported:

drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_BTCOEX' not handled in switch [-Wswitch]
drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_MGMT_TX_WMI' not handled in switch [-Wswitch]
drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_SPOOF_MAC_SUPPORT' not handled in switch [-Wswitch]
drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_VDEV_DISABLE_4_ADDR_SRC_LRN_SUPPORT' not handled in switch [-Wswitch]
drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_BB_TIMING_CONFIG_SUPPORT' not handled in switch [-Wswitch]
drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_THERM_THROT' not handled in switch [-Wswitch]

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/wmi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index b10ed523b99e..529f5a26f3f3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -475,6 +475,7 @@ static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_TX_MODE_PUSH_PULL);
 	SVCSTR(WMI_SERVICE_TX_MODE_DYNAMIC);
 	SVCSTR(WMI_SERVICE_VDEV_RX_FILTER);
+	SVCSTR(WMI_SERVICE_BTCOEX);
 	SVCSTR(WMI_SERVICE_CHECK_CAL_VERSION);
 	SVCSTR(WMI_SERVICE_DBGLOG_WARN2);
 	SVCSTR(WMI_SERVICE_BTCOEX_DUTY_CYCLE);
@@ -484,13 +485,18 @@ static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_SMART_LOGGING_SUPPORT);
 	SVCSTR(WMI_SERVICE_TDLS_CONN_TRACKER_IN_HOST_MODE);
 	SVCSTR(WMI_SERVICE_TDLS_EXPLICIT_MODE_ONLY);
+	SVCSTR(WMI_SERVICE_MGMT_TX_WMI);
 	SVCSTR(WMI_SERVICE_TDLS_WIDER_BANDWIDTH);
 	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
 	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
 	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
 	SVCSTR(WMI_SERVICE_RESET_CHIP);
+	SVCSTR(WMI_SERVICE_SPOOF_MAC_SUPPORT);
 	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
 	SVCSTR(WMI_SERVICE_VDEV_DIFFERENT_BEACON_INTERVAL_SUPPORT);
+	SVCSTR(WMI_SERVICE_VDEV_DISABLE_4_ADDR_SRC_LRN_SUPPORT);
+	SVCSTR(WMI_SERVICE_BB_TIMING_CONFIG_SUPPORT);
+	SVCSTR(WMI_SERVICE_THERM_THROT);
 	SVCSTR(WMI_SERVICE_RTT_RESPONDER_ROLE);
 	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
 	SVCSTR(WMI_SERVICE_REPORT_AIRTIME);
-- 
2.7.4


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

* [PATCH 2/6] ath10k: make wmi_service_name() warn about missing service ids
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
@ 2019-02-11 15:09 ` Kalle Valo
  2019-02-11 15:09 ` [PATCH 3/6] ath10k: change wmi.h to include only ieee80211.h Kalle Valo
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

When adding a new value to enum wmi_service it's very easy to miss that the new
value should be also added to wmi_service_name() mapping function. Modify the
function so that GCC can now warn about this:

drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_FOO' not handled in switch [-Wswitch]

And also add a reminder to the enum.

Thanks to Jouni Malinen for the idea.

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/wmi.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 529f5a26f3f3..74ce1a4c0e8f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -212,6 +212,8 @@ enum wmi_service {
 	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
 	WMI_SERVICE_REPORT_AIRTIME,
 
+	/* Remember to add the new value to wmi_service_name()! */
+
 	/* keep last */
 	WMI_SERVICE_MAX,
 };
@@ -378,7 +380,7 @@ enum wmi_10_4_service {
 	WMI_10_4_SERVICE_REPORT_AIRTIME,
 };
 
-static inline char *wmi_service_name(int service_id)
+static inline char *wmi_service_name(enum wmi_service service_id)
 {
 #define SVCSTR(x) case x: return #x
 
@@ -501,11 +503,13 @@ static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
 	SVCSTR(WMI_SERVICE_REPORT_AIRTIME);
 
-	default:
+	case WMI_SERVICE_MAX:
 		return NULL;
 	}
 
 #undef SVCSTR
+
+	return NULL;
 }
 
 #define WMI_SERVICE_IS_ENABLED(wmi_svc_bmap, svc_id, len) \
-- 
2.7.4


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

* [PATCH 3/6] ath10k: change wmi.h to include only ieee80211.h
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
  2019-02-11 15:09 ` [PATCH 2/6] ath10k: make wmi_service_name() warn about missing service ids Kalle Valo
@ 2019-02-11 15:09 ` Kalle Valo
  2019-02-11 15:09 ` [PATCH 4/6] ath10k: align ath10k_htt_txbuf structures Kalle Valo
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

wmi.h does not use anything from mac80211.h so change it to include only
ieee80211.h.

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/wmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 74ce1a4c0e8f..23fe482834b4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -20,7 +20,7 @@
 #define _WMI_H_
 
 #include <linux/types.h>
-#include <net/mac80211.h>
+#include <linux/ieee80211.h>
 
 /*
  * This file specifies the WMI interface for the Unified Software
-- 
2.7.4


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

* [PATCH 4/6] ath10k: align ath10k_htt_txbuf structures
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
  2019-02-11 15:09 ` [PATCH 2/6] ath10k: make wmi_service_name() warn about missing service ids Kalle Valo
  2019-02-11 15:09 ` [PATCH 3/6] ath10k: change wmi.h to include only ieee80211.h Kalle Valo
@ 2019-02-11 15:09 ` Kalle Valo
  2019-02-11 15:09 ` [PATCH 5/6] ath10k: fix documentation in ath10k_wow_convert_8023_to_80211() Kalle Valo
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

With W=1 GCC warns:

drivers/net/wireless/ath/ath10k/htt.h:1746:1: warning: alignment 1 of 'struct ath10k_htt_txbuf_32' is less than 4 [-Wpacked-not-aligned]
drivers/net/wireless/ath/ath10k/htt.h:1753:1: warning: alignment 1 of 'struct ath10k_htt_txbuf_64' is less than 4 [-Wpacked-not-aligned]

Fix that by using __align(4). Compile tested only.

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/htt.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 005fad94edad..d194bbe18fc3 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1743,14 +1743,14 @@ struct ath10k_htt_txbuf_32 {
 	struct ath10k_htc_hdr htc_hdr;
 	struct htt_cmd_hdr cmd_hdr;
 	struct htt_data_tx_desc cmd_tx;
-} __packed;
+} __packed __aligned(4);
 
 struct ath10k_htt_txbuf_64 {
 	struct htt_data_tx_desc_frag frags[2];
 	struct ath10k_htc_hdr htc_hdr;
 	struct htt_cmd_hdr cmd_hdr;
 	struct htt_data_tx_desc_64 cmd_tx;
-} __packed;
+} __packed __aligned(4);
 
 struct ath10k_htt {
 	struct ath10k *ar;
-- 
2.7.4


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

* [PATCH 5/6] ath10k: fix documentation in ath10k_wow_convert_8023_to_80211()
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
                   ` (2 preceding siblings ...)
  2019-02-11 15:09 ` [PATCH 4/6] ath10k: align ath10k_htt_txbuf structures Kalle Valo
@ 2019-02-11 15:09 ` Kalle Valo
  2019-02-11 15:09 ` [PATCH 6/6] ath10k: copy the whole struct ath10k_bus_params in ath10k_core_register() Kalle Valo
  2019-02-12 18:51 ` [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

With W=1 there's a warning:

drivers/net/wireless/ath/ath10k/wow.c:93: warning: Function parameter or member 'new' not described in 'ath10k_wow_convert_8023_to_80211'
drivers/net/wireless/ath/ath10k/wow.c:93: warning: Function parameter or member 'old' not described in 'ath10k_wow_convert_8023_to_80211'

Fix it by changing the documentation marker '/**' to a normal code comment.
While at it, clean up the line wrapping.

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/wow.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wow.c b/drivers/net/wireless/ath/ath10k/wow.c
index 36d4245c308e..b36fa0f8892e 100644
--- a/drivers/net/wireless/ath/ath10k/wow.c
+++ b/drivers/net/wireless/ath/ath10k/wow.c
@@ -77,7 +77,7 @@ static int ath10k_wow_cleanup(struct ath10k *ar)
 	return 0;
 }
 
-/**
+/*
  * Convert a 802.3 format to a 802.11 format.
  *         +------------+-----------+--------+----------------+
  * 802.3:  |dest mac(6B)|src mac(6B)|type(2B)|     body...    |
@@ -88,9 +88,8 @@ static int ath10k_wow_cleanup(struct ath10k *ar)
  * 802.11: |4B|dest mac(6B)| 6B |src mac(6B)|  8B  |type(2B)|  body...  |
  *         +--+------------+----+-----------+---------------+-----------+
  */
-static void ath10k_wow_convert_8023_to_80211
-					(struct cfg80211_pkt_pattern *new,
-					const struct cfg80211_pkt_pattern *old)
+static void ath10k_wow_convert_8023_to_80211(struct cfg80211_pkt_pattern *new,
+					     const struct cfg80211_pkt_pattern *old)
 {
 	u8 hdr_8023_pattern[ETH_HLEN] = {};
 	u8 hdr_8023_bit_mask[ETH_HLEN] = {};
-- 
2.7.4


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

* [PATCH 6/6] ath10k: copy the whole struct ath10k_bus_params in ath10k_core_register()
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
                   ` (3 preceding siblings ...)
  2019-02-11 15:09 ` [PATCH 5/6] ath10k: fix documentation in ath10k_wow_convert_8023_to_80211() Kalle Valo
@ 2019-02-11 15:09 ` Kalle Valo
  2019-02-12 18:51 ` [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-11 15:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

Instead of copying fields one by one copy the whole structure. This way there's
no need to modify the function every time we add a new field to the struct.

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 6c1dd5f8d012..a1b2aea4a77f 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2997,9 +2997,8 @@ static void ath10k_core_register_work(struct work_struct *work)
 int ath10k_core_register(struct ath10k *ar,
 			 const struct ath10k_bus_params *bus_params)
 {
-	ar->bus_param.chip_id = bus_params->chip_id;
-	ar->bus_param.dev_type = bus_params->dev_type;
-	ar->bus_param.link_can_suspend = bus_params->link_can_suspend;
+	ar->bus_param = *bus_params;
+
 	queue_work(ar->workqueue, &ar->register_work);
 
 	return 0;
-- 
2.7.4


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

* Re: [PATCH 1/6] ath10k: add missing values to wmi_service_name()
  2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
                   ` (4 preceding siblings ...)
  2019-02-11 15:09 ` [PATCH 6/6] ath10k: copy the whole struct ath10k_bus_params in ath10k_core_register() Kalle Valo
@ 2019-02-12 18:51 ` Kalle Valo
  5 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-02-12 18:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k, linux-wireless

Kalle Valo <kvalo@codeaurora.org> wrote:

> After implementing the next patch GCC reported:
> 
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_BTCOEX' not handled in switch [-Wswitch]
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_MGMT_TX_WMI' not handled in switch [-Wswitch]
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_SPOOF_MAC_SUPPORT' not handled in switch [-Wswitch]
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_VDEV_DISABLE_4_ADDR_SRC_LRN_SUPPORT' not handled in switch [-Wswitch]
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_BB_TIMING_CONFIG_SUPPORT' not handled in switch [-Wswitch]
> drivers/net/wireless/ath/ath10k/wmi.h:385:2: warning: enumeration value 'WMI_SERVICE_THERM_THROT' not handled in switch [-Wswitch]
> 
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

6 patches applied to ath-next branch of ath.git, thanks.

2321dd5d78fb ath10k: add missing values to wmi_service_name()
95cccf4d79fe ath10k: make wmi_service_name() warn about missing service ids
db3b6280f5f1 ath10k: change wmi.h to include only ieee80211.h
385bd8816cb5 ath10k: align ath10k_htt_txbuf structures
bdf2bd9aa684 ath10k: fix documentation in ath10k_wow_convert_8023_to_80211()
01dc76dfdc91 ath10k: copy the whole struct ath10k_bus_params in ath10k_core_register()

-- 
https://patchwork.kernel.org/patch/10806063/

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


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 15:09 [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo
2019-02-11 15:09 ` [PATCH 2/6] ath10k: make wmi_service_name() warn about missing service ids Kalle Valo
2019-02-11 15:09 ` [PATCH 3/6] ath10k: change wmi.h to include only ieee80211.h Kalle Valo
2019-02-11 15:09 ` [PATCH 4/6] ath10k: align ath10k_htt_txbuf structures Kalle Valo
2019-02-11 15:09 ` [PATCH 5/6] ath10k: fix documentation in ath10k_wow_convert_8023_to_80211() Kalle Valo
2019-02-11 15:09 ` [PATCH 6/6] ath10k: copy the whole struct ath10k_bus_params in ath10k_core_register() Kalle Valo
2019-02-12 18:51 ` [PATCH 1/6] ath10k: add missing values to wmi_service_name() Kalle Valo

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox