Re: [Patch net] nfc: check sock state in llcp_sock_getname()

2016-01-02 Thread Dmitry Vyukov 
On Sat, Jan 2, 2016 at 1:34 AM, Cong Wang  wrote:
> llcp_sock_getname() checks llcp_sock->dev to make sure
> llcp_sock is already connected or bound, however, we could
> be in the middle of llcp_sock_bind() where llcp_sock->dev
> is bound and llcp_sock->service_name_len is set,
> but llcp_sock->service_name is not, in this case we would
> lead to copy some bytes from a NULL pointer.
>
> We should just check if sk->sk_state is still closed since
> both connect() and bind() will update this state at the end.

Hi Cong,

This is still racy. If you want to play lock-free then you also need
proper memory barriers. Stores to sk_state need to be
smp_store_release, while the load needs to be smp_load_acquire.
Otherwise getname still can see partially initialized socket.


> Reported-by: Dmitry Vyukov 
> Cc: Lauro Ramos Venancio 
> Cc: Aloisio Almeida Jr 
> Cc: Samuel Ortiz 
> Signed-off-by: Cong Wang 
> ---
>  net/nfc/llcp_sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
> index ecf0a01..5a91997 100644
> --- a/net/nfc/llcp_sock.c
> +++ b/net/nfc/llcp_sock.c
> @@ -500,7 +500,7 @@ static int llcp_sock_getname(struct socket *sock, struct 
> sockaddr *uaddr,
> struct nfc_llcp_sock *llcp_sock = nfc_llcp_sock(sk);
> DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, llcp_addr, uaddr);
>
> -   if (llcp_sock == NULL || llcp_sock->dev == NULL)
> +   if (llcp_sock == NULL || sk->sk_state == LLCP_CLOSED)
> return -EBADFD;
>
> pr_debug("%p %d %d %d\n", sk, llcp_sock->target_idx,
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net-libertas: Better exception handling in if_spi_host_to_card_worker()

2016-01-02 Thread SF Markus Elfring 
>> Move the jump label directly before the desired log statement
>> so that the variable "err" will not be checked once more
>> after it was determined that a function call failed.
>> Use the identifier "report_failure" instead of the label "err".
> 
>Why?

I suggest to reconsider the places with which such a jump label
is connected.


> The code was smart enough

Which action should really be performed after a failure was detected
and handled a bit already?

* Another condition check

* Just additional error logging


> and you're making it uglier that it needs to be.

I assume that a software development taste can evolve, can't it?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net-libertas: Better exception handling in if_spi_host_to_card_worker()

2016-01-02 Thread Julia Lawall 


On Sat, 2 Jan 2016, SF Markus Elfring wrote:

> >> Move the jump label directly before the desired log statement
> >> so that the variable "err" will not be checked once more
> >> after it was determined that a function call failed.
> >> Use the identifier "report_failure" instead of the label "err".
> > 
> >Why?
> 
> I suggest to reconsider the places with which such a jump label
> is connected.
> 
> 
> > The code was smart enough
> 
> Which action should really be performed after a failure was detected
> and handled a bit already?
> 
> * Another condition check
> 
> * Just additional error logging
> 
> 
> > and you're making it uglier that it needs to be.
> 
> I assume that a software development taste can evolve, can't it?

So far, you have gotten several down votes for this kind of change, and no 
enthusiasm.

Admittedly, this is a trivial case, because there are no local variables, 
but do you actually know the semantics in C of a jump into a block?  And 
if you do know, do you think that this semantics is common knowledge?  And 
do you really think that introducing poorly understandable code is really 
worth saving an if test of a single variable on a non-critical path?

Most of the kernel code is not performance critical at the level of a 
single if test.  So the goal should be for the code to be easy to 
understand and robust to change.  The code that is performance critical, 
you should probably not touch, ever.  The people who wrote it knew what 
was important and what was not.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] brcmfmac: Add support for scheduled scan mac randomization

2016-01-02 Thread Arend van Spriel 
On 01-01-16 11:39, Kalle Valo wrote:
> Arend van Spriel  writes:
> 
>> On 29-12-15 21:47, Mathy Vanhoef wrote:
>>> On Mon, Dec 14, 2015 at 2:39 PM, Arend van Spriel  
>>> wrote:
 From: Hante Meuleman 

 Scheduled scan be requested to use mac randomization. This patch
 checks the flags and enables the randomization if desired.
>>>
>>> The driver also needs to advertise support for this using the
>>> NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR flag, if the firmware
>>> supports it. This can be done in brcmf_setup_wiphy. Otherwise
>>> tools like wpa_supplicant won't use randomization.
>>
>> Indeed. Hante found that out later on and we have a follow-up patch for
>> that.
>>
>> Kalle,
>>
>> How do you want to treat this? Should I squash the patches into one and
>> resubmit or can I just submit follow-up patch.
> 
> I would prefer that you squash the patches and resubmit the whole
> series.

Ok, thanks. Prepared that already.

Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/7] brcmfmac: add arp offload ip address table configuration support

2016-01-02 Thread Arend van Spriel 
From: Franky Lin 

Obtain ipv4 address through inetaddr notification for ARP offload host
ip table configuration.

Reviewed-by: Arend Van Spriel 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Hante Meuleman 
Signed-off-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 .../wireless/broadcom/brcm80211/brcmfmac/core.c| 108 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.h|   2 +
 2 files changed, 110 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 3a39192..4c8f7bf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -620,6 +621,8 @@ static int brcmf_netdev_stop(struct net_device *ndev)
 
brcmf_cfg80211_down(ndev);
 
+   brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear", NULL, 0);
+
brcmf_net_setcarrier(ifp, false);
 
return 0;
@@ -940,6 +943,98 @@ int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr)
return available ? bsscfgidx : -ENOMEM;
 }
 
+#ifdef CONFIG_INET
+#define ARPOL_MAX_ENTRIES  8
+static int brcmf_inetaddr_changed(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+   struct brcmf_pub *drvr = container_of(nb, struct brcmf_pub,
+ inetaddr_notifier);
+   struct in_ifaddr *ifa = data;
+   struct net_device *ndev = ifa->ifa_dev->dev;
+   struct brcmf_if *ifp;
+   int idx, i, ret;
+   u32 val;
+   __be32 addr_table[ARPOL_MAX_ENTRIES] = {0};
+
+   /* Find out if the notification is meant for us */
+   for (idx = 0; idx < BRCMF_MAX_IFS; idx++) {
+   ifp = drvr->iflist[idx];
+   if (ifp && ifp->ndev == ndev)
+   break;
+   if (idx == BRCMF_MAX_IFS - 1)
+   return NOTIFY_DONE;
+   }
+
+   /* check if arp offload is supported */
+   ret = brcmf_fil_iovar_int_get(ifp, "arpoe", &val);
+   if (ret)
+   return NOTIFY_OK;
+
+   /* old version only support primary index */
+   ret = brcmf_fil_iovar_int_get(ifp, "arp_version", &val);
+   if (ret)
+   val = 1;
+   if (val == 1)
+   ifp = drvr->iflist[0];
+
+   /* retrieve the table from firmware */
+   ret = brcmf_fil_iovar_data_get(ifp, "arp_hostip", addr_table,
+  sizeof(addr_table));
+   if (ret) {
+   brcmf_err("fail to get arp ip table err:%d\n", ret);
+   return NOTIFY_OK;
+   }
+
+   for (i = 0; i < ARPOL_MAX_ENTRIES; i++)
+   if (ifa->ifa_address == addr_table[i])
+   break;
+
+   switch (action) {
+   case NETDEV_UP:
+   if (i == ARPOL_MAX_ENTRIES) {
+   brcmf_dbg(TRACE, "add %pI4 to arp table\n",
+ &ifa->ifa_address);
+   /* set it directly */
+   ret = brcmf_fil_iovar_data_set(ifp, "arp_hostip",
+   &ifa->ifa_address, sizeof(ifa->ifa_address));
+   if (ret)
+   brcmf_err("add arp ip err %d\n", ret);
+   }
+   break;
+   case NETDEV_DOWN:
+   if (i < ARPOL_MAX_ENTRIES) {
+   addr_table[i] = 0;
+   brcmf_dbg(TRACE, "remove %pI4 from arp table\n",
+ &ifa->ifa_address);
+   /* clear the table in firmware */
+   ret = brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear",
+  NULL, 0);
+   if (ret) {
+   brcmf_err("fail to clear arp ip table err:%d\n",
+ ret);
+   return NOTIFY_OK;
+   }
+   for (i = 0; i < ARPOL_MAX_ENTRIES; i++) {
+   if (addr_table[i] != 0) {
+   brcmf_fil_iovar_data_set(ifp,
+   "arp_hostip", &addr_table[i],
+   sizeof(addr_table[i]));
+   if (ret)
+   brcmf_err("add arp ip err %d\n",
+ ret);
+   }
+   }
+   }
+   break;
+   default:
+   break;
+   }
+
+   return NOTIFY_OK;
+}
+#endif
+
 int brcmf_attach(struct device *dev)
 {
struct brcmf_pub *drvr = NULL;
@@ -1068,6 +1163,15 @@ int brcmf_bus_st

[PATCH V2 6/7] brcmfmac: Move all module parameters to one place

2016-01-02 Thread Arend van Spriel 
From: Hante Meuleman 

Module parameters are defined in several files. Move them in one
place and make them device specific or global. This makes it
easier to override device specific settings by external data like
platform data in the future.

Reviewed-by: Arend Van Spriel 
Reviewed-by: Franky (Zhenhui) Lin 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Hante Meuleman 
Signed-off-by: Arend van Spriel 
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 12 ++---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c | 13 ++---
 .../wireless/broadcom/brcm80211/brcmfmac/common.c  | 63 ++
 .../wireless/broadcom/brcm80211/brcmfmac/common.h  | 43 +++
 .../wireless/broadcom/brcm80211/brcmfmac/core.c| 22 
 .../wireless/broadcom/brcm80211/brcmfmac/core.h|  5 +-
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 13 ++---
 .../broadcom/brcm80211/brcmfmac/firmware.c | 16 +++---
 .../broadcom/brcm80211/brcmfmac/fwsignal.c |  9 ++--
 9 files changed, 144 insertions(+), 52 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 410a664..5363739 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,6 +47,8 @@
 #include "debug.h"
 #include "sdio.h"
 #include "of.h"
+#include "core.h"
+#include "common.h"
 
 #define SDIOH_API_ACCESS_RETRY_LIMIT   2
 
@@ -57,7 +59,6 @@
 /* Maximum milliseconds to wait for F2 to come up */
 #define SDIO_WAIT_F2RDY3000
 
-#define BRCMF_DEFAULT_TXGLOM_SIZE  32  /* max tx frames in glom chain */
 #define BRCMF_DEFAULT_RXGLOM_SIZE  32  /* max rx frames in glom chain */
 
 struct brcmf_sdiod_freezer {
@@ -68,10 +69,6 @@ struct brcmf_sdiod_freezer {
struct completion resumed;
 };
 
-static int brcmf_sdiod_txglomsz = BRCMF_DEFAULT_TXGLOM_SIZE;
-module_param_named(txglomsz, brcmf_sdiod_txglomsz, int, 0);
-MODULE_PARM_DESC(txglomsz, "maximum tx packet chain size [SDIO]");
-
 static irqreturn_t brcmf_sdiod_oob_irqhandler(int irq, void *dev_id)
 {
struct brcmf_bus *bus_if = dev_get_drvdata(dev_id);
@@ -890,7 +887,8 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev 
*sdiodev)
if (!sdiodev->sg_support)
return;
 
-   nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, brcmf_sdiod_txglomsz);
+   nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE,
+ sdiodev->bus_if->drvr->settings->sdiod_txglomsz);
nents += (nents >> 4) + 1;
 
WARN_ON(nents > sdiodev->max_segment_count);
@@ -902,7 +900,7 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev 
*sdiodev)
sdiodev->sg_support = false;
}
 
-   sdiodev->txglomsz = brcmf_sdiod_txglomsz;
+   sdiodev->txglomsz = sdiodev->bus_if->drvr->settings->sdiod_txglomsz;
 }
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index dc14dd4..6a7759f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -236,10 +236,6 @@ struct parsed_vndr_ies {
struct parsed_vndr_ie_info ie_info[VNDR_IE_PARSE_LIMIT];
 };
 
-static int brcmf_roamoff;
-module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
-MODULE_PARM_DESC(roamoff, "do not use internal roaming engine");
-
 
 static u16 chandef_to_chanspec(struct brcmu_d11inf *d11inf,
   struct cfg80211_chan_def *ch)
@@ -5395,7 +5391,7 @@ static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
__le32 roam_delta[2];
 
/* Configure beacon timeout value based upon roaming setting */
-   if (brcmf_roamoff)
+   if (ifp->drvr->settings->roamoff)
bcn_timeout = BRCMF_DEFAULT_BCN_TIMEOUT_ROAM_OFF;
else
bcn_timeout = BRCMF_DEFAULT_BCN_TIMEOUT_ROAM_ON;
@@ -5409,8 +5405,9 @@ static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
 * roaming.
 */
brcmf_dbg(INFO, "Internal Roaming = %s\n",
- brcmf_roamoff ? "Off" : "On");
-   err = brcmf_fil_iovar_int_set(ifp, "roam_off", !!(brcmf_roamoff));
+ ifp->drvr->settings->roamoff ? "Off" : "On");
+   err = brcmf_fil_iovar_int_set(ifp, "roam_off",
+ ifp->drvr->settings->roamoff);
if (err) {
brcmf_err("roam_off error (%d)\n", err);
goto roam_setup_done;
@@ -6082,7 +6079,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct 
brcmf_if *ifp)
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_TDLS))
wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS;
-   if (!brcmf_roamoff)
+   if (!ifp->drvr->settings->roamoff)
wi

[PATCH V2 0/7] brcmfmac: IBSS fixes and random mac scheduled scan

2016-01-02 Thread Arend van Spriel 
Start the new year setting things straight so resending series
incorporating review comments from Mathy Vanhoef. This replaces
the series with Message-ID:

<1450100394-17414-1-git-send-email-ar...@broadcom.com>

This series adds the following:
* query features through firmware command.
* IBSS fixes.
* ARP offload through inet notifier.
* force probe to succeed for debugging.
* random mac support for scheduled scan.

Arend van Spriel (2):
  brcmfmac: obtain feature info using 'cap' firmware command
  brcmfmac: introduce module parameter to force successful probe

Franky Lin (1):
  brcmfmac: add arp offload ip address table configuration support

