Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: ath10k: Fix soft lockup during firmware crash/hw-restart
From: Mohammed Shafi Shajakhan @ 2016-12-01  6:06 UTC (permalink / raw)
  To: Ben Greear; +Cc: Mohammed Shafi Shajakhan, ath10k, linux-wireless
In-Reply-To: <1a44a3e9-3d67-30a5-0325-a65003ebe1fc@candelatech.com>

On Tue, Nov 29, 2016 at 10:16:50AM -0800, Ben Greear wrote:
> Is this something for stable?  And if so, how far back should it be applied?

@Kalle,

[shafi] kindly suggest. If i am not wrong this is only needed for 4.9

> 
> I'll add this patch to my 4.9 tree and test it...
>

[shafi] thanks a lot Ben.

> 
> On 11/29/2016 06:46 AM, Mohammed Shafi Shajakhan wrote:
> >From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >
> >During firmware crash (or) user requested manual restart
> >the system gets into a soft lock up state because of the
> >below root cause.
> >
> >During user requested hardware restart / firmware crash
> >the system goes into a soft lockup state as 'napi_synchronize'
> >is called after 'napi_disable' (which sets 'NAPI_STATE_SCHED'
> >bit) and it sleeps into infinite loop as it waits for
> >'NAPI_STATE_SCHED' to be cleared. This condition is hit because
> >'ath10k_hif_stop' is called twice as below (resulting in calling
> >'napi_synchronize' after 'napi_disable')
> >
> >'ath10k_core_restart' -> 'ath10k_hif_stop' (ATH10K_STATE_ON) ->
> >-> 'ieee80211_restart_hw' -> 'ath10k_start' -> 'ath10k_halt' ->
> >'ath10k_core_stop' -> 'ath10k_hif_stop' (ATH10K_STATE_RESTARTING)
> >
> >Fix this by calling 'ath10k_halt' in ath10k_core_restart itself
> >as it makes more sense before informing mac80211 to restart h/w
> >Also remove 'ath10k_halt' in ath10k_start for the state of 'restarting'
> >
> >Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >---
> >[thanks to Kalle and Michal for their inputs]
> >
> > drivers/net/wireless/ath/ath10k/core.c | 2 +-
> > drivers/net/wireless/ath/ath10k/mac.c  | 1 -
> > 2 files changed, 1 insertion(+), 2 deletions(-)
> >
> >diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> >index 7005e2a..5bc6847 100644
> >--- a/drivers/net/wireless/ath/ath10k/core.c
> >+++ b/drivers/net/wireless/ath/ath10k/core.c
> >@@ -1536,7 +1536,7 @@ static void ath10k_core_restart(struct work_struct *work)
> > 	switch (ar->state) {
> > 	case ATH10K_STATE_ON:
> > 		ar->state = ATH10K_STATE_RESTARTING;
> >-		ath10k_hif_stop(ar);
> >+		ath10k_halt(ar);
> > 		ath10k_scan_finish(ar);
> > 		ieee80211_restart_hw(ar->hw);
> > 		break;
> >diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> >index 717b2fa..481842b 100644
> >--- a/drivers/net/wireless/ath/ath10k/mac.c
> >+++ b/drivers/net/wireless/ath/ath10k/mac.c
> >@@ -4449,7 +4449,6 @@ static int ath10k_start(struct ieee80211_hw *hw)
> > 		ar->state = ATH10K_STATE_ON;
> > 		break;
> > 	case ATH10K_STATE_RESTARTING:
> >-		ath10k_halt(ar);
> > 		ar->state = ATH10K_STATE_RESTARTED;
> > 		break;
> > 	case ATH10K_STATE_ON:
> >
> 
> 
> -- 
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
> 

^ permalink raw reply

* Re: pull-request: wireless-drivers 2016-11-29
From: Kalle Valo @ 2016-12-01  5:19 UTC (permalink / raw)
  To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20161130.143444.242053190479060640.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Tue, 29 Nov 2016 16:59:44 +0200
>
>> if there's still time here's one more patch to 3.9. I think this is good
>> to have in 3.9 as it fixes an issue where we were printing uninitialised
>> memory in mwifiex. I had this in wireless-drivers already for some time
>> as I was waiting for other fixes and nothing serious actually came up.
>> 
>> If this doesn't make it to 3.9 that's not a problem, I'll just merge
>> this to wireless-drivers-next. Let me know what you prefer.
>
> Unless you are in a time-machine of some sort I assume you mean "4.9" not
> "3.9" :-)
>
> Pulled, thanks.

Hah, I have no idea where I came up with this "3.9" :) I was trying mean
"4.9" of course, thanks for pulling.

-- 
Kalle Valo

^ permalink raw reply

* Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
From: Valo, Kalle @ 2016-12-01  5:13 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: k.eugene.e@gmail.com, Andy Gross, wcn36xx@lists.infradead.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <878ts0u7z3.fsf@kamboji.qca.qualcomm.com>

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> "Valo, Kalle" <kvalo@qca.qualcomm.com> writes:
>
>> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
>>
>>> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>>>
>>>> Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>>>> > The correct include file for getting errno constants and ERR_PTR() i=
s
>>>> > linux/err.h, rather than linux/errno.h, so fix the include.
>>>> >=20
>>>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled =
smem_state")
>>>> > Acked-by: Andy Gross <andy.gross@linaro.org>
>>>> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>>=20
>>>> For some reason this fails to compile now. Can you take a look, please=
?
>>>>=20
>>>> ERROR: "qcom_wcnss_open_channel" [drivers/net/wireless/ath/wcn36xx/wcn=
36xx.ko] undefined!
>>>> make[1]: *** [__modpost] Error 1
>>>> make: *** [modules] Error 2
>>>>=20
>>>> 5 patches set to Changes Requested.
>>>>=20
>>>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>>>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>>>
>>> This patch was updated with the necessary depends in Kconfig to catch
>>> this exact issue and when I pull in your .config (which has QCOM_SMD=3D=
n,
>>> QCOM_WCNSS_CTRL=3Dn and WCN36XX=3Dy) I can build this just fine.
>>>
>>> I've tested the various combinations and it seems to work fine. Do you
>>> have any other patches in your tree?
>>
>> This was with the pending branch of my ath.git tree. There are other
>> wireless patches (ath10k etc) but I would guess they don't affect here.
>>
>>> Any stale objects?
>>
>> Not sure what you mean with this question, but I didn't run 'make clean'
>> if that's what you are asking.
>>
>>> Would you mind retesting this, before I invest more time in trying to
>>> reproduce the issue you're seeing?
>>
>> Sure, I'll take a look but that might take few days.
>
> I didn't find enough time to look at this in detail. I applied this to
> my ath.git pending branch, let's see what the kbuild bot finds.

It found the same problem. Interestingly I'm also building x86 with 32
bit, maybe it's related?

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pendi=
ng
head:   1ea16a1c457939b4564643f7637d5cc639a8d3b7
commit: 5eb09c672b01460804fd49b1c9cc7d1072a102f0 [96/99] wcn36xx: Transitio=
n driver to SMD client
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        git checkout 5eb09c672b01460804fd49b1c9cc7d1072a102f0
        # save the attached .config to linux build tree
        make ARCH=3Di386=20

All errors (new ones prefixed by >>):

>> ERROR: "qcom_wcnss_open_channel" [drivers/net/wireless/ath/wcn36xx/wcn36=
xx.ko] undefined!

--=20
Kalle Valo=

^ permalink raw reply

* [PATCH] mac80211: Remove invalid flag operations in mesh TSF synchronization
From: Masashi Honma @ 2016-11-30 22:44 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, me, Masashi Honma

mesh_sync_offset_adjust_tbtt() implements Extensible synchronization
framework ([1] 13.13.2 Extensible synchronization framework). It shall
not operate the flag "TBTT Adjusting subfield" ([1] 8.4.2.100.8 Mesh
Capability), since it is used only for MBCA ([1] 13.13.4 Mesh beacon
collision avoidance, see 13.13.4.4.3 TBTT scanning and adjustment
procedures for detail). So this patch remove the flag operations.

[1] IEEE Std 802.11 2012

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 net/mac80211/mesh_sync.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/net/mac80211/mesh_sync.c b/net/mac80211/mesh_sync.c
index faca22c..836d791 100644
--- a/net/mac80211/mesh_sync.c
+++ b/net/mac80211/mesh_sync.c
@@ -172,11 +172,9 @@ static void mesh_sync_offset_adjust_tbtt(struct ieee80211_sub_if_data *sdata,
 					 struct beacon_data *beacon)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
-	u8 cap;
 
 	WARN_ON(ifmsh->mesh_sp_id != IEEE80211_SYNC_METHOD_NEIGHBOR_OFFSET);
 	WARN_ON(!rcu_read_lock_held());
-	cap = beacon->meshconf->meshconf_cap;
 
 	spin_lock_bh(&ifmsh->sync_offset_lock);
 
@@ -190,21 +188,13 @@ static void mesh_sync_offset_adjust_tbtt(struct ieee80211_sub_if_data *sdata,
 			  "TBTT : kicking off TBTT adjustment with clockdrift_max=%lld\n",
 			  ifmsh->sync_offset_clockdrift_max);
 		set_bit(MESH_WORK_DRIFT_ADJUST, &ifmsh->wrkq_flags);
-
-		ifmsh->adjusting_tbtt = true;
 	} else {
 		msync_dbg(sdata,
 			  "TBTT : max clockdrift=%lld; too small to adjust\n",
 			  (long long)ifmsh->sync_offset_clockdrift_max);
 		ifmsh->sync_offset_clockdrift_max = 0;
-
-		ifmsh->adjusting_tbtt = false;
 	}
 	spin_unlock_bh(&ifmsh->sync_offset_lock);
-
-	beacon->meshconf->meshconf_cap = ifmsh->adjusting_tbtt ?
-			IEEE80211_MESHCONF_CAPAB_TBTT_ADJUSTING | cap :
-			~IEEE80211_MESHCONF_CAPAB_TBTT_ADJUSTING & cap;
 }
 
 static const struct sync_method sync_methods[] = {
-- 
2.7.4

^ permalink raw reply related

* Re: pull-request: wireless-drivers 2016-11-29
From: David Miller @ 2016-11-30 19:34 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87r35utjq7.fsf@kamboji.qca.qualcomm.com>

From: Kalle Valo <kvalo@codeaurora.org>
Date: Tue, 29 Nov 2016 16:59:44 +0200

> if there's still time here's one more patch to 3.9. I think this is good
> to have in 3.9 as it fixes an issue where we were printing uninitialised
> memory in mwifiex. I had this in wireless-drivers already for some time
> as I was waiting for other fixes and nothing serious actually came up.
> 
> If this doesn't make it to 3.9 that's not a problem, I'll just merge
> this to wireless-drivers-next. Let me know what you prefer.

Unless you are in a time-machine of some sort I assume you mean "4.9" not
"3.9" :-)

