[ath9k-devel] [PATCH 0/5] ath10k: making static variables const

[ath9k-devel] [PATCH 0/5] ath10k: making static variables const

[Kalle Valo]

Did some review of static variables and noticed that
we had quite a few variables which were not const.

Please review carefully, I might have missed something.

---

Kalle Valo (5):
      ath10k: make rx_legacy_rate_idx variable const
      ath10k: make host_ce_config_wlan static variable const
      ath10k: make target_ce_config_wlan and target_service_to_ce_map_wlan const
      ath10k: make iee80211 iface combination structures static
      ath10k: copy ieee80211_supported_band for each instance


 drivers/net/wireless/ath/ath10k/ce.c   |    8 ++--
 drivers/net/wireless/ath/ath10k/ce.h   |    2 -
 drivers/net/wireless/ath/ath10k/core.h |    4 ++
 drivers/net/wireless/ath/ath10k/mac.c  |   67 ++++++++++++++++++++------------
 drivers/net/wireless/ath/ath10k/pci.c  |   12 +++---
 drivers/net/wireless/ath/ath10k/txrx.c |    2 -
 6 files changed, 57 insertions(+), 38 deletions(-)

[ath9k-devel] [PATCH 1/5] ath10k: make rx_legacy_rate_idx variable const

[Kalle Valo]

It's not modified in any way.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/txrx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 6a66dc2..239eff5 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -120,7 +120,7 @@ void ath10k_txrx_tx_completed(struct htt_struct *htt,
 	ath10k_txrx_tx_unref(htt, txdesc);
 }
 