Hante Meuleman (4):
  brcmfmac: Add get_station support for IBSS
  brcmfmac: Add support for scheduled scan mac randomization
  brcmfmac: Fix warn trace on module unload while in ibss mode
  brcmfmac: Move all module parameters to one place

 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  |  12 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c | 127 +---
 .../wireless/broadcom/brcm80211/brcmfmac/common.c  |  72 +++
 .../wireless/broadcom/brcm80211/brcmfmac/common.h  |  56 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.c| 132 +++--
 .../wireless/broadcom/brcm80211/brcmfmac/core.h|   7 +-
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c |  76 +++-
 .../wireless/broadcom/brcm80211/brcmfmac/feature.h |   4 +-
 .../broadcom/brcm80211/brcmfmac/firmware.c |  16 +--
 .../wireless/broadcom/brcm80211/brcmfmac/fwil.h|   1 +
 .../broadcom/brcm80211/brcmfmac/fwil_types.h   |  34 ++
 .../broadcom/brcm80211/brcmfmac/fwsignal.c |   9 +-
 12 files changed, 462 insertions(+), 84 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 7/7] brcmfmac: introduce module parameter to force successful probe

2016-01-02 Thread Arend van Spriel 
The module parameter can be used to ensure the probe succeeds thus
claiming the device and allowing post-mortem debugging in case of
firmware crash. It is only available when select CONFIG_BRCMDBG.

Reviewed-by: Hante Meuleman 
Reviewed-by: Franky (Zhenhui) Lin 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 11 ++-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 13 +
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c   |  2 ++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index bb9e2b3..4265b50 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -67,6 +67,13 @@ static int brcmf_roamoff;
 module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
 MODULE_PARM_DESC(roamoff, "Do not use internal roaming engine");
 
+#ifdef DEBUG
+/* always succeed brcmf_bus_start() */
+static int brcmf_ignore_probe_fail;
+module_param_named(ignore_probe_fail, brcmf_ignore_probe_fail, int, 0);
+MODULE_PARM_DESC(ignore_probe_fail, "always succeed probe for debugging");
+#endif
+
 struct brcmf_mp_global_t brcmf_mp_global;
 
 int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
@@ -232,7 +239,9 @@ int brcmf_mp_device_attach(struct brcmf_pub *drvr)
drvr->settings->feature_disable = brcmf_feature_disable;
drvr->settings->fcmode = brcmf_fcmode;
drvr->settings->roamoff = !!brcmf_roamoff;
-
+#ifdef DEBUG
+   drvr->settings->ignore_probe_fail = !!brcmf_ignore_probe_fail;
+#endif
return 0;
 }
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index abe3764..3b0a63b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -55,11 +55,24 @@ struct brcmf_mp_device {
int feature_disable;
int fcmode;
boolroamoff;
+   boolignore_probe_fail;
 };
 
 void brcmf_mp_attach(void);
 int brcmf_mp_device_attach(struct brcmf_pub *drvr);
 void brcmf_mp_device_detach(struct brcmf_pub *drvr);
+#ifdef DEBUG
+static inline bool brcmf_ignoring_probe_fail(struct brcmf_pub *drvr)
+{
+   return drvr->settings->ignore_probe_fail;
+}
+#else
+static inline bool brcmf_ignoring_probe_fail(struct brcmf_pub *drvr)
+{
+   return false;
+}
+#endif
+
 /* Sets dongle media info (drv_version, mac address). */
 int brcmf_c_preinit_dcmds(struct brcmf_if *ifp);
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 3fa7bc5..7c75b1a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1183,6 +1183,8 @@ fail:
brcmf_net_detach(p2p_ifp->ndev);
drvr->iflist[0] = NULL;
drvr->iflist[1] = NULL;
+   if (brcmf_ignoring_probe_fail(drvr))
+   ret = 0;
return ret;
}
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/7] brcmfmac: Add get_station support for IBSS

2016-01-02 Thread Arend van Spriel 
From: Hante Meuleman 

When get_station is requested for IBSS then an error will be
printed and no information will be returned. This patch adds
IBSS specific get station information function.

Reviewed-by: Arend Van Spriel 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Hante Meuleman 
Signed-off-by: Arend van Spriel 
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c | 51 ++
 .../wireless/broadcom/brcm80211/brcmfmac/fwil.h|  1 +
 .../broadcom/brcm80211/brcmfmac/fwil_types.h   | 17 
 3 files changed, 69 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 17658b3..6b9339b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2429,6 +2429,54 @@ static void brcmf_fill_bss_param(struct brcmf_if *ifp, 
struct station_info *si)
 }
 
 static s32