Pulled, thanks.

^ permalink raw reply

* [patch] mwifiex: clean up some messy indenting
From: Dan Carpenter @ 2016-11-30 19:21 UTC (permalink / raw)
  To: Amitkumar Karwar, Karthik D A
  Cc: Nishant Sarmukadam, Kalle Valo, linux-wireless, kernel-janitors

These lines were indented one tab extra.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index f5dffdf..fdc6cc2 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -1937,8 +1937,8 @@ int mwifiex_sta_prepare_cmd(struct mwifiex_private *priv, uint16_t cmd_no,
 		mwifiex_dbg(priv->adapter, ERROR,
 			    "0x%x command not supported by firmware\n",
 			    cmd_no);
-			return -EOPNOTSUPP;
-		}
+		return -EOPNOTSUPP;
+	}
 
 	/* Prepare command */
 	switch (cmd_no) {

^ permalink raw reply related

* Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
From: Valo, Kalle @ 2016-11-30 18:40 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: k.eugene.e@gmail.com, Andy Gross, wcn36xx@lists.infradead.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <877f7vimyg.fsf@kamboji.qca.qualcomm.com>

"Valo, Kalle" <kvalo@qca.qualcomm.com> writes:

> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
>
>> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>>
>>> Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>>> > The correct include file for getting errno constants and ERR_PTR() is
>>> > linux/err.h, rather than linux/errno.h, so fix the include.
>>> >=20
>>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled s=
mem_state")
>>> > Acked-by: Andy Gross <andy.gross@linaro.org>
>>> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>=20
>>> For some reason this fails to compile now. Can you take a look, please?
>>>=20
>>> ERROR: "qcom_wcnss_open_channel" [drivers/net/wireless/ath/wcn36xx/wcn3=
6xx.ko] undefined!
>>> make[1]: *** [__modpost] Error 1
>>> make: *** [modules] Error 2
>>>=20
>>> 5 patches set to Changes Requested.
>>>=20
>>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>>
>> This patch was updated with the necessary depends in Kconfig to catch
>> this exact issue and when I pull in your .config (which has QCOM_SMD=3Dn=
,
>> QCOM_WCNSS_CTRL=3Dn and WCN36XX=3Dy) I can build this just fine.
>>
>> I've tested the various combinations and it seems to work fine. Do you
>> have any other patches in your tree?
>
> This was with the pending branch of my ath.git tree. There are other
> wireless patches (ath10k etc) but I would guess they don't affect here.
>
>> Any stale objects?
>
> Not sure what you mean with this question, but I didn't run 'make clean'
> if that's what you are asking.
>
>> Would you mind retesting this, before I invest more time in trying to
>> reproduce the issue you're seeing?
>
> Sure, I'll take a look but that might take few days.

I didn't find enough time to look at this in detail. I applied this to
my ath.git pending branch, let's see what the kbuild bot finds.

--=20
Kalle Valo=

^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: get rid of global user_rmmod flag
From: Dmitry Torokhov @ 2016-11-30 18:38 UTC (permalink / raw)
  To: Amitkumar Karwar
  Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
	briannorris, Xinming Hu
In-Reply-To: <1480517537-9920-2-git-send-email-akarwar@marvell.com>

Hi Amitkumar,

On Wed, Nov 30, 2016 at 08:22:17PM +0530, Amitkumar Karwar wrote:
> @@ -3177,9 +3184,6 @@ static int mwifiex_pcie_init_module(void)
>  
>  	pr_debug("Marvell PCIe Driver\n");
>  
> -	/* Clear the flag in case user removes the card. */
> -	user_rmmod = 0;
> -
>  	ret = pci_register_driver(&mwifiex_pcie);
>  	if (ret)
>  		pr_err("Driver register failed!\n");
> @@ -3200,9 +3204,6 @@ static int mwifiex_pcie_init_module(void)
>   */
>  static void mwifiex_pcie_cleanup_module(void)
>  {
> -	/* Set the flag as user is removing this module. */
> -	user_rmmod = 1;
> -
>  	pci_unregister_driver(&mwifiex_pcie);
>  }

Now that your module init/exit code turns into wrapper around bus
driver registration calls, please consider using module_pci_driver(),
module_usb_driver(). Note that I do not see module_sdio_driver, but you
could still use

module_driver(mwifiex_sdio, sdio_register_driver, sdio_unregister_driver);

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process
From: Brian Norris @ 2016-11-30 18:33 UTC (permalink / raw)
  To: Amitkumar Karwar
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
	rajatja@google.com, dmitry.torokhov@gmail.com, Xinming Hu
In-Reply-To: <8e65d20f643a4c7f9a242c145ec8f24f@SC-EXCH04.marvell.com>

On Wed, Nov 30, 2016 at 12:39:11PM +0000, Amitkumar Karwar wrote:

> > Ugh, yet another band-aid? You might do better to still cancel any
> > pending work, just don't do it synchronously. i.e., do cancel_work()
> > after you've removed the card. It doesn't make sense to do a FW dump on
> > the "new" adapter when it was requested for the old one.
> 
> I could not find async version of cancel_work().

cancel_work() *is* asynchronous. It does not synchronize with the last
event, so you won't have the deadlock. (Remember: the synchronous
version is cancel_work_sync().)

> We can fix this problem with below change at the end of
> mwifiex_sdio_work(). All pending work requests would be ignored.
> 
> --------
> @ -2571,6 +2571,8 @@ static void mwifiex_sdio_work(struct work_struct *work)
>         if (test_and_clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET,
>                                &iface_work_flags))
>                 mwifiex_sdio_card_reset_work(save_adapter);
> +       clear_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP, &iface_work_flags);
> +       clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &iface_work_flags);
>  }
> ----------

I don't think that's exactly what you want. That might lose events,
won't it? I'd rather this sort of hack go into
mwifiex_recreate_adapter(), in between the remove() and probe() calls,
where you don't expect any new events to trigger. And maybe include a
comment as to why.

> > I think I've asked elsewhere but didn't receive an answer: why is
> > SDIO's mwifiex_recreate_adapter() so much different from PCIe's
> > mwifiex_do_flr()? It seems like the latter should be refactored to
> > remove some of the PCIe-specific cruft from main.c and then reused for
> > SDIO.
> 
> Our initial SDIO card reset implementation was based on MMC APIs where
> remove() and probe() would automatically get called by MMC subsystem
> after power cycle.
> https://www.spinics.net/lists/linux-wireless/msg98435.html
> Later it was improved by Andreas Fenkart by replacing those power
> cycle APIs with mmc_hw_reset().

Right.

> For PCIe, function level reset is standard feature. We implemented
> ".reset_notify" handler which gets called after and before FLR.

OK.

> You are right. We can have SDIO's handling similar to PCIe and avoid
> destroying+recreating adapter/card.

So all in all, you're saying it's just an artifact of history, and
there's no good reason they are so different? If so, then this looks
like another instance where you would have done well to refactor and
improve the existing mechanisms at the same time as you added new
features (i.e., PCIe FLR). I've seen this problem already several times,
where it seems development for your SDIO/PCIe/USB interface drivers
occur almost in isolation. IMO, it'd do you well to notice these
patterns while implementing features in the first place. The more code
you can share, the fewer bugs you (or I) will have to chase down.

> We have started working on this. We will get rid of global
> save_adapter, sdio_work etc.

Great!

> Meanwhile I will post above mentioned change if it looks good to you.

It's not perfect, but it's a start.

Brian

^ permalink raw reply

* Re: [PATCH] ath10k: Fix Tx DMA alloc failure during continuous wifi down/up
From: Adrian Chadd @ 2016-11-30 18:27 UTC (permalink / raw)
  To: Mohammed Shafi Shajakhan
  Cc: ath10k@lists.infradead.org, mohammed,
	linux-wireless@vger.kernel.org
In-Reply-To: <1480499414-19543-1-git-send-email-mohammed@qca.qualcomm.com>

Heh, I had to do something like this for freebsd too for my ath10k
port. So thanks. :)

Suggestion - take a look at where tasklets, completions, locks, etc
are all re-allocated multiple times, once upon initial VAP bringup. I
had to also undo this in FreeBSD, as we don't allow re-init of tasks,
completions, callouts and locks without first freeing/zero'ing them
appropriately. :)



-adrian


