* [PATCH] wl12xx: fix some sparse warnings
@ 2010-12-22 11:38 Eliad Peller
2011-01-10 16:02 ` Luciano Coelho
0 siblings, 1 reply; 6+ messages in thread
From: Eliad Peller @ 2010-12-22 11:38 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
Note that wl1271_write32() calls cpu_to_le32() by itself, so calling
wl1271_write32(addr, cpu_to_le32(val)) is in fact a bug on BE systems.
Fix the following sparse warnings:
drivers/net/wireless/wl12xx/cmd.c:662:16: warning: incorrect type in assignment (different base types)
drivers/net/wireless/wl12xx/cmd.c:662:16: expected unsigned short [unsigned] [addressable] [usertype] llc_type
drivers/net/wireless/wl12xx/cmd.c:662:16: got restricted __be16 [usertype] <noident>
drivers/net/wireless/wl12xx/cmd.c:674:17: warning: incorrect type in assignment (different base types)
drivers/net/wireless/wl12xx/cmd.c:674:17: expected unsigned int [unsigned] [addressable] [usertype] sender_ip
drivers/net/wireless/wl12xx/cmd.c:674:17: got restricted __be32 [usertype] ip_addr
drivers/net/wireless/wl12xx/rx.c:202:4: warning: incorrect type in argument 3 (different base types)
drivers/net/wireless/wl12xx/rx.c:202:4: expected unsigned int [unsigned] [usertype] val
drivers/net/wireless/wl12xx/rx.c:202:4: got restricted __le32 [usertype] <noident>
drivers/net/wireless/wl12xx/acx.c:1247:23: warning: incorrect type in assignment (different base types)
drivers/net/wireless/wl12xx/acx.c:1247:23: expected restricted __le32 [usertype] ht_capabilites
drivers/net/wireless/wl12xx/acx.c:1247:23: got unsigned long
drivers/net/wireless/wl12xx/acx.c:1250:24: warning: invalid assignment: |=
drivers/net/wireless/wl12xx/acx.c:1250:24: left side has type restricted __le32
drivers/net/wireless/wl12xx/acx.c:1250:24: right side has type unsigned long
drivers/net/wireless/wl12xx/acx.c:1253:24: warning: invalid assignment: |=
drivers/net/wireless/wl12xx/acx.c:1253:24: left side has type restricted __le32
drivers/net/wireless/wl12xx/acx.c:1253:24: right side has type unsigned long
drivers/net/wireless/wl12xx/acx.c:1256:24: warning: invalid assignment: |=
drivers/net/wireless/wl12xx/acx.c:1256:24: left side has type restricted __le32
drivers/net/wireless/wl12xx/acx.c:1256:24: right side has type unsigned long
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/acx.c | 13 +++++++------
drivers/net/wireless/wl12xx/cmd.c | 8 ++++----
drivers/net/wireless/wl12xx/rx.c | 3 +--
drivers/net/wireless/wl12xx/wl12xx_80211.h | 6 +++---
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index cc4068d..17f6a63 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1233,6 +1233,7 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
struct wl1271_acx_ht_capabilities *acx;
u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
int ret = 0;
+ u32 ht_capabilites = 0;
wl1271_debug(DEBUG_ACX, "acx ht capabilities setting");
@@ -1244,16 +1245,16 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
/* Allow HT Operation ? */
if (allow_ht_operation) {
- acx->ht_capabilites =
+ ht_capabilites =
WL1271_ACX_FW_CAP_HT_OPERATION;
if (ht_cap->cap & IEEE80211_HT_CAP_GRN_FLD)
- acx->ht_capabilites |=
+ ht_capabilites |=
WL1271_ACX_FW_CAP_GREENFIELD_FRAME_FORMAT;
if (ht_cap->cap & IEEE80211_HT_CAP_SGI_20)
- acx->ht_capabilites |=
+ ht_capabilites |=
WL1271_ACX_FW_CAP_SHORT_GI_FOR_20MHZ_PACKETS;
if (ht_cap->cap & IEEE80211_HT_CAP_LSIG_TXOP_PROT)
- acx->ht_capabilites |=
+ ht_capabilites |=
WL1271_ACX_FW_CAP_LSIG_TXOP_PROTECTION;
/* get data from A-MPDU parameters field */
@@ -1261,10 +1262,10 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
acx->ampdu_min_spacing = ht_cap->ampdu_density;
memcpy(acx->mac_address, mac_address, ETH_ALEN);
- } else { /* HT operations are not allowed */
- acx->ht_capabilites = 0;
}
+ acx->ht_capabilites = cpu_to_le32(ht_capabilites);
+
ret = wl1271_cmd_configure(wl, ACX_PEER_HT_CAP, acx, sizeof(*acx));
if (ret < 0) {
wl1271_warning("acx ht capabilities setting failed: %d", ret);
diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index 0106628..52a6bcd 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -659,15 +659,15 @@ int wl1271_cmd_build_arp_rsp(struct wl1271 *wl, __be32 ip_addr)
/* llc layer */
memcpy(tmpl.llc_hdr, rfc1042_header, sizeof(rfc1042_header));
- tmpl.llc_type = htons(ETH_P_ARP);
+ tmpl.llc_type = cpu_to_be16(ETH_P_ARP);
/* arp header */
arp_hdr = &tmpl.arp_hdr;
- arp_hdr->ar_hrd = htons(ARPHRD_ETHER);
- arp_hdr->ar_pro = htons(ETH_P_IP);
+ arp_hdr->ar_hrd = cpu_to_be16(ARPHRD_ETHER);
+ arp_hdr->ar_pro = cpu_to_be16(ETH_P_IP);
arp_hdr->ar_hln = ETH_ALEN;
arp_hdr->ar_pln = 4;
- arp_hdr->ar_op = htons(ARPOP_REPLY);
+ arp_hdr->ar_op = cpu_to_be16(ARPOP_REPLY);
/* arp payload */
memcpy(tmpl.sender_hw, wl->vif->addr, ETH_ALEN);
diff --git a/drivers/net/wireless/wl12xx/rx.c b/drivers/net/wireless/wl12xx/rx.c
index 682304c..e855c86 100644
--- a/drivers/net/wireless/wl12xx/rx.c
+++ b/drivers/net/wireless/wl12xx/rx.c
@@ -198,6 +198,5 @@ void wl1271_rx(struct wl1271 *wl, struct wl1271_fw_status *status)
pkt_offset += pkt_length;
}
}
- wl1271_write32(wl, RX_DRIVER_COUNTER_ADDRESS,
- cpu_to_le32(wl->rx_counter));
+ wl1271_write32(wl, RX_DRIVER_COUNTER_ADDRESS, wl->rx_counter);
}
diff --git a/drivers/net/wireless/wl12xx/wl12xx_80211.h b/drivers/net/wireless/wl12xx/wl12xx_80211.h
index be21032..e2b26fb 100644
--- a/drivers/net/wireless/wl12xx/wl12xx_80211.h
+++ b/drivers/net/wireless/wl12xx/wl12xx_80211.h
@@ -138,13 +138,13 @@ struct wl12xx_arp_rsp_template {
struct ieee80211_hdr_3addr hdr;
u8 llc_hdr[sizeof(rfc1042_header)];
- u16 llc_type;
+ __be16 llc_type;
struct arphdr arp_hdr;
u8 sender_hw[ETH_ALEN];
- u32 sender_ip;
+ __be32 sender_ip;
u8 target_hw[ETH_ALEN];
- u32 target_ip;
+ __be32 target_ip;
} __packed;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: fix some sparse warnings
2010-12-22 11:38 [PATCH] wl12xx: fix some sparse warnings Eliad Peller
@ 2011-01-10 16:02 ` Luciano Coelho
0 siblings, 0 replies; 6+ messages in thread
From: Luciano Coelho @ 2011-01-10 16:02 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
On Wed, 2010-12-22 at 12:38 +0100, Eliad Peller wrote:
> Note that wl1271_write32() calls cpu_to_le32() by itself, so calling
> wl1271_write32(addr, cpu_to_le32(val)) is in fact a bug on BE systems.
>
> Fix the following sparse warnings:
>
> drivers/net/wireless/wl12xx/cmd.c:662:16: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/wl12xx/cmd.c:662:16: expected unsigned short [unsigned] [addressable] [usertype] llc_type
> drivers/net/wireless/wl12xx/cmd.c:662:16: got restricted __be16 [usertype] <noident>
> drivers/net/wireless/wl12xx/cmd.c:674:17: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/wl12xx/cmd.c:674:17: expected unsigned int [unsigned] [addressable] [usertype] sender_ip
> drivers/net/wireless/wl12xx/cmd.c:674:17: got restricted __be32 [usertype] ip_addr
> drivers/net/wireless/wl12xx/rx.c:202:4: warning: incorrect type in argument 3 (different base types)
> drivers/net/wireless/wl12xx/rx.c:202:4: expected unsigned int [unsigned] [usertype] val
> drivers/net/wireless/wl12xx/rx.c:202:4: got restricted __le32 [usertype] <noident>
> drivers/net/wireless/wl12xx/acx.c:1247:23: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/wl12xx/acx.c:1247:23: expected restricted __le32 [usertype] ht_capabilites
> drivers/net/wireless/wl12xx/acx.c:1247:23: got unsigned long
> drivers/net/wireless/wl12xx/acx.c:1250:24: warning: invalid assignment: |=
> drivers/net/wireless/wl12xx/acx.c:1250:24: left side has type restricted __le32
> drivers/net/wireless/wl12xx/acx.c:1250:24: right side has type unsigned long
> drivers/net/wireless/wl12xx/acx.c:1253:24: warning: invalid assignment: |=
> drivers/net/wireless/wl12xx/acx.c:1253:24: left side has type restricted __le32
> drivers/net/wireless/wl12xx/acx.c:1253:24: right side has type unsigned long
> drivers/net/wireless/wl12xx/acx.c:1256:24: warning: invalid assignment: |=
> drivers/net/wireless/wl12xx/acx.c:1256:24: left side has type restricted __le32
> drivers/net/wireless/wl12xx/acx.c:1256:24: right side has type unsigned long
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
Applied, thank you!
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] wl12xx: fix some sparse warnings
@ 2011-01-16 7:49 Eliad Peller
2011-01-16 8:28 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Eliad Peller @ 2011-01-16 7:49 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
fix the following warnings:
CHECK drivers/net/wireless/wl12xx/cmd.c
drivers/net/wireless/wl12xx/cmd.c:987:20: warning: incorrect type in assignment (different base types)
drivers/net/wireless/wl12xx/cmd.c:987:20: expected restricted __le16 [usertype] aging_period
drivers/net/wireless/wl12xx/cmd.c:987:20: got int
CHECK drivers/net/wireless/wl12xx/tx.c
drivers/net/wireless/wl12xx/tx.c:197:2: warning: cast from restricted __le16
drivers/net/wireless/wl12xx/tx.c:197:2: warning: cast from restricted __le16
drivers/net/wireless/wl12xx/tx.c:197:2: warning: cast from restricted __le16
CHECK drivers/net/wireless/wl12xx/acx.c
drivers/net/wireless/wl12xx/acx.c:816:23: warning: incorrect type in assignment (different base types)
drivers/net/wireless/wl12xx/acx.c:816:23: expected restricted __le32 [usertype] rate_policy_idx
drivers/net/wireless/wl12xx/acx.c:816:23: got unsigned char [unsigned] [usertype] idx
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/acx.c | 2 +-
drivers/net/wireless/wl12xx/cmd.c | 2 +-
drivers/net/wireless/wl12xx/tx.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index 646d278..df73a21 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -813,7 +813,7 @@ int wl1271_acx_ap_rate_policy(struct wl1271 *wl, struct conf_tx_rate_class *c,
acx->rate_policy.long_retry_limit = c->long_retry_limit;
acx->rate_policy.aflags = c->aflags;
- acx->rate_policy_idx = idx;
+ acx->rate_policy_idx = __cpu_to_le32(idx);
ret = wl1271_cmd_configure(wl, ACX_RATE_POLICY, acx, sizeof(*acx));
if (ret < 0) {
diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index e28d9ca..6ce188d 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -984,7 +984,7 @@ int wl1271_cmd_start_bss(struct wl1271 *wl)
memcpy(cmd->bssid, bss_conf->bssid, ETH_ALEN);
- cmd->aging_period = WL1271_AP_DEF_INACTIV_SEC;
+ cmd->aging_period = __cpu_to_le16(WL1271_AP_DEF_INACTIV_SEC);
cmd->bss_index = WL1271_AP_BSS_INDEX;
cmd->global_hlid = WL1271_AP_GLOBAL_HLID;
cmd->broadcast_hlid = WL1271_AP_BROADCAST_HLID;
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 2347f25..209d4db 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -195,9 +195,9 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
desc->tx_attr = cpu_to_le16(tx_attr);
wl1271_debug(DEBUG_TX, "tx_fill_hdr: pad: %d hlid: %d "
- "tx_attr: 0x%x len: %d life: %d mem: %d", pad, (int)desc->hlid,
- (int)desc->tx_attr, (int)desc->length, (int)desc->life_time,
- (int)desc->total_mem_blocks);
+ "tx_attr: 0x%x len: %d life: %d mem: %d", pad, desc->hlid,
+ __le16_to_cpu(desc->tx_attr), __le16_to_cpu(desc->length),
+ __le16_to_cpu(desc->life_time), desc->total_mem_blocks);
}
/* caller must hold wl->mutex */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: fix some sparse warnings
2011-01-16 7:49 Eliad Peller
@ 2011-01-16 8:28 ` Johannes Berg
2011-01-16 8:48 ` Eliad Peller
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2011-01-16 8:28 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless
On Sun, 2011-01-16 at 09:49 +0200, Eliad Peller wrote:
> @@ -813,7 +813,7 @@ int wl1271_acx_ap_rate_policy(struct wl1271 *wl, struct conf_tx_rate_class *c,
> acx->rate_policy.long_retry_limit = c->long_retry_limit;
> acx->rate_policy.aflags = c->aflags;
>
> - acx->rate_policy_idx = idx;
> + acx->rate_policy_idx = __cpu_to_le32(idx);
The underscore version doesn't look appropriate at this point or in fact
anywhere in this patch.
Also, your patch subject and commit log is wrong -- this doesn't just
fix sparse warnings, it fixes actual bugs pointed out by sparse, so
commit log should reflect that.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: fix some sparse warnings
2011-01-16 8:28 ` Johannes Berg
@ 2011-01-16 8:48 ` Eliad Peller
2011-01-16 8:52 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Eliad Peller @ 2011-01-16 8:48 UTC (permalink / raw)
To: Johannes Berg; +Cc: Luciano Coelho, linux-wireless
On Sun, Jan 16, 2011 at 10:28 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sun, 2011-01-16 at 09:49 +0200, Eliad Peller wrote:
>
>> @@ -813,7 +813,7 @@ int wl1271_acx_ap_rate_policy(struct wl1271 *wl, struct conf_tx_rate_class *c,
>> acx->rate_policy.long_retry_limit = c->long_retry_limit;
>> acx->rate_policy.aflags = c->aflags;
>>
>> - acx->rate_policy_idx = idx;
>> + acx->rate_policy_idx = __cpu_to_le32(idx);
>
> The underscore version doesn't look appropriate at this point or in fact
> anywhere in this patch.
>
> Also, your patch subject and commit log is wrong -- this doesn't just
> fix sparse warnings, it fixes actual bugs pointed out by sparse, so
> commit log should reflect that.
>
thanks for your review!
i'll send a new version.
what is the difference between the normal and underscore version?
atomicity? (i didn't find a documentation about it)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wl12xx: fix some sparse warnings
2011-01-16 8:48 ` Eliad Peller
@ 2011-01-16 8:52 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2011-01-16 8:52 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless
On Sun, 2011-01-16 at 10:48 +0200, Eliad Peller wrote:
> >> - acx->rate_policy_idx = idx;
> >> + acx->rate_policy_idx = __cpu_to_le32(idx);
> >
> > The underscore version doesn't look appropriate at this point or in fact
> > anywhere in this patch.
> >
> > Also, your patch subject and commit log is wrong -- this doesn't just
> > fix sparse warnings, it fixes actual bugs pointed out by sparse, so
> > commit log should reflect that.
> >
> thanks for your review!
> i'll send a new version.
>
> what is the difference between the normal and underscore version?
> atomicity? (i didn't find a documentation about it)
Generally underscore versions are more "internal" APIs. In this case, I
believe the underscore version is usable in places where the compiler
needs a constant as the output, I'm not quite sure though.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-16 8:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-22 11:38 [PATCH] wl12xx: fix some sparse warnings Eliad Peller
2011-01-10 16:02 ` Luciano Coelho
-- strict thread matches above, loose matches on Subject: below --
2011-01-16 7:49 Eliad Peller
2011-01-16 8:28 ` Johannes Berg
2011-01-16 8:48 ` Eliad Peller
2011-01-16 8:52 ` Johannes Berg
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).