+brcmf_cfg80211_get_station_ibss(struct brcmf_if *ifp,
+   struct station_info *sinfo)
+{
+   struct brcmf_scb_val_le scbval;
+   struct brcmf_pktcnt_le pktcnt;
+   s32 err;
+   u32 rate;
+   u32 rssi;
+
+   /* Get the current tx rate */
+   err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_RATE, &rate);
+   if (err < 0) {
+   brcmf_err("BRCMF_C_GET_RATE error (%d)\n", err);
+   return err;
+   }
+   sinfo->filled |= BIT(NL80211_STA_INFO_TX_BITRATE);
+   sinfo->txrate.legacy = rate * 5;
+
+   memset(&scbval, 0, sizeof(scbval));
+   err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_RSSI, &scbval,
+sizeof(scbval));
+   if (err) {
+   brcmf_err("BRCMF_C_GET_RSSI error (%d)\n", err);
+   return err;
+   }
+   rssi = le32_to_cpu(scbval.val);
+   sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
+   sinfo->signal = rssi;
+
+   err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_GET_PKTCNTS, &pktcnt,
+sizeof(pktcnt));
+   if (err) {
+   brcmf_err("BRCMF_C_GET_GET_PKTCNTS error (%d)\n", err);
+   return err;
+   }
+   sinfo->filled |= BIT(NL80211_STA_INFO_RX_PACKETS) |
+BIT(NL80211_STA_INFO_RX_DROP_MISC) |
+BIT(NL80211_STA_INFO_TX_PACKETS) |
+BIT(NL80211_STA_INFO_TX_FAILED);
+   sinfo->rx_packets = le32_to_cpu(pktcnt.rx_good_pkt);
+   sinfo->rx_dropped_misc = le32_to_cpu(pktcnt.rx_bad_pkt);
+   sinfo->tx_packets = le32_to_cpu(pktcnt.tx_good_pkt);
+   sinfo->tx_failed  = le32_to_cpu(pktcnt.tx_bad_pkt);
+
+   return 0;
+}
+
+static s32
 brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
   const u8 *mac, struct station_info *sinfo)
 {
@@ -2445,6 +2493,9 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct 
net_device *ndev,
if (!check_vif_up(ifp->vif))
return -EIO;
 
+   if (brcmf_is_ibssmode(ifp->vif))
+   return brcmf_cfg80211_get_station_ibss(ifp, sinfo);
+
memset(&sta_info_le, 0, sizeof(sta_info_le));
memcpy(&sta_info_le, mac, ETH_ALEN);
err = brcmf_fil_iovar_data_get(ifp, "tdls_sta_info",
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
index b20fc0f..6b72df1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
@@ -70,6 +70,7 @@
 #define BRCMF_C_SET_WSEC   134
 #define BRCMF_C_GET_PHY_NOISE  135
 #define BRCMF_C_GET_BSS_INFO   136
+#define BRCMF_C_GET_GET_PKTCNTS137
 #define BRCMF_C_GET_BANDLIST   140
 #define BRCMF_C_SET_SCB_TIMEOUT158
 #define BRCMF_C_GET_ASSOCLIST  159
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 94d34ad..0b1e46d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -751,4 +751,21 @@ struct brcmf_pno_scanresults_le {
__le32 count;
 };
 
+/**
+ * struct brcmf_pktcnt_le - packet counters.
+ *
+ * @rx_good_pkt: packets (MSDUs & MMPDUs) received from this station
+ * @rx_bad_pkt: failed rx packets
+ * @tx_good_pkt: packets (MSDUs & MMPDUs) transmitted to this station
+ * @tx_bad_pkt: failed tx packets
+ * @rx_ocast_good_pkt: unicast packets destined for others
+ */
+struct brcmf_pktcnt_le {
+   __le32 rx_good_pkt;
+   __le32 rx_bad_pkt;
+   __le32 tx_good_pkt;
+   __le32 tx_bad_pkt;
+   __le32 rx_ocast_good_pkt;
+};
+
 #endif /* FWIL_TYPES_H_ */
-- 
1.9.1

--
To unsubscribe from this l

[PATCH V2 4/7] brcmfmac: obtain feature info using 'cap' firmware command

2016-01-02 Thread Arend van Spriel 
Several features in the driver directly map to a firmware feature
listed in response of the 'cap' firmware command. For those features
this response will be examined instead of attempting individual
firmware commands.

Reviewed-by: Hante Meuleman 
Reviewed-by: Franky (Zhenhui) Lin 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Arend van Spriel 
---
V2:
 - Correct feature bitmap update for 43362.
---
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 53 +-
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index e7ac8a2..d41f343 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -42,6 +42,17 @@ static const char *brcmf_feat_names[] = {
 };
 #undef BRCMF_FEAT_DEF
 
+struct brcmf_feat_fwcap {
+   enum brcmf_feat_id feature;
+   const char * const fwcap_id;
+};
+
+static const struct brcmf_feat_fwcap brcmf_fwcap_map[] = {
+   { BRCMF_FEAT_MBSS, "mbss" },
+   { BRCMF_FEAT_MCHAN, "mchan" },
+   { BRCMF_FEAT_P2P, "p2p" },
+};
+
 #ifdef DEBUG
 /*
  * expand quirk list to array of quirk strings.
@@ -106,25 +117,22 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if *ifp,
}
 }
 
-/**
- * brcmf_feat_iovar_int_set() - determine feature through iovar set.
- *
- * @ifp: interface to query.
- * @id: feature id.
- * @name: iovar name.
- */
-static void brcmf_feat_iovar_int_set(struct brcmf_if *ifp,
-enum brcmf_feat_id id, char *name, u32 val)
+static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
 {
-   int err;
-
-   err = brcmf_fil_iovar_int_set(ifp, name, val);
-   if (err == 0) {
-   brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
-   ifp->drvr->feat_flags |= BIT(id);
-   } else {
-   brcmf_dbg(TRACE, "%s feature check failed: %d\n",
- brcmf_feat_names[id], err);
+   char caps[256];
+   enum brcmf_feat_id id;
+   int i;
+
+   brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
+   brcmf_dbg(INFO, "[ %s]\n", caps);
+
+   for (i = 0; i < ARRAY_SIZE(brcmf_fwcap_map); i++) {
+   if (strnstr(caps, brcmf_fwcap_map[i].fwcap_id, sizeof(caps))) {
+   id = brcmf_fwcap_map[i].feature;
+   brcmf_dbg(INFO, "enabling feature: %s\n",
+ brcmf_feat_names[id]);
+   ifp->drvr->feat_flags |= BIT(id);
+   }
}
 }
 
@@ -134,13 +142,14 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
struct brcmf_pno_macaddr_le pfn_mac;
s32 err;
 
-   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_MCHAN, "mchan");
+   brcmf_feat_firmware_capabilities(ifp);
+
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
if (drvr->bus_if->wowl_supported)
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");
-   if (drvr->bus_if->chip != BRCM_CC_43362_CHIP_ID)
-   brcmf_feat_iovar_int_set(ifp, BRCMF_FEAT_MBSS, "mbss", 0);
-   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_P2P, "p2p");
+   /* MBSS does not work for 43362 */
+   if (drvr->bus_if->chip == BRCM_CC_43362_CHIP_ID)
+   ifp->drvr->feat_flags &= ~BIT(BRCMF_FEAT_MBSS);
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_RSDB, "rsdb_mode");
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_TDLS, "tdls_enable");
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 3/7] brcmfmac: Add support for scheduled scan mac randomization

2016-01-02 Thread Arend van Spriel 
From: Hante Meuleman 

Scheduled scan be requested to use mac randomization. This patch
checks the flags and enables the randomization if desired.

Reviewed-by: Arend Van Spriel 
Reviewed-by: Franky (Zhenhui) Lin 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Hante Meuleman 
Signed-off-by: Arend van Spriel 
---
V2:
 - set nl80211 feature flags in wiphy object.
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c | 54 +++---
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 10 
 .../wireless/broadcom/brcm80211/brcmfmac/feature.h |  4 +-
 .../broadcom/brcm80211/brcmfmac/fwil_types.h   | 17 +++
 4 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 6b9339b..05843b7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3544,9 +3544,14 @@ static int brcmf_dev_pno_clean(struct net_device *ndev)
return ret;
 }
 
-static int brcmf_dev_pno_config(struct net_device *ndev)
+static int brcmf_dev_pno_config(struct brcmf_if *ifp,
+   struct cfg80211_sched_scan_request *request)
 {
struct brcmf_pno_param_le pfn_param;
+   struct brcmf_pno_macaddr_le pfn_mac;
+   s32 err;
+   u8 *mac_mask;
+   int i;
 
memset(&pfn_param, 0, sizeof(pfn_param));
pfn_param.version = cpu_to_le32(BRCMF_PNO_VERSION);
@@ -3559,8 +3564,37 @@ static int brcmf_dev_pno_config(struct net_device *ndev)
/* set up pno scan fr */
pfn_param.scan_freq = cpu_to_le32(BRCMF_PNO_TIME);
 
-   return brcmf_fil_iovar_data_set(netdev_priv(ndev), "pfn_set",
-   &pfn_param, sizeof(pfn_param));
+   err = brcmf_fil_iovar_data_set(ifp, "pfn_set", &pfn_param,
+  sizeof(pfn_param));
+   if (err) {
+   brcmf_err("pfn_set failed, err=%d\n", err);
+   return err;
+   }
+
+   /* Find out if mac randomization should be turned on */
+   if (!(request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR))
+   return 0;
+
+   pfn_mac.version = BRCMF_PFN_MACADDR_CFG_VER;
+   pfn_mac.flags = BRCMF_PFN_MAC_OUI_ONLY | BRCMF_PFN_SET_MAC_UNASSOC;
+
+   memcpy(pfn_mac.mac, request->mac_addr, ETH_ALEN);
+   mac_mask = request->mac_addr_mask;
+   for (i = 0; i < ETH_ALEN; i++) {
+   pfn_mac.mac[i] &= mac_mask[i];
+   pfn_mac.mac[i] |= get_random_int() & ~(mac_mask[i]);
+   }
+   /* Clear multi bit */
+   pfn_mac.mac[0] &= 0xFE;
+   /* Set locally administered */
+   pfn_mac.mac[0] |= 0x02;
+
+   err = brcmf_fil_iovar_data_set(ifp, "pfn_macaddr", &pfn_mac,
+  sizeof(pfn_mac));
+   if (err)
+   brcmf_err("pfn_macaddr failed, err=%d\n", err);
+
+   return err;
 }
 
 static int