On 30 November 2016 at 01:50, Mohammed Shafi Shajakhan
<mohammed@qti.qualcomm.com> wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
>
> With maximum number of vap's configured in a two radio supported
> systems of ~256 Mb RAM, doing a continuous wifi down/up and
> intermittent traffic streaming from the connected stations results
> in failure to allocate contiguous memory for tx buffers. This results
> in the disappearance of all VAP's and a manual reboot is needed as
> this is not a crash (or) OOM(for OOM killer to be invoked). To address
> this allocate contiguous memory for tx buffers one time and re-use them
> until the modules are unloaded but this results in a slight increase in
> memory footprint of ath10k when the wifi is down, but the modules are
> still loaded. Also as of now we use a separate bool 'tx_mem_allocated'
> to keep track of the one time memory allocation, as we cannot come up
> with something like 'ath10k_tx_{register,unregister}' before
> 'ath10k_probe_fw' is called as 'ath10k_htt_tx_alloc_cont_frag_desc'
> memory allocation is dependent on the hw_param 'continuous_frag_desc'
>
> a) memory footprint of ath10k without the change
>
> lsmod | grep ath10k
> ath10k_core           414498  1 ath10k_pci
> ath10k_pci             38236  0
>
> b) memory footprint of ath10k with the change
>
> ath10k_core           414980  1 ath10k_pci
> ath10k_pci             38236  0
>
> Memory Failure Call trace:
>
> hostapd: page allocation failure: order:6, mode:0xd0
>  [<c021f150>] (__dma_alloc_buffer.isra.23) from
> [<c021f23c>] (__alloc_remap_buffer.isra.26+0x14/0xb8)
> [<c021f23c>] (__alloc_remap_buffer.isra.26) from
> [<c021f664>] (__dma_alloc+0x224/0x2b8)
> [<c021f664>] (__dma_alloc) from [<c021f810>]
> (arm_dma_alloc+0x84/0x90)
> [<c021f810>] (arm_dma_alloc) from [<bf954764>]
> (ath10k_htt_tx_alloc+0xe0/0x2e4 [ath10k_core])
> [<bf954764>] (ath10k_htt_tx_alloc [ath10k_core]) from
> [<bf94e6ac>] (ath10k_core_start+0x538/0xcf8 [ath10k_core])
> [<bf94e6ac>] (ath10k_core_start [ath10k_core]) from
> [<bf947eec>] (ath10k_start+0xbc/0x56c [ath10k_core])
> [<bf947eec>] (ath10k_start [ath10k_core]) from
> [<bf8a7a04>] (drv_start+0x40/0x5c [mac80211])
> [<bf8a7a04>] (drv_start [mac80211]) from [<bf8b7cf8>]
> (ieee80211_do_open+0x170/0x82c [mac80211])
> [<bf8b7cf8>] (ieee80211_do_open [mac80211]) from
> [<c056afc8>] (__dev_open+0xa0/0xf4)
> [21053.491752] Normal: 641*4kB (UEMR) 505*8kB (UEMR) 330*16kB (UEMR)
> 126*32kB (UEMR) 762*64kB (UEMR) 237*128kB (UEMR) 1*256kB (M) 0*512kB
> 0*1024kB 0*2048kB 0*4096kB = 95276kB
>
> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.c   |  5 +--
>  drivers/net/wireless/ath/ath10k/htt.h    |  6 +++-
>  drivers/net/wireless/ath/ath10k/htt_tx.c | 54 +++++++++++++++++++++++++-------
>  3 files changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 5bc6847..f7ea4de 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -1857,7 +1857,7 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
>                 goto err_wmi_detach;
>         }
>
> -       status = ath10k_htt_tx_alloc(&ar->htt);
> +       status = ath10k_htt_tx_start(&ar->htt);
>         if (status) {
>                 ath10k_err(ar, "failed to alloc htt tx: %d\n", status);
>                 goto err_wmi_detach;
> @@ -2052,7 +2052,7 @@ void ath10k_core_stop(struct ath10k *ar)
>                 ath10k_wait_for_suspend(ar, WMI_PDEV_SUSPEND_AND_DISABLE_INTR);
>
>         ath10k_hif_stop(ar);
> -       ath10k_htt_tx_free(&ar->htt);
> +       ath10k_htt_tx_stop(&ar->htt);
>         ath10k_htt_rx_free(&ar->htt);
>         ath10k_wmi_detach(ar);
>  }
> @@ -2385,6 +2385,7 @@ void ath10k_core_destroy(struct ath10k *ar)
>         destroy_workqueue(ar->workqueue_aux);
>
>         ath10k_debug_destroy(ar);
> +       ath10k_htt_tx_destroy(&ar->htt);
>         ath10k_wmi_free_host_mem(ar);
>         ath10k_mac_destroy(ar);
>  }
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index 0d2ed09..96bf7bf 100644
> --- a/drivers/net/wireless/ath/ath10k/htt.h
> +++ b/drivers/net/wireless/ath/ath10k/htt.h
> @@ -1692,6 +1692,8 @@ struct ath10k_htt {
>                 enum htt_tx_mode_switch_mode mode;
>                 enum htt_q_depth_type type;
>         } tx_q_state;
> +
> +       bool tx_mem_allocated;
>  };
>
>  #define RX_HTT_HDR_STATUS_LEN 64
> @@ -1754,7 +1756,9 @@ struct htt_rx_desc {
>  int ath10k_htt_init(struct ath10k *ar);
>  int ath10k_htt_setup(struct ath10k_htt *htt);
>
> -int ath10k_htt_tx_alloc(struct ath10k_htt *htt);
> +int ath10k_htt_tx_start(struct ath10k_htt *htt);
> +void ath10k_htt_tx_stop(struct ath10k_htt *htt);
> +void ath10k_htt_tx_destroy(struct ath10k_htt *htt);
>  void ath10k_htt_tx_free(struct ath10k_htt *htt);
>
>  int ath10k_htt_rx_alloc(struct ath10k_htt *htt);
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index ccbc8c03..27e49db 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -350,21 +350,15 @@ static int ath10k_htt_tx_alloc_txdone_fifo(struct ath10k_htt *htt)
>         return ret;
>  }
>
> -int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
> +static int ath10k_htt_tx_alloc_buf(struct ath10k_htt *htt)
>  {
>         struct ath10k *ar = htt->ar;
>         int ret;
>
> -       ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt tx max num pending tx %d\n",
> -                  htt->max_num_pending_tx);
> -
> -       spin_lock_init(&htt->tx_lock);
> -       idr_init(&htt->pending_tx);
> -
>         ret = ath10k_htt_tx_alloc_cont_txbuf(htt);
>         if (ret) {
>                 ath10k_err(ar, "failed to alloc cont tx buffer: %d\n", ret);
> -               goto free_idr_pending_tx;
> +               return ret;
>         }
>
>         ret = ath10k_htt_tx_alloc_cont_frag_desc(htt);
> @@ -396,6 +390,31 @@ int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
>  free_txbuf:
>         ath10k_htt_tx_free_cont_txbuf(htt);
>
> +       return ret;
> +}
> +
> +int ath10k_htt_tx_start(struct ath10k_htt *htt)
> +{
> +       struct ath10k *ar = htt->ar;
> +       int ret;
> +
> +       ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt tx max num pending tx %d\n",
> +                  htt->max_num_pending_tx);
> +
> +       spin_lock_init(&htt->tx_lock);
> +       idr_init(&htt->pending_tx);
> +
> +       if (htt->tx_mem_allocated)
> +               return 0;
> +
> +       ret = ath10k_htt_tx_alloc_buf(htt);
> +       if (ret)
> +               goto free_idr_pending_tx;
> +
> +       htt->tx_mem_allocated = true;
> +
> +       return 0;
> +
>  free_idr_pending_tx:
>         idr_destroy(&htt->pending_tx);
>
> @@ -418,15 +437,28 @@ static int ath10k_htt_tx_clean_up_pending(int msdu_id, void *skb, void *ctx)
>         return 0;
>  }
>
> -void ath10k_htt_tx_free(struct ath10k_htt *htt)
> +void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
>  {
> -       idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
> -       idr_destroy(&htt->pending_tx);
> +       if (!htt->tx_mem_allocated)
> +               return;
>
>         ath10k_htt_tx_free_cont_txbuf(htt);
>         ath10k_htt_tx_free_txq(htt);
>         ath10k_htt_tx_free_cont_frag_desc(htt);
>         ath10k_htt_tx_free_txdone_fifo(htt);
> +       htt->tx_mem_allocated = false;
> +}
> +
> +void ath10k_htt_tx_stop(struct ath10k_htt *htt)
> +{
> +       idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
> +       idr_destroy(&htt->pending_tx);
> +}
> +
> +void ath10k_htt_tx_free(struct ath10k_htt *htt)
> +{
> +       ath10k_htt_tx_stop(htt);
> +       ath10k_htt_tx_destroy(htt);
>  }
>
>  void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
> --
> 1.9.1
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply

* Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting
From: Jes Sorensen @ 2016-11-30 17:40 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Luca Coelho, linux-wireless, briselec
In-Reply-To: <87inr4ubc2.fsf@kamboji.qca.qualcomm.com>

Kalle Valo <kvalo@codeaurora.org> writes:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>
>> Luca Coelho <luca@coelho.fi> writes:
>>> On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
>>>> Jes.Sorensen@redhat.com writes:
>>>> 
>>>> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>> > 
>>>> > Hi,
>>>> > 
>>>> > This patch works around the issue with 8192eu and 8723bu devices not
>>>> > being able to reconnect. It is still not clear why this fails, and
>>>> > looking at rtlwifi it does issue the command for 8192ee and 8723be
>>>> > devices.
>>>> > 
>>>> > Until we have a better idea of what is going on, I have commented out
>>>> > the offending code. I prefer not deleting it until we have a better
>>>> > understanding of why this is causing issues.
>>>> > 
>>>> > Credits to Barry Day for tracking down and isolating the issue.
>>>> > 
>>>> > Kalle, not sure if this can make it to 4.9, but if it's possible it
>>>> > would be ideal since this is causing real issues in the field.
>>>> 
>>>> For 4.9 this is too late, sorry. I'll queue this to 4.10.
>>>
>>> You should then CC stable so it gets into 4.9.x when it reaches 4.10.
>>
>> Yeah good idea - Kalle do you want me to repost it with the CC line or
>> are you happy to add it?
>
> I can add it. To which stable releases should this go to?

Lets target 4.8 and 4.9 - I just verified it applies against 4.8.

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting
From: Kalle Valo @ 2016-11-30 17:27 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Luca Coelho, linux-wireless, briselec
In-Reply-To: <wrfja8ch6l1o.fsf@redhat.com>

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> Luca Coelho <luca@coelho.fi> writes:
>> On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
>>> Jes.Sorensen@redhat.com writes:
>>> 
>>> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>> > 
>>> > Hi,
>>> > 
>>> > This patch works around the issue with 8192eu and 8723bu devices not
>>> > being able to reconnect. It is still not clear why this fails, and
>>> > looking at rtlwifi it does issue the command for 8192ee and 8723be
>>> > devices.
>>> > 
>>> > Until we have a better idea of what is going on, I have commented out
>>> > the offending code. I prefer not deleting it until we have a better
>>> > understanding of why this is causing issues.
>>> > 
>>> > Credits to Barry Day for tracking down and isolating the issue.
>>> > 
>>> > Kalle, not sure if this can make it to 4.9, but if it's possible it
>>> > would be ideal since this is causing real issues in the field.
>>> 
>>> For 4.9 this is too late, sorry. I'll queue this to 4.10.
>>
>> You should then CC stable so it gets into 4.9.x when it reaches 4.10.
>
> Yeah good idea - Kalle do you want me to repost it with the CC line or
> are you happy to add it?

I can add it. To which stable releases should this go to?

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting
From: Jes Sorensen @ 2016-11-30 15:32 UTC (permalink / raw)
  To: Luca Coelho; +Cc: Kalle Valo, linux-wireless, briselec
In-Reply-To: <1480514067.29540.54.camel@coelho.fi>

