* Re: [PATCH] mwifiex: printk() overflow with 32-byte SSIDs
From: Kalle Valo @ 2016-11-09 22:20 UTC (permalink / raw)
To: Brian Norris
Cc: Amitkumar Karwar, Nishant Sarmukadam, linux-kernel,
linux-wireless, Cathy Luo, security, stable
In-Reply-To: <1478658504-31045-1-git-send-email-briannorris@chromium.org>
Brian Norris <briannorris@chromium.org> writes:
> SSIDs aren't guaranteed to be 0-terminated. Let's cap the max length
> when we print them out.
>
> This can be easily noticed by connecting to a network with a 32-octet
> SSID:
>
> [ 3903.502925] mwifiex_pcie 0000:01:00.0: info: trying to associate to
> '0123456789abcdef0123456789abcdef <uninitialized mem>' bssid
> xx:xx:xx:xx:xx:xx
>
> Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Cc: <stable@vger.kernel.org>
I'm planning to push this to 4.9 if no objections.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] mwifiex: printk() overflow with 32-byte SSIDs
From: Brian Norris @ 2016-11-09 22:22 UTC (permalink / raw)
To: Kalle Valo
Cc: Amitkumar Karwar, Nishant Sarmukadam, linux-kernel,
linux-wireless, Cathy Luo, security, stable
In-Reply-To: <87h97guwfu.fsf@kamboji.qca.qualcomm.com>
On Thu, Nov 10, 2016 at 12:20:53AM +0200, Kalle Valo wrote:
> Brian Norris <briannorris@chromium.org> writes:
>
> > SSIDs aren't guaranteed to be 0-terminated. Let's cap the max length
> > when we print them out.
> >
> > This can be easily noticed by connecting to a network with a 32-octet
> > SSID:
> >
> > [ 3903.502925] mwifiex_pcie 0000:01:00.0: info: trying to associate to
> > '0123456789abcdef0123456789abcdef <uninitialized mem>' bssid
> > xx:xx:xx:xx:xx:xx
> >
> > Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > Cc: <stable@vger.kernel.org>
>
> I'm planning to push this to 4.9 if no objections.
SGTM. Thanks.
^ permalink raw reply
* Re: [v6] ath9k: Switch to using mac80211 intermediate software queues.
From: Kalle Valo @ 2016-11-09 22:42 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: make-wifi-fast, linux-wireless, Toke Høiland-Jørgensen,
Tim Shepard, Felix Fietkau
In-Reply-To: <20161109113149.5724-1-toke@toke.dk>
Toke Høiland-Jørgensen wrote:
> This switches ath9k over to using the mac80211 intermediate software
> queueing mechanism for data packets. It removes the queueing inside the
> driver, except for the retry queue, and instead pulls from mac80211 when
> a packet is needed. The retry queue is used to store a packet that was
> pulled but can't be sent immediately.
>
> The old code path in ath_tx_start that would queue packets has been
> removed completely, as has the qlen limit tunables (since there's no
> longer a queue in the driver to limit).
>
> The mac80211 intermediate software queues offer significant latency
> reductions, and this patch allows ath9k to realise them. The exact gains
> from this varies with the test scenario, but in an access point scenario
> we have seen latency reductions ranging from 1/3 to as much as an order
> of magnitude. We also achieve slightly better aggregation.
>
> Median latency (ping) figures with this patch applied at the access point,
> with two high-rate stations and one low-rate station (HT20 5Ghz), running
> a Flent rtt_fair_var_up test with one TCP flow and one ping flow going to
> each station:
>
> Fast station Slow station
> Default pfifo_fast qdisc: 430.4 ms 638.7 ms
> fq_codel qdisc on iface: 35.5 ms 211.8 ms
> This patch set: 22.4 ms 38.2 ms
>
> Median aggregation sizes over the same test:
>
> Default pfifo_fast qdisc: 9.5 pkts 1.9 pkts
> fq_codel qdisc on iface: 11.2 pkts 1.9 pkts
> This patch set: 13.9 pkts 1.9 pkts
>
> This patch is based on Tim's original patch set, but reworked quite
> thoroughly.
>
> Cc: Tim Shepard <shep@alum.mit.edu>
> Cc: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Perfect, just what I was hoping to see :) Unless something really
surprising comes up I should apply this within the next few days.
--
https://patchwork.kernel.org/patch/9419029/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [v6] ath9k: Switch to using mac80211 intermediate software queues.
From: Toke Høiland-Jørgensen @ 2016-11-09 23:10 UTC (permalink / raw)
To: Kalle Valo; +Cc: make-wifi-fast, linux-wireless, Tim Shepard, Felix Fietkau
In-Reply-To: <689eea51fa77432cbe4c70c6c2fe7b13@euamsexm01a.eu.qualcomm.com>
Kalle Valo <kvalo@qca.qualcomm.com> writes:
> Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> This switches ath9k over to using the mac80211 intermediate software
>> queueing mechanism for data packets. It removes the queueing inside the
>> driver, except for the retry queue, and instead pulls from mac80211 when
>> a packet is needed. The retry queue is used to store a packet that was
>> pulled but can't be sent immediately.
>>=20
>> The old code path in ath_tx_start that would queue packets has been
>> removed completely, as has the qlen limit tunables (since there's no
>> longer a queue in the driver to limit).
>>=20
>> The mac80211 intermediate software queues offer significant latency
>> reductions, and this patch allows ath9k to realise them. The exact gains
>> from this varies with the test scenario, but in an access point scenario
>> we have seen latency reductions ranging from 1/3 to as much as an order
>> of magnitude. We also achieve slightly better aggregation.
>>=20
>> Median latency (ping) figures with this patch applied at the access poin=
t,
>> with two high-rate stations and one low-rate station (HT20 5Ghz), running
>> a Flent rtt_fair_var_up test with one TCP flow and one ping flow going to
>> each station:
>>=20
>> Fast station Slow station
>> Default pfifo_fast qdisc: 430.4 ms 638.7 ms
>> fq_codel qdisc on iface: 35.5 ms 211.8 ms
>> This patch set: 22.4 ms 38.2 ms
>>=20
>> Median aggregation sizes over the same test:
>>=20
>> Default pfifo_fast qdisc: 9.5 pkts 1.9 pkts
>> fq_codel qdisc on iface: 11.2 pkts 1.9 pkts
>> This patch set: 13.9 pkts 1.9 pkts
>>=20
>> This patch is based on Tim's original patch set, but reworked quite
>> thoroughly.
>>=20
>> Cc: Tim Shepard <shep@alum.mit.edu>
>> Cc: Felix Fietkau <nbd@nbd.name>
>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>
> Perfect, just what I was hoping to see :) Unless something really
> surprising comes up I should apply this within the next few days.
Awesome, thanks! :)
-Toke
^ permalink raw reply
* Re: [PATCH] mwifiex: fix memory leak in mwifiex_save_hidden_ssid_channels()
From: Brian Norris @ 2016-11-09 23:27 UTC (permalink / raw)
To: Ricky Liang
Cc: Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
open list:MARVELL MWIFIEX WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <1478662648-70698-1-git-send-email-jcliang@chromium.org>
On Wed, Nov 09, 2016 at 11:37:28AM +0800, Ricky Liang wrote:
> kmemleak reports memory leak in mwifiex_save_hidden_ssid_channels():
>
> unreferenced object 0xffffffc0a2914780 (size 192):
> comm "ksdioirqd/mmc2", pid 2004, jiffies 4307182506 (age 820.684s)
> hex dump (first 32 bytes):
> 00 06 47 49 4e 2d 32 67 01 03 c8 60 6c 03 01 40 ..GIN-2g...`l..@
> 07 10 54 57 20 34 04 1e 64 05 24 84 03 24 95 04 ..TW 4..d.$..$..
> backtrace:
> [<ffffffc0003375f4>] create_object+0x164/0x2b4
> [<ffffffc0008e3530>] kmemleak_alloc+0x50/0x88
> [<ffffffc000335120>] __kmalloc_track_caller+0x1bc/0x264
> [<ffffffc00030899c>] kmemdup+0x38/0x64
> [<ffffffbffc2311cc>] mwifiex_fill_new_bss_desc+0x3c/0x130 [mwifiex]
> [<ffffffbffc22ee9c>] mwifiex_save_curr_bcn+0x4ec/0x640 [mwifiex]
> [<ffffffbffc22f45c>] mwifiex_handle_event_ext_scan_report+0x1d4/0x268 [mwifiex]
> [<ffffffbffc2375d0>] mwifiex_process_sta_event+0x378/0x898 [mwifiex]
> [<ffffffbffc224dc8>] mwifiex_process_event+0x1a8/0x1e8 [mwifiex]
> [<ffffffbffc2228f0>] mwifiex_main_process+0x258/0x534 [mwifiex]
> [<ffffffbffc258858>] 0xffffffbffc258858
> [<ffffffc00071ee90>] process_sdio_pending_irqs+0xf8/0x160
> [<ffffffc00071efdc>] sdio_irq_thread+0x9c/0x1a4
> [<ffffffc000240d08>] kthread+0xf4/0x100
> [<ffffffc0002043fc>] ret_from_fork+0xc/0x50
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Signed-off-by: Ricky Liang <jcliang@chromium.org>
> ---
> drivers/net/wireless/marvell/mwifiex/scan.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 97c9765..98ce072 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1671,6 +1671,10 @@ static int mwifiex_save_hidden_ssid_channels(struct mwifiex_private *priv,
> }
>
> done:
> + /* beacon_ie buffer was allocated in function
> + * mwifiex_fill_new_bss_desc(). Free it now.
> + */
> + kfree(bss_desc->beacon_buf);
For a bit, I thought this was possibly a sort of double-free, since
mwifiex_fill_new_bss_desc() might actually fail to allocate
->beacon_buf, but kfree(NULL) is safe, so:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> kfree(bss_desc);
> return 0;
> }
> --
> 2.6.6
>
^ permalink raw reply
* Re: [PATCH 2/4] ath10k: Add support to update btcoex priority value via nl80211
From: Tamizh chelvam @ 2016-11-10 6:27 UTC (permalink / raw)
To: Julia Lawall; +Cc: c_traja, ath10k, linux-wireless, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1611092149120.2206@hadrien>
On 2016-11-10 02:20, Julia Lawall wrote:
> It seems possible that one might like to release the mutex lock before
> the
> return on line 7556.
>
yes mutex lock needs to release. I will send a v2 patch with the change.
> julia
>
>
>
> On Wed, 9 Nov 2016, kbuild test robot wrote:
>
>> Hi Tamizh,
>>
>> [auto build test WARNING on ath6kl/ath-next]
>> [cannot apply to v4.9-rc4 next-20161108]
>> [if your patch is applied to the wrong git tree, please drop us a note
>> to help improve the system]
>>
>> url:
>> https://github.com/0day-ci/linux/commits/c_traja-qti-qualcomm-com/ath10k-Add-support-for-BTCOEX-feature/20161109-043718
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>> ath-next
>> :::::: branch date: 2 hours ago
>> :::::: commit date: 2 hours ago
>>
>> >> drivers/net/wireless/ath/ath10k/mac.c:7556:2-8: preceding lock on line 7545
>>
>> git remote add linux-review https://github.com/0day-ci/linux
>> git remote update linux-review
>> git checkout 2fc922b27f111b9e8089a3e94a17ee827e769c55
>> vim +7556 drivers/net/wireless/ath/ath10k/mac.c
>>
>> 2fc922b2 Tamizh chelvam 2016-11-08 7539
>> 2fc922b2 Tamizh chelvam 2016-11-08 7540 if
>> (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags))) {
>> 2fc922b2 Tamizh chelvam 2016-11-08 7541 ret = -EINVAL;
>> 2fc922b2 Tamizh chelvam 2016-11-08 7542 goto exit;
>> 2fc922b2 Tamizh chelvam 2016-11-08 7543 }
>> 2fc922b2 Tamizh chelvam 2016-11-08 7544
>> 2fc922b2 Tamizh chelvam 2016-11-08 @7545
>> mutex_lock(&ar->conf_mutex);
>> 2fc922b2 Tamizh chelvam 2016-11-08 7546
>> 2fc922b2 Tamizh chelvam 2016-11-08 7547 if (ar->state !=
>> ATH10K_STATE_ON &&
>> 2fc922b2 Tamizh chelvam 2016-11-08 7548 ar->state !=
>> ATH10K_STATE_RESTARTED) {
>> 2fc922b2 Tamizh chelvam 2016-11-08 7549 ret = -ENETDOWN;
>> 2fc922b2 Tamizh chelvam 2016-11-08 7550 goto exit;
>> 2fc922b2 Tamizh chelvam 2016-11-08 7551 }
>> 2fc922b2 Tamizh chelvam 2016-11-08 7552
>> 2fc922b2 Tamizh chelvam 2016-11-08 7553 btcoex_prio =
>> ath10k_mac_get_btcoex_prio(btcoex_priority);
>> 2fc922b2 Tamizh chelvam 2016-11-08 7554
>> 2fc922b2 Tamizh chelvam 2016-11-08 7555 if (btcoex_prio > 0x3f)
>> 2fc922b2 Tamizh chelvam 2016-11-08 @7556 return -E2BIG;
>> 2fc922b2 Tamizh chelvam 2016-11-08 7557
>> 2fc922b2 Tamizh chelvam 2016-11-08 7558 ret =
>> ath10k_wmi_set_coex_param(ar, btcoex_prio);
>> 2fc922b2 Tamizh chelvam 2016-11-08 7559
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology
>> Center
>> https://lists.01.org/pipermail/kbuild-all Intel
>> Corporation
>>
^ permalink raw reply
* RE: [PATCH 5/5] mwifiex: wait for firmware dump completion in remove_card
From: Amitkumar Karwar @ 2016-11-10 10:01 UTC (permalink / raw)
To: Brian Norris
Cc: Kalle Valo, Dmitry Torokhov, linux-wireless@vger.kernel.org,
Cathy Luo, Nishant Sarmukadam, Xinming Hu
In-Reply-To: <20161109203710.GA118631@google.com>
Hi Brian,
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Thursday, November 10, 2016 2:07 AM
> To: Amitkumar Karwar
> Cc: Kalle Valo; Dmitry Torokhov; linux-wireless@vger.kernel.org; Cathy
> Luo; Nishant Sarmukadam; Xinming Hu
> Subject: Re: [PATCH 5/5] mwifiex: wait for firmware dump completion in
> remove_card
>
> On Wed, Nov 09, 2016 at 12:35:22PM +0000, Amitkumar Karwar wrote:
> > Hi Brian,
>
> Hi,
>
> > > From: Brian Norris [mailto:briannorris@chromium.org]
> > > Sent: Thursday, November 03, 2016 2:16 AM
> > > To: Kalle Valo
> > > Cc: Dmitry Torokhov; Amitkumar Karwar;
> > > linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> > > Xinming Hu
> > > Subject: Re: [PATCH 5/5] mwifiex: wait for firmware dump completion
> > > in remove_card
> > >
> > > On Thu, Oct 27, 2016 at 04:20:25PM +0300, Kalle Valo wrote:
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
> > > >
> > > > >> +/* reset_trigger variable is used to identify if
> > > > >> +mwifiex_sdio_remove()
> > > > >> + * is called by sdio_work during reset or the call is from
> > > > >> +sdio
> > > subsystem.
> > > > >> + * We will cancel sdio_work only if the call is from sdio
> > > subsystem.
> > > > >> + */
> > > > >> +static u8 reset_triggered;
> > > > >
> > > > > It would be really great if the driver supported multiple
> devices.
> > > > > IOW please avoid module-globals.
> > > >
> > > > Good catch, it's a hard requirement to support multiple devices at
> > > > the same time.
> > >
> > > BTW, this problem is repeated in several places throughout this
> driver.
> > > For instance, look for 'user_rmmod' (why? you shouldn't need to
> > > treat module unload differently...)
> >
> > We have 2 kinds of teardown cases.
> > 1) Chip is going to be powered off.
> > a) System reboot
> > b) Someone manually removed wifi card from system
> > 2) User unloaded the driver.
> >
> > In case 1. b), we can't have logic to terminate WIFI connection and
> > download SHUTDOWN command to firmware, as hardware itself is not
> > present.
>
> That seems like a poor way of determining the difference between hot
> unplug and clean shutdown. What if unplug happens concurrently with
> rmmod?
I agree. hotunplug thread may read the flag as "true" in this race and try to interact with hardware.
> Seems like it'd be better to identify hardware failures as such,
> and just skip talking to HW. Also, for interfaces that support unplug
> well (like USB), shouldn't you be able to get hotplug info from the
> driver core, instead of having to guess the inverse of that ("this was
> not a hotplug") from static variables like that?
You are right. We can identify hardware read/write failure and continue performing remaining cleanup.
This way "user_rmmod" flag can be avoided.
I will plan for this cleanup. Tests need to be performed with SDIO, PCIe use USB devices.
I just found hotunplug info is received in case of USB.(udev->state changed to USB_STATE_NOTATTACHED). We will make use of this.
>
> > This logic is useful when user just unloads and loads the driver.
> > Firmware download will be skipped in this case, as it's already
> > running. SHUTDOWN command sent during unload has cleared firmware's
> > state.
>
> Why is that useful?
Sending SHUTDOWN command helps firmware reset/clean it's state. As per our protocol, SHUTDOWN is the last command downloaded to firmware. After this, firmware's state would be same as freshly downloaded firmware.
Next time when driver is loaded, firmware download is skipped, as it's already active. Driver just need to send couple of initialization commands.
Regards,
Amitkumar
^ permalink raw reply
* cfg80211: add set/get link loss profile
From: Lazar, Alexei Avshalom @ 2016-11-10 11:33 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
>From b739abb6f29dc43a86b8b2b60e893b4441f8aa1f Mon Sep 17 00:00:00 2001
From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Date: Sun, 6 Nov 2016 16:21:20 +0200
Subject: [PATCH] cfg80211: add set/get link loss profile
Introduce NL80211_CMD_SET_LINK_LOSS_PROFILE and
NL80211_CMD_GET_LINK_LOSS_PROFILE as it required by the user space
to configure the link loss profile.
The link loss profile represents the priority of maintaining link up
in different link quality environments.
Three types of behavior for link loss defined:
LINK_LOSS_PROFILE_RELAXED: prefer maintaining link up even in poor
link quality environment.
LINK_LOSS_PROFILE_DEFAULT: The default behavior for maintaining link
up vs link quality.
LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link up in poor link
quality environment.
New cfg80211 API also been added, set/get_link_loss_profile.
Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 18 ++++++++
include/net/cfg80211.h | 13 ++++++
include/uapi/linux/nl80211.h | 32 +++++++++++++
net/wireless/nl80211.c | 70 +++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 28 ++++++++++++
net/wireless/trace.h | 39 ++++++++++++++++
6 files changed, 200 insertions(+)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index d117240..f8d576b 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -1424,6 +1424,22 @@ static void wil_cfg80211_stop_p2p_device(struct wiphy *wiphy,
mutex_unlock(&wil->mutex);
}
+static int
+wil_cfg80211_set_link_loss_profile(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr)
+{
+ return -ENOTSUPP;
+}
+
+static enum nl80211_link_loss_profile
+wil_cfg80211_get_link_loss_profile(struct wiphy *wiphy,
+ struct wireless_dev *wdev, const u8 *addr)
+{
+ return -ENOTSUPP;
+}
+
static struct cfg80211_ops wil_cfg80211_ops = {
.add_virtual_intf = wil_cfg80211_add_iface,
.del_virtual_intf = wil_cfg80211_del_iface,
@@ -1450,6 +1466,8 @@ static struct cfg80211_ops wil_cfg80211_ops = {
/* P2P device */
.start_p2p_device = wil_cfg80211_start_p2p_device,
.stop_p2p_device = wil_cfg80211_stop_p2p_device,
+ .set_link_loss_profile = wil_cfg80211_set_link_loss_profile,
+ .get_link_loss_profile = wil_cfg80211_get_link_loss_profile,
};
static void wil_wiphy_init(struct wiphy *wiphy)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 41ae3f5..2d7a0af 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2744,6 +2744,10 @@ struct cfg80211_nan_func {
* All other parameters must be ignored.
*
* @set_multicast_to_unicast: configure multicast to unicast conversion for BSS
+ *
+ * @set_link_loss_profile: Set link loss profile for specific connection.
+ * @get_link_loss_profile: Get the current link loss profile of specific
+ * connection.
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3024,6 +3028,15 @@ struct cfg80211_ops {
int (*set_multicast_to_unicast)(struct wiphy *wiphy,
struct net_device *dev,
const bool enabled);
+
+ int (*set_link_loss_profile)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr);
+ enum nl80211_link_loss_profile (*get_link_loss_profile)(
+ struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const u8 *addr);
};
/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index e21d23d..b68fad2 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -888,6 +888,11 @@
* This will contain a %NL80211_ATTR_NAN_MATCH nested attribute and
* %NL80211_ATTR_COOKIE.
*
+ * @NL80211_CMD_SET_LINK_LOSS_PROFILE: Set link loss profile (behavior) for
+ * specific connection.
+ * @NL80211_CMD_GET_LINK_LOSS_PROFILE: Get current link loss profile of specific
+ * connection.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -1085,6 +1090,9 @@ enum nl80211_commands {
NL80211_CMD_SET_MULTICAST_TO_UNICAST,
+ NL80211_CMD_SET_LINK_LOSS_PROFILE,
+ NL80211_CMD_GET_LINK_LOSS_PROFILE,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1969,6 +1977,9 @@ enum nl80211_commands {
* @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Indicates whether or not multicast
* packets should be send out as unicast to all stations (flag attribute).
*
+ * @NL80211_ATTR_LINK_LOSS_PROFILE: attribute that indicate the link loss
+ * behavior using &enum nl80211_link_loss_profile values.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2373,6 +2384,8 @@ enum nl80211_attrs {
NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED,
+ NL80211_ATTR_LINK_LOSS_PROFILE,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -5174,4 +5187,23 @@ enum nl80211_nan_match_attributes {
NL80211_NAN_MATCH_ATTR_MAX = NUM_NL80211_NAN_MATCH_ATTR - 1
};
+/**
+ * enum nl80211_link_loss_profile.
+ *
+ * Used by set_link_loss_profile() and get_link_loss_profile()
+ * to set/get link loss behavior.
+ *
+ * @NL80211_LINK_LOSS_PROFILE_RELAXED: prefer maintaining link
+ * up even in poor link quality environment
+ * @NL80211_LINK_LOSS_PROFILE_DEFAULT: The default behavior for
+ * maintaining link up vs link quality.
+ * @NL80211_LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link
+ * up in poor link quality environment
+ */
+enum nl80211_link_loss_profile {
+ NL80211_LINK_LOSS_PROFILE_RELAXED,
+ NL80211_LINK_LOSS_PROFILE_DEFAULT,
+ NL80211_LINK_LOSS_PROFILE_AGGRESSIVE
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 46b2e8c..c8dc7e2 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -418,6 +418,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
.len = FILS_MAX_KEK_LEN },
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
+ [NL80211_ATTR_LINK_LOSS_PROFILE] = { .type = NLA_U8 },
};
/* policy for the key attributes */
@@ -11792,6 +11793,60 @@ static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
return rdev_set_multicast_to_unicast(rdev, dev, enabled);
}
+static int nl80211_set_link_loss_profile(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ enum nl80211_link_loss_profile profile;
+ const u8 *addr;
+ int ret;
+
+ if (!info->attrs[NL80211_ATTR_LINK_LOSS_PROFILE] ||
+ !info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ profile = nla_get_u8(info->attrs[NL80211_ATTR_LINK_LOSS_PROFILE]);
+ addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+ if (profile != NL80211_LINK_LOSS_PROFILE_RELAXED &&
+ profile != NL80211_LINK_LOSS_PROFILE_DEFAULT &&
+ profile != NL80211_LINK_LOSS_PROFILE_AGGRESSIVE)
+ return -EINVAL;
+
+ if (!rdev->ops->set_link_loss_profile)
+ return -EOPNOTSUPP;
+
+ wdev_lock(wdev);
+ ret = rdev_set_link_loss_profile(rdev, wdev, profile, addr);
+ wdev_unlock(wdev);
+
+ return ret;
+}
+
+static int nl80211_get_link_loss_profile(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ const u8 *addr;
+ enum nl80211_link_loss_profile profile;
+
+ if (!info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+ if (!rdev->ops->get_link_loss_profile)
+ return -EOPNOTSUPP;
+
+ wdev_lock(wdev);
+ profile = rdev_get_link_loss_profile(rdev, wdev, addr);
+ wdev_unlock(wdev);
+
+ return profile;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -12659,6 +12714,21 @@ static const struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_SET_LINK_LOSS_PROFILE,
+ .doit = nl80211_set_link_loss_profile,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_GET_LINK_LOSS_PROFILE,
+ .doit = nl80211_get_link_loss_profile,
+ .policy = nl80211_policy,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};
/* notification functions */
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index e9ecf23..9a377c8 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1141,4 +1141,32 @@ rdev_set_coalesce(struct cfg80211_registered_device *rdev,
trace_rdev_return_int(&rdev->wiphy, ret);
return ret;
}
+
+static inline int
+rdev_set_link_loss_profile(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr)
+{
+ int ret;
+
+ trace_rdev_set_link_loss_profile(&rdev->wiphy, wdev, profile, addr);
+ ret = rdev->ops->set_link_loss_profile(&rdev->wiphy, wdev, profile,
+ addr);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
+static inline enum nl80211_link_loss_profile
+rdev_get_link_loss_profile(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ const u8 *addr)
+{
+ enum nl80211_link_loss_profile profile;
+
+ trace_rdev_get_link_loss_profile(&rdev->wiphy, wdev, addr);
+ profile = rdev->ops->get_link_loss_profile(&rdev->wiphy, wdev, addr);
+ trace_rdev_return_int(&rdev->wiphy, profile);
+ return profile;
+}
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 25f8318..d93a4b1 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2212,6 +2212,45 @@ TRACE_EVENT(rdev_tdls_cancel_channel_switch,
WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(addr))
);
+TRACE_EVENT(rdev_set_link_loss_profile,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile, const u8 *addr),
+ TP_ARGS(wiphy, wdev, profile, addr),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ __field(enum nl80211_link_loss_profile, profile)
+ MAC_ENTRY(addr)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ __entry->profile = profile;
+ MAC_ASSIGN(addr, addr);
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT ", " MAC_PR_FMT ", PROFILE %d",
+ WIPHY_PR_ARG, WDEV_PR_ARG, MAC_PR_ARG(addr),
+ __entry->profile)
+);
+
+TRACE_EVENT(rdev_get_link_loss_profile,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ const u8 *addr),
+ TP_ARGS(wiphy, wdev, addr),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ MAC_ENTRY(addr)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ MAC_ASSIGN(addr, addr);
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT ", " MAC_PR_FMT, WIPHY_PR_ARG,
+ WDEV_PR_ARG, MAC_PR_ARG(addr))
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
2.5.0
^ permalink raw reply related
* [PATCH RESEND v2 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Shengzhen Li, Xinming Hu
From: Shengzhen Li <szli@marvell.com>
We may get SLEEP event from firmware even if TXDone interrupt
for last Tx packet is still pending. In this case, we may
end up accessing PCIe memory for handling TXDone after power
save handshake is completed. This causes kernel crash with
external abort.
This patch will only allow downloading sleep confirm
when no tx done interrupt is pending in the hardware.
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Shengzhen Li <szli@marvell.com>
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: address format issues(Brain)
RESEND v2(Applicable for complete patch series):
1) Fixed syntax issue "changelog not placed after the Sign-offs"
pointed by Brian.
2) Dropped "[v2,03/12] mwifiex: don't do unbalanced free()'ing in
cleanup_if()" patch in this series. It was already sent by Brian
separately.
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 5 +++--
drivers/net/wireless/marvell/mwifiex/init.c | 1 +
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
drivers/net/wireless/marvell/mwifiex/pcie.c | 5 +++++
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 5347728..25a7475 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -1118,13 +1118,14 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
void
mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
{
- if (!adapter->cmd_sent &&
+ if (!adapter->cmd_sent && !atomic_read(&adapter->tx_hw_pending) &&
!adapter->curr_cmd && !IS_CARD_RX_RCVD(adapter))
mwifiex_dnld_sleep_confirm_cmd(adapter);
else
mwifiex_dbg(adapter, CMD,
- "cmd: Delay Sleep Confirm (%s%s%s)\n",
+ "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
(adapter->cmd_sent) ? "D" : "",
+ atomic_read(&adapter->tx_hw_pending) ? "T" : "",
(adapter->curr_cmd) ? "C" : "",
(IS_CARD_RX_RCVD(adapter)) ? "R" : "");
}
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 82839d9..b36cb3f 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -270,6 +270,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
adapter->adhoc_11n_enabled = false;
mwifiex_wmm_init(adapter);
+ atomic_set(&adapter->tx_hw_pending, 0);
sleep_cfm_buf = (struct mwifiex_opt_sleep_confirm *)
adapter->sleep_cfm->data;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 11abc49..b0c501d 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -857,6 +857,7 @@ struct mwifiex_adapter {
atomic_t rx_pending;
atomic_t tx_pending;
atomic_t cmd_pending;
+ atomic_t tx_hw_pending;
struct workqueue_struct *workqueue;
struct work_struct main_work;
struct workqueue_struct *rx_workqueue;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index ba1fa17..509c156 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -522,6 +522,7 @@ static int mwifiex_pcie_disable_host_int(struct mwifiex_adapter *adapter)
}
}
+ atomic_set(&adapter->tx_hw_pending, 0);
return 0;
}
@@ -721,6 +722,7 @@ static void mwifiex_cleanup_txq_ring(struct mwifiex_adapter *adapter)
card->tx_buf_list[i] = NULL;
}
+ atomic_set(&adapter->tx_hw_pending, 0);
return;
}
@@ -1158,6 +1160,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
-1);
else
mwifiex_write_data_complete(adapter, skb, 0, 0);
+ atomic_dec(&adapter->tx_hw_pending);
}
card->tx_buf_list[wrdoneidx] = NULL;
@@ -1250,6 +1253,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
wrindx = (card->txbd_wrptr & reg->tx_mask) >> reg->tx_start_ptr;
buf_pa = MWIFIEX_SKB_DMA_ADDR(skb);
card->tx_buf_list[wrindx] = skb;
+ atomic_inc(&adapter->tx_hw_pending);
if (reg->pfu_enabled) {
desc2 = card->txbd_ring[wrindx];
@@ -1327,6 +1331,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
done_unmap:
mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
card->tx_buf_list[wrindx] = NULL;
+ atomic_dec(&adapter->tx_hw_pending);
if (reg->pfu_enabled)
memset(desc2, 0, sizeof(*desc2));
else
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 02/11] mwifiex: complete blocked power save handshake in main process
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Shengzhen Li, Xinming Hu
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Shengzhen Li <szli@marvell.com>
Power save handshake with firmware might be blocked by on-going
data transfer.
this patch check the PS status in main process and complete
previous blocked PS handshake.
this patch also remove redudant check before call
mwifiex_check_ps_cond fuction.
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Shengzhen Li <szli@marvell.com>
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: 1. remove redudant check(Brain)
2. reorgnized to follow tx_hw_pending patch
---
drivers/net/wireless/marvell/mwifiex/main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 20c9b77..c710d5e 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -308,6 +308,9 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
/* We have tried to wakeup the card already */
if (adapter->pm_wakeup_fw_try)
break;
+ if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+ mwifiex_check_ps_cond(adapter);
+
if (adapter->ps_state != PS_STATE_AWAKE)
break;
if (adapter->tx_lock_flag) {
@@ -355,10 +358,8 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
/* Check if we need to confirm Sleep Request
received previously */
- if (adapter->ps_state == PS_STATE_PRE_SLEEP) {
- if (!adapter->cmd_sent && !adapter->curr_cmd)
- mwifiex_check_ps_cond(adapter);
- }
+ if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+ mwifiex_check_ps_cond(adapter);
/* * The ps_state may have been changed during processing of
* Sleep Request event.
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 03/11] mwifiex: resolve races between async FW init (failure) and device removal
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
It's possible for the FW init sequence to fail, which will trigger a
device cleanup sequence in mwifiex_fw_dpc(). This sequence can race with
device suspend() or remove() (e.g., reboot or unbind), and can trigger
use-after-free issues. Currently, this driver attempts (poorly) to
synchronize remove() using a semaphore, but it doesn't protect some of
the critical sections properly. Particularly, we grab a pointer to the
adapter struct (card->adapter) without checking if it's being freed or
not. We later do a NULL check on the adapter, but that doesn't work if
the adapter was freed.
Also note that the PCIe interface driver doesn't ever set card->adapter
to NULL, so even if we get the synchronization right, we still might try
to redo the cleanup in ->remove(), even if the FW init failure sequence
already did it.
This patch replaces the static semaphore with a per-device completion
struct, and uses that completion to synchronize the remove() thread with
the mwifiex_fw_dpc(). A future patch will utilize this completion to
synchronize the suspend() thread as well.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/main.c | 46 ++++++++++-------------------
drivers/net/wireless/marvell/mwifiex/main.h | 13 +++++---
drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++------
drivers/net/wireless/marvell/mwifiex/pcie.h | 2 ++
drivers/net/wireless/marvell/mwifiex/sdio.c | 18 +++++------
drivers/net/wireless/marvell/mwifiex/sdio.h | 2 ++
drivers/net/wireless/marvell/mwifiex/usb.c | 23 +++++++--------
drivers/net/wireless/marvell/mwifiex/usb.h | 2 ++
8 files changed, 55 insertions(+), 69 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index c710d5e..09d46d6 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -521,7 +521,6 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
struct mwifiex_private *priv;
struct mwifiex_adapter *adapter = context;
struct mwifiex_fw_image fw;
- struct semaphore *sem = adapter->card_sem;
bool init_failed = false;
struct wireless_dev *wdev;
@@ -670,7 +669,8 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
}
if (init_failed)
mwifiex_free_adapter(adapter);
- up(sem);
+ /* Tell all current and future waiters we're finished */
+ complete_all(adapter->fw_done);
return;
}
@@ -1365,7 +1365,7 @@ static void mwifiex_main_work_queue(struct work_struct *work)
* code is extracted from mwifiex_remove_card()
*/
static int
-mwifiex_shutdown_sw(struct mwifiex_adapter *adapter, struct semaphore *sem)
+mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
{
struct mwifiex_private *priv;
int i;
@@ -1373,8 +1373,9 @@ static void mwifiex_main_work_queue(struct work_struct *work)
if (!adapter)
goto exit_return;
- if (down_interruptible(sem))
- goto exit_sem_err;
+ wait_for_completion(adapter->fw_done);
+ /* Caller should ensure we aren't suspending while this happens */
+ reinit_completion(adapter->fw_done);
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
mwifiex_deauthenticate(priv, NULL);
@@ -1431,8 +1432,6 @@ static void mwifiex_main_work_queue(struct work_struct *work)
rtnl_unlock();
}
- up(sem);
-exit_sem_err:
mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
exit_return:
return 0;
@@ -1442,21 +1441,18 @@ static void mwifiex_main_work_queue(struct work_struct *work)
* code is extracted from mwifiex_add_card()
*/
static int
-mwifiex_reinit_sw(struct mwifiex_adapter *adapter, struct semaphore *sem,
+mwifiex_reinit_sw(struct mwifiex_adapter *adapter, struct completion *fw_done,
struct mwifiex_if_ops *if_ops, u8 iface_type)
{
char fw_name[32];
struct pcie_service_card *card = adapter->card;
- if (down_interruptible(sem))
- goto exit_sem_err;
-
mwifiex_init_lock_list(adapter);
if (adapter->if_ops.up_dev)
adapter->if_ops.up_dev(adapter);
adapter->iface_type = iface_type;
- adapter->card_sem = sem;
+ adapter->fw_done = fw_done;
adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING;
adapter->surprise_removed = false;
@@ -1507,7 +1503,8 @@ static void mwifiex_main_work_queue(struct work_struct *work)
}
strcpy(adapter->fw_name, fw_name);
mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
- up(sem);
+
+ complete_all(adapter->fw_done);
return 0;
err_init_fw:
@@ -1527,8 +1524,7 @@ static void mwifiex_main_work_queue(struct work_struct *work)
err_kmalloc:
mwifiex_terminate_workqueue(adapter);
adapter->surprise_removed = true;
- up(sem);
-exit_sem_err:
+ complete_all(adapter->fw_done);
mwifiex_dbg(adapter, INFO, "%s, error\n", __func__);
return -1;
@@ -1543,12 +1539,12 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
struct mwifiex_if_ops if_ops;
if (!prepare) {
- mwifiex_reinit_sw(adapter, adapter->card_sem, &if_ops,
+ mwifiex_reinit_sw(adapter, adapter->fw_done, &if_ops,
adapter->iface_type);
} else {
memcpy(&if_ops, &adapter->if_ops,
sizeof(struct mwifiex_if_ops));
- mwifiex_shutdown_sw(adapter, adapter->card_sem);
+ mwifiex_shutdown_sw(adapter);
}
}
EXPORT_SYMBOL_GPL(mwifiex_do_flr);
@@ -1618,15 +1614,12 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
* - Add logical interfaces
*/
int
-mwifiex_add_card(void *card, struct semaphore *sem,
+mwifiex_add_card(void *card, struct completion *fw_done,
struct mwifiex_if_ops *if_ops, u8 iface_type,
struct device *dev, bool of_node_valid)
{
struct mwifiex_adapter *adapter;
- if (down_interruptible(sem))
- goto exit_sem_err;
-
if (mwifiex_register(card, if_ops, (void **)&adapter)) {
pr_err("%s: software init failed\n", __func__);
goto err_init_sw;
@@ -1637,7 +1630,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
mwifiex_probe_of(adapter);
adapter->iface_type = iface_type;
- adapter->card_sem = sem;
+ adapter->fw_done = fw_done;
adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING;
adapter->surprise_removed = false;
@@ -1706,9 +1699,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
mwifiex_free_adapter(adapter);
err_init_sw:
- up(sem);
-exit_sem_err:
return -1;
}
EXPORT_SYMBOL_GPL(mwifiex_add_card);
@@ -1724,14 +1715,11 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
* - Unregister the device
* - Free the adapter structure
*/
-int mwifiex_remove_card(struct mwifiex_adapter *adapter, struct semaphore *sem)
+int mwifiex_remove_card(struct mwifiex_adapter *adapter)
{
struct mwifiex_private *priv = NULL;
int i;
- if (down_trylock(sem))
- goto exit_sem_err;
-
if (!adapter)
goto exit_remove;
@@ -1801,8 +1789,6 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter, struct semaphore *sem)
mwifiex_free_adapter(adapter);
exit_remove:
- up(sem);
-exit_sem_err:
return 0;
}
EXPORT_SYMBOL_GPL(mwifiex_remove_card);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index b0c501d..d0fbb2f 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -20,6 +20,7 @@
#ifndef _MWIFIEX_MAIN_H_
#define _MWIFIEX_MAIN_H_
+#include <linux/completion.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/sched.h>
@@ -985,7 +986,10 @@ struct mwifiex_adapter {
u32 usr_dot_11ac_mcs_support;
atomic_t pending_bridged_pkts;
- struct semaphore *card_sem;
+
+ /* For synchronizing FW initialization with device lifecycle. */
+ struct completion *fw_done;
+
bool ext_scan;
u8 fw_api_ver;
u8 key_api_major_ver, key_api_minor_ver;
@@ -1438,9 +1442,10 @@ static inline void mwifiex_enable_wake(struct mwifiex_adapter *adapter)
int mwifiex_init_shutdown_fw(struct mwifiex_private *priv,
u32 func_init_shutdown);
-int mwifiex_add_card(void *, struct semaphore *, struct mwifiex_if_ops *, u8,
- struct device *, bool);
-int mwifiex_remove_card(struct mwifiex_adapter *, struct semaphore *);
+int mwifiex_add_card(void *card, struct completion *fw_done,
+ struct mwifiex_if_ops *if_ops, u8 iface_type,
+ struct device *dev, bool of_node_valid);
+int mwifiex_remove_card(struct mwifiex_adapter *adapter);
void mwifiex_get_version(struct mwifiex_adapter *adapter, char *version,
int maxlen);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 509c156..c4bd64b 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -35,8 +35,6 @@
static struct mwifiex_if_ops pcie_ops;
-static struct semaphore add_remove_card_sem;
-
static const struct of_device_id mwifiex_pcie_of_match_table[] = {
{ .compatible = "pci11ab,2b42" },
{ .compatible = "pci1b4b,2b42" },
@@ -213,6 +211,8 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
card->dev = pdev;
if (ent->driver_data) {
@@ -234,7 +234,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
valid_of_node = true;
}
- ret = mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
+ ret = mwifiex_add_card(card, &card->fw_done, &pcie_ops,
MWIFIEX_PCIE, &pdev->dev, valid_of_node);
if (ret) {
pr_err("%s failed\n", __func__);
@@ -260,6 +260,8 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
if (!card)
return;
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
if (!adapter || !adapter->priv_num)
return;
@@ -279,7 +281,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
mwifiex_init_shutdown_fw(priv, MWIFIEX_FUNC_SHUTDOWN);
}
- mwifiex_remove_card(card->adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
}
static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
@@ -3181,8 +3183,7 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
/*
* This function initializes the PCIE driver module.
*
- * This initiates the semaphore and registers the device with
- * PCIE bus.
+ * This registers the device with PCIE bus.
*/
static int mwifiex_pcie_init_module(void)
{
@@ -3190,8 +3191,6 @@ static int mwifiex_pcie_init_module(void)
pr_debug("Marvell PCIe Driver\n");
- sema_init(&add_remove_card_sem, 1);
-
/* Clear the flag in case user removes the card. */
user_rmmod = 0;
@@ -3215,9 +3214,6 @@ static int mwifiex_pcie_init_module(void)
*/
static void mwifiex_pcie_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* Set the flag as user is removing this module. */
user_rmmod = 1;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
index 46f99ca..ae3365d 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
@@ -22,6 +22,7 @@
#ifndef _MWIFIEX_PCIE_H
#define _MWIFIEX_PCIE_H
+#include <linux/completion.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
@@ -345,6 +346,7 @@ struct pcie_service_card {
struct pci_dev *dev;
struct mwifiex_adapter *adapter;
struct mwifiex_pcie_device pcie;
+ struct completion fw_done;
u8 txbd_flush;
u32 txbd_wrptr;
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index ca5b0aa..90f45d9 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -49,8 +49,6 @@
static struct mwifiex_if_ops sdio_ops;
static unsigned long iface_work_flags;
-static struct semaphore add_remove_card_sem;
-
static struct memory_type_mapping generic_mem_type_map[] = {
{"DUMP", NULL, 0, 0xDD},
};
@@ -116,6 +114,8 @@ static int mwifiex_sdio_probe_of(struct device *dev)
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
card->func = func;
card->device_id = id;
@@ -156,7 +156,7 @@ static int mwifiex_sdio_probe_of(struct device *dev)
valid_of_node = true;
}
- ret = mwifiex_add_card(card, &add_remove_card_sem, &sdio_ops,
+ ret = mwifiex_add_card(card, &card->fw_done, &sdio_ops,
MWIFIEX_SDIO, &func->dev, valid_of_node);
if (ret) {
dev_err(&func->dev, "add card failed\n");
@@ -237,6 +237,8 @@ static int mwifiex_sdio_resume(struct device *dev)
if (!card)
return;
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
if (!adapter || !adapter->priv_num)
return;
@@ -254,7 +256,7 @@ static int mwifiex_sdio_resume(struct device *dev)
mwifiex_init_shutdown_fw(priv, MWIFIEX_FUNC_SHUTDOWN);
}
- mwifiex_remove_card(card->adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
}
/*
@@ -2716,14 +2718,11 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
/*
* This function initializes the SDIO driver.
*
- * This initiates the semaphore and registers the device with
- * SDIO bus.
+ * This registers the device with SDIO bus.
*/
static int
mwifiex_sdio_init_module(void)
{
- sema_init(&add_remove_card_sem, 1);
-
/* Clear the flag in case user removes the card. */
user_rmmod = 0;
@@ -2742,9 +2741,6 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
static void
mwifiex_sdio_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* Set the flag as user is removing this module. */
user_rmmod = 1;
cancel_work_sync(&sdio_work);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
index b9fbc5c..cdbf3a3a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -21,6 +21,7 @@
#define _MWIFIEX_SDIO_H
+#include <linux/completion.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/sdio_ids.h>
#include <linux/mmc/sdio_func.h>
@@ -238,6 +239,7 @@ struct sdio_mmc_card {
struct sdio_func *func;
struct mwifiex_adapter *adapter;
+ struct completion fw_done;
const char *firmware;
const struct mwifiex_sdio_card_reg *reg;
u8 max_ports;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 889203d..14f89fe 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -24,7 +24,6 @@
static u8 user_rmmod;
static struct mwifiex_if_ops usb_ops;
-static struct semaphore add_remove_card_sem;
static struct usb_device_id mwifiex_usb_table[] = {
/* 8766 */
@@ -386,6 +385,8 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
id_vendor = le16_to_cpu(udev->descriptor.idVendor);
id_product = le16_to_cpu(udev->descriptor.idProduct);
bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
@@ -475,7 +476,7 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, card);
- ret = mwifiex_add_card(card, &add_remove_card_sem, &usb_ops,
+ ret = mwifiex_add_card(card, &card->fw_done, &usb_ops,
MWIFIEX_USB, &card->udev->dev, false);
if (ret) {
pr_err("%s: mwifiex_add_card failed: %d\n", __func__, ret);
@@ -601,13 +602,15 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
struct usb_card_rec *card = usb_get_intfdata(intf);
struct mwifiex_adapter *adapter;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card) {
+ dev_err(&intf->dev, "%s: card is NULL\n", __func__);
return;
}
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
- if (!adapter->priv_num)
+ if (!adapter || !adapter->priv_num)
return;
if (user_rmmod && !adapter->mfg_mode) {
@@ -627,7 +630,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
mwifiex_dbg(adapter, FATAL,
"%s: removing card\n", __func__);
- mwifiex_remove_card(adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
usb_put_dev(interface_to_usbdev(intf));
}
@@ -1200,8 +1203,7 @@ static void mwifiex_usb_submit_rem_rx_urbs(struct mwifiex_adapter *adapter)
/* This function initializes the USB driver module.
*
- * This initiates the semaphore and registers the device with
- * USB bus.
+ * This registers the device with USB bus.
*/
static int mwifiex_usb_init_module(void)
{
@@ -1209,8 +1211,6 @@ static int mwifiex_usb_init_module(void)
pr_debug("Marvell USB8797 Driver\n");
- sema_init(&add_remove_card_sem, 1);
-
ret = usb_register(&mwifiex_usb_driver);
if (ret)
pr_err("Driver register failed!\n");
@@ -1230,9 +1230,6 @@ static int mwifiex_usb_init_module(void)
*/
static void mwifiex_usb_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* set the flag as user is removing this module */
user_rmmod = 1;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
index 30e8eb8..e5f204e 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.h
+++ b/drivers/net/wireless/marvell/mwifiex/usb.h
@@ -20,6 +20,7 @@
#ifndef _MWIFIEX_USB_H
#define _MWIFIEX_USB_H
+#include <linux/completion.h>
#include <linux/usb.h>
#define USB8XXX_VID 0x1286
@@ -75,6 +76,7 @@ struct usb_card_rec {
struct mwifiex_adapter *adapter;
struct usb_device *udev;
struct usb_interface *intf;
+ struct completion fw_done;
u8 rx_cmd_ep;
struct urb_context rx_cmd;
atomic_t rx_cmd_urb_pending;
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 04/11] mwifiex: remove redundant pdev check in suspend/resume handlers
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Amitkumar Karwar <akarwar@marvell.com>
to_pci_dev() would just do struct offset arithmetic on struct
device to get 'pdev' pointer. We never get NULL pdev pointer.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index c4bd64b..6152f08 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -117,14 +117,9 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pcie_service_card *card;
struct pci_dev *pdev = to_pci_dev(dev);
- if (pdev) {
- card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
- return 0;
- }
- } else {
- pr_err("PCIE device is not specified\n");
+ card = pci_get_drvdata(pdev);
+ if (!card || !card->adapter) {
+ pr_err("Card or adapter structure is not valid\n");
return 0;
}
@@ -162,14 +157,9 @@ static int mwifiex_pcie_resume(struct device *dev)
struct pcie_service_card *card;
struct pci_dev *pdev = to_pci_dev(dev);
- if (pdev) {
- card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
- return 0;
- }
- } else {
- pr_err("PCIE device is not specified\n");
+ card = pci_get_drvdata(pdev);
+ if (!card || !card->adapter) {
+ dev_err(dev, "Card or adapter structure is not valid\n");
return 0;
}
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 05/11] mwifiex: don't pretend to resume while remove()'ing
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
The device core will not allow suspend() to race with remove().
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 5 -----
drivers/net/wireless/marvell/mwifiex/sdio.c | 3 ---
drivers/net/wireless/marvell/mwifiex/usb.c | 5 -----
3 files changed, 13 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 6152f08..a2353f9 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -257,11 +257,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
return;
if (user_rmmod && !adapter->mfg_mode) {
-#ifdef CONFIG_PM_SLEEP
- if (adapter->is_suspended)
- mwifiex_pcie_resume(&pdev->dev);
-#endif
-
mwifiex_deauthenticate_all(adapter);
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 90f45d9..a2257a4 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -246,9 +246,6 @@ static int mwifiex_sdio_resume(struct device *dev)
mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
if (user_rmmod && !adapter->mfg_mode) {
- if (adapter->is_suspended)
- mwifiex_sdio_resume(adapter->dev);
-
mwifiex_deauthenticate_all(adapter);
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 14f89fe..558a7f1 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -614,11 +614,6 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
return;
if (user_rmmod && !adapter->mfg_mode) {
-#ifdef CONFIG_PM
- if (adapter->is_suspended)
- mwifiex_usb_resume(intf);
-#endif
-
mwifiex_deauthenticate_all(adapter);
mwifiex_init_shutdown_fw(mwifiex_get_priv(adapter,
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 06/11] mwifiex: resolve suspend() race with async FW init failure
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 12 ++++++++++--
drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++--
drivers/net/wireless/marvell/mwifiex/usb.c | 12 ++++++++++--
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index a2353f9..4d96683 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -118,12 +118,20 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
+ if (!card) {
+ dev_err(dev, "card structure is not valid\n");
return 0;
}
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(dev, "adapter is not valid\n");
+ return 0;
+ }
+
mwifiex_enable_wake(adapter);
/* Enable the Host Sleep */
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index a2257a4..39ffe7d 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -285,8 +285,8 @@ static int mwifiex_sdio_suspend(struct device *dev)
}
card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("suspend: invalid card or adapter\n");
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
} else {
@@ -294,7 +294,15 @@ static int mwifiex_sdio_suspend(struct device *dev)
return 0;
}
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(dev, "adapter is not valid\n");
+ return 0;
+ }
+
mwifiex_enable_wake(adapter);
/* Enable the Host Sleep */
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 558a7f1..671702c 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -503,11 +503,19 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
struct usb_tx_data_port *port;
int i, j;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card) {
+ dev_err(&intf->dev, "%s: card is NULL\n", __func__);
return 0;
}
+
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(&intf->dev, "card is not valid\n");
+ return 0;
+ }
if (unlikely(adapter->is_suspended))
mwifiex_dbg(adapter, WARN,
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 07/11] mwifiex: reset card->adapter during device unregister
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Xinming Hu, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Xinming Hu <huxm@marvell.com>
card->adapter gets initialized in mwifiex_register_dev(). As it's not
cleared in mwifiex_unregister_dev(), we may end up accessing the memory
which is already free in below scenario.
Scenario: Driver initialization is failed due to incorrect firmware or
some other reason. Meanwhile device reboot/unload occurs.
This is safe, now that we've properly synchronized suspend() and
remove() with the FW initialization thread; now that code can simply
check for 'card->adapter == NULL' and exit safely.
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 1 +
drivers/net/wireless/marvell/mwifiex/sdio.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 4d96683..1ab366c 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -3048,6 +3048,7 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
if (card->msi_enable)
pci_disable_msi(pdev);
}
+ card->adapter = NULL;
}
}
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 39ffe7d..4c4b012 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -2019,6 +2019,7 @@ static int mwifiex_alloc_sdio_mpa_buffers(struct mwifiex_adapter *adapter,
struct sdio_mmc_card *card = adapter->card;
if (adapter->card) {
+ card->adapter = NULL;
sdio_claim_host(card->func);
sdio_disable_func(card->func);
sdio_release_host(card->func);
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 08/11] mwifiex: usb: handle HS failures
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
SDIO and PCIe drivers handle this. Let's imitate it.
Not tested.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/usb.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 671702c..8bcfd92 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -521,7 +521,14 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
mwifiex_dbg(adapter, WARN,
"Device already suspended\n");
- mwifiex_enable_hs(adapter);
+ /* Enable the Host Sleep */
+ if (!mwifiex_enable_hs(adapter)) {
+ mwifiex_dbg(adapter, ERROR,
+ "cmd: failed to suspend\n");
+ adapter->hs_enabling = false;
+ return -EFAULT;
+ }
+
/* 'is_suspended' flag indicates device is suspended.
* It must be set here before the usb_kill_urb() calls. Reason
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 09/11] mwifiex: sdio: don't check for NULL sdio_func
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
sdio_func is retrieved via container_of() and should never be NULL.
Checking for NULL just makes the logic more confusing than necessary.
Stop doing that.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 40 +++++++++++------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 4c4b012..bee7326 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -190,15 +190,10 @@ static int mwifiex_sdio_resume(struct device *dev)
struct mwifiex_adapter *adapter;
mmc_pm_flag_t pm_flag = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("resume: invalid card or adapter\n");
- return 0;
- }
- } else {
- pr_err("resume: sdio_func is not specified\n");
+ pm_flag = sdio_get_host_pm_caps(func);
+ card = sdio_get_drvdata(func);
+ if (!card || !card->adapter) {
+ dev_err(dev, "resume: invalid card or adapter\n");
return 0;
}
@@ -274,23 +269,18 @@ static int mwifiex_sdio_suspend(struct device *dev)
mmc_pm_flag_t pm_flag = 0;
int ret = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
- sdio_func_id(func), pm_flag);
- if (!(pm_flag & MMC_PM_KEEP_POWER)) {
- pr_err("%s: cannot remain alive while host is"
- " suspended\n", sdio_func_id(func));
- return -ENOSYS;
- }
+ pm_flag = sdio_get_host_pm_caps(func);
+ pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
+ sdio_func_id(func), pm_flag);
+ if (!(pm_flag & MMC_PM_KEEP_POWER)) {
+ dev_err(dev, "%s: cannot remain alive while host is"
+ " suspended\n", sdio_func_id(func));
+ return -ENOSYS;
+ }
- card = sdio_get_drvdata(func);
- if (!card) {
- dev_err(dev, "suspend: invalid card\n");
- return 0;
- }
- } else {
- pr_err("suspend: sdio_func is not specified\n");
+ card = sdio_get_drvdata(func);
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 10/11] mwifiex: stop checking for NULL drvata/intfdata
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
These are never NULL, so stop making people think they might be.
I don't change this for SDIO because SDIO has a racy card-reset handler
that reallocates this struct. I'd rather not touch that mess right now.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 14 +++++---------
drivers/net/wireless/marvell/mwifiex/usb.c | 15 +++------------
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 1ab366c..3b6d908 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -118,10 +118,6 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card) {
- dev_err(dev, "card structure is not valid\n");
- return 0;
- }
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -166,8 +162,9 @@ static int mwifiex_pcie_resume(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- dev_err(dev, "Card or adapter structure is not valid\n");
+
+ if (!card->adapter) {
+ dev_err(dev, "adapter structure is not valid\n");
return 0;
}
@@ -255,8 +252,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
struct mwifiex_private *priv;
card = pci_get_drvdata(pdev);
- if (!card)
- return;
wait_for_completion(&card->fw_done);
@@ -2249,7 +2244,8 @@ static irqreturn_t mwifiex_pcie_interrupt(int irq, void *context)
}
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
+
+ if (!card->adapter) {
pr_err("info: %s: card=%p adapter=%p\n", __func__, card,
card ? card->adapter : NULL);
goto exit;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 8bcfd92..806ded6 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -503,11 +503,6 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
struct usb_tx_data_port *port;
int i, j;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return 0;
- }
-
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -574,8 +569,9 @@ static int mwifiex_usb_resume(struct usb_interface *intf)
struct mwifiex_adapter *adapter;
int i;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card->adapter) {
+ dev_err(&intf->dev, "%s: card->adapter is NULL\n",
+ __func__);
return 0;
}
adapter = card->adapter;
@@ -617,11 +613,6 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
struct usb_card_rec *card = usb_get_intfdata(intf);
struct mwifiex_adapter *adapter;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return;
- }
-
wait_for_completion(&card->fw_done);
adapter = card->adapter;
--
1.8.1.4
^ permalink raw reply related
* [PATCH RESEND v2 11/11] mwifiex: pcie: stop checking for NULL adapter->card
From: Xinming Hu @ 2016-11-10 11:57 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
It should never be NULL here, and to think otherwise makes things
confusing.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 55 +++++++++++++----------------
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 3b6d908..65f1f92 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -3021,31 +3021,28 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = card->dev;
int i;
- if (card) {
- pdev = card->dev;
- if (card->msix_enable) {
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- synchronize_irq(card->msix_entries[i].vector);
+ if (card->msix_enable) {
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ synchronize_irq(card->msix_entries[i].vector);
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- free_irq(card->msix_entries[i].vector,
- &card->msix_ctx[i]);
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ free_irq(card->msix_entries[i].vector,
+ &card->msix_ctx[i]);
- card->msix_enable = 0;
- pci_disable_msix(pdev);
- } else {
- mwifiex_dbg(adapter, INFO,
- "%s(): calling free_irq()\n", __func__);
- free_irq(card->dev->irq, &card->share_irq_ctx);
+ card->msix_enable = 0;
+ pci_disable_msix(pdev);
+ } else {
+ mwifiex_dbg(adapter, INFO,
+ "%s(): calling free_irq()\n", __func__);
+ free_irq(card->dev->irq, &card->share_irq_ctx);
- if (card->msi_enable)
- pci_disable_msi(pdev);
- }
- card->adapter = NULL;
- }
+ if (card->msi_enable)
+ pci_disable_msi(pdev);
+ }
+ card->adapter = NULL;
}
/* This function initializes the PCI-E host memory space, WCB rings, etc.
@@ -3128,18 +3125,14 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
adapter->seq_num = 0;
adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_4K;
- if (card) {
- if (reg->sleep_cookie)
- mwifiex_pcie_delete_sleep_cookie_buf(adapter);
-
- mwifiex_pcie_delete_cmdrsp_buf(adapter);
- mwifiex_pcie_delete_evtbd_ring(adapter);
- mwifiex_pcie_delete_rxbd_ring(adapter);
- mwifiex_pcie_delete_txbd_ring(adapter);
- card->cmdrsp_buf = NULL;
- }
+ if (reg->sleep_cookie)
+ mwifiex_pcie_delete_sleep_cookie_buf(adapter);
- return;
+ mwifiex_pcie_delete_cmdrsp_buf(adapter);
+ mwifiex_pcie_delete_evtbd_ring(adapter);
+ mwifiex_pcie_delete_rxbd_ring(adapter);
+ mwifiex_pcie_delete_txbd_ring(adapter);
+ card->cmdrsp_buf = NULL;
}
static struct mwifiex_if_ops pcie_ops = {
--
1.8.1.4
^ permalink raw reply related
* Re: cfg80211: add set/get link loss profile
From: Kalle Valo @ 2016-11-10 13:55 UTC (permalink / raw)
To: Lazar, Alexei Avshalom; +Cc: johannes, linux-wireless
In-Reply-To: <7742335b-f05e-edcd-ad80-fd09fb295eab@codeaurora.org>
"Lazar, Alexei Avshalom" <ailizaro@codeaurora.org> writes:
>>From b739abb6f29dc43a86b8b2b60e893b4441f8aa1f Mon Sep 17 00:00:00 2001
> From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
> Date: Sun, 6 Nov 2016 16:21:20 +0200
> Subject: [PATCH] cfg80211: add set/get link loss profile
>
> Introduce NL80211_CMD_SET_LINK_LOSS_PROFILE and
> NL80211_CMD_GET_LINK_LOSS_PROFILE as it required by the user space
> to configure the link loss profile.
> The link loss profile represents the priority of maintaining link up
> in different link quality environments.
> Three types of behavior for link loss defined:
> LINK_LOSS_PROFILE_RELAXED: prefer maintaining link up even in poor
> link quality environment.
> LINK_LOSS_PROFILE_DEFAULT: The default behavior for maintaining link
> up vs link quality.
> LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link up in poor link
> quality environment.
> New cfg80211 API also been added, set/get_link_loss_profile.
>
> Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
What does the term "link" mean in this context? A connection to the AP
or something else? The documentation doesn't give me any detailed
information nor any examples and lefts me guessing what this is supposed
to do.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH RESEND v2 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm
From: Kalle Valo @ 2016-11-10 14:06 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux Wireless, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Shengzhen Li, Xinming Hu
In-Reply-To: <1478779032-5165-1-git-send-email-huxinming820@marvell.com>
Xinming Hu <huxinming820@gmail.com> writes:
> From: Shengzhen Li <szli@marvell.com>
>
> We may get SLEEP event from firmware even if TXDone interrupt
> for last Tx packet is still pending. In this case, we may
> end up accessing PCIe memory for handling TXDone after power
> save handshake is completed. This causes kernel crash with
> external abort.
>
> This patch will only allow downloading sleep confirm
> when no tx done interrupt is pending in the hardware.
>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: address format issues(Brain)
> RESEND v2(Applicable for complete patch series):
> 1) Fixed syntax issue "changelog not placed after the Sign-offs"
> pointed by Brian.
> 2) Dropped "[v2,03/12] mwifiex: don't do unbalanced free()'ing in
> cleanup_if()" patch in this series. It was already sent by Brian
> separately.
I do not know where this "v2 RESEND" practise is coming from but I very
much prefer that people would not use it. If you need to resend
something keep things simple, just increase the version number and
document in the changelog that nothing changed. I don't care what the
patchset version number is, I always look at the latest version and
discard the older ones.
This is just for the future, no need resend anything because of this.
--
Kalle Valo
^ permalink raw reply
* Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
From: IgorMitsyanko @ 2016-11-10 14:02 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless, btherthala, hwang, smaksimenko, dlebed,
Igor Mitsyanko, Kamlesh Rath, Sergey Matyukevich, Avinash Patil
In-Reply-To: <1478725543.21403.4.camel@sipsolutions.net>
On 11/10/2016 12:05 AM, Johannes Berg wrote:
> I understand, and I understand that you/they are actually providing it
> when asked.
>
> However, the carl9170 project has its (entirely GPL) source tree out in
> the open, making it much *easier* and that was *still* thought to not
> be sufficient; I don't recall the discussions but I'm guessing that's
> because of something like redistributors having to make sure source is
> available, and guaranteeing that for a long time, etc.
>
> johannes
Johannes, from that perspective, who are the "redistributors"?
Specifically, is linux-firmware git
repository considered a redistributor or its just hosting files? I mean,
at what moment
someone else other then Quantenna will start to be legally obliged to
make GPL code
used in firmware available for others?
Personally I still hope that linux-firmware itself is not legally
concerned with what is the content of
firmware its hosting, but looks like there already was a precedent case
with carl9170 driver and
we have to somehow deal with it.
There still may be a difference though: Quantenna is semiconductor
company only, software
used on actual products based on Quantenna chipsets is released by other
companies.
I just want to present our legal team with a clear case (and position of
Linux maintainers) so that they can
work with it and make decision on how to proceed.
From technical perspective, as I mentioned, SDK is quite huge and
include a lot of opensource
components including full Linux, I don't think its reasonable to have it
inside linux-firmware tree.
What are the options to share it other then providing it on request basis:
- git repository
- store tarball somewhere on official website
Thanks!
^ permalink raw reply
* Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
From: Jes Sorensen @ 2016-11-08 14:29 UTC (permalink / raw)
To: Dave Jones; +Cc: Joe Perches, John Heenan, Kalle Valo, linux-wireless, netdev
In-Reply-To: <CANf5e8aQ+HZz47M3-4+qjsG=ZCaXhBFU_jVupLqT4rEYT2LQFQ@mail.gmail.com>
Dave Jones <s.dave.jones@gmail.com> writes:
> On Fri, Nov 04, 2016 at 09:56:00AM -0400, Jes Sorensen wrote:
>>
>> Joe Perches <joe@perches.com> writes:
>> > On Sun, 2016-10-30 at 19:02 -0400, Jes Sorensen wrote:
>> >> Code is 80 characters wide, and comments are /* */ never the ugly C++
>> >> crap.
>> >
>> > You might look at the recent Linus Torvalds authored commit
>> > 5e467652ffef (?printk: re-organize log_output() to be more legible")
>> > which does both of those: c99 // comments and > 80 columns.
>> >
>> > Absolutes are for zealots.
>>
>> What Linus does in his code, is totally up to him. What I pull into the
>> driver that *I* maintain, is up to me. It is perfectly normal to expect
>> submitters to respect the coding style of the piece of code they are
>> trying to edit.
>
> Bullshit. It's perfectly normal to respect Linux coding style described in
> Documentation/CodingStyle. Now let's back to the topic, could you
> apply John's patch or you just wanna improve your driver is 100% bug free?
First of all, I call for proper CodingStyle to be applied to my driver,
and I expect someone posting a patch to respect the codingstyle of the
driver in question. It is simple respect for the code. If you consider
that BS - that is on you!
Second I am NOT applying that patch as I have stated repeatedly because
I am not convinced it is safe to do so and it changes the code flow for
one type of chip and not the rest. In addition it uses a broken approach
to doing chip specific changes.
In short, the patch is broken!
Jes
^ permalink raw reply
* Problems getting mwifiex with sd8887 to work
From: Wolfram Sang @ 2016-11-10 17:59 UTC (permalink / raw)
To: linux-wireless; +Cc: Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo
[-- Attachment #1: Type: text/plain, Size: 10116 bytes --]
Hi,
I am trying to get a SD8887 based SDIO card to work with my Renesas H3
Salvator-X board. The card is either a u-blox emmy-w1-evk or a ZComax AC-180M,
I am evaluating both. I really prefer to run the upstream driver instead of the
paperwork protected and surprisingly old custom ones.
However, for both cards, I run into an error I cannot parse:
[ 4.028588] mwifiex_sdio mmc2:0001:1: mwifiex_process_cmdresp: cmd 0x242 failed during initialization
I use a v4.9-rc2 based tree called 'renesas-drives' [1] which has a few Renesas
specific patches on top but nothing mwifiex related AFAICT. I use the firmware from
the linux-firmware tree.
Any further use of the interface mlan0 or reading eeprom via debugfs will result in a timeout.
[ 18.146710] mwifiex_sdio mmc2:0001:1: mwifiex_cmd_timeout_func: Timeout cmd id = 0x10, act = 0x1
I assume this is a follow-up problem so I skip the logs for these timeouts (can
easily send them, of course, in case they are interesting). I'll attach a full
debug output from mwifiex from insertion until the initial 0x242 error.
For full disclaimer, I am the maintainer of the underlying Renesas SD driver
and I want to use this setup to create a test scenario for SDIO with UHS
speeds. So, if there is something wrong in the SD part, I'll happily hack on
that.
But for now, I simply would need a starting point :) 0x242 is
"HostCmd_CMD_CHAN_REGION_CFG". Could this mean it cannot load configuration
data? I did search around but couldn't find anything useful.
Thanks,
Wolfram
[1] https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/
[ 3.139424] mmc2: new ultra high speed SDR50 SDIO card at address 0001
[ 3.142867] mwifiex_sdio: info: vendor=0x02DF device=0x9135 class=0 function=1
[ 3.146696] mwifiex: rx work enabled, cpus 4
[ 3.267184] mwifiex_sdio mmc2:0001:1: info: downloading FW image (391772 bytes)
[ 3.452377] mwifiex_sdio mmc2:0001:1: info: FW download over, size 391772 bytes
[ 3.671191] mwifiex_sdio mmc2:0001:1: WLAN FW is active
[ 3.673910] mwifiex_sdio mmc2:0001:1: cmd: QUEUE_CMD: cmd=0xa9, cmd_pending=1
[ 3.677550] mwifiex_sdio mmc2:0001:1: cmd: DNLD_CMD: 0xa9, act 0x0, len 8, seqno 0x1
[ 3.681429] cmd buffer:00000000: a9 00 08 00 01 00 00 00
[ 3.684089] mwifiex_sdio mmc2:0001:1: info: mwifiex_host_to_card_mp_aggr: tx aggregation disabled
[ 3.688527] mwifiex_sdio mmc2:0001:1: data: mwifiex_host_to_card_mp_aggr: send current buffer 32768
[ 3.693652] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x80
[ 3.696443] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=1
[ 3.700321] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x82
[ 3.703106] mwifiex_sdio mmc2:0001:1: int: DNLD: wr_bitmap=0xffffffff
[ 3.706328] mwifiex_sdio mmc2:0001:1: info: <--- Tx DONE Interrupt --->
[ 3.709680] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.712811] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x40
[ 3.715600] mwifiex_sdio mmc2:0001:1: info: rx_len = 256
[ 3.718292] mwifiex_sdio mmc2:0001:1: info: --- Rx: Cmd Response ---
[ 3.721471] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.724563] mwifiex_sdio mmc2:0001:1: cmd: CMD_RESP: 0x80a9, result 0, len 8, seqno 0x1
[ 3.728565] CMD_RESP buffer:00000000: a9 00 08 00 01 00 00 00
[ 3.731439] mwifiex_sdio mmc2:0001:1: cmd completed: status=0
[ 3.734312] mwifiex_sdio mmc2:0001:1: cmd: FREE_CMD: cmd=0xa9, cmd_pending=0
[ 3.737930] mwifiex_sdio mmc2:0001:1: cmd: QUEUE_CMD: cmd=0x3, cmd_pending=1
[ 3.741459] mwifiex_sdio mmc2:0001:1: cmd: DNLD_CMD: 0x3, act 0x0, len 71, seqno 0x2
[ 3.745333] cmd buffer:00000000: 03 00 47 00 02 00 00 00 00 00 00 00 00 00 00 00
[ 3.749031] cmd buffer:00000010: ff ff ff ff ff ff 00 00 00 00 00 00 00 00 00 00
[ 3.752729] cmd buffer:00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 3.756426] cmd buffer:00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 3.760189] cmd buffer:00000040: 00 00 00 00 00 00 00
[ 3.762714] mwifiex_sdio mmc2:0001:1: info: mwifiex_host_to_card_mp_aggr: tx aggregation disabled
[ 3.767156] mwifiex_sdio mmc2:0001:1: data: mwifiex_host_to_card_mp_aggr: send current buffer 32768
[ 3.771814] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x80
[ 3.774599] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.777728] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x40
[ 3.780517] mwifiex_sdio mmc2:0001:1: info: rx_len = 256
[ 3.783210] mwifiex_sdio mmc2:0001:1: info: --- Rx: Cmd Response ---
[ 3.786386] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.789484] mwifiex_sdio mmc2:0001:1: cmd: CMD_RESP: 0x8003, result 0, len 79, seqno 0x2
[ 3.793530] CMD_RESP buffer:00000000: 03 00 4f 00 02 00 00 00 02 00 02 42 00 00 40 00
[ 3.797444] CMD_RESP buffer:00000010: d4 ca 6e 00 03 94 10 00 01 00 07 44 0f 05 00 00
[ 3.801359] CMD_RESP buffer:00000020: 00 00 00 00 00 00 00 00 00 00 03 ff 00 00 11 00
[ 3.805273] CMD_RESP buffer:00000030: d3 65 11 20 00 0e 00 00 00 00 00 00 00 00 00 30
[ 3.809188] CMD_RESP buffer:00000040: 71 c0 33 fe ff fe ff c7 01 04 00 01 00 02 00
[ 3.812974] mwifiex_sdio mmc2:0001:1: key_api v2.0
[ 3.815370] mwifiex_sdio mmc2:0001:1: info: GET_HW_SPEC: fw_release_number- 0x50f4407
[ 3.819285] mwifiex_sdio mmc2:0001:1: info: GET_HW_SPEC: permanent addr: d4:ca:6e:00:03:94
[ 3.823417] mwifiex_sdio mmc2:0001:1: info: GET_HW_SPEC: hw_if_version=0x2 version=0x4202
[ 3.827507] mwifiex_sdio mmc2:0001:1: cmd: mp_end_port 32, data port mask 0xffffffff
[ 3.831379] mwifiex_sdio mmc2:0001:1: cmd completed: status=0
[ 3.834251] mwifiex_sdio mmc2:0001:1: cmd: FREE_CMD: cmd=0x3, cmd_pending=0
[ 3.837886] mwifiex_sdio mmc2:0001:1: cmd: set tx_buf=2048
[ 3.840631] mwifiex_sdio mmc2:0001:1: cmd: QUEUE_CMD: cmd=0xd9, cmd_pending=1
[ 3.844228] mwifiex_sdio mmc2:0001:1: cmd: DNLD_CMD: 0xd9, act 0x1, len 16, seqno 0x3
[ 3.848151] cmd buffer:00000000: d9 00 10 00 03 00 00 00 01 00 00 08 00 00 00 00
[ 3.851850] mwifiex_sdio mmc2:0001:1: info: mwifiex_host_to_card_mp_aggr: tx aggregation disabled
[ 3.856286] mwifiex_sdio mmc2:0001:1: data: mwifiex_host_to_card_mp_aggr: send current buffer 32768
[ 3.861255] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x82
[ 3.864043] mwifiex_sdio mmc2:0001:1: int: DNLD: wr_bitmap=0xffffffff
[ 3.867266] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.870391] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x40
[ 3.873187] mwifiex_sdio mmc2:0001:1: info: rx_len = 256
[ 3.875881] mwifiex_sdio mmc2:0001:1: info: --- Rx: Cmd Response ---
[ 3.879057] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.882148] mwifiex_sdio mmc2:0001:1: cmd: CMD_RESP: 0x80d9, result 0, len 16, seqno 0x3
[ 3.886194] CMD_RESP buffer:00000000: d9 00 10 00 03 00 00 00 01 00 00 06 20 00 00 00
[ 3.890110] mwifiex_sdio mmc2:0001:1: cmd: curr_tx_buf_size=1536
[ 3.893113] mwifiex_sdio mmc2:0001:1: cmd: mp_end_port 32, data port mask 0xffffffff
[ 3.896984] mwifiex_sdio mmc2:0001:1: cmd completed: status=0
[ 3.899859] mwifiex_sdio mmc2:0001:1: cmd: FREE_CMD: cmd=0xd9, cmd_pending=0
[ 3.903480] mwifiex_sdio mmc2:0001:1: cmd: PS Command: Enter PS
[ 3.906439] mwifiex_sdio mmc2:0001:1: cmd: QUEUE_CMD: cmd=0xe4, cmd_pending=1
[ 3.910042] mwifiex_sdio mmc2:0001:1: cmd: DNLD_CMD: 0xe4, act 0xff, len 30, seqno 0x4
[ 3.914003] cmd buffer:00000000: e4 00 1e 00 04 00 00 00 ff 00 10 00 72 01 0e 00
[ 3.917701] cmd buffer:00000010: 00 00 01 00 05 00 00 00 00 00 01 00 e8 03
[ 3.921139] mwifiex_sdio mmc2:0001:1: info: mwifiex_host_to_card_mp_aggr: tx aggregation disabled
[ 3.925575] mwifiex_sdio mmc2:0001:1: data: mwifiex_host_to_card_mp_aggr: send current buffer 32768
[ 3.930274] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x80
[ 3.933065] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.936193] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x40
[ 3.938977] mwifiex_sdio mmc2:0001:1: info: rx_len = 256
[ 3.941672] mwifiex_sdio mmc2:0001:1: info: --- Rx: Cmd Response ---
[ 3.944851] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.947942] mwifiex_sdio mmc2:0001:1: cmd: CMD_RESP: 0x80e4, result 0, len 30, seqno 0x4
[ 3.951987] CMD_RESP buffer:00000000: e4 00 1e 00 04 00 00 00 ff 00 10 00 72 01 0e 00
[ 3.955902] CMD_RESP buffer:00000010: 00 00 01 00 05 00 00 00 00 00 01 00 e8 03
[ 3.959558] mwifiex_sdio mmc2:0001:1: info: mwifiex_ret_enh_power_mode: PS_MODE cmd reply result=0x0 action=0XFF
[ 3.964645] mwifiex_sdio mmc2:0001:1: cmd: Enabled STA power save
[ 3.967692] mwifiex_sdio mmc2:0001:1: cmd completed: status=0
[ 3.970563] mwifiex_sdio mmc2:0001:1: cmd: FREE_CMD: cmd=0xe4, cmd_pending=0
[ 3.974188] mwifiex_sdio mmc2:0001:1: cmd: QUEUE_CMD: cmd=0x242, cmd_pending=1
[ 3.977808] mwifiex_sdio mmc2:0001:1: cmd: DNLD_CMD: 0x242, act 0x0, len 10, seqno 0x5
[ 3.981768] cmd buffer:00000000: 42 02 0a 00 05 00 00 00 00 00
[ 3.984685] mwifiex_sdio mmc2:0001:1: info: mwifiex_host_to_card_mp_aggr: tx aggregation disabled
[ 3.989121] mwifiex_sdio mmc2:0001:1: data: mwifiex_host_to_card_mp_aggr: send current buffer 32768
[ 3.993759] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x80
[ 3.996547] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 3.999679] mwifiex_sdio mmc2:0001:1: int: sdio_ireg = 0x40
[ 4.002464] mwifiex_sdio mmc2:0001:1: info: rx_len = 256
[ 4.005160] mwifiex_sdio mmc2:0001:1: info: --- Rx: Cmd Response ---
[ 4.008339] mwifiex_sdio mmc2:0001:1: info: cmd_sent=0 data_sent=0
[ 4.011435] mwifiex_sdio mmc2:0001:1: cmd: CMD_RESP: 0x8242, result 2, len 10, seqno 0x5
[ 4.015481] CMD_RESP buffer:00000000: 42 02 0a 00 05 00 02 00 00 00
[ 4.018614] mwifiex_sdio mmc2:0001:1: CMD_RESP: cmd 0x242 error, result=0x2
[ 4.022099] mwifiex_sdio mmc2:0001:1: cmd completed: status=-1
[ 4.025017] mwifiex_sdio mmc2:0001:1: cmd: FREE_CMD: cmd=0x242, cmd_pending=0
[ 4.028588] mwifiex_sdio mmc2:0001:1: mwifiex_process_cmdresp: cmd 0x242 failed during initialization
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND v2 03/11] mwifiex: resolve races between async FW init (failure) and device removal
From: Brian Norris @ 2016-11-10 18:37 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux Wireless, Kalle Valo, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo
In-Reply-To: <1478779032-5165-3-git-send-email-huxinming820@marvell.com>
Hi,
On Thu, Nov 10, 2016 at 07:57:04PM +0800, Xinming Hu wrote:
> From: Brian Norris <briannorris@chromium.org>
>
> It's possible for the FW init sequence to fail, which will trigger a
> device cleanup sequence in mwifiex_fw_dpc(). This sequence can race with
> device suspend() or remove() (e.g., reboot or unbind), and can trigger
> use-after-free issues. Currently, this driver attempts (poorly) to
> synchronize remove() using a semaphore, but it doesn't protect some of
> the critical sections properly. Particularly, we grab a pointer to the
> adapter struct (card->adapter) without checking if it's being freed or
> not. We later do a NULL check on the adapter, but that doesn't work if
> the adapter was freed.
>
> Also note that the PCIe interface driver doesn't ever set card->adapter
> to NULL, so even if we get the synchronization right, we still might try
> to redo the cleanup in ->remove(), even if the FW init failure sequence
> already did it.
>
> This patch replaces the static semaphore with a per-device completion
> struct, and uses that completion to synchronize the remove() thread with
> the mwifiex_fw_dpc(). A future patch will utilize this completion to
> synchronize the suspend() thread as well.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: Same as v1
> ---
> drivers/net/wireless/marvell/mwifiex/main.c | 46 ++++++++++-------------------
> drivers/net/wireless/marvell/mwifiex/main.h | 13 +++++---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++------
> drivers/net/wireless/marvell/mwifiex/pcie.h | 2 ++
> drivers/net/wireless/marvell/mwifiex/sdio.c | 18 +++++------
> drivers/net/wireless/marvell/mwifiex/sdio.h | 2 ++
> drivers/net/wireless/marvell/mwifiex/usb.c | 23 +++++++--------
> drivers/net/wireless/marvell/mwifiex/usb.h | 2 ++
> 8 files changed, 55 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index c710d5e..09d46d6 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -521,7 +521,6 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> struct mwifiex_private *priv;
> struct mwifiex_adapter *adapter = context;
> struct mwifiex_fw_image fw;
> - struct semaphore *sem = adapter->card_sem;
> bool init_failed = false;
> struct wireless_dev *wdev;
>
> @@ -670,7 +669,8 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> }
> if (init_failed)
> mwifiex_free_adapter(adapter);
> - up(sem);
> + /* Tell all current and future waiters we're finished */
> + complete_all(adapter->fw_done);
This part introduces a new use-after-free. We need to dereference
adapter->fw_done *before* we free the adapter in mwifiex_free_adapter().
So you need a diff that looks something like this:
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -514,6 +514,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
struct mwifiex_fw_image fw;
bool init_failed = false;
struct wireless_dev *wdev;
+ struct completion *fw_done = adapter->fw_done;
if (!firmware) {
mwifiex_dbg(adapter, ERROR,
@@ -654,7 +655,7 @@ done:
if (init_failed)
mwifiex_free_adapter(adapter);
/* Tell all current and future waiters we're finished */
- complete_all(adapter->fw_done);
+ complete_all(fw_done);
return;
}
Brian
> return;
> }
>
...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox