Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-02-10 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
Mohammed Shafi Shajakhan  writes:

> the change suggested by you helps, and the device probe, scan
> is successful as well. Still good to have this change part of your
> basic sanity and regression testing !

Sure. I was sort of expecting that you would send v4 but I haven't seen
one so I guess you assumed I send that? :) I'll then submit v4.

[shafi] thanks Kalle, just saw your patch.

regards,
shafi

Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-02-06 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
Hi,

even with the below patch applied ?
https://patchwork.kernel.org/patch/9452265/

regards
shafi

From: Michael Ney <n...@vorklift.com>
Sent: 06 February 2017 17:46
To: Mohammed Shafi Shajakhan
Cc: Valo, Kalle; linux-wireless@vger.kernel.org; ath...@lists.infradead.org; 
Shajakhan, Mohammed Shafi (Mohammed Shafi)
Subject: Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

Symmetry is still broken on firmware crash (at least with 6174). 
ath10k_pci_hif_stop gets called twice, once from the driver restart (warm 
restart) and once from ieee80211 start (cold restart), resulting in 
napi_synchrionize/napi_disable getting called twice and sticking the driver in 
an infinite wait loop (napi_synchronize waits until NAPI_STATE_SCHED is off, 
while napi_disable leaves NAPI_STATE_SCHED to on when leaving).


> On Feb 6, 2017, at 5:04 AM, Mohammed Shafi Shajakhan 
> <moham...@codeaurora.org> wrote:
>
> Hi Kalle,
>
> the change suggested by you helps, and the device probe, scan
> is successful as well. Still good to have this change part of your
> basic sanity and regression testing !
>
> regards,
> shafi
>
> On Wed, Jan 25, 2017 at 01:46:28PM +, Valo, Kalle wrote:
>> Kalle Valo <kv...@qca.qualcomm.com> writes:
>>
>>> Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com> writes:
>>>
>>>> From: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
>>>>
>>>> This fixes the below crash when ath10k probe firmware fails,
>>>> NAPI polling tries to access a rx ring resource which was never
>>>> allocated, fix this by disabling NAPI right away once the probe
>>>> firmware fails by calling 'ath10k_hif_stop'. Its good to note
>>>> that the error is never propogated to 'ath10k_pci_probe' when
>>>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
>>>> PCI related things seems to be ok
>>>>
>>>> BUG: unable to handle kernel NULL pointer dereference at (null)
>>>> IP:  __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
>>>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
>>>>
>>>> Call Trace:
>>>>
>>>> [] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
>>>> [ath10k_core]
>>>> [] ath10k_htt_txrx_compl_task+0x433/0x17d0
>>>> [ath10k_core]
>>>> [] ? __wake_up_common+0x4d/0x80
>>>> [] ? cpu_load_update+0xdc/0x150
>>>> [] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
>>>> [] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
>>>> [] net_rx_action+0x20f/0x370
>>>>
>>>> Reported-by: Ben Greear <gree...@candelatech.com>
>>>> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
>>>> Signed-off-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
>>>
>>> Is there an easy way to reproduce this bug? I don't see it on my x86
>>> laptop with qca988x and I call rmmod all the time. I would like to test
>>> this myself.
>>>
>>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
>>>>ath10k_core_free_firmware_files(ar);
>>>>
>>>> err_power_down:
>>>> +  ath10k_hif_stop(ar);
>>>>ath10k_hif_power_down(ar);
>>>>
>>>>return ret;
>>>
>>> This breaks the symmetry, we should not be calling ath10k_hif_stop() if
>>> we haven't called ath10k_hif_start() from the same function. This can
>>> just create a bigger mess later, for example with other bus support like
>>> sdio or usb. In theory it should enough that we call
>>> ath10k_hif_power_down() and pci.c does the rest correctly "behind the
>>> scenes".
>>>
>>> I investigated this a bit and I think the real cause is that we call
>>> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from
>>> ath10k_pci_hif_stop(). Does anyone remember why?
>>>
>>> I was expecting that we would call napi_enable()/napi_disable() either
>>> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not
>>> mixed like it's currently.
>>
>> So below is something I was thinking of, now napi_enable() is called
>> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would
>> that work?
>>
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
>>
>>  ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
>>
>> +napi_enable(>napi);
>> +
>>  ath10k_pci_irq_enable(ar);
>>  ath10k_pci_rx_post(ar);
>>
>> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
>>  ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
>>  goto err_ce;
>>  }
>> -napi_enable(>napi);
>>
>>  return 0;
>>
>> --
>> Kalle Valo
>
> ___
> ath10k mailing list
> ath...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v4] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