Luca Coelho <luca@coelho.fi> writes:
> On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
>> Jes.Sorensen@redhat.com writes:
>> 
>> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> > 
>> > Hi,
>> > 
>> > This patch works around the issue with 8192eu and 8723bu devices not
>> > being able to reconnect. It is still not clear why this fails, and
>> > looking at rtlwifi it does issue the command for 8192ee and 8723be
>> > devices.
>> > 
>> > Until we have a better idea of what is going on, I have commented out
>> > the offending code. I prefer not deleting it until we have a better
>> > understanding of why this is causing issues.
>> > 
>> > Credits to Barry Day for tracking down and isolating the issue.
>> > 
>> > Kalle, not sure if this can make it to 4.9, but if it's possible it
>> > would be ideal since this is causing real issues in the field.
>> 
>> For 4.9 this is too late, sorry. I'll queue this to 4.10.
>
> You should then CC stable so it gets into 4.9.x when it reaches 4.10.

Yeah good idea - Kalle do you want me to repost it with the CC line or
are you happy to add it?

Cheers,
Jes

^ permalink raw reply

* [PATCH 1/2] mwifiex: code rearrangement in pcie.c and sdio.c
From: Amitkumar Karwar @ 2016-11-30 14:52 UTC (permalink / raw)
  To: linux-wireless
  Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
	dmitry.torokhov, Xinming Hu, Amitkumar Karwar

From: Xinming Hu <huxm@marvell.com>

Next patch in this series is going to use mwifiex_read_reg() in remove
handlers. The changes here are prerequisites to avoid forward
declarations.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/marvell/mwifiex/pcie.c |  73 +++---
 drivers/net/wireless/marvell/mwifiex/sdio.c | 336 ++++++++++++++--------------
 2 files changed, 201 insertions(+), 208 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 32fa4ed..7aa16a5 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -81,6 +81,42 @@ static void mwifiex_unmap_pci_memory(struct mwifiex_adapter *adapter,
 }
 
 /*
+ * This function writes data into PCIE card register.
+ */
+static int mwifiex_write_reg(struct mwifiex_adapter *adapter, int reg, u32 data)
+{
+	struct pcie_service_card *card = adapter->card;
+
+	iowrite32(data, card->pci_mmap1 + reg);
+
+	return 0;
+}
+
+/* This function reads data from PCIE card register.
+ */
+static int mwifiex_read_reg(struct mwifiex_adapter *adapter, int reg, u32 *data)
+{
+	struct pcie_service_card *card = adapter->card;
+
+	*data = ioread32(card->pci_mmap1 + reg);
+	if (*data == 0xffffffff)
+		return 0xffffffff;
+
+	return 0;
+}
+
+/* This function reads u8 data from PCIE card register. */
+static int mwifiex_read_reg_byte(struct mwifiex_adapter *adapter,
+				 int reg, u8 *data)
+{
+	struct pcie_service_card *card = adapter->card;
+
+	*data = ioread8(card->pci_mmap1 + reg);
+
+	return 0;
+}
+
+/*
  * This function reads sleep cookie and checks if FW is ready
  */
 static bool mwifiex_pcie_ok_to_access_hw(struct mwifiex_adapter *adapter)
@@ -374,43 +410,6 @@ static SIMPLE_DEV_PM_OPS(mwifiex_pcie_pm_ops, mwifiex_pcie_suspend,
 };
 
 /*
- * This function writes data into PCIE card register.
- */
-static int mwifiex_write_reg(struct mwifiex_adapter *adapter, int reg, u32 data)
-{
-	struct pcie_service_card *card = adapter->card;
-
-	iowrite32(data, card->pci_mmap1 + reg);
-
-	return 0;
-}
-
-/*
- * This function reads data from PCIE card register.
- */
-static int mwifiex_read_reg(struct mwifiex_adapter *adapter, int reg, u32 *data)
-{
-	struct pcie_service_card *card = adapter->card;
-
-	*data = ioread32(card->pci_mmap1 + reg);
-	if (*data == 0xffffffff)
-		return 0xffffffff;
-
-	return 0;
-}
-
-/* This function reads u8 data from PCIE card register. */
-static int mwifiex_read_reg_byte(struct mwifiex_adapter *adapter,
-				 int reg, u8 *data)
-{
-	struct pcie_service_card *card = adapter->card;
-
-	*data = ioread8(card->pci_mmap1 + reg);
-
-	return 0;
-}
-
-/*
  * This function adds delay loop to ensure FW is awake before proceeding.
  */
 static void mwifiex_pcie_dev_wakeup_delay(struct mwifiex_adapter *adapter)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 66d0dd6..6fcaf26 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -215,6 +215,171 @@ static int mwifiex_sdio_resume(struct device *dev)
 	return 0;
 }
 