@@ -3614,11 +3648,8 @@ brcmf_cfg80211_sched_scan_start(struct wiphy *wiphy,
}
 
/* configure pno */
-   ret = brcmf_dev_pno_config(ndev);
-   if (ret < 0) {
-   brcmf_err("PNO setup failed!! ret=%d\n", ret);
+   if (brcmf_dev_pno_config(ifp, request))
return -EINVAL;
-   }
 
/* configure each match set */
for (i = 0; i < request->n_match_sets; i++) {
@@ -6455,6 +6486,15 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct 
brcmf_pub *drvr,
goto wiphy_unreg_out;
}
 
+   /* Fill in some of the advertised nl80211 supported features */
+   if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_RANDOM_MAC)) {
+   wiphy->features |= NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR;
+#ifdef CONFIG_PM
+   if (wiphy->wowlan->flags & WIPHY_WOWLAN_NET_DETECT)
+   wiphy->features |= NL80211_FEATURE_ND_RANDOM_MAC_ADDR;
+#endif
+   }
+
return cfg;
 
 wiphy_unreg_out:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index d9d1ca4..e7ac8a2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -18,10 +18,12 @@
 #include 
 
 #include 
+#include 
 #include "core.h"
 #include "bus.h"
 #include "debug.h"
 #include "fwil.h"
+#include "fwil_types.h"
 #include "feature.h"
 
 
@@ -129,6 +131,8 @@ static void brcmf_feat_iovar_int_set(struct brcmf_if *ifp,
 void brcmf_feat_attach(struct brcmf_pub *drvr)
 {
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
+   struct brcmf_pno_macaddr_le pfn_mac;
+   s32 err;
 
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_MCHAN, "mchan");
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
@@ -1

[PATCH V2 5/7] brcmfmac: Fix warn trace on module unload while in ibss mode

2016-01-02 Thread Arend van Spriel 
From: Hante Meuleman 

When the driver is being unloaded a situation can occur where the
wirelesss core (cfg80211) wants to remove the ibss, but the state
of brcmfmac has already been set to down. When an error is
returned in that situation then that will result in a stack
trace on removal of the wiphy object. This is avoided by
returning 0 when device is down on a leave_ibss call.

Reviewed-by: Arend Van Spriel 
Reviewed-by: Pieter-Paul Giesberts 
Signed-off-by: Hante Meuleman 
Signed-off-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 05843b7..dc14dd4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1448,8 +1448,13 @@ brcmf_cfg80211_leave_ibss(struct wiphy *wiphy, struct 
net_device *ndev)
struct brcmf_if *ifp = netdev_priv(ndev);
 
brcmf_dbg(TRACE, "Enter\n");
-   if (!check_vif_up(ifp->vif))
-   return -EIO;
+   if (!check_vif_up(ifp->vif)) {
+   /* When driver is being unloaded, it can end up here. If an
+* error is returned then later on a debug trace in the wireless
+* core module will be printed. To avoid this 0 is returned.
+*/
+   return 0;
+   }
 
brcmf_link_down(ifp->vif, WLAN_REASON_DEAUTH_LEAVING);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net-brcmfmac: Delete an unnecessary variable initialisation in brcmf_sdio_download_firmware()

2016-01-02 Thread Arend van Spriel 

On 01/01/2016 08:26 PM, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Fri, 1 Jan 2016 20:20:15 +0100


I think it has been said over and over, but please use driver name only 
as prefix. I don't see value to prepend it with 'net-'.



Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.



That being said here is my

Acked-by: Arend van Spriel 

Signed-off-by: Markus Elfring 
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ceb2a75..c21eeb1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3260,7 +3260,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio 
*bus,
const struct firmware *fw,
void *nvram, u32 nvlen)
  {
-   int bcmerror = -EFAULT;
+   int bcmerror;
u32 rstvec;

sdio_claim_host(bus->sdiodev->func[1]);



--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

2016-01-02 Thread SF Markus Elfring 
>> I assume that a software development taste can evolve, can't it?
> 
> So far, you have gotten several down votes for this kind of change,

I am curious when more contributors will share corresponding opinions.


> and no enthusiasm.

How many software designers and developers can become enthusiastic
about better exception handling to some degree?


> The code that is performance critical, you should probably not touch, ever.

I imagine that technical evolution will result in further considerations
so that "unchangeable" components can be adjusted once more.


> The people who wrote it knew what was important and what was not.

I might come along at some places where the affected knowledge will also evolve.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

2016-01-02 Thread Arend van Spriel 


On 02-01-16 10:08, SF Markus Elfring wrote:
>>> I assume that a software development taste can evolve, can't it?
>>
>> So far, you have gotten several down votes for this kind of change,
> 
> I am curious when more contributors will share corresponding opinions.

Let's burn some cycles on this while the holidays give me time to do so.
"software development taste" is another term for "coding style". In
every project battles are fought over this between friends and foes. I
have never seen much evolution going on in this area.

>> and no enthusiasm.
> 
> How many software designers and developers can become enthusiastic
> about better exception handling to some degree?

I had to  take a look at this particular patch and I have to say that I
don't see, using your favorite term, evolution at work. It looks more
like the result of inbred. What the patch tries to do is avoid the extra
'if (err)'. Setting coding style aside, the question is whether there is
a good metric for the patch. So does it really safe processing time? Did
you look at the resulting assembly code for different target architectures?

You got pushed back on the change so you have to come up with solid
arguments for your change instead of spewing ideas about evolution in
software development. Running Coccinelle is one thing, but understanding
the results and what you are ultimately proposing to be changed is more
important.

Regards,
Arend

>> The code that is performance critical, you should probably not touch, ever.
> 
> I imagine that technical evolution will result in further considerations
> so that "unchangeable" components can be adjusted once more.
> 
> 
>> The people who wrote it knew what was important and what was not.
> 
> I might come along at some places where the affected knowledge will also 
> evolve.
> 
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

2016-01-02 Thread SF Markus Elfring 
> I have never seen much evolution going on in this area.

I can get an other impression from a specific document for example.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/Documentation/CodingStyle


> What the patch tries to do is avoid the extra 'if (err)'.

Yes. - I propose to look at related consequences together with the usage
of a popular short jump label once more.


> Setting coding style aside, the question is whether there is
> a good metric for the patch.

A software development challenge is to accept changes also around a topic
like "error handling". My update suggestion for the source file
"drivers/net/wireless/marvell/libertas/if_spi.c" should only improve
exception handling. (I came along other source files where more improvements
will eventually be possible.)

When will the run-time behaviour matter also for exceptional situations?


> Did you look at the resulting assembly code for different target 
> architectures?

Not yet. - Which execution system variants would you recommend for
further comparisons?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] net-rsi: Fine-tuning for two function implementations

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:36:25 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
  Delete unnecessary variable initialisations in rsi_send_data_pkt()
  Replace variable initialisations by assignments in rsi_send_data_pkt()

 drivers/net/wireless/rsi/rsi_91x_pkt.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 14:54:30 +0100

Omit explicit initialisation at the beginning for five local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..ee98f5b 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
  struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *wh = NULL;
+   struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = &hw->conf;
struct skb_info *tx_params;
-   int status = -E2BIG;
-   __le16 *msg = NULL;
-   u8 extnd_size = 0;
+   int status;
+   __le16 *msg;
+   u8 extnd_size;
u8 vap_id = 0;
 
info = IEEE80211_SKB_CB(skb);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:15:12 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ee98f5b..ec65e1c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -27,15 +27,15 @@
 int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *tmp_hdr = NULL;