2016-09-27 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
apologies please ignore this , wrong patch due to confusing v4 tag
 
please review,

[PATCH] ath10k: Cleanup calling ath10k_htt_rx_h_unchain


From: Shajakhan, Mohammed Shafi (Mohammed Shafi)
Sent: 27 September 2016 18:31
To: ath...@lists.infradead.org
Cc: moham...@codeaurora.org; linux-wireless@vger.kernel.org; Shajakhan, 
Mohammed Shafi (Mohammed Shafi)
Subject: [PATCH v4] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

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

'ath10k_htt_rx_h_unchain' is need to be called only if the return
value from 'ath10k_htt_rx_amsdu_pop' is 1('chained msdu's'), this
change makes it more explicit and avoids doing a skb_peek, fetching
rx descriptor pointer, checking rx msdu decap format for the case of
ret = 0 (unchained msdus). Found this change during code walk through,
not sure if this addresses any issue.

Signed-off-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7ae9349..e51dace 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1459,8 +1459,7 @@ static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
 }

 static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
-   struct sk_buff_head *amsdu,
-   bool chained)
+   struct sk_buff_head *amsdu)
 {
struct sk_buff *first;
struct htt_rx_desc *rxd;
@@ -1471,9 +1470,6 @@ static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
decap = MS(__le32_to_cpu(rxd->msdu_start.common.info1),
   RX_MSDU_START_INFO1_DECAP_FORMAT);

-   if (!chained)
-   return;
-
/* FIXME: Current unchaining logic can only handle simple case of raw
 * msdu chaining. If decapping is other than raw the chaining may be
 * more complex and this isn't handled by the current code. Don't even
@@ -1550,7 +1546,11 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt 
*htt)
}

ath10k_htt_rx_h_ppdu(ar, , rx_status, 0x);
-   ath10k_htt_rx_h_unchain(ar, , ret > 0);
+
+   /* only for ret = 1 indicates chained msdus */
+   if (ret > 0)
+   ath10k_htt_rx_h_unchain(ar, );
+
ath10k_htt_rx_h_filter(ar, , rx_status);
ath10k_htt_rx_h_mpdu(ar, , rx_status);
ath10k_htt_rx_h_deliver(ar, , rx_status);
--
1.9.1

RE: [PATCH] ath10k: Fix a typo in spectral code commments

2016-06-16 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
+ linux wireless, ath10k

Thanks, initially I could not get what you meant :(

-Original Message-
From: Arthur Jagiella [mailto:jagie...@vitracom.de] 
Sent: Tuesday, June 14, 2016 2:06 PM
To: Shajakhan, Mohammed Shafi (Mohammed Shafi) <moham...@qti.qualcomm.com>
Subject: Re: [PATCH] ath10k: Fix a typo in spectral code commments

smaples?



Am 14.06.2016 um 07:33 schrieb Mohammed Shafi Shajakhan:
> From: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
>
> Found this obvious typo while going through the spectral code design 
> in ath10k
>
> Signed-off-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath10k/spectral.c |2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/spectral.c 
> b/drivers/net/wireless/ath/ath10k/spectral.c
> index 4671cfb..4510906 100644
> --- a/drivers/net/wireless/ath/ath10k/spectral.c
> +++ b/drivers/net/wireless/ath/ath10k/spectral.c
> @@ -101,7 +101,7 @@ int ath10k_spectral_process_fft(struct ath10k *ar,
>   break;
>   case 80:
>   /* TODO: As experiments with an analogue sender and various
> -  * configuaritions (fft-sizes of 64/128/256 and 20/40/80 Mhz)
> +  * configurations (fft-sizes of 64/128/256 and 20/40/80 Mhz)
>* show, the particular configuration of 80 MHz/64 bins does
>* not match with the other smaples at all. Until the reason
>* for that is found, don't report these samples.

--
Arthur Jagiella
Praktikant
Forschung und Entwicklung
Vitracom AG

Erbprinzenstr. 4-12, A
D-76133 Karlsruhe

Email: jagie...@vitracom.de
Internet: www.vitracom.de




HRB Mannheim 111263
UST-ID: DE 247872702
Vorstand: Axel Stephan,
   Ralph Majer
Aufsichtsrat: Paul A. Stodden (Vors.)
   Christian Eckart, Sacha Grau


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] ath10k: Fix 10.4 extended peer stats update

2016-05-31 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
Hi Sven / Kalle,

seems the change got merged in pending branch
https://git.kernel.org/cgit/linux/kernel/git/kvalo/ath.git/commit/?h=master-pending=2a872321e07506e339fc1b3ca81aa3e88628cd6e

Request if you can send the patch to address this(incase the patch cannot be 
dropped)
and the v2 version cannot be picked

regards,
shafi

From: Mohammed Shafi Shajakhan <moham...@codeaurora.org>
Sent: 27 May 2016 14:36
To: Sven Eckelmann
Cc: Shajakhan, Mohammed Shafi (Mohammed Shafi); ath...@lists.infradead.org; 
linux-wireless@vger.kernel.org; Valo, Kalle
Subject: Re: [PATCH v1] ath10k: Fix 10.4 extended peer stats update

On Fri, May 27, 2016 at 10:12:45AM +0200, Sven Eckelmann wrote:
> On Wednesday 11 May 2016 11:54:30 Mohammed Shafi Shajakhan wrote:
> >  #else
> > -static inline void ath10k_sta_update_rx_duration(struct ath10k *ar,
> > -struct list_head *peer)
> > +static inline
> > +void ath10k_sta_update_rx_duration(struct ath10k *ar,
> > +  struct ath10k_fw_stats *stats);
> >  {
> >  }
>
> Please remove the ";" for the "static inline" version. I can also
> submit a patch in case patches in ath.git/pending cannot be dropped
> anymore.

[shafi] thanks a lot fixed it ! sorry for the typo, sent a v2 for
the same, kindly review the same.

>
> Kind regards,
>   Sven


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ath10k: Fix return value for btcoex and peer stats debugfs

2016-04-19 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
Applied, thanks.

I removed the line wrapping from "Fixes:" lines.

[shafi] thank you Kalle.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 3/3] ath10k: Enable parsing per station rx duration for 10.4

2016-03-23 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi) 
Hi Kalle,

I will make sure, I will run sparse before sending it for review
http://linuxwireless.org/en/users/Drivers/ath10k/CodingStyle/#Linux_style

make drivers/net/wireless/ath/ath10k/ C=2 CF="-D__CHECK_ENDIAN__"

regret the inconvenience so caused  (including the compilation error)

regards,
shafi

-Original Message-
From: Mohammed Shafi Shajakhan [mailto:moham...@codeaurora.org] 
Sent: Wednesday, March 23, 2016 6:34 PM
To: Valo, Kalle <kv...@qca.qualcomm.com>
Cc: Shajakhan, Mohammed Shafi (Mohammed Shafi) <moham...@qti.qualcomm.com>; 
ath...@lists.infradead.org; linux-wireless@vger.kernel.org
Subject: Re: [PATCH v1 3/3] ath10k: Enable parsing per station rx duration for 
10.4

Hi Kalle,

On Wed, Mar 23, 2016 at 01:00:01PM +, Valo, Kalle wrote:
> Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com> writes:
> 
> > From: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
> >
> > Rx duration support for per station is part of extended peer stats, 
> > enable provision to parse the same and provide backward 
> > compatibility based on the 'stats_id' event
> >
> > Signed-off-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
> 
> There was a new sparse warning:
> 
> drivers/net/wireless/ath/ath10k/wmi.c:2978:42: warning: incorrect type in 
> assignment (different base types)
> drivers/net/wireless/ath/ath10k/wmi.c:2978:42:expected unsigned int 
> [unsigned] [usertype] rx_duration
> drivers/net/wireless/ath/ath10k/wmi.c:2978:42:got restricted __le32 const 
> [usertype] rx_duration
> 
> I fixed it like this in the pending branch, please double check:

[shafi] thanks for fixing this, sorry i missed this.

> 
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -2975,7 +2975,7 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct 
> ath10k *ar,
> ath10k_wmi_10_4_pull_peer_stats(>common, dst);
> /* FIXME: expose 10.4 specific values */
> if (extd_peer_stats)
> -   dst->rx_duration = src->rx_duration;
> +   dst->rx_duration = 
> + __le32_to_cpu(src->rx_duration);
>  
> list_add_tail(>list, >peers);
> }
>
regards,
shafi
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html