* Re: [PATCH -next] wcn36xx: fix missing unlock on error in wcn36xx_smd_update_proberesp_tmpl()
From: Eugene Krasnikov @ 2013-10-30 7:12 UTC (permalink / raw)
To: Wei Yongjun; +Cc: John Linville, yongjun_wei, wcn36xx, linux-wireless
In-Reply-To: <CAPgLHd9HRb_o+hac5_RjKhK1K-jJFtQmrv9P2iy7zegQv=WOcw@mail.gmail.com>
Looks good to me! I assume this patch is the result of running smatch?
I made the same patch, just did not have time to send it out.
On Wed, Oct 30, 2013 at 5:30 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Add the missing unlock before return from function
> wcn36xx_smd_update_proberesp_tmpl() in the error
> handling case.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/wireless/ath/wcn36xx/smd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index f8c3a10..04df70b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -1327,7 +1327,8 @@ int wcn36xx_smd_update_proberesp_tmpl(struct wcn36xx *wcn,
> if (skb->len > BEACON_TEMPLATE_SIZE) {
> wcn36xx_warn("probe response template is too big: %d\n",
> skb->len);
> - return -E2BIG;
> + ret = -E2BIG;
> + goto out;
> }
>
> msg.probe_resp_template_len = skb->len;
>
--
Best regards,
Eugene
^ permalink raw reply
* Re: [PATCH -next] wcn36xx: fix missing unlock on error in wcn36xx_smd_update_proberesp_tmpl()
From: Wei Yongjun @ 2013-10-30 7:40 UTC (permalink / raw)
To: k.eugene.e; +Cc: linville, yongjun_wei, wcn36xx, linux-wireless
In-Reply-To: <CAFSJ42ZWiH0K7JKxBUHA8dqBLT++=Zu-mvyoF=egk4kP1+Detw@mail.gmail.com>
On 10/30/2013 03:12 PM, Eugene Krasnikov wrote:
> Looks good to me! I assume this patch is the result of running smatch?
> I made the same patch, just did not have time to send it out.
Hi,
I used coccinelle with script to found this. ^_^
>
> On Wed, Oct 30, 2013 at 5:30 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> Add the missing unlock before return from function
>> wcn36xx_smd_update_proberesp_tmpl() in the error
>> handling case.
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> ---
>> drivers/net/wireless/ath/wcn36xx/smd.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
>> index f8c3a10..04df70b 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
>> @@ -1327,7 +1327,8 @@ int wcn36xx_smd_update_proberesp_tmpl(struct wcn36xx *wcn,
>> if (skb->len > BEACON_TEMPLATE_SIZE) {
>> wcn36xx_warn("probe response template is too big: %d\n",
>> skb->len);
>> - return -E2BIG;
>> + ret = -E2BIG;
>> + goto out;
>> }
>>
>> msg.probe_resp_template_len = skb->len;
>>
>
>
^ permalink raw reply
* Re: iwlegacy (4965) - what would 0x8000 as the completed TX rate indicate?
From: Johannes Berg @ 2013-10-30 8:04 UTC (permalink / raw)
To: Adrian Chadd; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <CAJ-Vmom0P7bJYWtCAYfCBKmMhhGHnqpf51mwPn0xkgyS84Qt=A@mail.gmail.com>
On Tue, 2013-10-29 at 20:59 -0700, Adrian Chadd wrote:
> On 29 October 2013 01:03, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> >> Well, what's rate=0 mean?
> >>
> >> And 0x8000 is bit 11 set, which is HT40. I definitely don't have HT40 enabled.
> >
> > 0x8000 is bit 15 set, which means "antenna B", I think?
>
> Duh, sorry. was tired. :(
>
> > But then I can't seem to figure out what rate=0 means either. Should be
> > an OFDM rate (bit 9 not set) but 0 isn't a valid OFDM rate value. Hmm.
>
> That's why I'm confused too.
>
> I'm seeing some odd behaviour here where sometimes the NIC behaves
> fine, and sometimes it just gets long transmit failures on OFDM frames
> but is fine with CCK frames. That's why I'm digging into this.
>
> I also see the sensitivity tuning code lose the plot over time as OFDM
> errors are returned by the receiver and it keeps trying to compensate
> until it's maxed out the sensitivity tuning parameters. If I restart
> the NIC, it all comes back to normal for a while.
>
> Do any of these ring a bell?
Sorry, no.
johannes
^ permalink raw reply
* Re: [PATCH] iwl3945: do not print RFKILL message
From: Stanislaw Gruszka @ 2013-10-30 8:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Dietmar Rudolph, Pedro Francisco
In-Reply-To: <1382699879.12755.0.camel@jlt4.sipsolutions.net>
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
On Fri, Oct 25, 2013 at 01:17:59PM +0200, Johannes Berg wrote:
> On Fri, 2013-10-25 at 12:37 +0200, Stanislaw Gruszka wrote:
> > We can mess logs if user space try to open device again and again if
> > RFKILL switch is on. Do not print message and return ERFKILL error
> > instead to indicate where the problem is.
>
> Are you maybe not calling wiphy_rfkill_set_hw_state() in the right
> places?
This is done from work scheduled every 2 second if rfkill state change,
but is possible that initial state is not provided to upper layer.
Dietmar or Pedro, could you test attached patch (my old laptop with 3945
does not work any longer). It is alternative to previous patch posted
before, does rfkill work ok with it?
Stanislaw
[-- Attachment #2: iwl3945_rfkill.patch --]
[-- Type: text/plain, Size: 1206 bytes --]
diff --git a/drivers/net/wireless/iwlegacy/3945-mac.c b/drivers/net/wireless/iwlegacy/3945-mac.c
index dea3b50..74e2b0a 100644
--- a/drivers/net/wireless/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/iwlegacy/3945-mac.c
@@ -2202,6 +2202,9 @@ il3945_alive_start(struct il_priv *il)
} else
set_bit(S_RFKILL, &il->status);
+ wiphy_rfkill_set_hw_state(il->hw->wiphy,
+ test_bit(S_RFKILL, &il->status));
+
/* After the ALIVE response, we can send commands to 3945 uCode */
set_bit(S_ALIVE, &il->status);
@@ -2397,7 +2400,7 @@ __il3945_up(struct il_priv *il)
else {
set_bit(S_RFKILL, &il->status);
IL_WARN("Radio disabled by HW RF Kill switch\n");
- return -ENODEV;
+ return 0;
}
_il_wr(il, CSR_INT, 0xFFFFFFFF);
@@ -2825,6 +2828,9 @@ il3945_mac_start(struct ieee80211_hw *hw)
if (ret)
goto out_release_irq;
+ if (il_is_rfkill(il))
+ goto out;
+
D_INFO("Start UP work.\n");
/* Wait for START_ALIVE from ucode. Otherwise callbacks from
@@ -2845,6 +2851,7 @@ il3945_mac_start(struct ieee80211_hw *hw)
* no need to poll the killswitch state anymore */
cancel_delayed_work(&il->_3945.rfkill_poll);
+out:
il->is_open = 1;
D_MAC80211("leave\n");
return 0;
^ permalink raw reply related
* Re: [PATCH] iwl3945: do not print RFKILL message
From: Dietmar Rudolph @ 2013-10-30 8:51 UTC (permalink / raw)
To: Stanislaw Gruszka, Johannes Berg; +Cc: linux-wireless, Pedro Francisco
In-Reply-To: <20131030083432.GA1478@redhat.com>
> Dietmar or Pedro, could you test attached patch (my old laptop with 3945
> does not work any longer). It is alternative to previous patch posted
> before, does rfkill work ok with it?
Stanislaw,
I'm sorry a cannot be any help here as I don't have the time nor the
environment to compile and test your patch.
In regard to this, I'm just a plain end user who installed Ubuntu
out of the box. I had nticed the problem of the ever growing kernel
log and found this mailing list to be the right spot to post a bug
report. And I noticed your immediate reaction. Keep up the great work.
Kind regards
Dietmar
--
Dietmar Rudolph
Geschäftsführer/President
CR/LF GmbH
Obere Fuhr 27, 45136 Essen, Germany
phone: ++49 201 254566
mailto: dietmar@crlf.de, http://www.crlf.de
CR/LF Beratungsgesellschaft für EDV-Anwendungen mbH
Essen HRB 7016 - GF: Dipl.-Math. Dietmar Rudolph
^ permalink raw reply
* Re: [PATCH 2/3] brcm80211: fix usage of freq_reg_info()
From: Arend van Spriel @ 2013-10-30 9:10 UTC (permalink / raw)
To: Luis R. Rodriguez, linville, johannes
Cc: linux-wireless, Julia Lawall, Peter Senna Tschudin, Seth Forshee
In-Reply-To: <1383071666-26817-3-git-send-email-mcgrof@do-not-panic.com>
On 10/29/2013 07:34 PM, Luis R. Rodriguez wrote:
> freq_reg_info() expects KHz and not MHz, fix this. In
> this case we'll now be getting the no-ir flags cleared
> on channels for any channel when the country IE trusts
> that channel.
>
> @@
> struct ieee80211_channel *ch;
> struct wiphy *wiphy;
> const struct ieee80211_reg_rule *rule;
> @@
>
> -rule = freq_reg_info(wiphy, ch->center_freq);
> +rule = freq_reg_info(wiphy, MHZ_TO_KHZ(ch->center_freq));
>
> Generated-by: Coccinelle SmPL
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
> Cc: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Arend van Spriel <arend@broadcom.com>
> Reported-by: Mihir Shete <smihir@qti.qualcomm.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
> ---
> drivers/net/wireless/brcm80211/brcmsmac/channel.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/channel.c b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
> index c99364f..8272570 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/channel.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
> @@ -678,7 +678,8 @@ brcms_reg_apply_beaconing_flags(struct wiphy *wiphy,
> continue;
>
> if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
> - rule = freq_reg_info(wiphy, ch->center_freq);
> + rule = freq_reg_info(wiphy,
> + MHZ_TO_KHZ(ch->center_freq));
> if (IS_ERR(rule))
> continue;
>
>
^ permalink raw reply
* Re: [PATCH 0/2] ath10k: memory leak fixes
From: Kalle Valo @ 2013-10-30 9:53 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
In-Reply-To: <1382941094-3291-1-git-send-email-michal.kazior@tieto.com>
Michal Kazior <michal.kazior@tieto.com> writes:
> Michal Kazior (2):
> ath10k: plug memory leak in wmi mgmt tx worker
> ath10k: plug memory leak on beacon tx
Thanks, both patches applied.
--
Kalle Valo
^ permalink raw reply
* Monitor mode stopped working after kernel upgrade
From: Piotr Kaczorek @ 2013-10-30 11:10 UTC (permalink / raw)
To: linux-wireless
Update
I tried few more kernels (few longterm versions from kernel.org -
3.2.52, 3.4.67, 3.9.11 and current stable 3.11.6).
On 3.4.67 monitor mode still works.
On version 3.9.11 and above it doesn't.
Unfortunately I wanted support for Ralink 5572 which works since ~3.10.
Regards,
Piotr
^ permalink raw reply
* Re: [PATCH 02/16] wl1251: fix scan behaviour while not associated
From: Pavel Machek @ 2013-10-30 11:24 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1382819655-30430-3-git-send-email-pali.rohar@gmail.com>
On Sat 2013-10-26 22:34:01, Pali Rohár wrote:
> From: David Gnedt <david.gnedt@davizone.at>
>
> With a dissacociated card I often encoutered very long scan delays.
>
> My guess is that it has something to do with the cards DTIM handling and
> another firmware bug mentioned in the TI WLAN driver, which is described as
> the card may never end scanning if the channel is overloaded because it
> can't send probe requests. I think the firmware somehow also tries to
> receive DTIM messages when the BSSID is not set. Therefore most of the time
> it waits for DTIM messages and can't do scanning work.
>
> Anyway we can workaround this misbehaviour by setting the HIGH_PRIORITY
> bit for scans in disassociated state.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
I guess you should add your Signed-off-by: here (as you are sending
the patch forward).
Other than that:
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 05/16] wl1251: implement hardware ARP filtering
From: Pavel Machek @ 2013-10-30 11:28 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1382819655-30430-6-git-send-email-pali.rohar@gmail.com>
Hi!
> Update hardware ARP filter configuration on BSS_CHANGED_ARP_FILTER
> notification from mac80211.
> Ported from wl1271 driver.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
> ---
> drivers/net/wireless/ti/wl1251/acx.c | 31 +++++++++++++++++++++++++++++++
> drivers/net/wireless/ti/wl1251/acx.h | 15 +++++++++++++++
> drivers/net/wireless/ti/wl1251/main.c | 13 +++++++++++++
> 3 files changed, 59 insertions(+)
>
> diff --git a/drivers/net/wireless/ti/wl1251/acx.c b/drivers/net/wireless/ti/wl1251/acx.c
> index cce50e2..9295090 100644
> --- a/drivers/net/wireless/ti/wl1251/acx.c
> +++ b/drivers/net/wireless/ti/wl1251/acx.c
> @@ -1062,6 +1062,37 @@ out:
> return ret;
> }
>
> +int wl1251_acx_arp_ip_filter(struct wl1251 *wl, bool enable, __be32 address)
> +{
> + struct wl1251_acx_arp_filter *acx;
> + int ret;
> +
> + wl1251_debug(DEBUG_ACX, "acx arp ip filter, enable: %d", enable);
> +
> + acx = kzalloc(sizeof(*acx), GFP_KERNEL);
> + if (!acx) {
> + ret = -ENOMEM;
> + goto out;
> + }
I'd do "return -ENOMEM;" here. Trying to free NULL pointer is
unneccessary complication.
> + acx->version = ACX_IPV4_VERSION;
> + acx->enable = enable;
> +
> + if (enable == true)
if (enable) would be C way of writing stuff.
> + memcpy(acx->address, &address, ACX_IPV4_ADDR_SIZE);
> +
> + ret = wl1251_cmd_configure(wl, ACX_ARP_IP_FILTER,
> + acx, sizeof(*acx));
> + if (ret < 0) {
> + wl1251_warning("failed to set arp ip filter: %d", ret);
> + goto out;
> + }
> +
> +out:
No need for the out label now.
> diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
> index 46a2494..9752745 100644
> --- a/drivers/net/wireless/ti/wl1251/main.c
> +++ b/drivers/net/wireless/ti/wl1251/main.c
> @@ -1078,6 +1078,19 @@ static void wl1251_op_bss_info_changed(struct ieee80211_hw *hw,
> }
> }
>
> + if (changed & BSS_CHANGED_ARP_FILTER) {
> + __be32 addr = bss_conf->arp_addr_list[0];
> + WARN_ON(wl->bss_type != BSS_TYPE_STA_BSS);
> +
> + if (bss_conf->arp_addr_cnt == 1 && bss_conf->assoc)
> + ret = wl1251_acx_arp_ip_filter(wl, true, addr);
> + else
> + ret = wl1251_acx_arp_ip_filter(wl, false, addr);
enable = bss_conf->arp_addr_cnt == 1 && bss_conf->assoc;
ret = wl1251_acx_arp_ip_filter(wl, enable, addr);
?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 06/16] wl1251: split RX and TX data path initialisation
From: Pavel Machek @ 2013-10-30 11:31 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1382819655-30430-7-git-send-email-pali.rohar@gmail.com>
Hi!
> Split up data path initialisation into RX and TX data path initialisation
> functions. This change is required for channel switching in monitor mode.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
> ---
> drivers/net/wireless/ti/wl1251/cmd.c | 37 ++++++++++++++++++++++++++-------
> drivers/net/wireless/ti/wl1251/cmd.h | 3 ++-
> drivers/net/wireless/ti/wl1251/init.c | 9 ++++++--
> 3 files changed, 39 insertions(+), 10 deletions(-)
>
> @@ -238,6 +235,32 @@ int wl1251_cmd_data_path(struct wl1251 *wl, u8 channel, bool enable)
> wl1251_debug(DEBUG_BOOT, "rx %s cmd channel %d",
> enable ? "start" : "stop", channel);
>
> +out:
> + kfree(cmd);
> + return ret;
> +}
> +
> +int wl1251_cmd_data_path_tx(struct wl1251 *wl, u8 channel, bool enable)
> +{
> + struct cmd_enabledisable_path *cmd;
> + int ret;
> + u16 cmd_tx;
> +
> + wl1251_debug(DEBUG_CMD, "cmd data path");
> +
> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> + if (!cmd) {
> + ret = -ENOMEM;
> + goto out;
> + }
Again, doing jump just to kfree(NULL)... is probably not worth
it.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 07/16] wl1251: configure hardware en-/decryption for monitor mode
From: Pavel Machek @ 2013-10-30 11:35 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1382819655-30430-8-git-send-email-pali.rohar@gmail.com>
On Sat 2013-10-26 22:34:06, Pali Rohár wrote:
> From: David Gnedt <david.gnedt@davizone.at>
>
> Disable hardware encryption (DF_ENCRYPTION_DISABLE) and decryption
> (DF_SNIFF_MODE_ENABLE) via wl1251_acx_feature_cfg while monitor interface is
> present.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
You need to sign off the patch, otherwise looks ok.
Reviewed-by: Pavel Machek <pavel@ucw.cz>
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 08/16] wl1251: implement multicast address filtering
From: Pavel Machek @ 2013-10-30 11:41 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1382819655-30430-9-git-send-email-pali.rohar@gmail.com>
Hi!
> Port multicast address filtering from wl1271 driver.
> It sets up the hardware multicast address filter in configure_filter() with
> addresses supplied through prepare_multicast().
> +static u64 wl1251_op_prepare_multicast(struct ieee80211_hw *hw,
> + struct netdev_hw_addr_list *mc_list)
> +{
> + struct wl1251_filter_params *fp;
> + struct netdev_hw_addr *ha;
> + struct wl1251 *wl = hw->priv;
> +
> + if (unlikely(wl->state == WL1251_STATE_OFF))
> + return 0;
> +
> + fp = kzalloc(sizeof(*fp), GFP_ATOMIC);
> + if (!fp) {
> + wl1251_error("Out of memory setting filters.");
> + return 0;
> + }
So if there's not enough memory, we return 0.
> + /* update multicast filtering parameters */
> + fp->mc_list_length = 0;
> + if (netdev_hw_addr_list_count(mc_list) > ACX_MC_ADDRESS_GROUP_MAX) {
> + fp->enabled = false;
> + } else {
> + fp->enabled = true;
> + netdev_hw_addr_list_for_each(ha, mc_list) {
> + memcpy(fp->mc_list[fp->mc_list_length],
> + ha->addr, ETH_ALEN);
> + fp->mc_list_length++;
> + }
> + }
> +
> + return (u64)(unsigned long)fp;
> +}
Hiding pointers into u64 is not exactly nice, but I guess that's how
interface is designed? :-(.
> @@ -737,6 +779,15 @@ static void wl1251_op_configure_filter(struct ieee80211_hw *hw,
> if (ret < 0)
> goto out;
>
> + if (*total & FIF_ALLMULTI || *total & FIF_PROMISC_IN_BSS)
> + ret = wl1251_acx_group_address_tbl(wl, false, NULL, 0);
> + else if (fp)
> + ret = wl1251_acx_group_address_tbl(wl, fp->enabled,
> + fp->mc_list,
> + fp->mc_list_length);
Is it correct not to call anything in !fp case (for example because we
were out of memory?)
> + if (ret < 0)
> + goto out;
> +
> /* send filters to firmware */
> wl1251_acx_rx_config(wl, wl->rx_config, wl->rx_filter);
>
> @@ -744,6 +795,7 @@ static void wl1251_op_configure_filter(struct ieee80211_hw *hw,
>
> out:
> mutex_unlock(&wl->mutex);
> + kfree(fp);
> }
Umm, this is interesting. Who frees the memory in the success case?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* [PATCH/RFT 00/12] ath10k: pci fixes 2013-10-30
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
Hi,
This patchset contains a slew of PCI/CE cleanups
and fixes. The aim of the patchset is to deal with
CE null dereference bugs Ben has been reporting on
the mailing list for some time now.
I've done some brief testing and don't see any
major regressions. I wasn't able to reproduce the
reported bugs though. @Ben: Could you perhaps test
this, please?
Michal Kazior (12):
ath10k: remove ar_pci->ce_count
ath10k: don't forget to kill fw error tasklet
ath10k: split tasklet killing function
ath10k: rename function to match it's role
ath10k: make sure to mask all CE irqs
ath10k: fix ath10k_ce_init() failpath
ath10k: remove meaningless check
ath10k: use ath10k_do_pci_wake/sleep
ath10k: propagate ath10k_ce_disable_interrupts() errors
ath10k: guard against CE corruption from firmware
ath10k: re-arrange PCI init code
ath10k: add some debug prints
drivers/net/wireless/ath/ath10k/ce.c | 56 ++++++---
drivers/net/wireless/ath/ath10k/ce.h | 3 +-
drivers/net/wireless/ath/ath10k/htc.c | 5 +
drivers/net/wireless/ath/ath10k/pci.c | 221 ++++++++++++++++++----------------
drivers/net/wireless/ath/ath10k/pci.h | 3 -
5 files changed, 161 insertions(+), 127 deletions(-)
--
1.8.4.rc3
^ permalink raw reply
* [PATCH/RFT 01/12] ath10k: remove ar_pci->ce_count
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
It wasn't really useful to have it to begin with.
This makes it a little simpler to re-arrange PCI
init code as some function depended on
ar_pci->ce_count being set.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/ce.c | 9 +++------
drivers/net/wireless/ath/ath10k/pci.c | 18 ++++++++----------
drivers/net/wireless/ath/ath10k/pci.h | 3 ---
3 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index d243f28..2b99bd4 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -731,7 +731,6 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
void ath10k_ce_per_engine_service_any(struct ath10k *ar)
{
- struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int ce_id, ret;
u32 intr_summary;
@@ -741,7 +740,7 @@ void ath10k_ce_per_engine_service_any(struct ath10k *ar)
intr_summary = CE_INTERRUPT_SUMMARY(ar);
- for (ce_id = 0; intr_summary && (ce_id < ar_pci->ce_count); ce_id++) {
+ for (ce_id = 0; intr_summary && (ce_id < CE_COUNT); ce_id++) {
if (intr_summary & (1 << ce_id))
intr_summary &= ~(1 << ce_id);
else
@@ -785,16 +784,14 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
void ath10k_ce_disable_interrupts(struct ath10k *ar)
{
- struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int ce_id, ret;
ret = ath10k_pci_wake(ar);
if (ret)
return;
- for (ce_id = 0; ce_id < ar_pci->ce_count; ce_id++) {
- struct ath10k_ce_pipe *ce_state = &ar_pci->ce_states[ce_id];
- u32 ctrl_addr = ce_state->ctrl_addr;
+ for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
+ u32 ctrl_addr = ath10k_ce_base_address(ce_id);
ath10k_ce_copy_complete_intr_disable(ar, ctrl_addr);
}
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 42d2473..05cdbf0 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -831,7 +831,7 @@ static int ath10k_pci_start_ce(struct ath10k *ar)
spin_lock_init(&ar_pci->compl_lock);
INIT_LIST_HEAD(&ar_pci->compl_process);
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
pipe_info = &ar_pci->pipe_info[pipe_num];
spin_lock_init(&pipe_info->pipe_lock);
@@ -924,7 +924,7 @@ static void ath10k_pci_cleanup_ce(struct ath10k *ar)
spin_unlock_bh(&ar_pci->compl_lock);
/* Free unused completions for each pipe. */
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
pipe_info = &ar_pci->pipe_info[pipe_num];
spin_lock_bh(&pipe_info->pipe_lock);
@@ -1166,7 +1166,7 @@ static int ath10k_pci_post_rx(struct ath10k *ar)
const struct ce_attr *attr;
int pipe_num, ret = 0;
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
pipe_info = &ar_pci->pipe_info[pipe_num];
attr = &host_ce_config_wlan[pipe_num];
@@ -1295,7 +1295,7 @@ static void ath10k_pci_buffer_cleanup(struct ath10k *ar)
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int pipe_num;
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
struct ath10k_pci_pipe *pipe_info;
pipe_info = &ar_pci->pipe_info[pipe_num];
@@ -1310,7 +1310,7 @@ static void ath10k_pci_ce_deinit(struct ath10k *ar)
struct ath10k_pci_pipe *pipe_info;
int pipe_num;
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
pipe_info = &ar_pci->pipe_info[pipe_num];
if (pipe_info->ce_hdl) {
ath10k_ce_deinit(pipe_info->ce_hdl);
@@ -1755,7 +1755,7 @@ static int ath10k_pci_ce_init(struct ath10k *ar)
const struct ce_attr *attr;
int pipe_num;
- for (pipe_num = 0; pipe_num < ar_pci->ce_count; pipe_num++) {
+ for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
pipe_info = &ar_pci->pipe_info[pipe_num];
pipe_info->pipe_num = pipe_num;
pipe_info->hif_ce_state = ar;
@@ -1772,13 +1772,12 @@ static int ath10k_pci_ce_init(struct ath10k *ar)
return -1;
}
- if (pipe_num == ar_pci->ce_count - 1) {
+ if (pipe_num == CE_COUNT - 1) {
/*
* Reserve the ultimate CE for
* diagnostic Window support
*/
- ar_pci->ce_diag =
- ar_pci->pipe_info[ar_pci->ce_count - 1].ce_hdl;
+ ar_pci->ce_diag = pipe_info->ce_hdl;
continue;
}
@@ -2235,7 +2234,6 @@ static int ath10k_pci_start_intr(struct ath10k *ar)
exit:
ar_pci->num_msi_intrs = num;
- ar_pci->ce_count = CE_COUNT;
return ret;
}
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index a304c33..73a3d4e 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -199,9 +199,6 @@ struct ath10k_pci {
struct tasklet_struct intr_tq;
struct tasklet_struct msi_fw_err;
- /* Number of Copy Engines supported */
- unsigned int ce_count;
-
int started;
atomic_t keep_awake_count;
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 02/12] ath10k: don't forget to kill fw error tasklet
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
It was possible for FW error tasklet to be
executed during teardown. This could lead to
system crashes and/or memory corruption.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 05cdbf0..aa43466 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -888,6 +888,7 @@ static void ath10k_pci_stop_ce(struct ath10k *ar)
/* Cancel the pending tasklet */
tasklet_kill(&ar_pci->intr_tq);
+ tasklet_kill(&ar_pci->msi_fw_err);
for (i = 0; i < CE_COUNT; i++)
tasklet_kill(&ar_pci->pipe_info[i].intr);
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 03/12] ath10k: split tasklet killing function
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
The function will soon be called from more than 1
place.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index aa43466..d9467e5 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -877,21 +877,26 @@ static int ath10k_pci_start_ce(struct ath10k *ar)
return 0;
}
-static void ath10k_pci_stop_ce(struct ath10k *ar)
+static void ath10k_pci_kill_tasklet(struct ath10k *ar)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
- struct ath10k_pci_compl *compl;
- struct sk_buff *skb;
int i;
- ath10k_ce_disable_interrupts(ar);
-
- /* Cancel the pending tasklet */
tasklet_kill(&ar_pci->intr_tq);
tasklet_kill(&ar_pci->msi_fw_err);
for (i = 0; i < CE_COUNT; i++)
tasklet_kill(&ar_pci->pipe_info[i].intr);
+}
+
+static void ath10k_pci_stop_ce(struct ath10k *ar)
+{
+ struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+ struct ath10k_pci_compl *compl;
+ struct sk_buff *skb;
+
+ ath10k_ce_disable_interrupts(ar);
+ ath10k_pci_kill_tasklet(ar);
/* Mark pending completions as aborted, so that upper layers free up
* their associated resources */
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 04/12] ath10k: rename function to match it's role
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
What the function does is to actually wait for the
firmware indication bit to be set. Prerequisite
for this is having interrupts registered.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index d9467e5..1e430d0 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -53,7 +53,7 @@ static int ath10k_pci_post_rx_pipe(struct ath10k_pci_pipe *pipe_info,
static void ath10k_pci_rx_pipe_cleanup(struct ath10k_pci_pipe *pipe_info);
static void ath10k_pci_stop_ce(struct ath10k *ar);
static void ath10k_pci_device_reset(struct ath10k *ar);
-static int ath10k_pci_reset_target(struct ath10k *ar);
+static int ath10k_pci_wait_for_target_init(struct ath10k *ar);
static int ath10k_pci_start_intr(struct ath10k *ar);
static void ath10k_pci_stop_intr(struct ath10k *ar);
@@ -1857,7 +1857,7 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
*/
ath10k_pci_device_reset(ar);
- ret = ath10k_pci_reset_target(ar);
+ ret = ath10k_pci_wait_for_target_init(ar);
if (ret)
goto err_irq;
@@ -2257,7 +2257,7 @@ static void ath10k_pci_stop_intr(struct ath10k *ar)
pci_disable_msi(ar_pci->pdev);
}
-static int ath10k_pci_reset_target(struct ath10k *ar)
+static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int wait_limit = 300; /* 3 sec */
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 05/12] ath10k: make sure to mask all CE irqs
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
CE error interrupts were not disabled. This could
lead to invalid memory accesses / memory
corruption.
Also make sure CE watermark interrupts are also
disabled.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/ce.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 2b99bd4..7a49cde 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -243,6 +243,16 @@ static inline void ath10k_ce_error_intr_enable(struct ath10k *ar,
misc_ie_addr | CE_ERROR_MASK);
}
+static inline void ath10k_ce_error_intr_disable(struct ath10k *ar,
+ u32 ce_ctrl_addr)
+{
+ u32 misc_ie_addr = ath10k_pci_read32(ar,
+ ce_ctrl_addr + MISC_IE_ADDRESS);
+
+ ath10k_pci_write32(ar, ce_ctrl_addr + MISC_IE_ADDRESS,
+ misc_ie_addr & ~CE_ERROR_MASK);
+}
+
static inline void ath10k_ce_engine_int_status_clear(struct ath10k *ar,
u32 ce_ctrl_addr,
unsigned int mask)
@@ -794,6 +804,8 @@ void ath10k_ce_disable_interrupts(struct ath10k *ar)
u32 ctrl_addr = ath10k_ce_base_address(ce_id);
ath10k_ce_copy_complete_intr_disable(ar, ctrl_addr);
+ ath10k_ce_error_intr_disable(ar, ctrl_addr);
+ ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
}
ath10k_pci_sleep(ar);
}
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 06/12] ath10k: fix ath10k_ce_init() failpath
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
Make sure to put target back to sleep. This was a
minor issue as it didn't really matter if we put
target back to sleep at this point. It just looked
wrong.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/ce.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 7a49cde..657e8f2 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1077,7 +1077,7 @@ struct ath10k_ce_pipe *ath10k_ce_init(struct ath10k *ar,
ce_state = ath10k_ce_init_state(ar, ce_id, attr);
if (!ce_state) {
ath10k_err("Failed to initialize CE state for ID: %d\n", ce_id);
- return NULL;
+ goto out;
}
if (attr->src_nentries) {
@@ -1086,7 +1086,8 @@ struct ath10k_ce_pipe *ath10k_ce_init(struct ath10k *ar,
ath10k_err("Failed to initialize CE src ring for ID: %d (%d)\n",
ce_id, ret);
ath10k_ce_deinit(ce_state);
- return NULL;
+ ce_state = NULL;
+ goto out;
}
}
@@ -1096,15 +1097,16 @@ struct ath10k_ce_pipe *ath10k_ce_init(struct ath10k *ar,
ath10k_err("Failed to initialize CE dest ring for ID: %d (%d)\n",
ce_id, ret);
ath10k_ce_deinit(ce_state);
- return NULL;
+ ce_state = NULL;
+ goto out;
}
}
/* Enable CE error interrupts */
ath10k_ce_error_intr_enable(ar, ctrl_addr);
+out:
ath10k_pci_sleep(ar);
-
return ce_state;
}
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 08/12] ath10k: use ath10k_do_pci_wake/sleep
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
This removes some remaining direct use of the wake
register which could interfere with power state
tracking of the target device. This will allow
initialization code reordering.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 84 +++++++++++------------------------
1 file changed, 26 insertions(+), 58 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5a1ac9e..ffc63cc 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -52,7 +52,7 @@ static int ath10k_pci_post_rx_pipe(struct ath10k_pci_pipe *pipe_info,
int num);
static void ath10k_pci_rx_pipe_cleanup(struct ath10k_pci_pipe *pipe_info);
static void ath10k_pci_stop_ce(struct ath10k *ar);
-static void ath10k_pci_device_reset(struct ath10k *ar);
+static int ath10k_pci_device_reset(struct ath10k *ar);
static int ath10k_pci_wait_for_target_init(struct ath10k *ar);
static int ath10k_pci_start_intr(struct ath10k *ar);
static void ath10k_pci_stop_intr(struct ath10k *ar);
@@ -526,21 +526,6 @@ static bool ath10k_pci_target_is_awake(struct ath10k *ar)
return (RTC_STATE_V_GET(val) == RTC_STATE_V_ON);
}
-static int ath10k_pci_wait(struct ath10k *ar)
-{
- int n = 100;
-
- while (n-- && !ath10k_pci_target_is_awake(ar))
- msleep(10);
-
- if (n < 0) {
- ath10k_warn("Unable to wakeup target\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
int ath10k_do_pci_wake(struct ath10k *ar)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -1855,7 +1840,11 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
* is in an unexpected state. We try to catch that here in order to
* reset the Target and retry the probe.
*/
- ath10k_pci_device_reset(ar);
+ ret = ath10k_pci_device_reset(ar);
+ if (ret) {
+ ath10k_err("failed to reset target (%d)\n", ret);
+ goto err_irq;
+ }
ret = ath10k_pci_wait_for_target_init(ar);
if (ret)
@@ -2156,19 +2145,10 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
if (ret < 0)
return ret;
- /*
- * Make sure to wake the Target before enabling Legacy
- * Interrupt.
- */
- iowrite32(PCIE_SOC_WAKE_V_MASK,
- ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
- PCIE_SOC_WAKE_ADDRESS);
-
- ret = ath10k_pci_wait(ar);
+ ret = ath10k_do_pci_wake(ar);
if (ret) {
- ath10k_warn("Failed to enable legacy interrupt, target did not wake up: %d\n",
- ret);
free_irq(ar_pci->pdev->irq, ar);
+ ath10k_err("target didn't wake up (%d)\n", ret);
return ret;
}
@@ -2184,10 +2164,8 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
PCIE_INTR_CE_MASK_ALL,
ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
PCIE_INTR_ENABLE_ADDRESS));
- iowrite32(PCIE_SOC_WAKE_RESET,
- ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
- PCIE_SOC_WAKE_ADDRESS);
+ ath10k_do_pci_sleep(ar);
ath10k_info("legacy interrupt handling\n");
return 0;
}
@@ -2263,15 +2241,9 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
int wait_limit = 300; /* 3 sec */
int ret;
- /* Wait for Target to finish initialization before we proceed. */
- iowrite32(PCIE_SOC_WAKE_V_MASK,
- ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
- PCIE_SOC_WAKE_ADDRESS);
-
- ret = ath10k_pci_wait(ar);
+ ret = ath10k_do_pci_wake(ar);
if (ret) {
- ath10k_warn("Failed to reset target, target did not wake up: %d\n",
- ret);
+ ath10k_err("target didn't wake up (%d)\n", ret);
return ret;
}
@@ -2288,31 +2260,26 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
}
if (wait_limit < 0) {
- ath10k_err("Target stalled\n");
- iowrite32(PCIE_SOC_WAKE_RESET,
- ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
- PCIE_SOC_WAKE_ADDRESS);
- return -EIO;
+ ath10k_err("target stalled\n");
+ ret = -EIO;
+ goto out;
}
- iowrite32(PCIE_SOC_WAKE_RESET,
- ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
- PCIE_SOC_WAKE_ADDRESS);
-
- return 0;
+out:
+ ath10k_do_pci_sleep(ar);
+ return ret;
}
-static void ath10k_pci_device_reset(struct ath10k *ar)
+static int ath10k_pci_device_reset(struct ath10k *ar)
{
- int i;
+ int i, ret;
u32 val;
- ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
- PCIE_SOC_WAKE_V_MASK);
- for (i = 0; i < ATH_PCI_RESET_WAIT_MAX; i++) {
- if (ath10k_pci_target_is_awake(ar))
- break;
- msleep(1);
+ ret = ath10k_do_pci_wake(ar);
+ if (ret) {
+ ath10k_err("failed to reset target - couldn't wake target (%d)\n",
+ ret);
+ return ret;
}
/* Put Target, including PCIe, into RESET. */
@@ -2338,7 +2305,8 @@ static void ath10k_pci_device_reset(struct ath10k *ar)
msleep(1);
}
- ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS, PCIE_SOC_WAKE_RESET);
+ ath10k_do_pci_sleep(ar);
+ return 0;
}
static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 07/12] ath10k: remove meaningless check
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
The check doesn't make much sense. If the address
were to be 0x0000 the check would fail. In this
case a 0 address isn't wrong.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 1e430d0..5a1ac9e 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2307,9 +2307,6 @@ static void ath10k_pci_device_reset(struct ath10k *ar)
int i;
u32 val;
- if (!SOC_GLOBAL_RESET_ADDRESS)
- return;
-
ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
PCIE_SOC_WAKE_V_MASK);
for (i = 0; i < ATH_PCI_RESET_WAIT_MAX; i++) {
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 09/12] ath10k: propagate ath10k_ce_disable_interrupts() errors
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
This shouldn't be silenced. This will be necessary
for PCI init code reordering.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/ce.c | 6 ++++--
drivers/net/wireless/ath/ath10k/ce.h | 2 +-
drivers/net/wireless/ath/ath10k/pci.c | 6 +++++-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 657e8f2..3e8395d 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -792,13 +792,13 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
ath10k_pci_sleep(ar);
}
-void ath10k_ce_disable_interrupts(struct ath10k *ar)
+int ath10k_ce_disable_interrupts(struct ath10k *ar)
{
int ce_id, ret;
ret = ath10k_pci_wake(ar);
if (ret)
- return;
+ return ret;
for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
u32 ctrl_addr = ath10k_ce_base_address(ce_id);
@@ -807,7 +807,9 @@ void ath10k_ce_disable_interrupts(struct ath10k *ar)
ath10k_ce_error_intr_disable(ar, ctrl_addr);
ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
}
+
ath10k_pci_sleep(ar);
+ return ret;
}
void ath10k_ce_send_cb_register(struct ath10k_ce_pipe *ce_state,
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 15d45b5..67dbde6 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -234,7 +234,7 @@ void ath10k_ce_deinit(struct ath10k_ce_pipe *ce_state);
/*==================CE Interrupt Handlers====================*/
void ath10k_ce_per_engine_service_any(struct ath10k *ar);
void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id);
-void ath10k_ce_disable_interrupts(struct ath10k *ar);
+int ath10k_ce_disable_interrupts(struct ath10k *ar);
/* ce_attr.flags values */
/* Use NonSnooping PCIe accesses? */
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index ffc63cc..63ad250 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -879,8 +879,12 @@ static void ath10k_pci_stop_ce(struct ath10k *ar)
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
struct ath10k_pci_compl *compl;
struct sk_buff *skb;
+ int ret;
+
+ ret = ath10k_ce_disable_interrupts(ar);
+ if (ret)
+ ath10k_warn("failed to disable CE interrupts (%d)\n", ret);
- ath10k_ce_disable_interrupts(ar);
ath10k_pci_kill_tasklet(ar);
/* Mark pending completions as aborted, so that upper layers free up
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 10/12] ath10k: guard against CE corruption from firmware
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
In case firmware crashes it may report CE
completions for entries that were never
submitted/filled with meaningful data. This in
turn led to NULL dereferences.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/htc.c | 5 +++++
drivers/net/wireless/ath/ath10k/pci.c | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 3118d75..c59f5b4 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -191,6 +191,11 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
struct ath10k_htc *htc = &ar->htc;
struct ath10k_htc_ep *ep = &htc->endpoint[eid];
+ if (!skb) {
+ ath10k_warn("invalid sk_buff completion - NULL pointer. firmware crashed?");
+ return 0;
+ }
+
ath10k_htc_notify_tx_completion(ep, skb);
/* the skb now belongs to the completion handler */
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 63ad250..43cdc35 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1270,6 +1270,13 @@ static void ath10k_pci_tx_pipe_cleanup(struct ath10k_pci_pipe *pipe_info)
* Indicate the completion to higer layer to free
* the buffer
*/
+
+ if (!netbuf) {
+ ath10k_warn("invalid sk_buff on CE %d - NULL pointer. firmware crashed?",
+ ce_hdl->id);
+ continue;
+ }
+
ATH10K_SKB_CB(netbuf)->is_aborted = true;
ar_pci->msg_callbacks_current.tx_completion(ar,
netbuf,
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH/RFT 11/12] ath10k: re-arrange PCI init code
From: Michal Kazior @ 2013-10-30 11:42 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, greearb, Michal Kazior
In-Reply-To: <1383133346-8135-1-git-send-email-michal.kazior@tieto.com>
This patch moves irq registering after necessary
structures have been allocated and initialized.
This should prevent interrupts from causing
tasklet access invalid memory pointers.
Reported-By: Ben Greear <greearb@candelatech.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/ce.c | 19 +++++++--
drivers/net/wireless/ath/ath10k/ce.h | 1 +
drivers/net/wireless/ath/ath10k/pci.c | 79 +++++++++++++++++++++--------------
3 files changed, 64 insertions(+), 35 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 3e8395d..7517b94 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -792,6 +792,21 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
ath10k_pci_sleep(ar);
}
+int ath10k_ce_enable_err_irq(struct ath10k *ar)
+{
+ int i, ret;
+
+ ret = ath10k_pci_wake(ar);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < CE_COUNT; i++)
+ ath10k_ce_error_intr_enable(ar, ath10k_ce_base_address(i));
+
+ ath10k_pci_sleep(ar);
+ return 0;
+}
+
int ath10k_ce_disable_interrupts(struct ath10k *ar)
{
int ce_id, ret;
@@ -1058,7 +1073,6 @@ struct ath10k_ce_pipe *ath10k_ce_init(struct ath10k *ar,
const struct ce_attr *attr)
{
struct ath10k_ce_pipe *ce_state;
- u32 ctrl_addr = ath10k_ce_base_address(ce_id);
int ret;
/*
@@ -1104,9 +1118,6 @@ struct ath10k_ce_pipe *ath10k_ce_init(struct ath10k *ar,
}
}
- /* Enable CE error interrupts */
- ath10k_ce_error_intr_enable(ar, ctrl_addr);
-
out:
ath10k_pci_sleep(ar);
return ce_state;
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 67dbde6..8a58bac 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -235,6 +235,7 @@ void ath10k_ce_deinit(struct ath10k_ce_pipe *ce_state);
void ath10k_ce_per_engine_service_any(struct ath10k *ar);
void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id);
int ath10k_ce_disable_interrupts(struct ath10k *ar);
+int ath10k_ce_enable_err_irq(struct ath10k *ar);
/* ce_attr.flags values */
/* Use NonSnooping PCIe accesses? */
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 43cdc35..7b606d0 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1786,18 +1786,6 @@ static int ath10k_pci_ce_init(struct ath10k *ar)
pipe_info->buf_sz = (size_t) (attr->src_sz_max);
}
- /*
- * Initially, establish CE completion handlers for use with BMI.
- * These are overwritten with generic handlers after we exit BMI phase.
- */
- pipe_info = &ar_pci->pipe_info[BMI_CE_NUM_TO_TARG];
- ath10k_ce_send_cb_register(pipe_info->ce_hdl,
- ath10k_pci_bmi_send_done, 0);
-
- pipe_info = &ar_pci->pipe_info[BMI_CE_NUM_TO_HOST];
- ath10k_ce_recv_cb_register(pipe_info->ce_hdl,
- ath10k_pci_bmi_recv_data);
-
return 0;
}
@@ -1830,17 +1818,27 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
ath10k_pci_sleep(ar);
}
+static void ath10k_pci_start_bmi(struct ath10k *ar)
+{
+ struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+ struct ath10k_pci_pipe *pipe;
+
+ /*
+ * Initially, establish CE completion handlers for use with BMI.
+ * These are overwritten with generic handlers after we exit BMI phase.
+ */
+ pipe = &ar_pci->pipe_info[BMI_CE_NUM_TO_TARG];
+ ath10k_ce_send_cb_register(pipe->ce_hdl, ath10k_pci_bmi_send_done, 0);
+
+ pipe = &ar_pci->pipe_info[BMI_CE_NUM_TO_HOST];
+ ath10k_ce_recv_cb_register(pipe->ce_hdl, ath10k_pci_bmi_recv_data);
+}
+
static int ath10k_pci_hif_power_up(struct ath10k *ar)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int ret;
- ret = ath10k_pci_start_intr(ar);
- if (ret) {
- ath10k_err("could not start interrupt handling (%d)\n", ret);
- goto err;
- }
-
/*
* Bring the target up cleanly.
*
@@ -1854,13 +1852,9 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
ret = ath10k_pci_device_reset(ar);
if (ret) {
ath10k_err("failed to reset target (%d)\n", ret);
- goto err_irq;
+ goto err;
}
- ret = ath10k_pci_wait_for_target_init(ar);
- if (ret)
- goto err_irq;
-
if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
/* Force AWAKE forever */
ath10k_do_pci_wake(ar);
@@ -1869,25 +1863,48 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
if (ret)
goto err_ps;
+ ret = ath10k_ce_disable_interrupts(ar);
+ if (ret) {
+ ath10k_err("could not disable CE interrupts (%d)\n", ret);
+ goto err_ce;
+ }
+
+ ret = ath10k_pci_start_intr(ar);
+ if (ret) {
+ ath10k_err("could not start interrupt handling (%d)\n", ret);
+ goto err_ce;
+ }
+
+ ret = ath10k_pci_wait_for_target_init(ar);
+ if (ret)
+ goto err_irq;
+
+ ret = ath10k_ce_enable_err_irq(ar);
+ if (ret)
+ goto err_irq;
+
ret = ath10k_pci_init_config(ar);
if (ret)
- goto err_ce;
+ goto err_irq;
ret = ath10k_pci_wake_target_cpu(ar);
if (ret) {
ath10k_err("could not wake up target CPU (%d)\n", ret);
- goto err_ce;
+ goto err_irq;
}
+ ath10k_pci_start_bmi(ar);
return 0;
+err_irq:
+ ath10k_ce_disable_interrupts(ar);
+ ath10k_pci_stop_intr(ar);
+ ath10k_pci_kill_tasklet(ar);
err_ce:
ath10k_pci_ce_deinit(ar);
err_ps:
if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
ath10k_do_pci_sleep(ar);
-err_irq:
- ath10k_pci_stop_intr(ar);
err:
return ret;
}
@@ -2156,7 +2173,7 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
if (ret < 0)
return ret;
- ret = ath10k_do_pci_wake(ar);
+ ret = ath10k_pci_wake(ar);
if (ret) {
free_irq(ar_pci->pdev->irq, ar);
ath10k_err("target didn't wake up (%d)\n", ret);
@@ -2176,7 +2193,7 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
PCIE_INTR_ENABLE_ADDRESS));
- ath10k_do_pci_sleep(ar);
+ ath10k_pci_sleep(ar);
ath10k_info("legacy interrupt handling\n");
return 0;
}
@@ -2252,7 +2269,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
int wait_limit = 300; /* 3 sec */
int ret;
- ret = ath10k_do_pci_wake(ar);
+ ret = ath10k_pci_wake(ar);
if (ret) {
ath10k_err("target didn't wake up (%d)\n", ret);
return ret;
@@ -2277,7 +2294,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
}
out:
- ath10k_do_pci_sleep(ar);
+ ath10k_pci_sleep(ar);
return ret;
}
--
1.8.4.rc3
^ 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