+   struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
int status = -EINVAL;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
-   u8 extnd_size = 0;
+   u8 extnd_size;
__le16 *frame_desc;
-   u16 seq_num = 0;
+   u16 seq_num;
 
info = IEEE80211_SKB_CB(skb);
bss = &info->control.vif->bss_conf;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:25:34 +0100

Replace explicit initialisation for two local variables at the beginning
by assignments.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ec65e1c..fe36e7d 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -26,12 +26,12 @@
  */
 int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
 {
-   struct rsi_hw *adapter = common->priv;
+   struct rsi_hw *adapter;
struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
struct ieee80211_bss_conf *bss;
-   int status = -EINVAL;
+   int status;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
u8 extnd_size;
__le16 *frame_desc;
@@ -41,8 +41,10 @@ int rsi_send_data_pkt(struct rsi_common *common, struct 
sk_buff *skb)
bss = &info->control.vif->bss_conf;
tx_params = (struct skb_info *)info->driver_data;
 
-   if (!bss->assoc)
+   if (!bss->assoc) {
+   status = -EINVAL;
goto err;
+   }
 
tmp_hdr = (struct ieee80211_hdr *)&skb->data[0];
seq_num = (le16_to_cpu(tmp_hdr->seq_ctrl) >> 4);
@@ -97,7 +99,7 @@ int rsi_send_data_pkt(struct rsi_common *common, struct 
sk_buff *skb)
frame_desc[7] = cpu_to_le16(((tx_params->tid & 0xf) << 4) |
(skb->priority & 0xf) |
(tx_params->sta_id << 8));
-
+   adapter = common->priv;
status = adapter->host_intf_write_pkt(common->priv,
  skb->data,
  skb->len);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

2016-01-02 Thread Francois Romieu 
SF Markus Elfring  :
> From: Markus Elfring 
> Date: Sat, 2 Jan 2016 15:25:34 +0100
> 
> Replace explicit initialisation for two local variables at the beginning
> by assignments.

It makes no sense for the 'adapter' variable.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

net-rsi: Reconsider usage of variable "vap_id" in rsi_send_mgmt_pkt()

2016-01-02 Thread SF Markus Elfring 
Hello,

I have taken another look at the implementation of the function 
"rsi_send_mgmt_pkt".
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/net/wireless/rsi/rsi_91x_pkt.c?id=e8c58e7a5a106c3d557fccd01cd4d1128f9bab38#n114

I find the following statement combination interesting there.

…
u8 vap_id = 0;
…
msg[7] |= cpu_to_le16(vap_id << 8);
…

I would appreciate a further clarification.
Does a shift operation for a variable which contains zero indicate an open 
issue?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

2016-01-02 Thread kbuild test robot 
Hi Markus,

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on v4.4-rc7 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/net-rsi-Fine-tuning-for-two-function-implementations/20160102-224740
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
master
config: x86_64-randconfig-s2-01030012 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/wireless/rsi/rsi_91x_pkt.c: In function 'rsi_send_mgmt_pkt':
>> drivers/net/wireless/rsi/rsi_91x_pkt.c:211:2: warning: 'status' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
 rsi_indicate_tx_status(common->priv, skb, status);
 ^

vim +/status +211 drivers/net/wireless/rsi/rsi_91x_pkt.c

dad0d04f Fariya Fatima 2014-03-16  195  /* Indicate to firmware to give 
cfm */
dad0d04f Fariya Fatima 2014-03-16  196  if ((skb->data[16] == 
IEEE80211_STYPE_PROBE_REQ) && (!bss->assoc)) {
dad0d04f Fariya Fatima 2014-03-16  197  msg[1] |= 
cpu_to_le16(BIT(10));
dad0d04f Fariya Fatima 2014-03-16  198  msg[7] = 
cpu_to_le16(PROBEREQ_CONFIRM);
dad0d04f Fariya Fatima 2014-03-16  199  common->mgmt_q_block = 
true;
dad0d04f Fariya Fatima 2014-03-16  200  }
dad0d04f Fariya Fatima 2014-03-16  201  
dad0d04f Fariya Fatima 2014-03-16  202  msg[7] |= cpu_to_le16(vap_id << 
8);
dad0d04f Fariya Fatima 2014-03-16  203  
dad0d04f Fariya Fatima 2014-03-16  204  status = 
adapter->host_intf_write_pkt(common->priv,
dad0d04f Fariya Fatima 2014-03-16  205  
  (u8 *)msg,
dad0d04f Fariya Fatima 2014-03-16  206  
  skb->len);
dad0d04f Fariya Fatima 2014-03-16  207  if (status)
dad0d04f Fariya Fatima 2014-03-16  208  rsi_dbg(ERR_ZONE, "%s: 
Failed to write the packet\n", __func__);
dad0d04f Fariya Fatima 2014-03-16  209  
dad0d04f Fariya Fatima 2014-03-16  210  err:
dad0d04f Fariya Fatima 2014-03-16 @211  
rsi_indicate_tx_status(common->priv, skb, status);
dad0d04f Fariya Fatima 2014-03-16  212  return status;
dad0d04f Fariya Fatima 2014-03-16  213  }

:: The code at line 211 was first introduced by commit
:: dad0d04fa7ba41ce603a01e8e64967650303e9a2 rsi: Add RS9113 wireless driver

:: TO: Fariya Fatima 
:: CC: John W. Linville 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Reading the current monitor flags of an interface

2016-01-02 Thread Roger James 

Hi,

I am trying to get at the current monitor flags setting of a wireless 
monitor mode virtual interface. I have tried to do this by hacking the 
iw source. However I never see a NL80211_ATTR_MNTR_FLAGS attribute 
returned in response to the


TOPLEVEL(info, NULL, NL80211_CMD_GET_INTERFACE, 0, CIB_NETDEV, 
handle_interface_info,

 "Show information for this interface.");

command in iw's interface.c code.

Questions.

1. Am I wasting my time? (e.g. the rt2800usb driver never returns this 
info, or there is another easy way of viewing this info, etc)


2. Am I doing something stupid? (e.g. I need to specify some additional 
flags to the get command.)


3. Has anybody else tried this before?

All help will be gratefully received.

Roger


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()

2016-01-02 Thread Souptick Joarder 
On Sat, Jan 2, 2016 at 2:02 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Fri, 1 Jan 2016 21:16:01 +0100
>
> Rename a jump label according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/net/wireless/intel/iwlegacy/common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c 
> b/drivers/net/wireless/intel/iwlegacy/common.c
> index ae45fd3..660ab2b 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
> @@ -759,7 +759,7 @@ il_eeprom_init(struct il_priv *il)
>  IL_EEPROM_ACCESS_TIMEOUT);
> if (ret < 0) {
> IL_ERR("Time out reading EEPROM[%d]\n", addr);
> -   goto done;
> +   goto release_semaphore;

Current code looks good.
> }
> r = _il_rd(il, CSR_EEPROM_REG);
> e[addr / 2] = cpu_to_le16(r >> 16);
> @@ -769,7 +769,7 @@ il_eeprom_init(struct il_priv *il)
>  il_eeprom_query16(il, EEPROM_VERSION));
>
> ret = 0;
> -done:
> +release_semaphore:
> il->ops->eeprom_release_semaphore(il);
>
> if (ret) {
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-Souptick
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 19:22:36 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..571eaba 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
  struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *wh = NULL;
+   struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = &hw->conf;
struct skb_info *tx_params;
int status = -E2BIG;
-   __le16 *msg = NULL;
-   u8 extnd_size = 0;
+   __le16 *msg;
+   u8 extnd_size;
u8 vap_id = 0;
 
info = IEEE80211_SKB_CB(skb);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] brcmfmac: obtain feature info using 'cap' firmware command