+/* Write data into SDIO card register. Caller claims SDIO device. */
+static int
+mwifiex_write_reg_locked(struct sdio_func *func, u32 reg, u8 data)
+{
+	int ret = -1;
+
+	sdio_writeb(func, data, reg, &ret);
+	return ret;
+}
+
+/* This function writes data into SDIO card register.
+ */
+static int
+mwifiex_write_reg(struct mwifiex_adapter *adapter, u32 reg, u8 data)
+{
+	struct sdio_mmc_card *card = adapter->card;
+	int ret;
+
+	sdio_claim_host(card->func);
+	ret = mwifiex_write_reg_locked(card->func, reg, data);
+	sdio_release_host(card->func);
+
+	return ret;
+}
+
+/* This function reads data from SDIO card register.
+ */
+static int
+mwifiex_read_reg(struct mwifiex_adapter *adapter, u32 reg, u8 *data)
+{
+	struct sdio_mmc_card *card = adapter->card;
+	int ret = -1;
+	u8 val;
+
+	sdio_claim_host(card->func);
+	val = sdio_readb(card->func, reg, &ret);
+	sdio_release_host(card->func);
+
+	*data = val;
+
+	return ret;
+}
+
+/* This function writes multiple data into SDIO card memory.
+ *
+ * This does not work in suspended mode.
+ */
+static int
+mwifiex_write_data_sync(struct mwifiex_adapter *adapter,
+			u8 *buffer, u32 pkt_len, u32 port)
+{
+	struct sdio_mmc_card *card = adapter->card;
+	int ret;
+	u8 blk_mode =
+		(port & MWIFIEX_SDIO_BYTE_MODE_MASK) ? BYTE_MODE : BLOCK_MODE;
+	u32 blk_size = (blk_mode == BLOCK_MODE) ? MWIFIEX_SDIO_BLOCK_SIZE : 1;
+	u32 blk_cnt =
+		(blk_mode ==
+		 BLOCK_MODE) ? (pkt_len /
+				MWIFIEX_SDIO_BLOCK_SIZE) : pkt_len;
+	u32 ioport = (port & MWIFIEX_SDIO_IO_PORT_MASK);
+
+	if (adapter->is_suspended) {
+		mwifiex_dbg(adapter, ERROR,
+			    "%s: not allowed while suspended\n", __func__);
+		return -1;
+	}
+
+	sdio_claim_host(card->func);
+
+	ret = sdio_writesb(card->func, ioport, buffer, blk_cnt * blk_size);
+
+	sdio_release_host(card->func);
+
+	return ret;
+}
+
+/* This function reads multiple data from SDIO card memory.
+ */
+static int mwifiex_read_data_sync(struct mwifiex_adapter *adapter, u8 *buffer,
+				  u32 len, u32 port, u8 claim)
+{
+	struct sdio_mmc_card *card = adapter->card;
+	int ret;
+	u8 blk_mode = (port & MWIFIEX_SDIO_BYTE_MODE_MASK) ? BYTE_MODE
+		       : BLOCK_MODE;
+	u32 blk_size = (blk_mode == BLOCK_MODE) ? MWIFIEX_SDIO_BLOCK_SIZE : 1;
+	u32 blk_cnt = (blk_mode == BLOCK_MODE) ? (len / MWIFIEX_SDIO_BLOCK_SIZE)
+			: len;
+	u32 ioport = (port & MWIFIEX_SDIO_IO_PORT_MASK);
+
+	if (claim)
+		sdio_claim_host(card->func);
+
+	ret = sdio_readsb(card->func, buffer, ioport, blk_cnt * blk_size);
+
+	if (claim)
+		sdio_release_host(card->func);
+
+	return ret;
+}
+
+/* This function reads the firmware status.
+ */
+static int
+mwifiex_sdio_read_fw_status(struct mwifiex_adapter *adapter, u16 *dat)
+{
+	struct sdio_mmc_card *card = adapter->card;
+	const struct mwifiex_sdio_card_reg *reg = card->reg;
+	u8 fws0, fws1;
+
+	if (mwifiex_read_reg(adapter, reg->status_reg_0, &fws0))
+		return -1;
+
+	if (mwifiex_read_reg(adapter, reg->status_reg_1, &fws1))
+		return -1;
+
+	*dat = (u16)((fws1 << 8) | fws0);
+	return 0;
+}
+
+/* This function checks the firmware status in card.
+ */
+static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
+				   u32 poll_num)
+{
+	int ret = 0;
+	u16 firmware_stat;
+	u32 tries;
+
+	for (tries = 0; tries < poll_num; tries++) {
+		ret = mwifiex_sdio_read_fw_status(adapter, &firmware_stat);
+		if (ret)
+			continue;
+		if (firmware_stat == FIRMWARE_READY_SDIO) {
+			ret = 0;
+			break;
+		}
+
+		msleep(100);
+		ret = -1;
+	}
+
+	return ret;
+}
+
+/* This function checks if WLAN is the winner.
+ */
+static int mwifiex_check_winner_status(struct mwifiex_adapter *adapter)
+{
+	int ret = 0;
+	u8 winner = 0;
+	struct sdio_mmc_card *card = adapter->card;
+
+	if (mwifiex_read_reg(adapter, card->reg->status_reg_0, &winner))
+		return -1;
+
+	if (winner)
+		adapter->winner = 0;
+	else
+		adapter->winner = 1;
+
+	return ret;
+}
+
 /*
  * SDIO remove.
  *
@@ -374,111 +539,6 @@ static int mwifiex_sdio_suspend(struct device *dev)
 	}
 };
 
-/* Write data into SDIO card register. Caller claims SDIO device. */
-static int
-mwifiex_write_reg_locked(struct sdio_func *func, u32 reg, u8 data)
-{
-	int ret = -1;
-	sdio_writeb(func, data, reg, &ret);
-	return ret;
-}
-
-/*
- * This function writes data into SDIO card register.
- */
-static int
-mwifiex_write_reg(struct mwifiex_adapter *adapter, u32 reg, u8 data)
-{
-	struct sdio_mmc_card *card = adapter->card;
-	int ret;
-
-	sdio_claim_host(card->func);
-	ret = mwifiex_write_reg_locked(card->func, reg, data);
-	sdio_release_host(card->func);
-
-	return ret;
-}
-
-/*
- * This function reads data from SDIO card register.
- */
-static int
-mwifiex_read_reg(struct mwifiex_adapter *adapter, u32 reg, u8 *data)
-{
-	struct sdio_mmc_card *card = adapter->card;
-	int ret = -1;
-	u8 val;
-
-	sdio_claim_host(card->func);
-	val = sdio_readb(card->func, reg, &ret);
-	sdio_release_host(card->func);
-
-	*data = val;
-
-	return ret;
-}
-
-/*
- * This function writes multiple data into SDIO card memory.
- *
- * This does not work in suspended mode.
- */
-static int
-mwifiex_write_data_sync(struct mwifiex_adapter *adapter,
-			u8 *buffer, u32 pkt_len, u32 port)
-{
-	struct sdio_mmc_card *card = adapter->card;
-	int ret;
-	u8 blk_mode =
-		(port & MWIFIEX_SDIO_BYTE_MODE_MASK) ? BYTE_MODE : BLOCK_MODE;
-	u32 blk_size = (blk_mode == BLOCK_MODE) ? MWIFIEX_SDIO_BLOCK_SIZE : 1;
-	u32 blk_cnt =
-		(blk_mode ==
-		 BLOCK_MODE) ? (pkt_len /
-				MWIFIEX_SDIO_BLOCK_SIZE) : pkt_len;
-	u32 ioport = (port & MWIFIEX_SDIO_IO_PORT_MASK);
-
-	if (adapter->is_suspended) {
-		mwifiex_dbg(adapter, ERROR,
-			    "%s: not allowed while suspended\n", __func__);
-		return -1;
-	}
-
-	sdio_claim_host(card->func);
-
-	ret = sdio_writesb(card->func, ioport, buffer, blk_cnt * blk_size);
-
-	sdio_release_host(card->func);
-
-	return ret;
-}
-
-/*
- * This function reads multiple data from SDIO card memory.
- */
-static int mwifiex_read_data_sync(struct mwifiex_adapter *adapter, u8 *buffer,
-				  u32 len, u32 port, u8 claim)
-{
-	struct sdio_mmc_card *card = adapter->card;
-	int ret;
-	u8 blk_mode = (port & MWIFIEX_SDIO_BYTE_MODE_MASK) ? BYTE_MODE
-		       : BLOCK_MODE;
-	u32 blk_size = (blk_mode == BLOCK_MODE) ? MWIFIEX_SDIO_BLOCK_SIZE : 1;
-	u32 blk_cnt = (blk_mode == BLOCK_MODE) ? (len / MWIFIEX_SDIO_BLOCK_SIZE)
-			: len;
-	u32 ioport = (port & MWIFIEX_SDIO_IO_PORT_MASK);
-
-	if (claim)
-		sdio_claim_host(card->func);
-
-	ret = sdio_readsb(card->func, buffer, ioport, blk_cnt * blk_size);
-
-	if (claim)
-		sdio_release_host(card->func);
-
-	return ret;
-}
-
 /*
  * This function wakes up the card.
  *
@@ -765,27 +825,6 @@ static int mwifiex_get_wr_port_data(struct mwifiex_adapter *adapter, u32 *port)
 }
 
 /*
- * This function reads the firmware status.
- */
-static int
-mwifiex_sdio_read_fw_status(struct mwifiex_adapter *adapter, u16 *dat)
-{
-	struct sdio_mmc_card *card = adapter->card;
-	const struct mwifiex_sdio_card_reg *reg = card->reg;
-	u8 fws0, fws1;
-
-	if (mwifiex_read_reg(adapter, reg->status_reg_0, &fws0))
-		return -1;
-
-	if (mwifiex_read_reg(adapter, reg->status_reg_1, &fws1))
-		return -1;
-
-	*dat = (u16) ((fws1 << 8) | fws0);
-
-	return 0;
-}
-
-/*
  * This function disables the host interrupt.
  *
  * The host interrupt mask is read, the disable bit is reset and
@@ -1090,51 +1129,6 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
 }
 
 /*
- * This function checks the firmware status in card.
- */
-static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
-				   u32 poll_num)
-{
-	int ret = 0;
-	u16 firmware_stat;
-	u32 tries;
-
-	for (tries = 0; tries < poll_num; tries++) {
-		ret = mwifiex_sdio_read_fw_status(adapter, &firmware_stat);
-		if (ret)
-			continue;
-		if (firmware_stat == FIRMWARE_READY_SDIO) {
-			ret = 0;
-			break;
-		} else {
-			msleep(100);
-			ret = -1;
-		}
-	}
-
-	return ret;
-}
-
-/* This function checks if WLAN is the winner.
- */
-static int mwifiex_check_winner_status(struct mwifiex_adapter *adapter)
-{
-	int ret = 0;
-	u8 winner = 0;
-	struct sdio_mmc_card *card = adapter->card;
-
-	if (mwifiex_read_reg(adapter, card->reg->status_reg_0, &winner))
-		return -1;
-
-	if (winner)
-		adapter->winner = 0;
-	else
-		adapter->winner = 1;
-
-	return ret;
-}
-
-/*
  * This function decode sdio aggreation pkt.
  *
  * Based on the the data block size and pkt_len,
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/2] mwifiex: get rid of global user_rmmod flag
From: Amitkumar Karwar @ 2016-11-30 14:52 UTC (permalink / raw)
  To: linux-wireless
  Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
	dmitry.torokhov, Xinming Hu, Amitkumar Karwar
In-Reply-To: <1480517537-9920-1-git-send-email-akarwar@marvell.com>

From: Xinming Hu <huxm@marvell.com>

bus.remove() callback function is called when user removes this module
from kernel space or ejects the card from the slot. The driver handles
these 2 cases differently. Few commands (FUNC_SHUTDOWN etc.) are sent to
the firmware only for module unload case.

The variable 'user_rmmod' is used to distinguish between these two
scenarios.

This patch checks hardware status and get rid of global variable
user_rmmod.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 23 ++++++++++++-----------
 drivers/net/wireless/marvell/mwifiex/sdio.c | 27 ++++-----------------------
 drivers/net/wireless/marvell/mwifiex/usb.c  |  6 +-----
 3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 7aa16a5..079bb09 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -31,8 +31,6 @@
 #define PCIE_VERSION	"1.0"
 #define DRV_NAME        "Marvell mwifiex PCIe"
 
-static u8 user_rmmod;
-
 static struct mwifiex_if_ops pcie_ops;
 
 static const struct of_device_id mwifiex_pcie_of_match_table[] = {
@@ -284,6 +282,9 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
 	struct pcie_service_card *card;
 	struct mwifiex_adapter *adapter;
 	struct mwifiex_private *priv;
+	const struct mwifiex_pcie_card_reg *reg;
+	u32 fw_status;
+	int ret;
 
 	card = pci_get_drvdata(pdev);
 
@@ -295,7 +296,11 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
 
 	cancel_work_sync(&card->work);
 
-	if (user_rmmod && !adapter->mfg_mode) {
+	reg = card->pcie.reg;
+	if (reg)
+		ret = mwifiex_read_reg(adapter, reg->fw_status, &fw_status);
+
+	if (fw_status == FIRMWARE_READY_PCIE && !adapter->mfg_mode) {
 		mwifiex_deauthenticate_all(adapter);
 
 		priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
@@ -310,7 +315,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
 
 static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
 {
-	user_rmmod = 1;
 	mwifiex_pcie_remove(pdev);
 
 	return;
@@ -2864,8 +2868,11 @@ static void mwifiex_pcie_cleanup(struct mwifiex_adapter *adapter)
 	struct pcie_service_card *card = adapter->card;
 	struct pci_dev *pdev = card->dev;
 	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
+	int ret;
+	u32 fw_status;
 
-	if (user_rmmod) {
+	ret = mwifiex_read_reg(adapter, reg->fw_status, &fw_status);
+	if (fw_status == FIRMWARE_READY_PCIE) {
 		mwifiex_dbg(adapter, INFO,
 			    "Clearing driver ready signature\n");
 		if (mwifiex_write_reg(adapter, reg->drv_rdy, 0x00000000))
@@ -3177,9 +3184,6 @@ static int mwifiex_pcie_init_module(void)
 
 	pr_debug("Marvell PCIe Driver\n");
 
-	/* Clear the flag in case user removes the card. */
-	user_rmmod = 0;
-
 	ret = pci_register_driver(&mwifiex_pcie);
 	if (ret)
 		pr_err("Driver register failed!\n");
