* Re: [ath9k-devel] ath: Unable to remove station entry
From: Michael Tokarev @ 2013-07-06 6:47 UTC (permalink / raw)
To: Oleksij Rempel; +Cc: linux-wireless, ath9k-devel
In-Reply-To: <51D7ABC6.40307@rempel-privat.de>
06.07.2013 09:31, Oleksij Rempel wrote:
> Hi Michael,
Hello!
> there is no way to avoid latest wireless-testing kernel. I had similar problems for some kernel version. But now it seems to work more or less ok. For example it will nuke complete system only by triggering some bug in xhcd driver :)
Hmm. Are you saying that no one cares about -stable, and
bugs are only fixed in latest prereleases? If that's true,
it is very unfortunate situation for the wireless subsystem...
Thanks,
/mjt
> Am 05.07.2013 11:01, schrieb Michael Tokarev:
>> Hello.
>>
>> Recently I bought a TP-Link TL-WN821N v3 802.11n USB adaptor,
>> and tried to use it as an access point for a small Wireless LAN.
>>
>> It works fine so far, except of one issue.
>>
>> Quite often to be really annoying, it stops working with the
>> following message in kernel log:
>>
>> Jul 5 09:51:26 gnome vmunix: [133814.449408] ath: Unable to remove station entry for: 38:aa:3c:02:07:f1
>>
>> after this, the interface is stuck, it can't be seen over WIFI,
>> and any attempt to do anything with it inside the host results
>> in more processes entering D state (initially right when this
>> happens, there's a kworker process in D state).
>>
>> For example, `rmmod ath9k_htc' - which appears to be a topmost
>> module on the stack - results in rmmod entering D state, with
>> the following stack:
>>
>> rmmod D 000000010266c10c 0 10684 10643 0x00000000
>> ffffffff8148b020 0000000000000082 0000000000012400 ffff88012ae9d7d0
>> ffff880114843fd8 ffff880114843fd8 ffff880114843fd8 ffff88012ae9d7d0
>> 0000000125aa5040 ffffffff8148b020 0000000000012400 ffff8801298cb500
>> Call Trace:
>> [<ffffffff81367a59>] ? __schedule+0x3a9/0x960
>> [<ffffffff810558f0>] ? usleep_range+0x40/0x40
>> [<ffffffff81368e38>] ? __mutex_lock_slowpath+0xc8/0x140
>> [<ffffffff813689ba>] ? mutex_lock+0x1a/0x40
>> [<ffffffffa025bc66>] ? ath9k_wmi_cmd+0xc6/0x200 [ath9k_htc]
>> [<ffffffffa02614d8>] ? ath9k_regread+0x38/0x50 [ath9k_htc]
>> [<ffffffffa0076849>] ? ath_hw_keyreset+0x59/0x220 [ath]
>> [<ffffffffa0076a2d>] ? ath_key_delete+0x1d/0xdc [ath]
>> [<ffffffffa025edc5>] ? ath9k_htc_set_key+0x85/0x130 [ath9k_htc]
>> [<ffffffffa0230d19>] ? ieee80211_key_disable_hw_accel+0x89/0x130 [mac80211]
>> [<ffffffffa0230ddc>] ? __ieee80211_key_destroy+0x1c/0x80 [mac80211]
>> [<ffffffffa0231505>] ? ieee80211_free_keys+0x45/0x80 [mac80211]
>> [<ffffffffa02233a7>] ? ieee80211_do_stop+0x1f7/0x5c0 [mac80211]
>> [<ffffffff812d9f40>] ? dev_deactivate_many+0x1f0/0x240
>> [<ffffffffa0223785>] ? ieee80211_stop+0x15/0x20 [mac80211]
>> [<ffffffff812bc505>] ? __dev_close_many+0x85/0xd0
>> [<ffffffff812bc638>] ? dev_close_many+0x98/0x110
>> [<ffffffff812bc788>] ? rollback_registered_many+0xd8/0x250
>> [<ffffffff812bc90e>] ? unregister_netdevice_many+0xe/0x60
>> [<ffffffffa0222f00>] ? ieee80211_remove_interfaces+0xc0/0x100 [mac80211]
>> [<ffffffffa0211096>] ? ieee80211_unregister_hw+0x46/0x110 [mac80211]
>> [<ffffffffa02624c4>] ? ath9k_htc_disconnect_device+0x54/0xd0 [ath9k_htc]
>> [<ffffffffa025b3a2>] ? ath9k_hif_usb_disconnect+0x52/0x150 [ath9k_htc]
>> [<ffffffffa005dd52>] ? usb_unbind_interface+0x42/0x150 [usbcore]
>> [<ffffffff81274546>] ? __device_release_driver+0x76/0xe0
>> [<ffffffff81274bf0>] ? driver_detach+0xa0/0xb0
>> [<ffffffff812743d0>] ? bus_remove_driver+0x70/0xc0
>> [<ffffffffa005d7a6>] ? usb_deregister+0xa6/0xc0 [usbcore]
>> [<ffffffffa0262d36>] ? ath9k_htc_exit+0x6/0x16 [ath9k_htc]
>> [<ffffffff81080ee2>] ? sys_delete_module+0x132/0x260
>> [<ffffffff8136a245>] ? page_fault+0x25/0x30
>> [<ffffffff81371552>] ? system_call_fastpath+0x16/0x1b
>>
>> followed by:
>>
>> Jul 5 10:02:27 gnome vmunix: [134474.473451] ath: Unable to remove interface at idx: 0
>>
>> (rmmod is stuck forever).
>>
>> Now, in order to make the interface to work again, the only way I
>> found so far is to _reboot_ the machine. For example, re-plugging
>> the USB cord does not help, because, as far as I can see, the driver
>> is in some weird state and can't initialize the new interface.
>>
>> This is a 3.2.0-stable kernel (right now 3.2.46), x86-64 (amd64),
>> self-compiled, without additional patches.
>>
>>
>> There are a few references to this message on the 'net, including
>> one mentioning this very card (in russian) --
>>
>> http://forums.opensuse.org/p-russian/dhydhdhdhdhundhdhdh/1046-1077-1083-1077-1079-1086/469022-wifi-usb-tp-link-tl-wn821n.html
>>
>> they claim the problem has been fixed for _some_ by upgrading the
>> BIOS on the motherboard. Maybe this is actually related, because
>> as far as I can tell, this started happening _after_ I upgraded
>> BIOS on my motherboard, so it may be related to the bios changes.
>> I don't recall whenever I noticed this erratic behavour before I
>> upgraded BIOS. Looking at the BIOS history, I don't see anything
>> interesting about USB in the changelog, except this:
>>
>> * Fixed issue with Fast Boot so USB devices still work under DOS
>> if USB Optimization is enabled.
>>
>> This is an intel atom-based D2500CC board, with the latest BIOS.
>> I had to update bios because of another issue which is now fixed,
>> but I can't go back to the old bios because the old one was too
>> old and current motherboard refuses to flash it.
>>
>>
>> What can be done to diagnose the problem? I can give a more recent
>> kernel a try, but I'd love to see it fixed for a -stable kernel which
>> is used by several major distributions. Also, the problem is not
>> easy to trigger, the system may work for a few days without issues
>> or may stop working in a few hours, irrespective of the load (f.e.
>> the above example at 09:51 was me awakening my android phone just
>> to see what time it is now, and it trying to connect/disconnect
>> to/from the default wifi network -- there were no other devices
>> using wifi at that time).
>>
>> Thanks,
>>
>> /mjt
>> _______________________________________________
>> ath9k-devel mailing list
>> ath9k-devel@lists.ath9k.org
>> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>>
>
>
^ permalink raw reply
* Re: [ath9k-devel] ath: Unable to remove station entry
From: Oleksij Rempel @ 2013-07-06 5:31 UTC (permalink / raw)
To: Michael Tokarev; +Cc: linux-wireless, ath9k-devel
In-Reply-To: <51D68B55.3070309@msgid.tls.msk.ru>
Hi Michael,
there is no way to avoid latest wireless-testing kernel. I had similar
problems for some kernel version. But now it seems to work more or less
ok. For example it will nuke complete system only by triggering some bug
in xhcd driver :)
Am 05.07.2013 11:01, schrieb Michael Tokarev:
> Hello.
>
> Recently I bought a TP-Link TL-WN821N v3 802.11n USB adaptor,
> and tried to use it as an access point for a small Wireless LAN.
>
> It works fine so far, except of one issue.
>
> Quite often to be really annoying, it stops working with the
> following message in kernel log:
>
> Jul 5 09:51:26 gnome vmunix: [133814.449408] ath: Unable to remove station entry for: 38:aa:3c:02:07:f1
>
> after this, the interface is stuck, it can't be seen over WIFI,
> and any attempt to do anything with it inside the host results
> in more processes entering D state (initially right when this
> happens, there's a kworker process in D state).
>
> For example, `rmmod ath9k_htc' - which appears to be a topmost
> module on the stack - results in rmmod entering D state, with
> the following stack:
>
> rmmod D 000000010266c10c 0 10684 10643 0x00000000
> ffffffff8148b020 0000000000000082 0000000000012400 ffff88012ae9d7d0
> ffff880114843fd8 ffff880114843fd8 ffff880114843fd8 ffff88012ae9d7d0
> 0000000125aa5040 ffffffff8148b020 0000000000012400 ffff8801298cb500
> Call Trace:
> [<ffffffff81367a59>] ? __schedule+0x3a9/0x960
> [<ffffffff810558f0>] ? usleep_range+0x40/0x40
> [<ffffffff81368e38>] ? __mutex_lock_slowpath+0xc8/0x140
> [<ffffffff813689ba>] ? mutex_lock+0x1a/0x40
> [<ffffffffa025bc66>] ? ath9k_wmi_cmd+0xc6/0x200 [ath9k_htc]
> [<ffffffffa02614d8>] ? ath9k_regread+0x38/0x50 [ath9k_htc]
> [<ffffffffa0076849>] ? ath_hw_keyreset+0x59/0x220 [ath]
> [<ffffffffa0076a2d>] ? ath_key_delete+0x1d/0xdc [ath]
> [<ffffffffa025edc5>] ? ath9k_htc_set_key+0x85/0x130 [ath9k_htc]
> [<ffffffffa0230d19>] ? ieee80211_key_disable_hw_accel+0x89/0x130 [mac80211]
> [<ffffffffa0230ddc>] ? __ieee80211_key_destroy+0x1c/0x80 [mac80211]
> [<ffffffffa0231505>] ? ieee80211_free_keys+0x45/0x80 [mac80211]
> [<ffffffffa02233a7>] ? ieee80211_do_stop+0x1f7/0x5c0 [mac80211]
> [<ffffffff812d9f40>] ? dev_deactivate_many+0x1f0/0x240
> [<ffffffffa0223785>] ? ieee80211_stop+0x15/0x20 [mac80211]
> [<ffffffff812bc505>] ? __dev_close_many+0x85/0xd0
> [<ffffffff812bc638>] ? dev_close_many+0x98/0x110
> [<ffffffff812bc788>] ? rollback_registered_many+0xd8/0x250
> [<ffffffff812bc90e>] ? unregister_netdevice_many+0xe/0x60
> [<ffffffffa0222f00>] ? ieee80211_remove_interfaces+0xc0/0x100 [mac80211]
> [<ffffffffa0211096>] ? ieee80211_unregister_hw+0x46/0x110 [mac80211]
> [<ffffffffa02624c4>] ? ath9k_htc_disconnect_device+0x54/0xd0 [ath9k_htc]
> [<ffffffffa025b3a2>] ? ath9k_hif_usb_disconnect+0x52/0x150 [ath9k_htc]
> [<ffffffffa005dd52>] ? usb_unbind_interface+0x42/0x150 [usbcore]
> [<ffffffff81274546>] ? __device_release_driver+0x76/0xe0
> [<ffffffff81274bf0>] ? driver_detach+0xa0/0xb0
> [<ffffffff812743d0>] ? bus_remove_driver+0x70/0xc0
> [<ffffffffa005d7a6>] ? usb_deregister+0xa6/0xc0 [usbcore]
> [<ffffffffa0262d36>] ? ath9k_htc_exit+0x6/0x16 [ath9k_htc]
> [<ffffffff81080ee2>] ? sys_delete_module+0x132/0x260
> [<ffffffff8136a245>] ? page_fault+0x25/0x30
> [<ffffffff81371552>] ? system_call_fastpath+0x16/0x1b
>
> followed by:
>
> Jul 5 10:02:27 gnome vmunix: [134474.473451] ath: Unable to remove interface at idx: 0
>
> (rmmod is stuck forever).
>
> Now, in order to make the interface to work again, the only way I
> found so far is to _reboot_ the machine. For example, re-plugging
> the USB cord does not help, because, as far as I can see, the driver
> is in some weird state and can't initialize the new interface.
>
> This is a 3.2.0-stable kernel (right now 3.2.46), x86-64 (amd64),
> self-compiled, without additional patches.
>
>
> There are a few references to this message on the 'net, including
> one mentioning this very card (in russian) --
>
> http://forums.opensuse.org/p-russian/dhydhdhdhdhundhdhdh/1046-1077-1083-1077-1079-1086/469022-wifi-usb-tp-link-tl-wn821n.html
>
> they claim the problem has been fixed for _some_ by upgrading the
> BIOS on the motherboard. Maybe this is actually related, because
> as far as I can tell, this started happening _after_ I upgraded
> BIOS on my motherboard, so it may be related to the bios changes.
> I don't recall whenever I noticed this erratic behavour before I
> upgraded BIOS. Looking at the BIOS history, I don't see anything
> interesting about USB in the changelog, except this:
>
> * Fixed issue with Fast Boot so USB devices still work under DOS
> if USB Optimization is enabled.
>
> This is an intel atom-based D2500CC board, with the latest BIOS.
> I had to update bios because of another issue which is now fixed,
> but I can't go back to the old bios because the old one was too
> old and current motherboard refuses to flash it.
>
>
> What can be done to diagnose the problem? I can give a more recent
> kernel a try, but I'd love to see it fixed for a -stable kernel which
> is used by several major distributions. Also, the problem is not
> easy to trigger, the system may work for a few days without issues
> or may stop working in a few hours, irrespective of the load (f.e.
> the above example at 09:51 was me awakening my android phone just
> to see what time it is now, and it trying to connect/disconnect
> to/from the default wifi network -- there were no other devices
> using wifi at that time).
>
> Thanks,
>
> /mjt
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel@lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>
--
Regards,
Oleksij
^ permalink raw reply
* Re: Passing a custom MAC to ath6kl
From: Kalle Valo @ 2013-07-05 20:23 UTC (permalink / raw)
To: Arend van Spriel
Cc: Hector Palacios, linux-wireless@vger.kernel.org,
ath6kl-devel@qca.qualcomm.com
In-Reply-To: <51D70A22.20607@broadcom.com>
Arend van Spriel <arend@broadcom.com> writes:
> On 07/05/2013 05:56 PM, Hector Palacios wrote:
>> Dear Kalle,
>>
>> On 07/05/2013 05:49 PM, Kalle Valo wrote:
>>> Hector Palacios <hector.palacios@digi.com> writes:
>>>
>>>> Is there a way to pass a custom MAC address to the ath6kl upstream
>>>> driver?
>>>
>>> Not right now. It has been a common feature request so I'm hoping
>>> someone will implement it soon.
>>>
>>>> In the ISC version of the driver, a softmac.c file is provided that
>>>> reads a MAC from a file if a module parameter softmac_enable=1 is
>>>> passed. Is there any reason why this has not made it upstream?
>>>
>>> Providing mac address through a file is too ugly for upstream. We need
>>> to implement it properly using the proper interface.
>>
>> I totally agree. What would be the proper interface for that?
>
> Add .ndo_set_mac_address() callback in main.c and go from there.
>
> int (*ndo_set_mac_address)(struct net_device *dev, void *addr);
And the challenge with ath6kl is that we need to stop the firmware,
change mac address in the firmware and restart the firmware. So it's not
trivial to add that but it shouldn't be too difficult either. Patches
more than welcome :)
--
Kalle Valo
^ permalink raw reply
* Re: Passing a custom MAC to ath6kl
From: Arend van Spriel @ 2013-07-05 18:02 UTC (permalink / raw)
To: Hector Palacios
Cc: Kalle Valo, linux-wireless@vger.kernel.org,
ath6kl-devel@qca.qualcomm.com
In-Reply-To: <51D6EC90.40009@digi.com>
On 07/05/2013 05:56 PM, Hector Palacios wrote:
> Dear Kalle,
>
> On 07/05/2013 05:49 PM, Kalle Valo wrote:
>> Hector Palacios <hector.palacios@digi.com> writes:
>>
>>> Is there a way to pass a custom MAC address to the ath6kl upstream
>>> driver?
>>
>> Not right now. It has been a common feature request so I'm hoping
>> someone will implement it soon.
>>
>>> In the ISC version of the driver, a softmac.c file is provided that
>>> reads a MAC from a file if a module parameter softmac_enable=1 is
>>> passed. Is there any reason why this has not made it upstream?
>>
>> Providing mac address through a file is too ugly for upstream. We need
>> to implement it properly using the proper interface.
>
> I totally agree. What would be the proper interface for that?
Add .ndo_set_mac_address() callback in main.c and go from there.
int (*ndo_set_mac_address)(struct net_device *dev, void *addr);
Regards,
Arend
> Best regards,
> --
> Hector Palacios
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* RE: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Amitkumar Karwar @ 2013-07-05 17:05 UTC (permalink / raw)
To: 'Daniel Drake'
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <CAMLZHHSZvKWnaBXycNsh64oExhpa0amcUi-6eC-FOaeNZ-TP1A@mail.gmail.com>
Hi Daniel,
>There is no point in the one case that you mention. But there is a
>very good reason to do so in the case that I am talking about. So why
>not do it? There is no harm to do it if the hardware is not present,
>right?
I got your point. We didn't test the behavior of sdio_read/sdio_write API's when hardware is not present. But they should just return an error.
>This doesn't make much sense to me either. Why don't we just disable
>the interrupt handler at the point when interrupts can no longer be
>correctly handled by the driver?
Agreed. Following change will fix the issue. Right? Instead of going for your earlier changes to ack interrupts until we disable them.
@@ -152,6 +152,9 @@ mwifiex_sdio_remove(struct sdio_func *func)
mwifiex_init_shutdown_fw(priv, MWIFIEX_FUNC_SHUTDOWN);
}
+ /* Disable host interrupt mask register for SDIO */
+ mwifiex_sdio_disable_host_int(adapter);
+
mwifiex_remove_card(card->adapter, &add_remove_card_sem);
kfree(card);
}
Thanks,
Amitkumar Karwar
^ permalink raw reply
* Re: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Daniel Drake @ 2013-07-05 16:48 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <5FF020A1CFFEEC49BD1E09530C4FF5951035636353@SC-VEXCH1.marvell.com>
On Fri, Jul 5, 2013 at 10:43 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
>
> Hi Daniel,
>
>>I don't see any external reason why we are not allowed to communicate to
>>the hardware here. The other suspend paths do run some communication.
>
> The reason is mwifiex_sdio_remove() gets called even if user removes an externally connected wifi card. In that case the hardware is not present so there is no point in reading/writing hardware registers.
There is no point in the one case that you mention. But there is a
very good reason to do so in the case that I am talking about. So why
not do it? There is no harm to do it if the hardware is not present,
right?
> This approach looks correct to me. While returning failure in suspend handler, 'pm_flag & MMC_PM_KEEP_POWER' is false. Card is going to be powered off soon. There won't be any further communication with card, so disabling interrupts at this point of time is logical.
> When system is resumed, bus rescan etc. will happen and card will be redetected.
This doesn't make much sense to me either. Why don't we just disable
the interrupt handler at the point when interrupts can no longer be
correctly handled by the driver?
Daniel
^ permalink raw reply
* RE: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Amitkumar Karwar @ 2013-07-05 16:43 UTC (permalink / raw)
To: 'Daniel Drake'
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <CAMLZHHQuHAZGW0M0qhQi5PoCW78jgF8rw2ZsmbApT2REBRXZtA@mail.gmail.com>
Hi Daniel,
>I don't see any external reason why we are not allowed to communicate to
>the hardware here. The other suspend paths do run some communication.
The reason is mwifiex_sdio_remove() gets called even if user removes an externally connected wifi card. In that case the hardware is not present so there is no point in reading/writing hardware registers.
If mwifiex_sdio_remove() is called when card is not powered off or unplugged, we do take care of hardware de-initialization using user_rmmod flag. Interrupt storm issue doesn't exist there.
>However, disabling interrupts at this point does seem like a bad
>solution to me, and something that would come back and bite us in the
>future. mwifiex_sdio_suspend() is not directly related to the cause of
>the problem.
This approach looks correct to me. While returning failure in suspend handler, 'pm_flag & MMC_PM_KEEP_POWER' is false. Card is going to be powered off soon. There won't be any further communication with card, so disabling interrupts at this point of time is logical.
When system is resumed, bus rescan etc. will happen and card will be redetected.
Can you please try this change?
Thanks and regards,
Amitkumar Karwar
^ permalink raw reply
* Re: Passing a custom MAC to ath6kl
From: Hector Palacios @ 2013-07-05 15:56 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless@vger.kernel.org, ath6kl-devel@qca.qualcomm.com
In-Reply-To: <87vc4pf1ln.fsf@kamboji.qca.qualcomm.com>
Dear Kalle,
On 07/05/2013 05:49 PM, Kalle Valo wrote:
> Hector Palacios <hector.palacios@digi.com> writes:
>
>> Is there a way to pass a custom MAC address to the ath6kl upstream driver?
>
> Not right now. It has been a common feature request so I'm hoping
> someone will implement it soon.
>
>> In the ISC version of the driver, a softmac.c file is provided that
>> reads a MAC from a file if a module parameter softmac_enable=1 is
>> passed. Is there any reason why this has not made it upstream?
>
> Providing mac address through a file is too ugly for upstream. We need
> to implement it properly using the proper interface.
I totally agree. What would be the proper interface for that?
Best regards,
--
Hector Palacios
^ permalink raw reply
* Re: Passing a custom MAC to ath6kl
From: Kalle Valo @ 2013-07-05 15:49 UTC (permalink / raw)
To: Hector Palacios; +Cc: linux-wireless, ath6kl-devel
In-Reply-To: <51D6E484.9060000@digi.com>
Hector Palacios <hector.palacios@digi.com> writes:
> Is there a way to pass a custom MAC address to the ath6kl upstream driver?
Not right now. It has been a common feature request so I'm hoping
someone will implement it soon.
> In the ISC version of the driver, a softmac.c file is provided that
> reads a MAC from a file if a module parameter softmac_enable=1 is
> passed. Is there any reason why this has not made it upstream?
Providing mac address through a file is too ugly for upstream. We need
to implement it properly using the proper interface.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Daniel Drake @ 2013-07-05 15:37 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <5FF020A1CFFEEC49BD1E09530C4FF5951035636352@SC-VEXCH1.marvell.com>
On Fri, Jul 5, 2013 at 9:24 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
> In your case, as card is powering off, is it ok to access hardware registers to ACK interrupts? If yes, we can disable interrupts before returning "-ENOSYS" in suspend handler.
You tell me - you know the hardware and the driver better than I do. I
don't see any external reason why we are not allowed to communicate to
the hardware here. The other suspend paths do run some communication.
However, disabling interrupts at this point does seem like a bad
solution to me, and something that would come back and bite us in the
future. mwifiex_sdio_suspend() is not directly related to the cause of
the problem.
The problem that we should focus on is that mwifiex_remove_card()
makes the driver enter a state where if an interrupt arrives, it will
be handled badly. Therefore it should be mwifiex_remove_card() or
something directly related to it (maybe the callsite,
mwifiex_sdio_remove) that takes the necessary steps to make sure that
interrupts will not arrive in future.
> Modifying mwifiex_sdio_remove() to fix interrupt storm issue would break card unplugged scenario.
Why would it break that scenario?
Daniel
^ permalink raw reply
* RE: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Amitkumar Karwar @ 2013-07-05 15:24 UTC (permalink / raw)
To: 'Daniel Drake'
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <CAMLZHHSduu3guHuw__XKZfhrFWc8r7W6DwLp3SMdtLQ56CRh6Q@mail.gmail.com>
Hi Daniel,
>mwifiex_sdio_remove() calls mwifiex_remove_card() without having taken
>any care to finish pending commands, etc. mwifiex_remove_card()
>immediately sets surprise_removed which triggers the questionable
>"ignore all interrupts" behaviour. If there were any async commands
>pending, they complete now, with an interrupt. The interrupt doesn't
>get acked, so it becomes an interrupt storm.
>Hope that is clearer.
Thanks for the clarification.
As per our current design, mwifiex_sdio_remove() will be called in following scenarios.
1) Card is powered off / card is unplugged
As we are not allowed to interact with hardware (read/disable interrupts), other driver specific cleanup work is performed.
2) Driver is unloaded
Here apart from cleanup work in case 1, we do send SHUTDOWN etc. command to firmware to perform hardware cleanup(this code is under if(user_rmmod) check).
In your case, as card is powering off, is it ok to access hardware registers to ACK interrupts? If yes, we can disable interrupts before returning "-ENOSYS" in suspend handler.
Modifying mwifiex_sdio_remove() to fix interrupt storm issue would break card unplugged scenario.
Please let me know your thoughts.
Thanks,
Amit
^ permalink raw reply
* Passing a custom MAC to ath6kl
From: Hector Palacios @ 2013-07-05 15:21 UTC (permalink / raw)
To: linux-wireless; +Cc: kvalo, ath6kl-devel
Greetings,
Is there a way to pass a custom MAC address to the ath6kl upstream driver?
In the ISC version of the driver, a softmac.c file is provided that reads a MAC from a
file if a module parameter softmac_enable=1 is passed.
Is there any reason why this has not made it upstream?
Best regards,
--
Hector Palacios
^ permalink raw reply
* [PATCH] ath10k: remove unneeded semicolon
From: Kalle Valo @ 2013-07-05 14:46 UTC (permalink / raw)
To: ath9k-devel; +Cc: fengguang.wu, linux-wireless
Coccinelly found:
drivers/net/wireless/ath/ath10k/htt_rx.c:1163:2-3: Unneeded semicolon
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index de058d7..ad28866 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1160,7 +1160,7 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt event: ",
skb->data, skb->len);
break;
- };
+ }
/* Free the indication buffer */
dev_kfree_skb_any(skb);
^ permalink raw reply related
* Re: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Daniel Drake @ 2013-07-05 14:45 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: Bing Zhao, linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <5FF020A1CFFEEC49BD1E09530C4FF5951035636350@SC-VEXCH1.marvell.com>
On Fri, Jul 5, 2013 at 8:26 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
> I think, we should probably look into mwifiex_sdio_suspend() routine instead of mwifiex_sdio_remove() to debug interrupt storm issue. Please correct me if I am missing something.
The system is going into an unpowered suspend. This means that
mwifiex_sdio_suspend() returns -ENOSYS.
The card then gets removed by mwifiex_sdio_remove().
mwifiex_sdio_remove() calls mwifiex_remove_card() without having taken
any care to finish pending commands, etc. mwifiex_remove_card()
immediately sets surprise_removed which triggers the questionable
"ignore all interrupts" behaviour. If there were any async commands
pending, they complete now, with an interrupt. The interrupt doesn't
get acked, so it becomes an interrupt storm.
Hope that is clearer.
Daniel
^ permalink raw reply
* RE: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Amitkumar Karwar @ 2013-07-05 14:44 UTC (permalink / raw)
To: 'Daniel Drake', Bing Zhao
Cc: linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <CAMLZHHSSfPnWK957Yd+vrLrsPHtUDW3b4ZxSjMe2SdpaQPk5hQ@mail.gmail.com>
Hi Daniel,
>Also, at which point do we wait upon all async commands to complete
>before shutting down?
All sync and async commands are queued into same queue. Sync and async type indicates if the thread just queues the command or it queues the command and waits until command response is received.
Next command is sent to firmware only after receiving response of previous command. As we don't queue any command after SHUTDOWN command is queued, SHUTDOWN command is the last command sent to firmware. SHUTDOWN is a sync command. Hence remove handler thread waits until it's response.
Please let me know if there are any doubts.
Thanks,
Amit
^ permalink raw reply
* RE: [PATCH V2] vlan priority handling in WMM
From: voncken @ 2013-07-05 14:39 UTC (permalink / raw)
To: 'Johannes Berg'; +Cc: linux-wireless
In-Reply-To: <1373018378.8548.6.camel@jlt4.sipsolutions.net>
> >+ vlan_priority = (skb->vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
> >+ if (vlan_priority > 0)
>> + return vlan_priority;
>Is this really correct? For once, checking vlan_priority for non-zero seems weird since that ought to be a valid value -- is there no other way to determine that the value is valid?
The vlan Tag contain three bit for priority. The value 0 indicate no priority (on this case the VLAN tag contain only VID). The vlan_tci field is set to zero if the frame do not contain the vlan tag. So if we have not a vlan tag or no priority in VLAN tag the priority value is always 0.
>Also, this gives you a 2-bit value, while the return value should be a 3-bit value, maybe you need to shift this up by one to spread into the correct buckets?
Sorry but I don't understand. The vlan_tci field it is a __u16 value (defined in include/linux/skbuff.h), the VLAN_PRIO_MASK is set to 0xE000 and VLAN_PRIO_SHIFT is set to 13 (defined in include/linux/if_vlan.h), the vlan_priority is an unsigned char. For me the vlan_priority contain a 3-bit value (0xE000 >>13 = 0x0003), why 2 ?
If you are agree with me, I'll resend a V3 patch with a correct subject.
Cedric
>johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Amitkumar Karwar @ 2013-07-05 14:26 UTC (permalink / raw)
To: 'Daniel Drake', Bing Zhao
Cc: linux-wireless@vger.kernel.org, linville@tuxdriver.com
In-Reply-To: <CAMLZHHSSfPnWK957Yd+vrLrsPHtUDW3b4ZxSjMe2SdpaQPk5hQ@mail.gmail.com>
Hi Daniel,
>> Followings are the actions taken for driver unload.
>>
>> a) If device is connected, sync deauth command is sent.
>> b) Auto deep sleep is cancelled by sending sync command
>> c) Sync shutdown command is queued and HW_STATUS is changed to reset.
>> d) Now no other command gets queued based on HW_STATUS.
>> e) Wait for shutdown command response and handle it.
>> f) Set surprise_removed flag which blocks SDIO interrupts
>>
>> As per our design, we don't send any command to firmware after SHUTDOWN command. Also, firmware doesn't send any interrupt after SHUTDOWN command response.
>I cannot see how the driver behaviour matches the above description
>for when the card is removed as the system is suspended. For example I
>cannot see where the SHUTDOWN command gets sent in such cases.
This behavior (i.e. mwifiex_sdio_remove() handler) is for driver unload(rmmod mwifiex_sdio) or when the system is powering down.
Our mwifiex_sdio_suspend() handler gets called when system is supended. It just informs firmware/hardware that host is going into sleep state by sending a command. The sleep is cancelled in mwifiex_sdio_resume() handler when sytem resumes.
We support wake-on-lan feature. User can configure wol conditions(using 'ethtool wol' or 'iw wowlan') before system suspend. Firmware wakes up the host if Rx packet matching configured condition is received.
I think, we should probably look into mwifiex_sdio_suspend() routine instead of mwifiex_sdio_remove() to debug interrupt storm issue. Please correct me if I am missing something.
We have wait_event_interruptible() call in suspend handler to make sure that we don't return from the handler until we get host sleep activated event from firmware. Firmware is not supposed to send any event after this.
We can't disable interrupts at this point, because we expect wakeup event from firmware upon packet arrival
Can you please share system suspend logs with dynamic debug enabled for mwifiex?
Thanks,
Amitkumar Karwar
^ permalink raw reply
* Re: [patch] ath10k: off by one sanity check
From: Kalle Valo @ 2013-07-05 13:36 UTC (permalink / raw)
To: Dan Carpenter
Cc: John W. Linville, linux-wireless, kernel-janitors, ath9k-devel
In-Reply-To: <20130618072846.GC12329@elgon.mountain>
Dan Carpenter <dan.carpenter@oracle.com> writes:
> This should be >= ARRAY_SIZE() instead of > ARRAY_SIZE().
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks for the patch. John already applied this to his wireless-next
tree.
--
Kalle Valo
^ permalink raw reply
* Re: [ath9k-devel] [PATCH v2] ath10k: leave MMIC generation to the HW
From: Kalle Valo @ 2013-07-05 13:25 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath9k-devel, linux-wireless
In-Reply-To: <1372229844-7987-1-git-send-email-michal.kazior@tieto.com>
Michal Kazior <michal.kazior@tieto.com> writes:
> Apparently HW doesn't require us to generate MMIC
> for TKIP suite.
>
> Each frame was 8 bytes longer than it should be
> and some APs would drop frames that exceed 1520
> bytes of 802.11 payload. This could be observed
> during throughput tests or fragmented IP traffic.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
This is also applied.
--
Kalle Valo
^ permalink raw reply
* Re: [ath9k-devel] [PATCH v2] ath10k: fix 5ghz channel definitions
From: Kalle Valo @ 2013-07-05 13:24 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath9k-devel, linux-wireless
In-Reply-To: <1372229694-6769-1-git-send-email-michal.kazior@tieto.com>
Michal Kazior <michal.kazior@tieto.com> writes:
> Nonsense channel flags were being set.
>
> Although it doesn't seem this was visible to the
> user the patch makes sure that channel
> availability won't be crippled in the future if
> ath_common behaviour changes.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
John also applied this already.
--
Kalle Valo
^ permalink raw reply
* Re: [ath9k-devel] [PATCH v2] ath10k: fix MSI-X setup failpath
From: Kalle Valo @ 2013-07-05 13:23 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath9k-devel, linux-wireless
In-Reply-To: <1372229450-4575-1-git-send-email-michal.kazior@tieto.com>
Michal Kazior <michal.kazior@tieto.com> writes:
> Irqs were not freed up correctly upon msi-x setup
> failure.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
John has already applied this.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCHv5 04/18] mac80211: select and adjust bitrates according to channel mode
From: Johannes Berg @ 2013-07-05 13:17 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: linux-wireless, Mathias Kretschmer, Simon Wunderlich
In-Reply-To: <1372352340-20663-5-git-send-email-siwu@hrz.tu-chemnitz.de>
>
> +
> +
> +
> static int sta_apply_parameters(struct ieee80211_local *local,
Hmm? :)
johannes
^ permalink raw reply
* Re: [PATCHv5 03/18] mac80211: fix timing for 5 MHz and 10 MHz channels
From: Johannes Berg @ 2013-07-05 13:15 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: linux-wireless, Mathias Kretschmer, Simon Wunderlich
In-Reply-To: <1372352340-20663-4-git-send-email-siwu@hrz.tu-chemnitz.de>
On Thu, 2013-06-27 at 18:58 +0200, Simon Wunderlich wrote:
> * change divisor to shift to avoid unnecesary integer divisions
That didn't really do much ;-)
> d->perfect_tx_time = ieee80211_frame_duration(band, 1200,
> - rate->bitrate, erp, 1);
> + DIV_ROUND_UP(rate->bitrate, 1 << shift), erp, 1,
That's still the integer division, unless the compiler can do something
about it?
> + DIV_ROUND_UP(rate->bitrate, 1 << shift), erp, 1,
ditto
> + mr->bitrate = DIV_ROUND_UP(sband->bitrates[i].bitrate,
> + (1 << shift) * 5);
That looks even weirder?
> + /* IEEE 802.11-2012 18.3.2.4: all values above are:
> + * * times 4 for 5 MHz
> + * * times 2 for 10 MHz
> + */
> + dur *= 1 << shift;
Isn't that just "dur <<= shift"?
I think "DIV_ROUND_UP(x, 1 << shift)" can probably be written more
efficiently? Unless the compiler will do something by itself?
Something like
DIV_ROUND_UP(x, 1<<shift) == (x + (1<<shift) -1) >> shift
I think?
OK, it looks like if "x" is *unsigned* then the compiler will do this,
but with *signed* values it can't actually know because for negative
values it's obviously not actually equivalent.
johannes
^ permalink raw reply
* [PATCH 13/16] ath10k: embed HTT struct inside ath10k
From: Kalle Valo @ 2013-07-05 13:15 UTC (permalink / raw)
To: linux-wireless; +Cc: ath9k-devel, Michal Kazior
In-Reply-To: <1373030117-18077-1-git-send-email-kvalo@qca.qualcomm.com>
From: Michal Kazior <michal.kazior@tieto.com>
This reduces number of allocations and simplifies
memory managemnt.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/core.c | 12 ++++++------
drivers/net/wireless/ath/ath10k/core.h | 7 +++----
drivers/net/wireless/ath/ath10k/htt.c | 25 +++++++++++++------------
drivers/net/wireless/ath/ath10k/htt.h | 3 +--
drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
drivers/net/wireless/ath/ath10k/htt_tx.c | 2 +-
drivers/net/wireless/ath/ath10k/mac.c | 16 ++++++++--------
7 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 4199560..f1312fa 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -556,9 +556,9 @@ int ath10k_core_register(struct ath10k *ar)
if (status)
goto err_wmi_detach;
- ar->htt = ath10k_htt_attach(ar);
- if (!ar->htt) {
- status = -ENOMEM;
+ status = ath10k_htt_attach(ar);
+ if (status) {
+ ath10k_err("could not attach htt (%d)\n", status);
goto err_wmi_detach;
}
@@ -585,7 +585,7 @@ int ath10k_core_register(struct ath10k *ar)
goto err_disconnect_htc;
}
- status = ath10k_htt_attach_target(ar->htt);
+ status = ath10k_htt_attach_target(&ar->htt);
if (status)
goto err_disconnect_htc;
@@ -606,7 +606,7 @@ err_unregister_mac:
err_disconnect_htc:
ath10k_htc_stop(&ar->htc);
err_htt_detach:
- ath10k_htt_detach(ar->htt);
+ ath10k_htt_detach(&ar->htt);
err_wmi_detach:
ath10k_wmi_detach(ar);
err:
@@ -621,7 +621,7 @@ void ath10k_core_unregister(struct ath10k *ar)
* unhappy about callback failures. */
ath10k_mac_unregister(ar);
ath10k_htc_stop(&ar->htc);
- ath10k_htt_detach(ar->htt);
+ ath10k_htt_detach(&ar->htt);
ath10k_wmi_detach(ar);
}
EXPORT_SYMBOL(ath10k_core_unregister);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index dfd4fe1..2f7065c 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -23,6 +23,7 @@
#include <linux/types.h>
#include <linux/pci.h>
+#include "htt.h"
#include "htc.h"
#include "hw.h"
#include "targaddrs.h"
@@ -273,15 +274,13 @@ struct ath10k {
const struct ath10k_hif_ops *ops;
} hif;
- struct ath10k_wmi wmi;
-
wait_queue_head_t event_queue;
bool is_target_paused;
struct ath10k_bmi bmi;
+ struct ath10k_wmi wmi;
struct ath10k_htc htc;
-
- struct ath10k_htt *htt;
+ struct ath10k_htt htt;
struct ath10k_hw_params {
u32 id;
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 2bfb9b4..39342c5 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -16,6 +16,7 @@
*/
#include <linux/slab.h>
+#include <linux/if_ether.h>
#include "htt.h"
#include "core.h"
@@ -47,15 +48,11 @@ static int ath10k_htt_htc_attach(struct ath10k_htt *htt)
return 0;
}
-struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar)
+int ath10k_htt_attach(struct ath10k *ar)
{
- struct ath10k_htt *htt;
+ struct ath10k_htt *htt = &ar->htt;
int ret;
- htt = kzalloc(sizeof(*htt), GFP_KERNEL);
- if (!htt)
- return NULL;
-
htt->ar = ar;
htt->max_throughput_mbps = 800;
@@ -65,8 +62,11 @@ struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar)
* since ath10k_htt_rx_attach involves sending a rx ring configure
* message to the target.
*/
- if (ath10k_htt_htc_attach(htt))
+ ret = ath10k_htt_htc_attach(htt);
+ if (ret) {
+ ath10k_err("could not attach htt htc (%d)\n", ret);
goto err_htc_attach;
+ }
ret = ath10k_htt_tx_attach(htt);
if (ret) {
@@ -74,8 +74,11 @@ struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar)
goto err_htc_attach;
}
- if (ath10k_htt_rx_attach(htt))
+ ret = ath10k_htt_rx_attach(htt);
+ if (ret) {
+ ath10k_err("could not attach htt rx (%d)\n", ret);
goto err_rx_attach;
+ }
/*
* Prefetch enough data to satisfy target
@@ -89,13 +92,12 @@ struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar)
8 + /* llc snap */
2; /* ip4 dscp or ip6 priority */
- return htt;
+ return 0;
err_rx_attach:
ath10k_htt_tx_detach(htt);
err_htc_attach:
- kfree(htt);
- return NULL;
+ return ret;
}
#define HTT_TARGET_VERSION_TIMEOUT_HZ (3*HZ)
@@ -148,5 +150,4 @@ void ath10k_htt_detach(struct ath10k_htt *htt)
{
ath10k_htt_rx_detach(htt);
ath10k_htt_tx_detach(htt);
- kfree(htt);
}
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index a7a7aa0..318be46 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -20,7 +20,6 @@
#include <linux/bug.h>
-#include "core.h"
#include "htc.h"
#include "rx_desc.h"
@@ -1317,7 +1316,7 @@ struct htt_rx_desc {
#define HTT_LOG2_MAX_CACHE_LINE_SIZE 7 /* 2^7 = 128 */
#define HTT_MAX_CACHE_LINE_SIZE_MASK ((1 << HTT_LOG2_MAX_CACHE_LINE_SIZE) - 1)
-struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar);
+int ath10k_htt_attach(struct ath10k *ar);
int ath10k_htt_attach_target(struct ath10k_htt *htt);
void ath10k_htt_detach(struct ath10k_htt *htt);
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index de058d7..04f08d9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -15,6 +15,7 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include "core.h"
#include "htc.h"
#include "htt.h"
#include "txrx.h"
@@ -1036,7 +1037,7 @@ end:
void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
{
- struct ath10k_htt *htt = ar->htt;
+ struct ath10k_htt *htt = &ar->htt;
struct htt_resp *resp = (struct htt_resp *)skb->data;
/* confirm alignment */
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index fa568f1..dc3f3e8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -153,7 +153,7 @@ void ath10k_htt_tx_detach(struct ath10k_htt *htt)
void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
{
struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
- struct ath10k_htt *htt = ar->htt;
+ struct ath10k_htt *htt = &ar->htt;
if (skb_cb->htt.is_conf) {
dev_kfree_skb_any(skb);
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cdee800..7bc746f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1394,15 +1394,15 @@ static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb)
int ret;
if (ieee80211_is_mgmt(hdr->frame_control))
- ret = ath10k_htt_mgmt_tx(ar->htt, skb);
+ ret = ath10k_htt_mgmt_tx(&ar->htt, skb);
else if (ieee80211_is_nullfunc(hdr->frame_control))
/* FW does not report tx status properly for NullFunc frames
* unless they are sent through mgmt tx path. mac80211 sends
* those frames when it detects link/beacon loss and depends on
* the tx status to be correct. */
- ret = ath10k_htt_mgmt_tx(ar->htt, skb);
+ ret = ath10k_htt_mgmt_tx(&ar->htt, skb);
else
- ret = ath10k_htt_tx(ar->htt, skb);
+ ret = ath10k_htt_tx(&ar->htt, skb);
if (ret) {
ath10k_warn("tx failed (%d). dropping packet.\n", ret);
@@ -2656,12 +2656,12 @@ static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
if (drop)
return;
- ret = wait_event_timeout(ar->htt->empty_tx_wq, ({
+ ret = wait_event_timeout(ar->htt.empty_tx_wq, ({
bool empty;
- spin_lock_bh(&ar->htt->tx_lock);
- empty = bitmap_empty(ar->htt->used_msdu_ids,
- ar->htt->max_num_pending_tx);
- spin_unlock_bh(&ar->htt->tx_lock);
+ spin_lock_bh(&ar->htt.tx_lock);
+ empty = bitmap_empty(ar->htt.used_msdu_ids,
+ ar->htt.max_num_pending_tx);
+ spin_unlock_bh(&ar->htt.tx_lock);
(empty);
}), ATH10K_FLUSH_TIMEOUT_HZ);
if (ret <= 0)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 14/16] ath10k: improve locking
From: Kalle Valo @ 2013-07-05 13:15 UTC (permalink / raw)
To: linux-wireless; +Cc: ath9k-devel, Michal Kazior
In-Reply-To: <1373030117-18077-1-git-send-email-kvalo@qca.qualcomm.com>
From: Michal Kazior <michal.kazior@tieto.com>
Add more lockdep asserts and a few conf_mutex
locks. It's better to be on the safe side.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 57 +++++++++++++++++++++++++++++++--
1 file changed, 55 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7bc746f..def1e68 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -43,6 +43,8 @@ static int ath10k_send_key(struct ath10k_vif *arvif,
.macaddr = macaddr,
};
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE)
arg.key_flags = WMI_KEY_PAIRWISE;
else
@@ -88,6 +90,8 @@ static int ath10k_install_key(struct ath10k_vif *arvif,
struct ath10k *ar = arvif->ar;
int ret;
+ lockdep_assert_held(&ar->conf_mutex);
+
INIT_COMPLETION(ar->install_key_done);
ret = ath10k_send_key(arvif, key, cmd, macaddr);
@@ -369,6 +373,8 @@ static inline int ath10k_vdev_setup_sync(struct ath10k *ar)
{
int ret;
+ lockdep_assert_held(&ar->conf_mutex);
+
ret = wait_for_completion_timeout(&ar->vdev_setup_done,
ATH10K_VDEV_SETUP_TIMEOUT_HZ);
if (ret == 0)
@@ -602,6 +608,8 @@ static void ath10k_control_beaconing(struct ath10k_vif *arvif,
{
int ret = 0;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
if (!info->enable_beacon) {
ath10k_vdev_stop(arvif);
return;
@@ -628,6 +636,8 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
{
int ret = 0;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
if (!info->ibss_joined) {
ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, self_peer);
if (ret)
@@ -677,6 +687,8 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
enum wmi_sta_ps_mode psmode;
int ret;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
if (vif->type != NL80211_IFTYPE_STATION)
return;
@@ -719,6 +731,8 @@ static void ath10k_peer_assoc_h_basic(struct ath10k *ar,
struct ieee80211_bss_conf *bss_conf,
struct wmi_peer_assoc_complete_arg *arg)
{
+ lockdep_assert_held(&ar->conf_mutex);
+
memcpy(arg->addr, sta->addr, ETH_ALEN);
arg->vdev_id = arvif->vdev_id;
arg->peer_aid = sta->aid;
@@ -761,6 +775,8 @@ static void ath10k_peer_assoc_h_crypto(struct ath10k *ar,
const u8 *rsnie = NULL;
const u8 *wpaie = NULL;
+ lockdep_assert_held(&ar->conf_mutex);
+
bss = cfg80211_get_bss(ar->hw->wiphy, ar->hw->conf.chandef.chan,
info->bssid, NULL, 0, 0, 0);
if (bss) {
@@ -801,6 +817,8 @@ static void ath10k_peer_assoc_h_rates(struct ath10k *ar,
u32 ratemask;
int i;
+ lockdep_assert_held(&ar->conf_mutex);
+
sband = ar->hw->wiphy->bands[ar->hw->conf.chandef.chan->band];
ratemask = sta->supp_rates[ar->hw->conf.chandef.chan->band];
rates = sband->bitrates;
@@ -824,6 +842,8 @@ static void ath10k_peer_assoc_h_ht(struct ath10k *ar,
int smps;
int i, n;
+ lockdep_assert_held(&ar->conf_mutex);
+
if (!ht_cap->ht_supported)
return;
@@ -902,6 +922,8 @@ static void ath10k_peer_assoc_h_qos_ap(struct ath10k *ar,
u32 uapsd = 0;
u32 max_sp = 0;
+ lockdep_assert_held(&ar->conf_mutex);
+
if (sta->wme)
arg->peer_flags |= WMI_PEER_QOS;
@@ -1053,6 +1075,8 @@ static int ath10k_peer_assoc(struct ath10k *ar,
{
struct wmi_peer_assoc_complete_arg arg;
+ lockdep_assert_held(&ar->conf_mutex);
+
memset(&arg, 0, sizeof(struct wmi_peer_assoc_complete_arg));
ath10k_peer_assoc_h_basic(ar, arvif, sta, bss_conf, &arg);
@@ -1076,6 +1100,8 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
struct ieee80211_sta *ap_sta;
int ret;
+ lockdep_assert_held(&ar->conf_mutex);
+
rcu_read_lock();
ap_sta = ieee80211_find_sta(vif, bss_conf->bssid);
@@ -1116,6 +1142,8 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
int ret;
+ lockdep_assert_held(&ar->conf_mutex);
+
/*
* For some reason, calling VDEV-DOWN before VDEV-STOP
* makes the FW to send frames via HTT after disassociation.
@@ -1149,6 +1177,8 @@ static int ath10k_station_assoc(struct ath10k *ar, struct ath10k_vif *arvif,
{
int ret = 0;
+ lockdep_assert_held(&ar->conf_mutex);
+
ret = ath10k_peer_assoc(ar, arvif, sta, NULL);
if (ret) {
ath10k_warn("WMI peer assoc failed for %pM\n", sta->addr);
@@ -1169,6 +1199,8 @@ static int ath10k_station_disassoc(struct ath10k *ar, struct ath10k_vif *arvif,
{
int ret = 0;
+ lockdep_assert_held(&ar->conf_mutex);
+
ret = ath10k_clear_peer_keys(arvif, sta->addr);
if (ret) {
ath10k_warn("could not clear all peer wep keys (%d)\n", ret);
@@ -1195,6 +1227,8 @@ static int ath10k_update_channel_list(struct ath10k *ar)
int ret;
int i;
+ lockdep_assert_held(&ar->conf_mutex);
+
bands = hw->wiphy->bands;
for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
if (!bands[band])
@@ -1281,6 +1315,8 @@ static void ath10k_reg_notifier(struct wiphy *wiphy,
struct ath10k *ar = hw->priv;
int ret;
+ mutex_lock(&ar->conf_mutex);
+
ath_reg_notifier_apply(wiphy, request, &ar->ath_common.regulatory);
ret = ath10k_update_channel_list(ar);
@@ -1298,6 +1334,8 @@ static void ath10k_reg_notifier(struct wiphy *wiphy,
regpair->reg_5ghz_ctl);
if (ret)
ath10k_warn("could not set pdev regdomain (%d)\n", ret);
+
+ mutex_unlock(&ar->conf_mutex);
}
/***************/
@@ -1675,6 +1713,8 @@ static int ath10k_start(struct ieee80211_hw *hw)
struct ath10k *ar = hw->priv;
int ret;
+ mutex_lock(&ar->conf_mutex);
+
ret = ath10k_wmi_pdev_set_param(ar, WMI_PDEV_PARAM_PMF_QOS, 1);
if (ret)
ath10k_warn("could not enable WMI_PDEV_PARAM_PMF_QOS (%d)\n",
@@ -1685,6 +1725,7 @@ static int ath10k_start(struct ieee80211_hw *hw)
ath10k_warn("could not init WMI_PDEV_PARAM_DYNAMIC_BW (%d)\n",
ret);
+ mutex_unlock(&ar->conf_mutex);
return 0;
}
@@ -1692,9 +1733,11 @@ static void ath10k_stop(struct ieee80211_hw *hw)
{
struct ath10k *ar = hw->priv;
- /* avoid leaks in case FW never confirms scan for offchannel */
- cancel_work_sync(&ar->offchan_tx_work);
+ mutex_lock(&ar->conf_mutex);
ath10k_offchan_tx_purge(ar);
+ mutex_unlock(&ar->conf_mutex);
+
+ cancel_work_sync(&ar->offchan_tx_work);
}
static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2378,6 +2421,8 @@ static int ath10k_conf_tx_uapsd(struct ath10k *ar, struct ieee80211_vif *vif,
u32 value = 0;
int ret = 0;
+ lockdep_assert_held(&ar->conf_mutex);
+
if (arvif->vdev_type != WMI_VDEV_TYPE_STA)
return 0;
@@ -2573,6 +2618,8 @@ static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
rts = min_t(u32, rts, ATH10K_RTS_MAX);
ar_iter->ret = ath10k_wmi_vdev_set_param(ar_iter->ar, arvif->vdev_id,
@@ -2611,6 +2658,8 @@ static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
int ret;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
frag = clamp_t(u32, frag,
ATH10K_FRAGMT_THRESHOLD_MIN,
ATH10K_FRAGMT_THRESHOLD_MAX);
@@ -2656,6 +2705,8 @@ static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
if (drop)
return;
+ mutex_lock(&ar->conf_mutex);
+
ret = wait_event_timeout(ar->htt.empty_tx_wq, ({
bool empty;
spin_lock_bh(&ar->htt.tx_lock);
@@ -2666,6 +2717,8 @@ static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
}), ATH10K_FLUSH_TIMEOUT_HZ);
if (ret <= 0)
ath10k_warn("tx not flushed\n");
+
+ mutex_unlock(&ar->conf_mutex);
}
/* TODO: Implement this function properly
--
1.7.9.5
^ permalink raw reply related
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