2016-01-02 Thread Mathy Vanhoef 
On Fri, Jan 1, 2016 at 8:05 AM, Arend van Spriel  wrote:
> On 12/29/2015 09:58 PM, Mathy Vanhoef wrote:
>>
>> On Mon, Dec 14, 2015 at 2:39 PM, Arend van Spriel 
>> wrote:
>>>
>>> Several features in the driver directly map to a firmware feature
>>> listed in response of the 'cap' firmware command. For those features
>>> this response will be examined instead of attempting individual
>>> firmware commands.
>>>
>>> Reviewed-by: Hante Meuleman 
>>> Reviewed-by: Franky (Zhenhui) Lin 
>>> Reviewed-by: Pieter-Paul Giesberts 
>
>
> Hi Mathy,
>
> Do you want credits, ie. have "Reviewed-by:" tag added?

Sure, glad I could help.

Kr,
Mathy

>
> Regards,
> Arend
>
>
>>> Signed-off-by: Arend van Spriel 
>>> ---
>>>   .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 51
>>> +-
>>>   1 file changed, 30 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>>> index d9d1ca4..08b7200 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>>> @@ -40,6 +40,17 @@ static const char *brcmf_feat_names[] = {
>>>   };
>>>   #undef BRCMF_FEAT_DEF
>>>
>>> +struct brcmf_feat_fwcap {
>>> +   enum brcmf_feat_id feature;
>>> +   const char * const fwcap_id;
>>> +};
>>> +
>>> +static const struct brcmf_feat_fwcap brcmf_fwcap_map[] = {
>>> +   { BRCMF_FEAT_MBSS, "mbss" },
>>> +   { BRCMF_FEAT_MCHAN, "mchan" },
>>> +   { BRCMF_FEAT_P2P, "p2p" },
>>> +};
>>> +
>>>   #ifdef DEBUG
>>>   /*
>>>* expand quirk list to array of quirk strings.
>>> @@ -104,25 +115,22 @@ static void brcmf_feat_iovar_int_get(struct
>>> brcmf_if *ifp,
>>>  }
>>>   }
>>>
>>> -/**
>>> - * brcmf_feat_iovar_int_set() - determine feature through iovar set.
>>> - *
>>> - * @ifp: interface to query.
>>> - * @id: feature id.
>>> - * @name: iovar name.
>>> - */
>>> -static void brcmf_feat_iovar_int_set(struct brcmf_if *ifp,
>>> -enum brcmf_feat_id id, char *name,
>>> u32 val)
>>> +static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
>>>   {
>>> -   int err;
>>> -
>>> -   err = brcmf_fil_iovar_int_set(ifp, name, val);
>>> -   if (err == 0) {
>>> -   brcmf_dbg(INFO, "enabling feature: %s\n",
>>> brcmf_feat_names[id]);
>>> -   ifp->drvr->feat_flags |= BIT(id);
>>> -   } else {
>>> -   brcmf_dbg(TRACE, "%s feature check failed: %d\n",
>>> - brcmf_feat_names[id], err);
>>> +   char caps[256];
>>> +   enum brcmf_feat_id id;
>>> +   int i;
>>> +
>>> +   brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
>>> +   brcmf_dbg(INFO, "[ %s]\n", caps);
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(brcmf_fwcap_map); i++) {
>>> +   if (strnstr(caps, brcmf_fwcap_map[i].fwcap_id,
>>> sizeof(caps))) {
>>> +   id = brcmf_fwcap_map[i].feature;
>>> +   brcmf_dbg(INFO, "enabling feature: %s\n",
>>> + brcmf_feat_names[id]);
>>> +   ifp->drvr->feat_flags |= BIT(id);
>>> +   }
>>>  }
>>>   }
>>>
>>> @@ -130,13 +138,14 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
>>>   {
>>>  struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
>>>
>>> -   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_MCHAN, "mchan");
>>> +   brcmf_feat_firmware_capabilities(ifp);
>>> +
>>>  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
>>>  if (drvr->bus_if->wowl_supported)
>>>  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");
>>> +   /* MBSS does not work for 43362 */
>>>  if (drvr->bus_if->chip != BRCM_CC_43362_CHIP_ID)
>>> -   brcmf_feat_iovar_int_set(ifp, BRCMF_FEAT_MBSS, "mbss",
>>> 0);
>>> -   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_P2P, "p2p");
>>> +   ifp->drvr->feat_flags &= BIT(BRCMF_FEAT_MBSS);
>>
>>
>> Missing ~ before the BIT() declaration? If the if-test fails, all bits are
>> cleared except BRCMF_FEAT_MBSS. I think the if-test also needs to be
>> updated
>> to an equals `==` instead of the old inequality. So one would get:
>>
>> /* clear MBSS feature for the 43362 (does not work) */
>> if (drvr->bus_if->chip == BRCM_CC_43362_CHIP_ID)
>>  ifp->drvr->feat_flags &= ~BIT(BRCMF_FEAT_MBSS);
>>
>> Also, happy holidays!
>> Mathy
>>
>>>  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_RSDB, "rsdb_mode");
>>>  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_TDLS, "tdls_enable");
>>>
>>> --
>>> 1.9.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
>>> in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wire

Re: [PATCH V2 4/7] brcmfmac: obtain feature info using 'cap' firmware command