@@ -3200,9 +3204,6 @@ static int mwifiex_pcie_init_module(void)
  */
 static void mwifiex_pcie_cleanup_module(void)
 {
-	/* Set the flag as user is removing this module. */
-	user_rmmod = 1;
-
 	pci_unregister_driver(&mwifiex_pcie);
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 6fcaf26..9a16e61 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -31,21 +31,6 @@
 
 #define SDIO_VERSION	"1.0"
 
-/* The mwifiex_sdio_remove() callback function is called when
- * user removes this module from kernel space or ejects
- * the card from the slot. The driver handles these 2 cases
- * differently.
- * If the user is removing the module, the few commands (FUNC_SHUTDOWN,
- * HS_CANCEL etc.) are sent to the firmware.
- * If the card is removed, there is no need to send these command.
- *
- * The variable 'user_rmmod' is used to distinguish these two
- * scenarios. This flag is initialized as FALSE in case the card
- * is removed, and will be set to TRUE for module removal when
- * module_exit function is called.
- */
-static u8 user_rmmod;
-
 static void mwifiex_sdio_work(struct work_struct *work);
 static DECLARE_WORK(sdio_work, mwifiex_sdio_work);
 
@@ -391,6 +376,8 @@ static int mwifiex_check_winner_status(struct mwifiex_adapter *adapter)
 	struct sdio_mmc_card *card;
 	struct mwifiex_adapter *adapter;
 	struct mwifiex_private *priv;
+	int ret = 0;
+	u16 firmware_stat;
 
 	card = sdio_get_drvdata(func);
 	if (!card)
@@ -404,7 +391,8 @@ static int mwifiex_check_winner_status(struct mwifiex_adapter *adapter)
 
 	mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
 
-	if (user_rmmod && !adapter->mfg_mode) {
+	ret = mwifiex_sdio_read_fw_status(adapter, &firmware_stat);
+	if (firmware_stat == FIRMWARE_READY_SDIO && !adapter->mfg_mode) {
 		mwifiex_deauthenticate_all(adapter);
 
 		priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
@@ -2718,9 +2706,6 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
 static int
 mwifiex_sdio_init_module(void)
 {
-	/* Clear the flag in case user removes the card. */
-	user_rmmod = 0;
-
 	return sdio_register_driver(&mwifiex_sdio);
 }
 
@@ -2736,10 +2721,6 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
 static void
 mwifiex_sdio_cleanup_module(void)
 {
-	/* Set the flag as user is removing this module. */
-	user_rmmod = 1;
-	cancel_work_sync(&sdio_work);
-
 	sdio_unregister_driver(&mwifiex_sdio);
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index c563160..53881c4 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -22,7 +22,6 @@
 
 #define USB_VERSION	"1.0"
 
-static u8 user_rmmod;
 static struct mwifiex_if_ops usb_ops;
 
 static struct usb_device_id mwifiex_usb_table[] = {
@@ -618,7 +617,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
 	if (!adapter || !adapter->priv_num)
 		return;
 
-	if (user_rmmod && !adapter->mfg_mode) {
+	if (card->udev->state != USB_STATE_NOTATTACHED && !adapter->mfg_mode) {
 		mwifiex_deauthenticate_all(adapter);
 
 		mwifiex_init_shutdown_fw(mwifiex_get_priv(adapter,
@@ -1230,9 +1229,6 @@ static int mwifiex_usb_init_module(void)
  */
 static void mwifiex_usb_cleanup_module(void)
 {
-	/* set the flag as user is removing this module */
-	user_rmmod = 1;
-
 	usb_deregister(&mwifiex_usb_driver);
 }
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
From: Ulf Hansson @ 2016-11-30 14:39 UTC (permalink / raw)
  To: Javier Martinez Canillas, Matt Ranostay
  Cc: linux-wireless@vger.kernel.org, Linux Kernel,
	devicetree@vger.kernel.org, linux-mmc@vger.kernel.org,
	Tony Lindgren, Mark Rutland, Srinivas Kandagatla
In-Reply-To: <CABxcv=nc1qjtSjzfcuGTo-zUpuWdRT6OR7MZCCWaoeq4Co3Uew@mail.gmail.com>

On 30 November 2016 at 14:11, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Matt,
>
> On Tue, Nov 29, 2016 at 10:20 PM, Matt Ranostay
> <matt@ranostay.consulting> wrote:
>> On Tue, Nov 29, 2016 at 9:13 AM, Javier Martinez Canillas
>
> [snip]
>
>>
>>
>>>> +- pwndn-gpio: contains a power down GPIO specifier.
>>>> +- reset-gpio: contains a reset GPIO specifier.
>>>> +
>>>
>>> I wonder if we really need a custom power sequence provider for just
>>> this SDIO WiFI chip though. AFAICT the only missing piece in
>>> mmc-pwrseq-simple is the power down GPIO property, so maybe
>>> mmc-pwrseq-simple could be extended instead to have an optional
>>> powerdown-gpios property and instead in the Marvell SD8787 DT binding
>>> can be mentioned which mmc-pwrseq-simple properties are required for
>>> the device.
>>>
>>
>> The reason we didn't do that is we need delay between the two
>> assertions/desertions of GPIOs. It wouldn't seems good practice to
>> hack the pwrseq-simple for this...
>>
>
> Yes, I noticed that. I wouldn't say that it would be a hack for the
> pwrseq-simple since it already has a "post-power-on-delay-ms" DT
> property, so AFAICT it would just be adding a "pre-power-on-delay-ms"
> property for your use case.
>
> It would also be more consistent since it would support a delay for
> pre and post power callbacks. It would also make you avoid hardcoding
> the 300 msec wait, in case other device has a similar need but with a
> different wait time.
>
> In summary, I think that devices having a power (or power down) and
> enable GPIO, and needing to wait between the GPIO toggling are common.
> So I would prefer to make pwrseq-simple usable for these instead of
> adding device specific power sequence providers. But it's just my
> opinion and not my call :-)

This is a good idea. Please try out this approach.

[...]

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting
From: Luca Coelho @ 2016-11-30 13:54 UTC (permalink / raw)
  To: Kalle Valo, Jes.Sorensen; +Cc: linux-wireless, briselec
In-Reply-To: <87mvghtkwd.fsf@kamboji.qca.qualcomm.com>

On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
> Jes.Sorensen@redhat.com writes:
> 
> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
> > 
> > Hi,
> > 
> > This patch works around the issue with 8192eu and 8723bu devices not
> > being able to reconnect. It is still not clear why this fails, and
> > looking at rtlwifi it does issue the command for 8192ee and 8723be
> > devices.
> > 
> > Until we have a better idea of what is going on, I have commented out
> > the offending code. I prefer not deleting it until we have a better
> > understanding of why this is causing issues.
> > 
> > Credits to Barry Day for tracking down and isolating the issue.
> > 
> > Kalle, not sure if this can make it to 4.9, but if it's possible it
> > would be ideal since this is causing real issues in the field.
> 
> For 4.9 this is too late, sorry. I'll queue this to 4.10.

You should then CC stable so it gets into 4.9.x when it reaches 4.10.

--
Luca.

^ permalink raw reply

* Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
From: Javier Martinez Canillas @ 2016-11-30 13:11 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: linux-wireless@vger.kernel.org, Linux Kernel,
	devicetree@vger.kernel.org, linux-mmc@vger.kernel.org,
	Tony Lindgren, Ulf Hansson, Mark Rutland, Srinivas Kandagatla
In-Reply-To: <CAJ_EiST_bpBm+spPuWH-a+47t4qVsDVFjUm=a+TtL-BWN3DHdw@mail.gmail.com>

Hello Matt,

On Tue, Nov 29, 2016 at 10:20 PM, Matt Ranostay
<matt@ranostay.consulting> wrote:
> On Tue, Nov 29, 2016 at 9:13 AM, Javier Martinez Canillas

[snip]

>
>
>>> +- pwndn-gpio: contains a power down GPIO specifier.
>>> +- reset-gpio: contains a reset GPIO specifier.
>>> +
>>
>> I wonder if we really need a custom power sequence provider for just
>> this SDIO WiFI chip though. AFAICT the only missing piece in
>> mmc-pwrseq-simple is the power down GPIO property, so maybe
>> mmc-pwrseq-simple could be extended instead to have an optional
>> powerdown-gpios property and instead in the Marvell SD8787 DT binding
>> can be mentioned which mmc-pwrseq-simple properties are required for
>> the device.
>>
>
> The reason we didn't do that is we need delay between the two
> assertions/desertions of GPIOs. It wouldn't seems good practice to
> hack the pwrseq-simple for this...
>

Yes, I noticed that. I wouldn't say that it would be a hack for the
pwrseq-simple since it already has a "post-power-on-delay-ms" DT
property, so AFAICT it would just be adding a "pre-power-on-delay-ms"
property for your use case.

It would also be more consistent since it would support a delay for
pre and post power callbacks. It would also make you avoid hardcoding
the 300 msec wait, in case other device has a similar need but with a
different wait time.

In summary, I think that devices having a power (or power down) and
enable GPIO, and needing to wait between the GPIO toggling are common.
So I would prefer to make pwrseq-simple usable for these instead of
adding device specific power sequence providers. But it's just my
opinion and not my call :-)

>>> +Example:
>>> +
>>> +       wifi_pwrseq: wifi_pwrseq {
>>> +               compatible = "mmc-pwrseq-sd8787";
>>> +               pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>;
>>> +               reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>;
>>> +       }
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>
>> Does this patch depend on a previous posted series? I don't see this
>> file in today's linux-next...
>
> Got renamed to ->
> Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt it
> seems very recently.
>

I see, that's what I thought but I wasn't able to find the commit /
patch that did it.

>>
>>> index c421aba0a5bc..08fd65d35725 100644
>>> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>> @@ -32,6 +32,9 @@ Optional properties:
>>>                  so that the wifi chip can wakeup host platform under certain condition.
>>>                  during system resume, the irq will be disabled to make sure
>>>                  unnecessary interrupt is not received.
>>> +  - vmmc-supply: a phandle of a regulator, supplying VCC to the card
>>> +  - mmc-pwrseq:  phandle to the MMC power sequence node. See "mmc-pwrseq-*"
>>> +                for documentation of MMC power sequence bindings.
>>>
>>>  Example:
>>>
>>> @@ -44,6 +47,7 @@ so that firmware can wakeup host using this device side pin.
>>>  &mmc3 {
>>>         status = "okay";
>>>         vmmc-supply = <&wlan_en_reg>;
>>> +       mmc-pwrseq = <&wifi_pwrseq>;
>>>         bus-width = <4>;
>>>         cap-power-off-card;
>>>         keep-power-in-suspend;
>>
>> I think this change should be split in a separate patch as well.
>>

You didn't answer about this, but I guess you agree it should be in a
separate patch.

Best regards,
Javier

^ permalink raw reply

* RE: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process
From: Amitkumar Karwar @ 2016-11-30 12:39 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
	rajatja@google.com, dmitry.torokhov@gmail.com, Xinming Hu
In-Reply-To: <20161128212706.GA45985@google.com>

Hi Brian,

> > > >   *
> > > > @@ -2227,7 +2237,7 @@ static void mwifiex_recreate_adapter(struct
> > > sdio_mmc_card *card)
> > > >  	 * discovered and initializes them from scratch.
> > > >  	 */
> > > >
> > > > -	mwifiex_sdio_remove(func);
> > > > +	__mwifiex_sdio_remove(func);
> > >
> > > ^^ So here, you're trying to avoid syncing with the card-reset work
> > > event, except that function will free up all your resources
> > > (including the static save_adapter). Thus, you're explicitly
> > > allowing a use-after- free error here. That seems unwise.
> >
> > Even if firmware dump is triggered after card reset is started, it
> > will execute after card reset is completed as discussed above. Only
> > problem I can see is with "save_adapter". We can put new_adapter
> > pointer in "save_adapter" at the end of mwifiex_recreate_adapter() to
> > solve the issue.
> 
> Ugh, yet another band-aid? You might do better to still cancel any
> pending work, just don't do it synchronously. i.e., do cancel_work()
> after you've removed the card. It doesn't make sense to do a FW dump on
> the "new" adapter when it was requested for the old one.

I could not find async version of cancel_work().
We can fix this problem with below change at the end of mwifiex_sdio_work(). All pending work requests would be ignored.

--------
@ -2571,6 +2571,8 @@ static void mwifiex_sdio_work(struct work_struct *work)
        if (test_and_clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET,
                               &iface_work_flags))
                mwifiex_sdio_card_reset_work(save_adapter);
+       clear_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP, &iface_work_flags);
+       clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &iface_work_flags);
 }
----------

> 
> > > Instead, you should actually retain the invariant that you're doing
> > > a full remove/reinitialize here, which includes doing the *same*
> > > cancel_work_sync() here in mwifiex_recreate_adapter() as you would
> > > in any other remove().
> >
> > We are executing mwifiex_recreate_adapter() as a part of sdio_work().
> > Calling
> > cancel_work_sync() here would cause deadlock. The API is supposed to
> > waits until sdio_work() is finished.
> 
> You are correct. So using the _sync() version would be wrong.
> 
> > >
> > > IOW, kill the __mwifiex_sdio_remove() and just call
> > > mwifiex_sdio_remove() as you were.
> > >
> > > That also means that you can do the same per-adapter cleanup in the
> > > following patch as you do for PCIe.
> >
> > Currently as sdio_work recreates "card", the work structure can't be
> moved inside card structure.
> > Let me know your suggestions.
> 
> If you address the TODO in mwifiex_create_adapter() instead, you can
> get past this problem:
> 
>         /* TODO mmc_hw_reset does not require destroying and re-probing
> the
>          * whole adapter. Hence there was no need to for this rube-
> goldberg
>          * design to reload the fw from an external workqueue. If we
> don't
>          * destroy the adapter we could reload the fw from
>          * mwifiex_main_work_queue directly.
> 
> The "save_adapter" is an abomination that should be terminated swiftly,
> but it is perpetuated in part by the hacks noted in the TODO.
> 
> So I'd recommend addressing the TODO ASAP, but in the meantime, a hack
> like my suggestion (cancel the FW dump work w/o synchronizing) or --
> less preferably -- yours (manually set 'save_adapter' again) might be
> OK.
> 
> I think I've asked elsewhere but didn't receive an answer: why is
> SDIO's mwifiex_recreate_adapter() so much different from PCIe's
> mwifiex_do_flr()? It seems like the latter should be refactored to
> remove some of the PCIe-specific cruft from main.c and then reused for
> SDIO.

Our initial SDIO card reset implementation was based on MMC APIs where remove() and probe() would automatically get called by MMC subsystem after power cycle.
https://www.spinics.net/lists/linux-wireless/msg98435.html
Later it was improved by Andreas Fenkart by replacing those power cycle APIs with mmc_hw_reset().

For PCIe, function level reset is standard feature. We implemented ".reset_notify" handler which gets called after and before FLR.

You are right. We can have SDIO's handling similar to PCIe and avoid destroying+recreating adapter/card.
We have started working on this. We will get rid of global save_adapter, sdio_work etc. Meanwhile I will post above mentioned change if it looks good to you.

Regards,
Amitkumar

^ permalink raw reply

* [PATCH 2/2] net: rfkill: Add rfkill-any LED trigger
From: Michał Kępień @ 2016-11-30 12:03 UTC (permalink / raw)
  To: Johannes Berg, David S . Miller; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20161130120317.11851-1-kernel@kempniu.pl>

This patch adds a new "global" (i.e. not per-rfkill device) LED trigger,
rfkill-any, which may be useful for laptops with a single "radio LED"
and multiple radio transmitters.  The trigger is meant to turn a LED on
whenever there is at least one radio transmitter active and turn it off
otherwise.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
Note that the search for any active radio will have quadratic complexity
whenever __rfkill_switch_all() is used (as it calls rfkill_set_block()
for every affected rfkill device), but I intentionally refrained from
implementing rfkill_any_led_trigger_event() using struct work_struct to
keep things simple, given the average number of rfkill devices in
hardware these days.  Please let me know in case this should be
reworked.

 net/rfkill/core.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f28e441..5275f2f 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -176,6 +176,47 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill)
 {
 	led_trigger_unregister(&rfkill->led_trigger);
 }
+
+static struct led_trigger rfkill_any_led_trigger;
+
+static void __rfkill_any_led_trigger_event(void)
+{
+	enum led_brightness brightness = LED_OFF;
+	struct rfkill *rfkill;
+
+	list_for_each_entry(rfkill, &rfkill_list, node) {
+		if (!(rfkill->state & RFKILL_BLOCK_ANY)) {
+			brightness = LED_FULL;
+			break;
+		}
+	}
+
+	led_trigger_event(&rfkill_any_led_trigger, brightness);
+}
+
+static void rfkill_any_led_trigger_event(void)
+{
+	mutex_lock(&rfkill_global_mutex);
+	__rfkill_any_led_trigger_event();
+	mutex_unlock(&rfkill_global_mutex);
+}
+
+static void rfkill_any_led_trigger_activate(struct led_classdev *led_cdev)
+{
+	rfkill_any_led_trigger_event();
+}
+
+static int rfkill_any_led_trigger_register(void)
+{
+	rfkill_any_led_trigger.name = "rfkill-any";
+	rfkill_any_led_trigger.activate = rfkill_any_led_trigger_activate;
+	return led_trigger_register(&rfkill_any_led_trigger);
+}
+
+static void rfkill_any_led_trigger_unregister(void)
+{
+	led_trigger_unregister(&rfkill_any_led_trigger);
+}
 #else
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
@@ -189,6 +230,19 @@ static inline int rfkill_led_trigger_register(struct rfkill *rfkill)
 static inline void rfkill_led_trigger_unregister(struct rfkill *rfkill)
 {
 }
+
+static void rfkill_any_led_trigger_event(void)
+{
+}
+
+static int rfkill_any_led_trigger_register(void)
+{
+	return 0;
+}
+
+static void rfkill_any_led_trigger_unregister(void)
+{
+}
 #endif /* CONFIG_RFKILL_LEDS */
 
 static void rfkill_fill_event(struct rfkill_event *ev, struct rfkill *rfkill,
@@ -297,6 +351,7 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
 	spin_unlock_irqrestore(&rfkill->lock, flags);
 
 	rfkill_led_trigger_event(rfkill);
+	__rfkill_any_led_trigger_event();
 
 	if (prev != curr)
 		rfkill_event(rfkill);
@@ -477,6 +532,7 @@ bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
 	spin_unlock_irqrestore(&rfkill->lock, flags);
 
 	rfkill_led_trigger_event(rfkill);
+	rfkill_any_led_trigger_event();
 
 	if (!rfkill->registered)
 		return ret;
@@ -523,6 +579,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
 		schedule_work(&rfkill->uevent_work);
 
 	rfkill_led_trigger_event(rfkill);
+	rfkill_any_led_trigger_event();
 
 	return blocked;
 }
@@ -572,6 +629,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
 			schedule_work(&rfkill->uevent_work);
 
 		rfkill_led_trigger_event(rfkill);
+		rfkill_any_led_trigger_event();
 	}
 }
 EXPORT_SYMBOL(rfkill_set_states);
@@ -988,6 +1046,7 @@ int __must_check rfkill_register(struct rfkill *rfkill)
 #endif
 	}
 
+	__rfkill_any_led_trigger_event();
 	rfkill_send_events(rfkill, RFKILL_OP_ADD);
 
 	mutex_unlock(&rfkill_global_mutex);
@@ -1020,6 +1079,7 @@ void rfkill_unregister(struct rfkill *rfkill)
 	mutex_lock(&rfkill_global_mutex);
 	rfkill_send_events(rfkill, RFKILL_OP_DEL);
 	list_del_init(&rfkill->node);
+	__rfkill_any_led_trigger_event();
 	mutex_unlock(&rfkill_global_mutex);
 
 	rfkill_led_trigger_unregister(rfkill);
@@ -1278,8 +1338,18 @@ static int __init rfkill_init(void)
 		goto error_input;
 #endif
 
+#ifdef CONFIG_RFKILL_LEDS
+	error = rfkill_any_led_trigger_register();
+	if (error)
+		goto error_led_trigger;
+#endif
+
 	return 0;
 
+error_led_trigger:
+#ifdef CONFIG_RFKILL_INPUT
+	rfkill_handler_exit();
+#endif
 error_input:
 	misc_deregister(&rfkill_miscdev);
 error_misc:
@@ -1291,6 +1361,9 @@ subsys_initcall(rfkill_init);
 
 static void __exit rfkill_exit(void)
 {
+#ifdef CONFIG_RFKILL_LEDS
+	rfkill_any_led_trigger_unregister();
+#endif
 #ifdef CONFIG_RFKILL_INPUT
 	rfkill_handler_exit();
 #endif
-- 
2.10.2

^ permalink raw reply related

* [PATCH 1/2] net: rfkill: Cleanup error handling in rfkill_init()
From: Michał Kępień @ 2016-11-30 12:03 UTC (permalink / raw)
  To: Johannes Berg, David S . Miller; +Cc: linux-wireless, netdev, linux-kernel

Use a separate label per error condition in rfkill_init() to make it a
bit cleaner and easier to extend.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 net/rfkill/core.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 884027f..f28e441 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1266,24 +1266,25 @@ static int __init rfkill_init(void)
 
 	error = class_register(&rfkill_class);
 	if (error)
-		goto out;
+		goto error_class;
 
 	error = misc_register(&rfkill_miscdev);
-	if (error) {
-		class_unregister(&rfkill_class);
-		goto out;
-	}
+	if (error)
+		goto error_misc;
 
 #ifdef CONFIG_RFKILL_INPUT
 	error = rfkill_handler_init();
-	if (error) {
-		misc_deregister(&rfkill_miscdev);
-		class_unregister(&rfkill_class);
-		goto out;
-	}
+	if (error)
+		goto error_input;
 #endif
 
- out:
+	return 0;
+
+error_input:
+	misc_deregister(&rfkill_miscdev);
+error_misc:
+	class_unregister(&rfkill_class);
+error_class:
 	return error;
 }
 subsys_initcall(rfkill_init);
-- 
2.10.2

^ permalink raw reply related

* Re: Atheros ETSI DFS pattern
From: Zefir Kurtisi @ 2016-11-30 10:54 UTC (permalink / raw)
  To: voncken, linux-wireless
In-Reply-To: <012101d24af3$40bb97f0$c232c7d0$@acksys.fr>

On 11/30/2016 11:19 AM, voncken wrote:
> In the file drivers/net/wireless/ath/dfs_pattern_detector.c we can found the
> ETSI pattern definition.
> 
> In this definition we can found 7 patterns (0 to 6).
> 
> The ETSI standard EN 301893 Annex D only specifies 6 patterns. Only the
> pattern 1 to 6 defined in the ath dfs pattern match with the ETSI standard.
> 
> Why the pattern 0 is defined?
> 
> Is it possible to remove this pattern and keep compliant with the ETSI
> standard?
> 
> Thanks for your help.
> 
> Cedric.
> 
Hi,

pattern 0 is the 'reference DFS test signal', the 700Hz pattern with 18 pulses
each 1us wide.

At implementation time, EN301893 v1.5 was the current revision and the reference
pattern is defined in Table D.3. I double checked that v1.8.1 has this one still
defined, so it remains required.

Given that most basic DFS features in certification labs are tested with this
pattern, I don't expect it to be obsoleted any time.

Out of curiosity: why would you like it removed? From the false-detection
probability, pattern 0 is uncritical (the tricky one is pattern 1).


Cheers,
Zefir

^ permalink raw reply

* Atheros ETSI DFS pattern
From: voncken @ 2016-11-30 10:19 UTC (permalink / raw)
  To: linux-wireless

In the file drivers/net/wireless/ath/dfs_pattern_detector.c we can found the
ETSI pattern definition.

In this definition we can found 7 patterns (0 to 6).

The ETSI standard EN 301893 Annex D only specifies 6 patterns. Only the
pattern 1 to 6 defined in the ath dfs pattern match with the ETSI standard.

Why the pattern 0 is defined?

Is it possible to remove this pattern and keep compliant with the ETSI
standard?

Thanks for your help.

Cedric.

^ permalink raw reply

* Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting
From: Kalle Valo @ 2016-11-30  8:46 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-wireless, briselec
In-Reply-To: <20161129235902.11882-1-Jes.Sorensen@redhat.com>

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Hi,
>
> This patch works around the issue with 8192eu and 8723bu devices not
> being able to reconnect. It is still not clear why this fails, and
> looking at rtlwifi it does issue the command for 8192ee and 8723be
> devices.
>
> Until we have a better idea of what is going on, I have commented out
> the offending code. I prefer not deleting it until we have a better
> understanding of why this is causing issues.
>
> Credits to Barry Day for tracking down and isolating the issue.
>
> Kalle, not sure if this can make it to 4.9, but if it's possible it
> would be ideal since this is causing real issues in the field.

For 4.9 this is too late, sorry. I'll queue this to 4.10.

-- 
Kalle Valo

^ permalink raw reply

* [PATCH] ath10k: Fix Tx DMA alloc failure during continuous wifi down/up
From: Mohammed Shafi Shajakhan @ 2016-11-30  9:50 UTC (permalink / raw)
  To: ath10k; +Cc: mohammed, linux-wireless, Mohammed Shafi Shajakhan

From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>

With maximum number of vap's configured in a two radio supported
systems of ~256 Mb RAM, doing a continuous wifi down/up and
intermittent traffic streaming from the connected stations results
in failure to allocate contiguous memory for tx buffers. This results
in the disappearance of all VAP's and a manual reboot is needed as
this is not a crash (or) OOM(for OOM killer to be invoked). To address
this allocate contiguous memory for tx buffers one time and re-use them
until the modules are unloaded but this results in a slight increase in
memory footprint of ath10k when the wifi is down, but the modules are
still loaded. Also as of now we use a separate bool 'tx_mem_allocated'
to keep track of the one time memory allocation, as we cannot come up
with something like 'ath10k_tx_{register,unregister}' before
'ath10k_probe_fw' is called as 'ath10k_htt_tx_alloc_cont_frag_desc'
memory allocation is dependent on the hw_param 'continuous_frag_desc'

a) memory footprint of ath10k without the change

