Re: [Patch net] nfc: check sock state in llcp_sock_getname()
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()
>> 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()
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
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
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
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
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
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
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
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
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
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()
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()
>> 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()
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()
> 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
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()
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()
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()
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()
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()
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()
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
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()
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()
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
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
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
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()
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()
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()
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()
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()
>> 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()
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