2016-01-02 Thread Mathy Vanhoef 
On Sat, Jan 2, 2016 at 9:41 AM, Arend van Spriel  wrote:
> Several features in the driver directly map to a firmware feature
> listed in response of the 'cap' firmware command. For those features
> this response will be examined instead of attempting individual
> firmware commands.
>
> Reviewed-by: Hante Meuleman 
> Reviewed-by: Franky (Zhenhui) Lin 
> Reviewed-by: Pieter-Paul Giesberts 
> Signed-off-by: Arend van Spriel 
> ---
> V2:
>  - Correct feature bitmap update for 43362.
> ---
>  .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 53 
> +-
>  1 file changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> index e7ac8a2..d41f343 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> @@ -42,6 +42,17 @@ static const char *brcmf_feat_names[] = {
>  };
>  #undef BRCMF_FEAT_DEF
>
> +struct brcmf_feat_fwcap {
> +   enum brcmf_feat_id feature;
> +   const char * const fwcap_id;
> +};
> +
> +static const struct brcmf_feat_fwcap brcmf_fwcap_map[] = {
> +   { BRCMF_FEAT_MBSS, "mbss" },
> +   { BRCMF_FEAT_MCHAN, "mchan" },
> +   { BRCMF_FEAT_P2P, "p2p" },
> +};
> +
>  #ifdef DEBUG
>  /*
>   * expand quirk list to array of quirk strings.
> @@ -106,25 +117,22 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if 
> *ifp,
> }
>  }
>
> -/**
> - * brcmf_feat_iovar_int_set() - determine feature through iovar set.
> - *
> - * @ifp: interface to query.
> - * @id: feature id.
> - * @name: iovar name.
> - */
> -static void brcmf_feat_iovar_int_set(struct brcmf_if *ifp,
> -enum brcmf_feat_id id, char *name, u32 
> val)
> +static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
>  {
> -   int err;
> -
> -   err = brcmf_fil_iovar_int_set(ifp, name, val);
> -   if (err == 0) {
> -   brcmf_dbg(INFO, "enabling feature: %s\n", 
> brcmf_feat_names[id]);
> -   ifp->drvr->feat_flags |= BIT(id);
> -   } else {
> -   brcmf_dbg(TRACE, "%s feature check failed: %d\n",
> - brcmf_feat_names[id], err);
> +   char caps[256];
> +   enum brcmf_feat_id id;
> +   int i;
> +
> +   brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
> +   brcmf_dbg(INFO, "[ %s]\n", caps);
> +
> +   for (i = 0; i < ARRAY_SIZE(brcmf_fwcap_map); i++) {
> +   if (strnstr(caps, brcmf_fwcap_map[i].fwcap_id, sizeof(caps))) 
> {
> +   id = brcmf_fwcap_map[i].feature;
> +   brcmf_dbg(INFO, "enabling feature: %s\n",
> + brcmf_feat_names[id]);
> +   ifp->drvr->feat_flags |= BIT(id);
> +   }
> }
>  }
>
> @@ -134,13 +142,14 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
> struct brcmf_pno_macaddr_le pfn_mac;
> s32 err;
>
> -   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_MCHAN, "mchan");
> +   brcmf_feat_firmware_capabilities(ifp);
> +
> brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
> if (drvr->bus_if->wowl_supported)
> brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");
> -   if (drvr->bus_if->chip != BRCM_CC_43362_CHIP_ID)
> -   brcmf_feat_iovar_int_set(ifp, BRCMF_FEAT_MBSS, "mbss", 0);
> -   brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_P2P, "p2p");
> +   /* MBSS does not work for 43362 */
> +   if (drvr->bus_if->chip == BRCM_CC_43362_CHIP_ID)
> +   ifp->drvr->feat_flags &= ~BIT(BRCMF_FEAT_MBSS);
> brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_RSDB, "rsdb_mode");
> brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_TDLS, "tdls_enable");
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Mathy Vanhoef 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] NFC-mei_phy: Fine-tuning for two function implementations

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 21:47:30 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Refactoring for mei_nfc_connect()
  Refactoring for mei_nfc_if_version()
  Delete an unnecessary variable initialisation in mei_nfc_if_version()

 drivers/nfc/mei_phy.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] NFC-mei_phy: Refactoring for mei_nfc_connect()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 21:21:24 +0100

This issue was detected by using the Coccinelle software.

Adjust jump targets according to the current Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/nfc/mei_phy.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 83deda4..8e3a69f 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -173,8 +173,8 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
 
reply = kzalloc(connect_resp_length, GFP_KERNEL);
if (!reply) {
-   kfree(cmd);
-   return -ENOMEM;
+   r = -ENOMEM;
+   goto free_cmd;
}
 
connect_resp = (struct mei_nfc_connect_resp *)reply->data;
@@ -189,7 +189,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
r = mei_cldev_send(phy->cldev, (u8 *)cmd, connect_length);
if (r < 0) {
pr_err("Could not send connect cmd %d\n", r);
-   goto err;
+   goto free_reply;
}
 
bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
@@ -197,7 +197,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
if (bytes_recv < 0) {
r = bytes_recv;
pr_err("Could not read connect response %d\n", r);
-   goto err;
+   goto free_reply;
}
 
MEI_DUMP_NFC_HDR("connect reply", &reply->hdr);
@@ -210,11 +210,10 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
connect_resp->me_hotfix, connect_resp->me_build);
 
r = 0;
-
-err:
+free_reply:
kfree(reply);
+free_cmd:
kfree(cmd);
-
return r;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] NFC-mei_phy: Delete an unnecessary variable initialisation in mei_nfc_if_version()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 21:40:10 +0100

Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.

Signed-off-by: Markus Elfring 
---
 drivers/nfc/mei_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 3c74028..99fd87d 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -105,7 +105,7 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
 {
 
struct mei_nfc_cmd cmd;
-   struct mei_nfc_reply *reply = NULL;
+   struct mei_nfc_reply *reply;
struct mei_nfc_if_version *version;
size_t if_version_length;
int bytes_recv, r;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] NFC-mei_phy: Refactoring for mei_nfc_if_version()

2016-01-02 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sat, 2 Jan 2016 21:33:04 +0100

Rename a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/nfc/mei_phy.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 8e3a69f..3c74028 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -136,7 +136,7 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
pr_err("Could not read IF version\n");
r = -EIO;
-   goto err;
+   goto free_reply;
}
 
version = (struct mei_nfc_if_version *)reply->data;
@@ -144,8 +144,7 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
phy->fw_ivn = version->fw_ivn;
phy->vendor_id = version->vendor_id;
phy->radio_type = version->radio_type;
-
-err:
+free_reply:
kfree(reply);
return r;
 }
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] NFC-mei_phy: Refactoring for mei_nfc_connect()

2016-01-02 Thread Julian Calaby 
Hi Markus,

On Sun, Jan 3, 2016 at 7:54 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sat, 2 Jan 2016 21:21:24 +0100
>
> This issue was detected by using the Coccinelle software.
>
> Adjust jump targets according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/nfc/mei_phy.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
> index 83deda4..8e3a69f 100644
> --- a/drivers/nfc/mei_phy.c
> +++ b/drivers/nfc/mei_phy.c
> @@ -173,8 +173,8 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
>
> reply = kzalloc(connect_resp_length, GFP_KERNEL);
> if (!reply) {
> -   kfree(cmd);
> -   return -ENOMEM;
> +   r = -ENOMEM;
> +   goto free_cmd;
> }
>
> connect_resp = (struct mei_nfc_connect_resp *)reply->data;
> @@ -189,7 +189,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
> r = mei_cldev_send(phy->cldev, (u8 *)cmd, connect_length);
> if (r < 0) {
> pr_err("Could not send connect cmd %d\n", r);
> -   goto err;
> +   goto free_reply;
> }
>
> bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> @@ -197,7 +197,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
> if (bytes_recv < 0) {
> r = bytes_recv;
> pr_err("Could not read connect response %d\n", r);
> -   goto err;
> +   goto free_reply;
> }
>
> MEI_DUMP_NFC_HDR("connect reply", &reply->hdr);
> @@ -210,11 +210,10 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
> connect_resp->me_hotfix, connect_resp->me_build);
>
> r = 0;
> -
> -err:
> +free_reply:
> kfree(reply);
> +free_cmd:
> kfree(cmd);
> -

Why are you deleting the two blank lines here?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] NFC-mei_phy: Refactoring for mei_nfc_connect()

2016-01-02 Thread SF Markus Elfring 
>> r = 0;
>> -
>> -err:
>> +free_reply:
>> kfree(reply);
>> +free_cmd:
>> kfree(cmd);
>> -
> 
> Why are you deleting the two blank lines here?

Can they be unnecessary at this source code place
according to the Linux coding style convention?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] NFC-mei_phy: Refactoring for mei_nfc_connect()

2016-01-02 Thread Joe Perches 
On Sun, 2016-01-03 at 08:00 +0100, SF Markus Elfring wrote:
> > > r = 0;
> > > -
> > > -err:
> > > +free_reply:
> > > kfree(reply);
> > > +free_cmd:
> > > kfree(cmd);
> > > -
> > 
> > Why are you deleting the two blank lines here?
> 
> Can they be unnecessary at this source code place
> according to the Linux coding style convention?

As far as I know, there's no linux specific accepted
convention for blank lines preceding labels.

My personal preference is for a blank line before a
new block, but not before the second and subsequent
labels in an error handling block.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html