lsmod | grep ath10k
ath10k_core           414498  1 ath10k_pci
ath10k_pci             38236  0

b) memory footprint of ath10k with the change

ath10k_core           414980  1 ath10k_pci
ath10k_pci             38236  0

Memory Failure Call trace:

hostapd: page allocation failure: order:6, mode:0xd0
 [<c021f150>] (__dma_alloc_buffer.isra.23) from
[<c021f23c>] (__alloc_remap_buffer.isra.26+0x14/0xb8)
[<c021f23c>] (__alloc_remap_buffer.isra.26) from
[<c021f664>] (__dma_alloc+0x224/0x2b8)
[<c021f664>] (__dma_alloc) from [<c021f810>]
(arm_dma_alloc+0x84/0x90)
[<c021f810>] (arm_dma_alloc) from [<bf954764>]
(ath10k_htt_tx_alloc+0xe0/0x2e4 [ath10k_core])
[<bf954764>] (ath10k_htt_tx_alloc [ath10k_core]) from
[<bf94e6ac>] (ath10k_core_start+0x538/0xcf8 [ath10k_core])
[<bf94e6ac>] (ath10k_core_start [ath10k_core]) from
[<bf947eec>] (ath10k_start+0xbc/0x56c [ath10k_core])
[<bf947eec>] (ath10k_start [ath10k_core]) from
[<bf8a7a04>] (drv_start+0x40/0x5c [mac80211])
[<bf8a7a04>] (drv_start [mac80211]) from [<bf8b7cf8>]
(ieee80211_do_open+0x170/0x82c [mac80211])
[<bf8b7cf8>] (ieee80211_do_open [mac80211]) from
[<c056afc8>] (__dev_open+0xa0/0xf4)
[21053.491752] Normal: 641*4kB (UEMR) 505*8kB (UEMR) 330*16kB (UEMR)
126*32kB (UEMR) 762*64kB (UEMR) 237*128kB (UEMR) 1*256kB (M) 0*512kB
0*1024kB 0*2048kB 0*4096kB = 95276kB

Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.c   |  5 +--
 drivers/net/wireless/ath/ath10k/htt.h    |  6 +++-
 drivers/net/wireless/ath/ath10k/htt_tx.c | 54 +++++++++++++++++++++++++-------
 3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5bc6847..f7ea4de 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1857,7 +1857,7 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		goto err_wmi_detach;
 	}
 
