* [PATCH] net/wireless/brcm80211/brcmfmac: Make return type reflect actual semantics
@ 2014-06-20 21:32 Rasmus Villemoes
2014-06-21 10:20 ` Arend van Spriel
2014-06-21 22:55 ` [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name " Rasmus Villemoes
0 siblings, 2 replies; 6+ messages in thread
From: Rasmus Villemoes @ 2014-06-20 21:32 UTC (permalink / raw)
To: Brett Rudley, Arend van Spriel
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rasmus Villemoes
Applying ++ to a bool is equivalent to setting it true, regardless of
its initial value (bools are not uint1_t). Hence the function
wl_get_vif_state_all can only ever return true/false. The only
in-tree caller uses its return value as a boolean. So update its
return type, and since the list traversal and bit testing have no side
effects, just return true immediately.
Signed-off-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
---
Notes:
Alternatively, if the function is supposed to return a count, the
one-line fix would be
- bool result = 0;
+ u32 result = 0;
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index d8fa276..ec5f8c5 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5625,16 +5625,15 @@ enum nl80211_iftype brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
return wdev->iftype;
}
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
+bool wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
{
struct brcmf_cfg80211_vif *vif;
- bool result = 0;
list_for_each_entry(vif, &cfg->vif_list, list) {
if (test_bit(state, &vif->sme_state))
- result++;
+ return true;
}
- return result;
+ return false;
}
static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
index 283c525..c702e4e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
@@ -477,7 +477,7 @@ const struct brcmf_tlv *
brcmf_parse_tlvs(const void *buf, int buflen, uint key);
u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
struct ieee80211_channel *ch);
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
+bool wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
struct brcmf_cfg80211_vif *vif);
bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/wireless/brcm80211/brcmfmac: Make return type reflect actual semantics
2014-06-20 21:32 [PATCH] net/wireless/brcm80211/brcmfmac: Make return type reflect actual semantics Rasmus Villemoes
@ 2014-06-21 10:20 ` Arend van Spriel
2014-06-21 22:55 ` [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name " Rasmus Villemoes
1 sibling, 0 replies; 6+ messages in thread
From: Arend van Spriel @ 2014-06-21 10:20 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Brett Rudley, linux-wireless, brcm80211-dev-list, netdev,
linux-kernel
On 06/20/14 23:32, Rasmus Villemoes wrote:
> Applying ++ to a bool is equivalent to setting it true, regardless of
> its initial value (bools are not uint1_t). Hence the function
> wl_get_vif_state_all can only ever return true/false. The only
> in-tree caller uses its return value as a boolean. So update its
> return type, and since the list traversal and bit testing have no side
> effects, just return true immediately.
Hi Rasmus,
At the moment the function is indeed only used to determine whether any
vif is in connected state. I am ok with your patch if you would also
rename the function to brcmf_get_vif_state_any(). You may add
'Reviewed-by: Arend van Spriel <arend@broadcom.com>' to the patch.
Regards,
Arend
> Signed-off-by: Rasmus Villemoes<linux@rasmusvillemoes.dk>
> ---
>
> Notes:
> Alternatively, if the function is supposed to return a count, the
> one-line fix would be
>
> - bool result = 0;
> + u32 result = 0;
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> index d8fa276..ec5f8c5 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -5625,16 +5625,15 @@ enum nl80211_iftype brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
> return wdev->iftype;
> }
>
> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
> +bool wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
> {
> struct brcmf_cfg80211_vif *vif;
> - bool result = 0;
>
> list_for_each_entry(vif,&cfg->vif_list, list) {
> if (test_bit(state,&vif->sme_state))
> - result++;
> + return true;
> }
> - return result;
> + return false;
> }
>
> static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> index 283c525..c702e4e 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> @@ -477,7 +477,7 @@ const struct brcmf_tlv *
> brcmf_parse_tlvs(const void *buf, int buflen, uint key);
> u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
> struct ieee80211_channel *ch);
> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
> +bool wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
> void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
> struct brcmf_cfg80211_vif *vif);
> bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name reflect actual semantics
2014-06-20 21:32 [PATCH] net/wireless/brcm80211/brcmfmac: Make return type reflect actual semantics Rasmus Villemoes
2014-06-21 10:20 ` Arend van Spriel
@ 2014-06-21 22:55 ` Rasmus Villemoes
2014-06-22 9:27 ` Arend van Spriel
2014-06-22 18:50 ` [PATCH v3] " Rasmus Villemoes
1 sibling, 2 replies; 6+ messages in thread
From: Rasmus Villemoes @ 2014-06-21 22:55 UTC (permalink / raw)
To: Arend van Spriel, Brett Rudley
Cc: linux-wireless, brcm80211-dev-list, netdev, linux-kernel,
Rasmus Villemoes
Applying ++ to a bool is equivalent to setting it true, regardless of
its initial value (bools are not uint1_t). Hence the function
wl_get_vif_state_all can only ever return true/false. The only in-tree
caller uses its return value as a boolean. So update its return type,
and since the list traversal and bit testing have no side effects,
just return true immediately. Its return value tells if any vif is up,
so also rename it to brcmf_get_vif_state_any.
Reviewed-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Notes:
v2: Rename wl_get_vif_state_all => brcmf_get_vif_state_any as
requested by Arend.
drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 2 +-
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index f3445ac..588fdbd 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -708,7 +708,7 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
active = P2PAPI_SCAN_SOCIAL_DWELL_TIME_MS;
else if (num_chans == AF_PEER_SEARCH_CNT)
active = P2PAPI_SCAN_AF_SEARCH_DWELL_TIME_MS;
- else if (wl_get_vif_state_all(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
+ else if (brcmf_get_vif_state_any(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
active = -1;
else
active = P2PAPI_SCAN_DWELL_TIME_MS;
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index d8fa276..93b1809 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5625,16 +5625,15 @@ enum nl80211_iftype brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
return wdev->iftype;
}
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
+bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state)
{
struct brcmf_cfg80211_vif *vif;
- bool result = 0;
list_for_each_entry(vif, &cfg->vif_list, list) {
if (test_bit(state, &vif->sme_state))
- result++;
+ return true;
}
- return result;
+ return false;
}
static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
index 283c525..f9fb109 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
@@ -477,7 +477,7 @@ const struct brcmf_tlv *
brcmf_parse_tlvs(const void *buf, int buflen, uint key);
u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
struct ieee80211_channel *ch);
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
+bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state);
void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
struct brcmf_cfg80211_vif *vif);
bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
--
1.9.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name reflect actual semantics
2014-06-21 22:55 ` [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name " Rasmus Villemoes
@ 2014-06-22 9:27 ` Arend van Spriel
2014-06-22 9:37 ` Arend van Spriel
2014-06-22 18:50 ` [PATCH v3] " Rasmus Villemoes
1 sibling, 1 reply; 6+ messages in thread
From: Arend van Spriel @ 2014-06-22 9:27 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Brett Rudley, linux-wireless, brcm80211-dev-list, netdev,
linux-kernel
On 06/22/14 00:55, Rasmus Villemoes wrote:
> Applying ++ to a bool is equivalent to setting it true, regardless of
> its initial value (bools are not uint1_t). Hence the function
> wl_get_vif_state_all can only ever return true/false. The only in-tree
> caller uses its return value as a boolean. So update its return type,
> and since the list traversal and bit testing have no side effects,
> just return true immediately. Its return value tells if any vif is up,
Now I may be really nit-picking, but the return value if any vif is *in
the specified state*.
Regards,
Arend
> so also rename it to brcmf_get_vif_state_any.
>
> Reviewed-by: Arend van Spriel<arend@broadcom.com>
> Signed-off-by: Rasmus Villemoes<linux@rasmusvillemoes.dk>
> ---
>
> Notes:
> v2: Rename wl_get_vif_state_all => brcmf_get_vif_state_any as
> requested by Arend.
>
> drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 2 +-
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
> 3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
> index f3445ac..588fdbd 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
> @@ -708,7 +708,7 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> active = P2PAPI_SCAN_SOCIAL_DWELL_TIME_MS;
> else if (num_chans == AF_PEER_SEARCH_CNT)
> active = P2PAPI_SCAN_AF_SEARCH_DWELL_TIME_MS;
> - else if (wl_get_vif_state_all(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
> + else if (brcmf_get_vif_state_any(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
> active = -1;
> else
> active = P2PAPI_SCAN_DWELL_TIME_MS;
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> index d8fa276..93b1809 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -5625,16 +5625,15 @@ enum nl80211_iftype brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
> return wdev->iftype;
> }
>
> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
> +bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state)
> {
> struct brcmf_cfg80211_vif *vif;
> - bool result = 0;
>
> list_for_each_entry(vif,&cfg->vif_list, list) {
> if (test_bit(state,&vif->sme_state))
> - result++;
> + return true;
> }
> - return result;
> + return false;
> }
>
> static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> index 283c525..f9fb109 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
> @@ -477,7 +477,7 @@ const struct brcmf_tlv *
> brcmf_parse_tlvs(const void *buf, int buflen, uint key);
> u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
> struct ieee80211_channel *ch);
> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
> +bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state);
> void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
> struct brcmf_cfg80211_vif *vif);
> bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name reflect actual semantics
2014-06-22 9:27 ` Arend van Spriel
@ 2014-06-22 9:37 ` Arend van Spriel
0 siblings, 0 replies; 6+ messages in thread
From: Arend van Spriel @ 2014-06-22 9:37 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Brett Rudley, linux-wireless, brcm80211-dev-list, netdev,
linux-kernel
On 06/22/14 11:27, Arend van Spriel wrote:
> On 06/22/14 00:55, Rasmus Villemoes wrote:
>> Applying ++ to a bool is equivalent to setting it true, regardless of
>> its initial value (bools are not uint1_t). Hence the function
>> wl_get_vif_state_all can only ever return true/false. The only in-tree
>> caller uses its return value as a boolean. So update its return type,
>> and since the list traversal and bit testing have no side effects,
>> just return true immediately. Its return value tells if any vif is up,
>
- Now I may be really nit-picking, but the return value if any vif is *in
- the specified state*.
+ Now I may be really nit-picking, but the return value tells if any
+ vif is *in the specified state*.
>
> Regards,
> Arend
>
>> so also rename it to brcmf_get_vif_state_any.
>>
>> Reviewed-by: Arend van Spriel<arend@broadcom.com>
>> Signed-off-by: Rasmus Villemoes<linux@rasmusvillemoes.dk>
>> ---
>>
>> Notes:
>> v2: Rename wl_get_vif_state_all => brcmf_get_vif_state_any as
>> requested by Arend.
>>
>> drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 2 +-
>> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
>> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
>> 3 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
>> b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
>> index f3445ac..588fdbd 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
>> @@ -708,7 +708,7 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info
>> *p2p, u32 num_chans,
>> active = P2PAPI_SCAN_SOCIAL_DWELL_TIME_MS;
>> else if (num_chans == AF_PEER_SEARCH_CNT)
>> active = P2PAPI_SCAN_AF_SEARCH_DWELL_TIME_MS;
>> - else if (wl_get_vif_state_all(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
>> + else if (brcmf_get_vif_state_any(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
>> active = -1;
>> else
>> active = P2PAPI_SCAN_DWELL_TIME_MS;
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>> b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>> index d8fa276..93b1809 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>> @@ -5625,16 +5625,15 @@ enum nl80211_iftype
>> brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
>> return wdev->iftype;
>> }
>>
>> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned
>> long state)
>> +bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg,
>> unsigned long state)
>> {
>> struct brcmf_cfg80211_vif *vif;
>> - bool result = 0;
>>
>> list_for_each_entry(vif,&cfg->vif_list, list) {
>> if (test_bit(state,&vif->sme_state))
>> - result++;
>> + return true;
>> }
>> - return result;
>> + return false;
>> }
>>
>> static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event
>> *event,
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
>> b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
>> index 283c525..f9fb109 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
>> @@ -477,7 +477,7 @@ const struct brcmf_tlv *
>> brcmf_parse_tlvs(const void *buf, int buflen, uint key);
>> u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
>> struct ieee80211_channel *ch);
>> -u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned
>> long state);
>> +bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg,
>> unsigned long state);
>> void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
>> struct brcmf_cfg80211_vif *vif);
>> bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] net/wireless/brcm80211/brcmfmac: Make return type and name reflect actual semantics
2014-06-21 22:55 ` [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name " Rasmus Villemoes
2014-06-22 9:27 ` Arend van Spriel
@ 2014-06-22 18:50 ` Rasmus Villemoes
1 sibling, 0 replies; 6+ messages in thread
From: Rasmus Villemoes @ 2014-06-22 18:50 UTC (permalink / raw)
To: Arend van Spriel, Brett Rudley
Cc: linux-wireless, brcm80211-dev-list, netdev, linux-kernel,
Rasmus Villemoes
Applying ++ to a bool is equivalent to setting it true, regardless of
its initial value (bools are not uint1_t). Hence the function
wl_get_vif_state_all can only ever return true/false. The only in-tree
caller uses its return value as a boolean. So update its return type,
and since the list traversal and bit testing have no side effects,
just return true immediately. Its return value tells if any vif is in
the specified state, so also rename it to brcmf_get_vif_state_any.
Reviewed-by: Arend van Spriel<arend@broadcom.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Notes:
v3: Fix thinko in commit message.
v2: Rename wl_get_vif_state_all => brcmf_get_vif_state_any as
requested by Arend.
drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 2 +-
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index f3445ac..588fdbd 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -708,7 +708,7 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
active = P2PAPI_SCAN_SOCIAL_DWELL_TIME_MS;
else if (num_chans == AF_PEER_SEARCH_CNT)
active = P2PAPI_SCAN_AF_SEARCH_DWELL_TIME_MS;
- else if (wl_get_vif_state_all(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
+ else if (brcmf_get_vif_state_any(p2p->cfg, BRCMF_VIF_STATUS_CONNECTED))
active = -1;
else
active = P2PAPI_SCAN_DWELL_TIME_MS;
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index d8fa276..93b1809 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5625,16 +5625,15 @@ enum nl80211_iftype brcmf_cfg80211_get_iftype(struct brcmf_if *ifp)
return wdev->iftype;
}
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state)
+bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state)
{
struct brcmf_cfg80211_vif *vif;
- bool result = 0;
list_for_each_entry(vif, &cfg->vif_list, list) {
if (test_bit(state, &vif->sme_state))
- result++;
+ return true;
}
- return result;
+ return false;
}
static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
index 283c525..f9fb109 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
@@ -477,7 +477,7 @@ const struct brcmf_tlv *
brcmf_parse_tlvs(const void *buf, int buflen, uint key);
u16 channel_to_chanspec(struct brcmu_d11inf *d11inf,
struct ieee80211_channel *ch);
-u32 wl_get_vif_state_all(struct brcmf_cfg80211_info *cfg, unsigned long state);
+bool brcmf_get_vif_state_any(struct brcmf_cfg80211_info *cfg, unsigned long state);
void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
struct brcmf_cfg80211_vif *vif);
bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg);
--
1.9.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-22 18:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-20 21:32 [PATCH] net/wireless/brcm80211/brcmfmac: Make return type reflect actual semantics Rasmus Villemoes
2014-06-21 10:20 ` Arend van Spriel
2014-06-21 22:55 ` [PATCH v2] net/wireless/brcm80211/brcmfmac: Make return type and name " Rasmus Villemoes
2014-06-22 9:27 ` Arend van Spriel
2014-06-22 9:37 ` Arend van Spriel
2014-06-22 18:50 ` [PATCH v3] " Rasmus Villemoes
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).