-static u8 rx_legacy_rate_idx[] = {
+static const u8 rx_legacy_rate_idx[] = {
 	3,	/* 0x00  - 11Mbps  */
 	2,	/* 0x01  - 5.5Mbps */
 	1,	/* 0x02  - 2Mbps   */

[ath9k-devel] [PATCH 2/5] ath10k: make host_ce_config_wlan static variable const

[Kalle Valo]

Nothing modifies it.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/ce.c  |    8 ++++----
 drivers/net/wireless/ath/ath10k/ce.h  |    2 +-
 drivers/net/wireless/ath/ath10k/pci.c |    8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index d5351a3..d3aa515 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -703,7 +703,7 @@ void ath10k_ce_recv_cb_register(struct ce_state *ce_state,
 static int ath10k_ce_init_src_ring(struct ath10k *ar,
 				   unsigned int ce_id,
 				   struct ce_state *ce_state,
-				   struct ce_attr *attr)
+				   const struct ce_attr *attr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_ring_state *src_ring;
@@ -795,7 +795,7 @@ static int ath10k_ce_init_src_ring(struct ath10k *ar,
 static int ath10k_ce_init_dest_ring(struct ath10k *ar,
 				    unsigned int ce_id,
 				    struct ce_state *ce_state,
-				    struct ce_attr *attr)
+				    const struct ce_attr *attr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_ring_state *dest_ring;
@@ -877,7 +877,7 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar,
 
 static struct ce_state *ath10k_ce_init_state(struct ath10k *ar,
 				      unsigned int ce_id,
-				      struct ce_attr *attr)
+				      const struct ce_attr *attr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_state *ce_state = NULL;
@@ -916,7 +916,7 @@ static struct ce_state *ath10k_ce_init_state(struct ath10k *ar,
  */
 struct ce_state *ath10k_ce_init(struct ath10k *ar,
 				unsigned int ce_id,
-				struct ce_attr *attr)
+				const struct ce_attr *attr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_state *ce_state;
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 084b997..01879eb 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -275,7 +275,7 @@ int ath10k_ce_completed_send_next(struct ce_state *ce_state,
 /* Initialize an instance of a CE */
 struct ce_state *ath10k_ce_init(struct ath10k *ar,
 				unsigned int ce_id,
-				struct ce_attr *attr);
+				const struct ce_attr *attr);
 
 /*==================CE Engine Shutdown=======================*/
 /*
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 0d1523b..22a5451 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -52,7 +52,7 @@ static int ath10k_pci_post_recv_buffers(struct ath10k *ar);
 static int ath10k_pci_post_recv_buffers_pipe(struct hif_ce_pipe_info *pipe_info,
 					     int num);
 
-static struct ce_attr host_ce_config_wlan[] = {
+static const struct ce_attr host_ce_config_wlan[] = {
 	/* host->target HTC control and raw streams */
 	{ /* CE0 */ CE_ATTR_FLAGS, 0, 16, 256, 0, NULL,},
 	/* could be moved to share CE3 */
@@ -770,7 +770,7 @@ static void ath10k_pci_start_ce(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_state *ce_diag = ar_pci->ce_diag;
-	struct ce_attr *attr;
+	const struct ce_attr *attr;
 	struct hif_ce_pipe_info *pipe_info;
 	struct hif_ce_completion_state *compl;
 	int i, pipe_num, completions;
@@ -1075,7 +1075,7 @@ static int ath10k_pci_post_recv_buffers(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct hif_ce_pipe_info *pipe_info;
-	struct ce_attr *attr;
+	const struct ce_attr *attr;
 	int pipe_num, ret = 0;
 
 	for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
@@ -1622,7 +1622,7 @@ static void ath10k_pci_ce_init(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct hif_ce_pipe_info *pipe_info;
-	struct ce_attr *attr;
+	const struct ce_attr *attr;
 	int pipe_num;
 
 	for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {

[ath9k-devel] [PATCH 3/5] ath10k: make target_ce_config_wlan and target_service_to_ce_map_wlan const

[Kalle Valo]

Nothing modifies them.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 22a5451..7a35234 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -74,7 +74,7 @@ static const struct ce_attr host_ce_config_wlan[] = {
 };
 
 /* Target firmware's Copy Engine configuration. */
-static struct ce_pipe_config target_ce_config_wlan[] = {
+static const struct ce_pipe_config target_ce_config_wlan[] = {
 	/* host->target HTC control and raw streams */
 	{ /* CE0 */ 0, PIPEDIR_OUT, 32, 256, CE_ATTR_FLAGS, 0,},
 	/* target->host HTT + HTC control */
@@ -1358,7 +1358,7 @@ static void ath10k_pci_bmi_recv_data(struct ce_state *ce_state,
  * This table is derived from the CE_PCI TABLE, above.
  * It is passed to the Target at startup for use by firmware.
  */
-static struct service_to_pipe target_service_to_ce_map_wlan[] = {
+static const struct service_to_pipe target_service_to_ce_map_wlan[] = {
 	{
 		 ATH10K_HTC_SVC_ID_WMI_DATA_VO,
 		 PIPEDIR_OUT,		/* out = UL = host -> target */

[ath9k-devel] [PATCH 4/5] ath10k: make iee80211 iface combination structures static

[Kalle Valo]

Nothing modifies them.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 60855fb..8f58dda 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2531,7 +2531,7 @@ void ath10k_mac_destroy(struct ath10k *ar)
 	ieee80211_free_hw(ar->hw);
 }
 
-static struct ieee80211_iface_limit ath10k_if_limits[] = {
+static const struct ieee80211_iface_limit ath10k_if_limits[] = {
 	{
 	.max	= 8,
 	.types	= BIT(NL80211_IFTYPE_STATION)
@@ -2541,7 +2541,7 @@ static struct ieee80211_iface_limit ath10k_if_limits[] = {
 	}
 };
 
-static struct ieee80211_iface_combination ath10k_if_comb = {
+static const struct ieee80211_iface_combination ath10k_if_comb = {
 	.limits = ath10k_if_limits,
 	.n_limits = ARRAY_SIZE(ath10k_if_limits),
 	.max_interfaces = 8,

[ath9k-devel] [PATCH 5/5] ath10k: copy ieee80211_supported_band for each instance

[Kalle Valo]

If we run multiple instances of ath10k the supported band structures were shared
with all instances. This is wrong and would cause corruption. Fix it by making
a private copy for each instance.

I didn't copy the rate structure ath10k_rates as ath9k also doesn't copy it.
Most likely nothing modifies that structure, but that needs to be verified.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.h |    4 ++
 drivers/net/wireless/ath/ath10k/mac.c  |   63 ++++++++++++++++++++------------
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 42d9e72..c50ef59 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -322,6 +322,10 @@ struct ath10k {
 		int vdev_id;
 	} scan;
 
+	struct {
+		struct ieee80211_supported_band sbands[IEEE80211_NUM_BANDS];
+	} mac;
+
 	/* should never be NULL; needed for regular htt rx */
 	struct ieee80211_channel __rcu *rx_channel;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8f58dda..20fba5d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2429,7 +2429,7 @@ static const struct ieee80211_ops ath10k_ops = {
 	.max_power		= 30, \
 }
 
-static struct ieee80211_channel ath10k_2ghz_channels[] = {
+static const struct ieee80211_channel ath10k_2ghz_channels[] = {
 	CHAN2G(1, 2412, 0),
 	CHAN2G(2, 2417, 0),
 	CHAN2G(3, 2422, 0),
@@ -2446,7 +2446,7 @@ static struct ieee80211_channel ath10k_2ghz_channels[] = {
 	CHAN2G(14, 2484, 0),
 };
 
-static struct ieee80211_channel ath10k_5ghz_channels[] = {
+static const struct ieee80211_channel ath10k_5ghz_channels[] = {
 	CHAN5G(36, 5180, 14),
 	CHAN5G(40, 5200, 15),
 	CHAN5G(44, 5220, 16),
@@ -2495,22 +2495,6 @@ static struct ieee80211_rate ath10k_rates[] = {
 #define ath10k_g_rates (ath10k_rates + 0)
 #define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))
 
-static struct ieee80211_supported_band ath10k_band_2ghz = {
-	.n_channels = ARRAY_SIZE(ath10k_2ghz_channels),
-	.channels = ath10k_2ghz_channels,
-	.n_bitrates = ath10k_g_rates_size,
-	.bitrates = ath10k_g_rates,
-	/* .ht_cap depends on FW capabilities */
-};
-
-static struct ieee80211_supported_band ath10k_band_5ghz = {
-	.n_channels = ARRAY_SIZE(ath10k_5ghz_channels),
-	.channels = ath10k_5ghz_channels,
-	.n_bitrates = ath10k_a_rates_size,
-	.bitrates = ath10k_a_rates,
-	/* .ht_cap depends on FW capabilities */
-};
-
 struct ath10k *ath10k_mac_create(void)
 {
 	struct ieee80211_hw *hw;
@@ -2652,7 +2636,9 @@ struct ath10k_vif *ath10k_get_arvif(struct ath10k *ar, u32 vdev_id)
 int ath10k_mac_register(struct ath10k *ar)
 {
 	struct ath_common *common = ath10k_common(ar);
+	struct ieee80211_supported_band *band;
 	struct ieee80211_sta_ht_cap ht_cap;
+	void *channels;
 	int ret;
 
 	SET_IEEE80211_PERM_ADDR(ar->hw, ar->mac_addr);
@@ -2660,14 +2646,40 @@ int ath10k_mac_register(struct ath10k *ar)
 	SET_IEEE80211_DEV(ar->hw, ar->dev);
 
 	ht_cap = ath10k_get_ht_cap(ar);
-	ath10k_band_2ghz.ht_cap = ht_cap;
-	ath10k_band_5ghz.ht_cap = ht_cap;
 
-	if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY)
-		ar->hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &ath10k_band_2ghz;
+	if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY) {
+		channels = kmemdup(ath10k_2ghz_channels,
+				   sizeof(ath10k_2ghz_channels),
+				   GFP_KERNEL);
+		if (!channels)
+			return -ENOMEM;
+
+		band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
+		band->n_channels = ARRAY_SIZE(ath10k_2ghz_channels);
+		band->channels = channels;
+		band->n_bitrates = ath10k_g_rates_size;
+		band->bitrates = ath10k_g_rates;
+		band->ht_cap = ht_cap;
+		ar->hw->wiphy->bands[IEEE80211_BAND_2GHZ] = band;
+	}
 
-	if (ar->phy_capability & WHAL_WLAN_11A_CAPABILITY)
-		ar->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = &ath10k_band_5ghz;
+
+
+	if (ar->phy_capability & WHAL_WLAN_11A_CAPABILITY) {
+		channels = kmemdup(ath10k_5ghz_channels,
+				   sizeof(ath10k_5ghz_channels),
+				   GFP_KERNEL);
+		if (!channels)
+			return -ENOMEM;
+
+		band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
+		band->n_channels = ARRAY_SIZE(ath10k_5ghz_channels);
+		band->channels = channels;
+		band->n_bitrates = ath10k_a_rates_size;
+		band->bitrates = ath10k_a_rates;
+		band->ht_cap = ht_cap;
+		ar->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = band;
+	}
 
 	ar->hw->wiphy->interface_modes =
 		BIT(NL80211_IFTYPE_STATION) |
@@ -2745,5 +2757,8 @@ void ath10k_mac_unregister(struct ath10k *ar)
 {
 	ieee80211_unregister_hw(ar->hw);
 
+	kfree(ar->mac.sbands[IEEE80211_BAND_2GHZ].channels);
+	kfree(ar->mac.sbands[IEEE80211_BAND_5GHZ].channels);
+
 	SET_IEEE80211_DEV(ar->hw, NULL);
 }

[ath9k-devel] [PATCH 4/5] ath10k: make iee80211 iface combination structures static

[Michal Kazior]

On 24/04/13 08:37, Kalle Valo wrote:
> Nothing modifies them.

I think the title should be s/static/const/


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH 4/5] ath10k: make iee80211 iface combination structures static

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 24/04/13 08:37, Kalle Valo wrote:
>> Nothing modifies them.
>
> I think the title should be s/static/const/

Yup, I fixed that in my branch.

Thanks for review.

-- 
Kalle Valo

[ath9k-devel] [PATCH 0/5] ath10k: making static variables const

[Kalle Valo]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Did some review of static variables and noticed that
> we had quite a few variables which were not const.
>
> Please review carefully, I might have missed something.
>
> ---
>
> Kalle Valo (5):
>       ath10k: make rx_legacy_rate_idx variable const
>       ath10k: make host_ce_config_wlan static variable const
>       ath10k: make target_ce_config_wlan and target_service_to_ce_map_wlan const
>       ath10k: make iee80211 iface combination structures static
>       ath10k: copy ieee80211_supported_band for each instance

Applied.

-- 
Kalle Valo