-	status = ath10k_htt_tx_alloc(&ar->htt);
+	status = ath10k_htt_tx_start(&ar->htt);
 	if (status) {
 		ath10k_err(ar, "failed to alloc htt tx: %d\n", status);
 		goto err_wmi_detach;
@@ -2052,7 +2052,7 @@ void ath10k_core_stop(struct ath10k *ar)
 		ath10k_wait_for_suspend(ar, WMI_PDEV_SUSPEND_AND_DISABLE_INTR);
 
 	ath10k_hif_stop(ar);
-	ath10k_htt_tx_free(&ar->htt);
+	ath10k_htt_tx_stop(&ar->htt);
 	ath10k_htt_rx_free(&ar->htt);
 	ath10k_wmi_detach(ar);
 }
@@ -2385,6 +2385,7 @@ void ath10k_core_destroy(struct ath10k *ar)
 	destroy_workqueue(ar->workqueue_aux);
 
 	ath10k_debug_destroy(ar);
+	ath10k_htt_tx_destroy(&ar->htt);
 	ath10k_wmi_free_host_mem(ar);
 	ath10k_mac_destroy(ar);
 }
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 0d2ed09..96bf7bf 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1692,6 +1692,8 @@ struct ath10k_htt {
 		enum htt_tx_mode_switch_mode mode;
 		enum htt_q_depth_type type;
 	} tx_q_state;
+
+	bool tx_mem_allocated;
 };
 
 #define RX_HTT_HDR_STATUS_LEN 64
@@ -1754,7 +1756,9 @@ struct htt_rx_desc {
 int ath10k_htt_init(struct ath10k *ar);
 int ath10k_htt_setup(struct ath10k_htt *htt);
 
-int ath10k_htt_tx_alloc(struct ath10k_htt *htt);
+int ath10k_htt_tx_start(struct ath10k_htt *htt);
+void ath10k_htt_tx_stop(struct ath10k_htt *htt);
+void ath10k_htt_tx_destroy(struct ath10k_htt *htt);
 void ath10k_htt_tx_free(struct ath10k_htt *htt);
 
 int ath10k_htt_rx_alloc(struct ath10k_htt *htt);
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index ccbc8c03..27e49db 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -350,21 +350,15 @@ static int ath10k_htt_tx_alloc_txdone_fifo(struct ath10k_htt *htt)
 	return ret;
 }
 
-int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
+static int ath10k_htt_tx_alloc_buf(struct ath10k_htt *htt)
 {
 	struct ath10k *ar = htt->ar;
 	int ret;
 
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt tx max num pending tx %d\n",
-		   htt->max_num_pending_tx);
-
-	spin_lock_init(&htt->tx_lock);
-	idr_init(&htt->pending_tx);
-
 	ret = ath10k_htt_tx_alloc_cont_txbuf(htt);
 	if (ret) {
 		ath10k_err(ar, "failed to alloc cont tx buffer: %d\n", ret);
-		goto free_idr_pending_tx;
+		return ret;
 	}
 
 	ret = ath10k_htt_tx_alloc_cont_frag_desc(htt);
@@ -396,6 +390,31 @@ int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
 free_txbuf:
 	ath10k_htt_tx_free_cont_txbuf(htt);
 
+	return ret;
+}
+
+int ath10k_htt_tx_start(struct ath10k_htt *htt)
+{
+	struct ath10k *ar = htt->ar;
+	int ret;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt tx max num pending tx %d\n",
+		   htt->max_num_pending_tx);
+
+	spin_lock_init(&htt->tx_lock);
+	idr_init(&htt->pending_tx);
+
+	if (htt->tx_mem_allocated)
+		return 0;
+
+	ret = ath10k_htt_tx_alloc_buf(htt);
+	if (ret)
+		goto free_idr_pending_tx;
+
+	htt->tx_mem_allocated = true;
+
+	return 0;
+
 free_idr_pending_tx:
 	idr_destroy(&htt->pending_tx);
 
@@ -418,15 +437,28 @@ static int ath10k_htt_tx_clean_up_pending(int msdu_id, void *skb, void *ctx)
 	return 0;
 }
 
-void ath10k_htt_tx_free(struct ath10k_htt *htt)
+void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
 {
-	idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
-	idr_destroy(&htt->pending_tx);
+	if (!htt->tx_mem_allocated)
+		return;
 
 	ath10k_htt_tx_free_cont_txbuf(htt);
 	ath10k_htt_tx_free_txq(htt);
 	ath10k_htt_tx_free_cont_frag_desc(htt);
 	ath10k_htt_tx_free_txdone_fifo(htt);
+	htt->tx_mem_allocated = false;
+}
+
+void ath10k_htt_tx_stop(struct ath10k_htt *htt)
+{
+	idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
+	idr_destroy(&htt->pending_tx);
+}
+
+void ath10k_htt_tx_free(struct ath10k_htt *htt)
+{
+	ath10k_htt_tx_stop(htt);
+	ath10k_htt_tx_destroy(htt);
 }
 
 void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox