Re: iwlwifi: mvm: set the TX disable bit when doing a chanctx switch

[Coelho, Luciano]

Hi Dan,

Sorry for the late reply.

On Tue, 2014-09-09 at 12:40 +0300, Dan Carpenter wrote:
 Hello Luciano Coelho,
 
 The patch 0ce04ce797f8: iwlwifi: mvm: set the TX disable bit when
 doing a chanctx switch from May 8, 2014, leads to the following
 static checker warning:
 
   drivers/net/wireless/iwlwifi/mvm/mac80211.c:2768 
 __iwl_mvm_assign_vif_chanctx()
   warn: missing error code here? 'iwl_mvm_sta_from_staid_protected()' 
 failed. 'ret' = '0'
 
 drivers/net/wireless/iwlwifi/mvm/mac80211.c
   2761  if (vif-csa_active  vif-type == NL80211_IFTYPE_STATION) {
   2762  struct iwl_mvm_sta *mvmsta;
   2763  
   2764  mvmsta = iwl_mvm_sta_from_staid_protected(mvm,
   2765
 mvmvif-ap_sta_id);
   2766  
   2767  if (WARN_ON(!mvmsta))
   2768  goto out;
 
 Did you want to set an error code here?  I don't know the code well
 enough to say.

Good catch!

This gets fixed in another patch I implemented (that Emmanuel is
probably going to send out soon) where I move the code to another
function.

Since this should not really happen in normal cases, and since this will
be fixed very soon anyway, we could probably leave this patch as it is.

The patch that fixes this is called iwlwifi: mvm: finalize on
post_switch instead of unassign and you should see it coming soon to a
theater near you. ;)

Thanks for running your cool scripts! :)

--
Cheers,
Luca.
N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj��!�i

Re: iwlwifi/iwlmvm dies on resume when rfkill is set

[Coelho, Luciano]

On Mon, 2015-06-01 at 23:30 +, Grumbach, Emmanuel wrote:
 On Mon, 2015-06-01 at 15:51 -0700, Andy Lutomirski wrote:
  If I switch my laptop into airplane mode (hardware switch) and then
  suspend and resume, my wireless is dead on resume and it spews all
  over the kernel log (see below).  I can rescue it by switching
  airplane mode off and then suspending and resuming again.
  
 
 This one is tracked here:
 https://bugzilla.kernel.org/show_bug.cgi?id=98591
 
 
  This problem has existed on all kernel versions I've tried, although I
  haven't tried a 4.0 kernel yet.
  
 
 This is a tricky HW timing issues which (as you can imagine) we can't
 reproduce internally.
 You seem to have a LENOVO whereas the submitted of the bugzilla above
 has some other OEM.
 We are currently checking with our System people (who know the HW) how
 we should address that.

Right, I'm investigating this.  The problem seems to be a quick toggle
of the rfkill interrupt that may be caused by the way we reset the
device when resuming.  I have involved our System and HW people and I'll
let you know how it is going as soon as I have more info.

Thanks for reporting!

--
Luca.
N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj��!�i

Re: [PATCH] iwlwifi:Fix incorrect fallthrough in switch statement in the function iwl_mvm_check_running_scans

[Coelho, Luciano]

On Tue, 2015-09-22 at 20:24 -0400, Nicholas Krause wrote:
> This fixes incorrect fallthrough in the switch statment checking
> the scan type passed by the caller to iwl_mvm_check_running_scans
> for the switch case IWL_MVM_SCAN_SCHED to return directly after
> the call to iwl_mvm_scan_stop in order to avoid fallthrough into
> the next case and incorrectly return zero indicating success to
> the caller even if the function call to iwl_mvm_scan_stop fails.
> 
> Signed-off-by: Nicholas Krause 
> ---

Applied to our internal tree with a modified commit message.

Thanks!

--
Luca.

Re: wireless-testing on 4.7

[Coelho, Luciano]

On Wed, 2016-06-01 at 16:08 -0600, Reinoud Koornstra wrote:
> On Wed, Jun 1, 2016 at 7:19 AM, Coelho, Luciano
> <luciano.coe...@intel.com> wrote:
> > On Wed, 2016-06-01 at 08:54 -0400, Bob Copeland wrote:
> > > + Luca, Emmanuel
> > 
> > Thanks, Bob!
> > 
> > 
> > > On Tue, May 31, 2016 at 10:06:57PM -0600, Reinoud Koornstra
> > > wrote:
> > > > Today I compiled 4.6+ and pulled sources today
> > > > iwlwifi isn't super smooth.
> > > 
> > > I assume you mean wireless-testing, based on 4.7-rc1 (as this
> > > email
> > > is
> > > in reply to my announcement of same).
> > 
> > Yes, we need to know exactly what kernel you're using so we know
> > what
> > we're debugging.
> > 
> 
> Yesterday this is what I did to obtain the latest source:
> 
> git clone
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> kernel_current
> 
> This is the latest commit in that tree:
> 
> commit 367d3fd50566a313946fa9c5b2116a81bf3807e4
> Merge: 5eca831 cf0d44d
> Author: Linus Torvalds <torva...@linux-foundation.org>
> Date:   Tue May 31 09:43:24 2016 -0700
> 
>    Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> 
>    Pull s390 fixes from Martin Schwidefsky:
> "Three bugs fixes and an update for the default configuration"
> 
>    * 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux:
>  s390: fix info leak in do_sigsegv
>  s390/config: update default configuration
>  s390/bpf: fix recache skb->data/hlen for skb_vlan_push/pop
>  s390/bpf: reduce maximum program size to 64 KB

Ah, okay.  This has nothing to do with the wireless-testing tree.  This
is random commit in Linus' tree.  I suggest that you use a release tag
or such.  For instance you could do this in Linus' tree to get the
current release candidate for 4.7:

git checkout v4.7-rc1

Or you could use the wireless-testing tree that Bob is maintaining,
which is always based on an official release candidate (currently the
above mentioned v4.7-rc1 release) plus the latest and greatest (and
probably "brokenest" :P) wireless changes:

git clone 
git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-testing.git


> > > > 
> > > > [  112.345457] wlp4s0: authenticate with 00:30:44:1d:cf:2b
> > > > [  112.349002] wlp4s0: send auth to 00:30:44:1d:cf:2b (try 1/3)
> > > > [  112.349647] wlp4s0: authenticated
> > > > [  112.352456] wlp4s0: associate with 00:30:44:1d:cf:2b (try
> > > > 1/3)
> > > > [  112.353764] wlp4s0: RX AssocResp from 00:30:44:1d:cf:2b
> > > > (capab=0x11
> > > > status=0 aid=3)
> > > > [  112.355035] wlp4s0: associated
> > > > [  112.355393] IPv6: ADDRCONF(NETDEV_CHANGE): wlp4s0: link
> > > > becomes
> > > > ready
> > > > [  118.616327] wlp4s0: deauthenticated from 00:30:44:1d:cf:2b
> > > > (Reason:
> > > > 15=4WAY_HANDSHAKE_TIMEOUT)
> > > > [  128.673812] wlp4s0: authenticate with 00:30:44:1d:cf:2b
> > > > [  128.676997] wlp4s0: send auth to 00:30:44:1d:cf:2b (try 1/3)
> > > > [  128.677648] wlp4s0: authenticated
> > > > [  128.678691] wlp4s0: associate with 00:30:44:1d:cf:2b (try
> > > > 1/3)
> > > > [  128.679807] wlp4s0: RX AssocResp from 00:30:44:1d:cf:2b
> > > > (capab=0x11
> > > > status=0 aid=3)
> > > > [  128.681198] wlp4s0: associated
> > > > 
> > > > I never experienced the 4 way handshake 802.11i timeout in my
> > > > home
> > > > setup.
> > > > With other setups plenty of times, especially with mediatek
> > > > drivers. :)
> > > > If you wish I can sniff and see which side is at fault.
> > > 
> > > I have not noticed any problems on my laptop with iwldvm on a
> > > 6205,
> > > with
> > > latest wireless-testing.  But maybe intel devs can weigh in.
> > 
> > I'm not going to put all my weight in lest I break things even
> > more. :P
> > 
> > I can't really see much from the log extract you posted.  Can you
> > get
> > some logs (especially trace-cmd and a sniffer capture) and report a
> > bug
> > in bugzilla? We have some instructions on how to do it here:
> > 
> > https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/debugging
> > 
> 
> Ok, will do so, thanks,

Great, thanks! Please use one of the more-well-known trees/tags I
mentioned above and report if you still have trouble.

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: wireless-testing on 4.7

[Coelho, Luciano]

On Wed, 2016-06-01 at 08:54 -0400, Bob Copeland wrote:
> + Luca, Emmanuel

Thanks, Bob!


> On Tue, May 31, 2016 at 10:06:57PM -0600, Reinoud Koornstra wrote:
> > Today I compiled 4.6+ and pulled sources today
> > iwlwifi isn't super smooth.
> 
> I assume you mean wireless-testing, based on 4.7-rc1 (as this email
> is
> in reply to my announcement of same).

Yes, we need to know exactly what kernel you're using so we know what
we're debugging.

> > 
> > [  112.345457] wlp4s0: authenticate with 00:30:44:1d:cf:2b
> > [  112.349002] wlp4s0: send auth to 00:30:44:1d:cf:2b (try 1/3)
> > [  112.349647] wlp4s0: authenticated
> > [  112.352456] wlp4s0: associate with 00:30:44:1d:cf:2b (try 1/3)
> > [  112.353764] wlp4s0: RX AssocResp from 00:30:44:1d:cf:2b
> > (capab=0x11
> > status=0 aid=3)
> > [  112.355035] wlp4s0: associated
> > [  112.355393] IPv6: ADDRCONF(NETDEV_CHANGE): wlp4s0: link becomes
> > ready
> > [  118.616327] wlp4s0: deauthenticated from 00:30:44:1d:cf:2b
> > (Reason:
> > 15=4WAY_HANDSHAKE_TIMEOUT)
> > [  128.673812] wlp4s0: authenticate with 00:30:44:1d:cf:2b
> > [  128.676997] wlp4s0: send auth to 00:30:44:1d:cf:2b (try 1/3)
> > [  128.677648] wlp4s0: authenticated
> > [  128.678691] wlp4s0: associate with 00:30:44:1d:cf:2b (try 1/3)
> > [  128.679807] wlp4s0: RX AssocResp from 00:30:44:1d:cf:2b
> > (capab=0x11
> > status=0 aid=3)
> > [  128.681198] wlp4s0: associated
> > 
> > I never experienced the 4 way handshake 802.11i timeout in my home
> > setup.
> > With other setups plenty of times, especially with mediatek
> > drivers. :)
> > If you wish I can sniff and see which side is at fault.
> 
> I have not noticed any problems on my laptop with iwldvm on a 6205,
> with
> latest wireless-testing.  But maybe intel devs can weigh in.

I'm not going to put all my weight in lest I break things even more. :P

I can't really see much from the log extract you posted.  Can you get
some logs (especially trace-cmd and a sniffer capture) and report a bug
in bugzilla? We have some instructions on how to do it here:

https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/debugging

Thanks for reporting!

--
Cheers,
Luca.

Re: pull request: iwlwifi-2016-02-16

[Coelho, Luciano]

Hi Arend,

On Fri, 2016-03-18 at 20:52 +0100, Arend Van Spriel wrote:
> On 15-2-2016 12:57, Grumbach, Emmanuel wrote:
> > 
> > * Luca fixes a very visible bug in scheduled scan: our firmware
> >   doesn't support scheduled scan with no profile configured and
> >   the supplicant sometimes requests such scheduled scans.
> Hi Luca,
> 
> I have seen this happening as well, but did not find the exact
> scenario
> in which supplicant does this. Would you happen to know?

I don't remember exactly what was the scenario, but I think it was when
wpa_s had a single AP configure and failed to connect to it (and
blacklisted it).  Then it would try a couple of regular scans with
broadcast SSID then switch to sched scan.

I'll dig the bug up next week so I can tell you for sure.

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH] iwlwifi: fix fw version reading for DVM devices

[Coelho, Luciano]

On Mon, 2016-04-25 at 20:02 +0300, Luca Coelho wrote:
> From: Luca Coelho 
> 
> In commit 97f95c93c8ed ("iwlwifi: remove support for fw older than
> -16.ucode") we accidentally changed the fw version reading code for
> DVM devices.  The code intended to remove the old fw version API,
> because all MVM firmwares version 16 and above that we support don't
> use it anymore.  But DVM devices still use the old FW API.
> 
> Fix that by bringing the code back in.
> 
> Reported-by: Pat Erley 
> Fixes: 97f95c93c8ed ("iwlwifi: remove support for fw older than-
> 16.ucode")
> Signed-off-by: Luca Coelho 
> --- 

Kalle, I forgot to add your Tested-by.  Feel free to add it if you want
(as if I have the authority of saying this to the maintainer :P).

--
Cheers,
Luca.

Re: [PATCH 1/1] iwlwifi: rs: remove superfluous check

[Coelho, Luciano]

On Wed, 2016-05-18 at 01:31 +0200, Heinrich Schuchardt wrote:
> If we dereference a variable anyway in other parts of the code,
> there is no need to check against NULL in a single place.

NACK.  This is not true.

If lq_sta is NULL, it means that mvm_sta is also NULL.  Then we call
the rate_control_send with mvm_sta == NULL:

if (rate_control_send_low(sta, mvm_sta, txrc))
return;

The rate_control_send_low() function looks like this:


bool rate_control_send_low(struct ieee80211_sta *pubsta,
   void *priv_sta,
   struct ieee80211_tx_rate_control *txrc)
{
[...]
if (!pubsta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {
[...]
return true;
}
[...]
}


Which means that if priv_sta (aka mvm_sta) is NULL, we will return
without running the rest of rs_get_rate() where lq_sta is accessed
without checks.

I admit that the rs_get_rate() function is a bit hard to read, but
removing the lq_sta check as you did doesn't help, but makes things
worse.

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2016-05-18 at 11:45 -0700, Linus Torvalds wrote:
> On Wed, May 18, 2016 at 7:23 AM, Coelho, Luciano
> <luciano.coe...@intel.com> wrote:
> > 
> > 
> > I can confirm that 4.6 contains the same bug.  And reverting the
> > patch
> > I mentioned does solve the problem...
> > 
> > The same patch works fine in our internal tree.  I'll have to
> > figure
> > out together with Emmanuel what the problem actually is.
> Hmm.
> 
> From what I can tell, there's a merge bug in commit 909b27f70643,
> where David seems to have lost some of the changes to
> iwl_mvm_set_tx_cmd().
> 
> The reason seems to be a conflict with d8fe484470dd, where David
> missed the fact that "info->driver_data[0]" had become
> "skb_info->driver_data[0]", and then he removed the skb_info because
> it was unused.
> 
> I do not know if that's the reason for the problem I see. But I will
> test.
> 
> David, do you happen to recall that merge conflict? I think you must
> have removed that "skb_info" variable declaration and initialization
> manually (due to the "unused variable" warning, which in turn was due
> to the incorrect merge of the actual conflict), because I think git
> would have merged that line into the result.

Actually I just tested it and indeed it seems to be the merge damage
(which we discussed extensively in the linux-wireless mailing list)
that causes this problem.  The "4.6 doesn't work either" thing was a
false alarm.

If the merge damage is fixed this way, the problem is gone:

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index b5f7c36..ae2ecf6 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -211,6 +211,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct
sk_buff *skb,
struct iwl_tx_cmd *tx_cmd,
struct ieee80211_tx_info *info, u8 sta_id)
 {
+   struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb);
struct ieee80211_hdr *hdr = (void *)skb->data;
__le16 fc = hdr->frame_control;
u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags);
@@ -294,7 +295,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct
sk_buff *skb,
tx_cmd->tx_flags = cpu_to_le32(tx_flags);
/* Total # bytes to be transmitted */
tx_cmd->len = cpu_to_le16((u16)skb->len +
-   (uintptr_t)info->driver_data[0]);
+   (uintptr_t)skb_info->driver_data[0]);
tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
tx_cmd->sta_id = sta_id;


Kalle, David, what is the status with the fix that is on the way via
your trees?

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2016-05-18 at 12:00 -0700, Linus Torvalds wrote:
> On Wed, May 18, 2016 at 11:58 AM, Kalle Valo 
> wrote:
> > 
> > 
> > It would be best if you could send a patch either directly to Dave
> > or
> > Linus to resolve this quickly.
> I'm committing my patch myself right now, since this bug makes my
> laptop useless, and I will take credit for finding and testing it on
> my own even if it was apparently also discussed independently on the
> networking list ;)

Great! :)

You beat me by a few minutes, even though I had the whole day to play
with it. :\

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: linux-next: manual merge of the wireless-drivers-next tree with the net-next tree

[Coelho, Luciano]

Hi Kalle,

On Mon, 2016-05-16 at 16:10 +0300, Kalle Valo wrote:
> (Adding Luca and linux-wireless)
> 
> Stephen Rothwell  writes:
> 
> > 
> > Today's linux-next merge of the wireless-drivers-next tree got a
> > conflict in:
> > 
> >   drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> > 
> > between commit:
> > 
> >   909b27f70643 ("Merge
> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
> > 
> > from the net-next tree and commit:
> > 
> >   a525d0eab17d ("Merge tag 'iwlwifi-for-kalle-2016-05-04' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-
> > fixes")
> > 
> > from the wireless-drivers-next tree.
> > 
> > I fixed it up (I think that the net-next tree merge lost the
> > changes
> > to iwl_mvm_set_tx_cmd() associated with commit 5c08b0f5026f
> > ("iwlwifi:
> > mvm: don't override the rate with the AMSDU len")) and can carry
> > the
> > fix as necessary.
> Hmm, I'm starting to suspect something is wrong. I did a test merge
> of
> net-next and wireless-drivers-next and got this as a diff after the
> merge:
> 
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> @@ -211,7 +211,6 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm,
> struct sk_buff *skb,
> struct iwl_tx_cmd *tx_cmd,
> struct ieee80211_tx_info *info, u8 sta_id)
>  {
> -   struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb);
> struct ieee80211_hdr *hdr = (void *)skb->data;
> __le16 fc = hdr->frame_control;
> u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags);
> @@ -295,7 +294,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm,
> struct sk_buff *skb,
> tx_cmd->tx_flags = cpu_to_le32(tx_flags);
> /* Total # bytes to be transmitted */
> tx_cmd->len = cpu_to_le16((u16)skb->len +
> -   (uintptr_t)skb_info->driver_data[0]);
> +   (uintptr_t)info->driver_data[0]);
> tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
> tx_cmd->sta_id = sta_id;
> 
> But commit 5c08b0f5026f ("iwlwifi: mvm: don't override the rate with
> the
> AMSDU len") specifically added skb_info variable to that function:
> 
> @@ -105,6 +105,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm,
> struct sk_buff *skb,
> struct iwl_tx_cmd *tx_cmd,
> struct ieee80211_tx_info *info, u8 sta_id)
>  {
> +   struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb);
> struct ieee80211_hdr *hdr = (void *)skb->data;
> __le16 fc = hdr->frame_control;
> u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags);
> @@ -185,7 +186,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm,
> struct sk_buff *skb,
> tx_cmd->tx_flags = cpu_to_le32(tx_flags);
> /* Total # bytes to be transmitted */
> tx_cmd->len = cpu_to_le16((u16)skb->len +
> -   (uintptr_t)info->driver_data[0]);
> +   (uintptr_t)skb_info->driver_data[0]);
> tx_cmd->next_frame_len = 0;
> tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
> tx_cmd->sta_id = sta_id;
> 
> I wasn't expecting that skb_info variable is removed. Do we now have
> merge damage somewhere? Luca, what do you think?

As we discussed on IRC, it seems to me that there was a merge damage
when Dave merged net.git into net-next.git (as you mostly found out ;).

I'm not sure how to solve that, but I'm sure you and Dave can figure
something out. :) Please let me know if you need any help with it.

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: pull-request: wireless-drivers-next 2016-05-13

[Coelho, Luciano]

On Mon, 2016-05-16 at 17:08 +0300, Kalle Valo wrote:
> Kalle Valo  writes:
> 
> > 
> > Kalle Valo  writes:
> > 
> > > 
> > > The following changes since commit
> > > ede00a5ceb4d903a8c137a52bb77d574baaef8bd:
> > > 
> > >   Merge tag 'wireless-drivers-next-for-davem-2016-05-02' of
> > > git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-
> > > drivers-next (2016-05-03 00:35:16 -0400)
> > > 
> > > are available in the git repository at:
> > > 
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-
> > > drivers-next.git tags/wireless-drivers-next-for-davem-2016-05-13
> > Please don't pull this yet, there might be something wrong now with
> > merges and need to check that first.
> Ok, like discussed in thread "linux-next: manual merge of the
> wireless-drivers-next tree with the net-next tree" there seems to be
> a
> problem on net-next in function iwl_mvm_set_tx_cmd(). Here is how I
> propose to fix this.
> 
> When pulling the tag above you should get a conflict like this:
> 
> diff --cc drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> index 880210917a6f,779bafcbc9a1..
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> @@@ -294,7 -295,7 +294,11 @@@ void iwl_mvm_set_tx_cmd(struct iwl_mvm 
> tx_cmd->tx_flags = cpu_to_le32(tx_flags);
> /* Total # bytes to be transmitted */
> tx_cmd->len = cpu_to_le16((u16)skb->len +
> ++<<< HEAD
>  +  (uintptr_t)info->driver_data[0]);
> ++===
> +   (uintptr_t)skb_info->driver_data[0]);
> ++>>> master
> tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
> tx_cmd->sta_id = sta_id;
> 
> Pick the latter with skb_info and then add skb_info to the beginning
> of
> the same function. So the function should be:
> 
> void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct sk_buff *skb,
>   struct iwl_tx_cmd *tx_cmd,
>   struct ieee80211_tx_info *info, u8 sta_id)
> {
>   struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb);
>   struct ieee80211_hdr *hdr = (void *)skb->data;
>   __le16 fc = hdr->frame_control;
>   u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags);
>   u32 len = skb->len + FCS_LEN;
>   u8 ac;
> 
> [...]
> 
>   tx_cmd->tx_flags = cpu_to_le32(tx_flags);
>   /* Total # bytes to be transmitted */
>   tx_cmd->len = cpu_to_le16((u16)skb->len +
>   (uintptr_t)skb_info->driver_data[0]);
>   tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
>   tx_cmd->sta_id = sta_id;
> 
> Sorry about the hassle and please let me know if you have any
> problems.
> Adding Luca and Emmanuel just in case I missed something.

ACK.  This looks correct.  I just diffed the iwlwifi-next.git tree (at
commit a525d0eab17d -- which is where I merge iwlwifi-fixes into
iwlwifi-next) with net-next.git master and the difference [1] is
exactly what you proposed to fix.

[1] http://pastebin.coelho.fi/1b6907cdb9a25413.txt

--
Cheers,
Luca.

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2016-05-18 at 06:20 -0600, Reinoud Koornstra wrote:
> On Wed, May 18, 2016 at 4:51 AM, Coelho, Luciano
> <luciano.coe...@intel.com> wrote:
> > 
> > Hi Emmanuel, Linus,
> > 
> > 
> > On Wed, 2016-05-18 at 06:37 +0300, Emmanuel Grumbach wrote:
> > > 
> > > On Wed, May 18, 2016 at 4:00 AM, Linus Torvalds
> > > <torva...@linux-foundation.org> wrote:
> > > > 
> > > > 
> > > > On Tue, May 17, 2016 at 12:11 PM, David Miller <davem@davemloft
> > > > .net
> > > > > 
> > > > > wrote:
> > > > > 
> > > > > 
> > > > > Highlights:
> > > > Lowlights:
> > > > 
> > > >  1) the iwlwifi driver seems to be broken
> > > > 
> > > > My laptop that uses the intel 7680 iwlwifi module no longer
> > > > connects
> > > > to the network. It fails with a "Microcode SW error detected."
> > > > and
> > > > spews out register state over and over again.
> > > Can we have the register state and the ASSERT / NMI / whatever
> > > that
> > > goes along with it?
> > > This clearly means that the firmware is crashing, but I don't
> > > know
> > > why,
> > > I copied here the lines that I need from another bug with another
> > > device with another firmware,
> > > but the log that we will still explain what I need:
> > I managed to reproduce this bug locally with Linus' master.  I'm
> > investigating the cause and I'll let you how it goes.
> I did run the latest git code as well 4.6+
> iwlwifi went pearshape in my case as well.
> I just updated the microcode as well, it didn't matter.
> 4.6-rc7 works fine and no errors are reported with iwlwifi.
> 
> Here's output that might come in handy

Thanks! That is helpful.

Since you say that 4.6-rc7 works but 4.6 doesn't, the prime suspect is
commit 5c08b0f5026f ("iwlwifi: mvm: don't override the rate with the
AMSDU len"), which is the only iwlwifi patch between those two
releases.

Could you try to revert that and see if the error is gone?

--
Cheers,
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [GIT] Networking

[Coelho, Luciano]

Hi Emmanuel, Linus,


On Wed, 2016-05-18 at 06:37 +0300, Emmanuel Grumbach wrote:
> On Wed, May 18, 2016 at 4:00 AM, Linus Torvalds
>  wrote:
> > 
> > On Tue, May 17, 2016 at 12:11 PM, David Miller  > > wrote:
> > > 
> > > 
> > > Highlights:
> > Lowlights:
> > 
> >  1) the iwlwifi driver seems to be broken
> > 
> > My laptop that uses the intel 7680 iwlwifi module no longer
> > connects
> > to the network. It fails with a "Microcode SW error detected." and
> > spews out register state over and over again.
> Can we have the register state and the ASSERT / NMI / whatever that
> goes along with it?
> This clearly means that the firmware is crashing, but I don't know
> why,
> I copied here the lines that I need from another bug with another
> device with another firmware,
> but the log that we will still explain what I need:

I managed to reproduce this bug locally with Linus' master.  I'm
investigating the cause and I'll let you how it goes.

--
Cheers,
Luca.

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2016-05-18 at 06:51 -0600, Reinoud Koornstra wrote:
> On Wed, May 18, 2016 at 6:41 AM, Coelho, Luciano
> <luciano.coe...@intel.com> wrote:
> > 
> > On Wed, 2016-05-18 at 06:20 -0600, Reinoud Koornstra wrote:
> > > 
> > > On Wed, May 18, 2016 at 4:51 AM, Coelho, Luciano
> > > <luciano.coe...@intel.com> wrote:
> > > > 
> > > > 
> > > > Hi Emmanuel, Linus,
> > > > 
> > > > 
> > > > On Wed, 2016-05-18 at 06:37 +0300, Emmanuel Grumbach wrote:
> > > > > 
> > > > > 
> > > > > On Wed, May 18, 2016 at 4:00 AM, Linus Torvalds
> > > > > <torva...@linux-foundation.org> wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Tue, May 17, 2016 at 12:11 PM, David Miller <davem@davem
> > > > > > loft
> > > > > > .net
> > > > > > > 
> > > > > > > 
> > > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Highlights:
> > > > > > Lowlights:
> > > > > > 
> > > > > >  1) the iwlwifi driver seems to be broken
> > > > > > 
> > > > > > My laptop that uses the intel 7680 iwlwifi module no longer
> > > > > > connects
> > > > > > to the network. It fails with a "Microcode SW error
> > > > > > detected."
> > > > > > and
> > > > > > spews out register state over and over again.
> > > > > Can we have the register state and the ASSERT / NMI /
> > > > > whatever
> > > > > that
> > > > > goes along with it?
> > > > > This clearly means that the firmware is crashing, but I don't
> > > > > know
> > > > > why,
> > > > > I copied here the lines that I need from another bug with
> > > > > another
> > > > > device with another firmware,
> > > > > but the log that we will still explain what I need:
> > > > I managed to reproduce this bug locally with Linus'
> > > > master.  I'm
> > > > investigating the cause and I'll let you how it goes.
> > > I did run the latest git code as well 4.6+
> > > iwlwifi went pearshape in my case as well.
> > > I just updated the microcode as well, it didn't matter.
> > > 4.6-rc7 works fine and no errors are reported with iwlwifi.
> > > 
> > > Here's output that might come in handy
> > Thanks! That is helpful.
> > 
> > Since you say that 4.6-rc7 works but 4.6 doesn't, the prime suspect
> > is
> > commit 5c08b0f5026f ("iwlwifi: mvm: don't override the rate with
> > the
> > AMSDU len"), which is the only iwlwifi patch between those two
> > releases.
> > 
> > Could you try to revert that and see if the error is gone?
> Will do, since git revert failed I'll revert manually and report
> back.

I can confirm that 4.6 contains the same bug.  And reverting the patch
I mentioned does solve the problem...

The same patch works fine in our internal tree.  I'll have to figure
out together with Emmanuel what the problem actually is.

--
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [patch] iwlwifi: mvm: remove an unused variable

[Coelho, Luciano]

On Wed, 2016-05-04 at 09:19 +0300, Dan Carpenter wrote:
> We never initalize ampdu_status so it causes a static checker warning
> when we pass it to iwl_mvm_pass_packet_to_mac80211().  Fortunately,
> it's
> never used so we can just remove it.
> 
> Signed-off-by: Dan Carpenter 

Thanks Dan!

I'll push this patch through our internal tree.

--
Cheers,
Luca.

Re: pull request: iwlwifi 2016-05-04

[Coelho, Luciano]

On Fri, 2016-05-06 at 14:34 +0300, Kalle Valo wrote:
> "Grumbach, Emmanuel"  writes:
> 
> > 
> > I know it is extremely late in the cycle, but this patch is
> > intended
> > for 4.6... It fixes a regression I introduced: a P2P specification
> > violation as mentioned in the commit message.
> > The patch is rather big in terms of number of lines changed, but
> > the
> > semantics is very simple. I have to copy the control block of the
> > skb
> > to the stack whereas we accessed we used to access it directly
> > through
> > a pointer.
> > This obviously introduces a lot of line changes, but don't really
> > change the behavior of the code (since most of the accesses are
> > read
> > only).
> > 
> > Let me know how it goes.
> [...]
> 
> > 
> > The following changes since commit
> > f742aaf36edf0390c54d0614bc4d20fd4cd3762a:
> > 
> >   iwlwifi: mvm: fix accessing Null pointer during fw dump
> > collection (2016-04-12 11:52:39 +0300)
> > 
> > are available in the git repository at:
> > 
> >   https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-f
> > ixes.git tags/iwlwifi-for-kalle-2016-05-04
> Pulled, thanks. Let's see if this makes it to 4.6.
> 
> > 
> > I take this opportunity to inform you that I will be replaced by
> > Luca
> > for a few months at least. I will be working full steam on a side
> > project that will not allow me to do the maintainer job in
> > parallel.
> Ok, have fun in your new project.
> 
> Luca, as we _finally_ got sun here in Finland we have to have a "kick
> off meeting about the new organisation structure" by your grill ;)

Sounds good! When are you going to be coming to "The South"? :)

--
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH 4/7] nl80211: Add info to scan results to support beacon report

[Coelho, Luciano]

On Tue, 2016-07-05 at 15:23 +0300, Luca Coelho wrote:
> From: Avraham Stern 
> 
> Beacon report radio measurement requires reporting observed BSSs
> on the channels specified in the beacon request. If the measurement
> mode is set to passive or active, it requires actually performing a
> scan (passive or active, accordingly), and reporting the time that
> the scan was started and the time each beacon/probe was received
> (both in terms of TSF of the BSS of the requesting AP). If the
> request mode is table, this information is optional.
> In addition, the radio measurement request specifies the channel
> dwell time for the measurement.
> 
> In order to use scan for beacon report when the mode is active or
> passive, add a parameter to scan request that specifies the
> channel dwell time, and add scan start time and beacon received time
> to scan results information.
> 
> Supporting beacon report is required for Multi Band Operation (MBO).
> 
> Signed-off-by: Assaf Krauss 
> Signed-off-by: David Spinadel 
> Signed-off-by: Avraham Stern 
> Signed-off-by: Luca Coelho 
> ---
> 
 include/net/cfg80211.h   | 40
> +++-
>  include/uapi/linux/nl80211.h | 42
> ++
>  net/mac80211/scan.c  |  9 +++--
>  net/wireless/core.c  |  4 ++--
>  net/wireless/core.h  | 12 
>  net/wireless/nl80211.c   | 27 +++
>  net/wireless/scan.c  | 18 --
>  net/wireless/trace.h | 33 +
>  8 files changed, 162 insertions(+), 23 deletions(-)

Ouch, I missed changing the APIs of the non-cfg80211 drivers here.
 I'll fix and resend.

Thanks to kbuild test robot for reporting. :)

--
Luca.N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded

[Coelho, Luciano]

On Mon, 2016-07-11 at 11:18 -0400, Prarit Bhargava wrote:
> Didn't get any feedback or review comments on this patch.  Resending
> ...
> 
> P.

Sorry, this got flooded down my inbox.


> ---8<---
> 
> The iwlwifi driver implements a thermal zone and hwmon device, but
> returns -EIO on temperature reads if the firmware isn't loaded.  This
> results in the error
> 
> iwlwifi-virtual-0
> Adapter: Virtual device
> ERROR: Can't get value of subfeature temp1_input: I/O error
> temp1:N/A
> 
> being output when using sensors from the lm-sensors package.  Since
> the temperature cannot be read unless the ucode is loaded there is no
> reason to add the interface only to have it return an error 100% of
> the time.
> 
> This patch moves the firmware check to
> iwl_mvm_thermal_zone_register() and
> stops the thermal zone from being created if the ucode hasn't been
> loaded.
> 
> Signed-off-by: Prarit Bhargava 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: Chaya Rachel Ivgi 
> Cc: Sara Sharon 
> Cc: linux-wireless@vger.kernel.org
> Cc: net...@vger.kernel.org
> ---

I have now sent it for review on our internal tree.

--
Luca.

Re: [PATCH 00/17] iwlwifi: updates intended for v4.11 2017-02-08

[Coelho, Luciano]

On Wed, 2017-02-08 at 16:29 +0200, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > This is the third and final pull-request before 4.11's merge window.
> > This time I concentrated in bugfixes:
> > 
> > * Fix 802.11w, which was failing to due an IGTK bug;
> > * A few more bugzilla bug fixes;
> > * A channel-switch race condition fix;
> > * Some fixes related to suspend/resume with new HW;
> > * The RF-kill saga continues;
> > * And some other fixes here and there...
> > 
> > As usual, I'm pushing this to a pending branch, for kbuild bot, and
> > will send a pull-request later.
> > 
> > Please review.
> 
> I only see patches 1-8 in the mailing list or in my inbox, what happened
> to the rest?

Not sure, maybe they are still queued in my server? Let me check.

--
Luca.

Re: [PATCH 0/6] iwlwifi: updates intended for v4.10 2017-01-13

[Coelho, Luciano]

On Fri, 2017-01-13 at 14:39 +0200, Luca Coelho wrote:
> From: Luca Coelho 
> 
> Hi,
> 
> Here are a few fixes that I intend to send for v4.10:
> 
> * fix suspend failure with unified firmware images
> * fix a potential crash when dumping debug data
> * fix a couple of mistakes in FW names exported by the modules
> * small fix for PS-Poll enabling via debugfs
> * fix for dynamic queue allocation offchannel data

As discussed on IRC, it got a bit late for some of these patches (due to
my trip last week).  So, let's ignore this and I'll send a new version
with less patches today.

--
Luca.

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Coelho, Luciano]

(resending, sorry for the HTML version my phone decided to use for this)

On Jan 21, 2017 09:56, Kalle Valo  wrote:
Jens Axboe  writes: 
> > A recent firmware change seems to have enabled thermal zones on the 
> > iwlwifi driver. Unfortunately, my device fails when registering the 
> > thermal zone. This doesn't stop the driver from attempting to unregister 
> > the thermal zone at unload time, triggering a NULL pointer deference in 
> > strlen() off the thermal_zone_device_unregister() path. 
> > 
> > Don't unregister if name is NULL, for that case we failed registering. 
> > Do the same for the cooling zone. 
> > 
> > Signed-off-by: Jens Axboe  
> > 
> > --- 
> > 
> > Would be great if this could go into the current series, as sometimes I 
> > have to reload the driver. Right now I can't, since it crashes... 
> 
> Luca, can I take this directly to wireless-drivers? This is an important 
> fix and we should get it to Linus' tree ASAP.

Yes, please go ahead and take it directly.

Acked-by: Luca Coelho 

Thanks!

--
Cheers,
Luca.

Re: [PATCH 07/10] iwlwifi: remove support for fw older than -17 and -22

[Coelho, Luciano]

On Thu, 2016-09-22 at 23:52 +0300, Luca Coelho wrote:
> From: Luca Coelho 
> 
> FW versions older than -17 for 3160 and 7260 and older than -22 for
> newer NICs are not supported anymore.  Don't load these versions
> and remove code that handles them.
> 
> Signed-off-by: Luca Coelho 
> Signed-off-by: Luca Coelho 
> ---

Heh, double S-O-B.  I'll fix this before sending the pull request.

--
Luca.

Re: [PATCH] nl80211: validate number of probe response CSA counters

[Coelho, Luciano]

On Tue, 2016-09-13 at 15:56 +0200, Johannes Berg wrote:
> From: Johannes Berg 
> 
> Due to an apparent copy/paste bug, the number of counters for the
> beacon configuration were checked twice, instead of checking the
> number of probe response counters. Fix this to check the number of
> probe response counters before parsing those.
> 
> Signed-off-by: Johannes Berg 
> ---

Acked-by: Luciano Coelho 

--
Luca.

Re: [PATCH v2 0/3] Fix -Wunused-but-set-variable in iwlwifi/pcie/trans and iwlwifi/mvm/rs

[Coelho, Luciano]

On Tue, 2016-11-08 at 21:49 -0800, Kirtika Ruchandani wrote:
> This patchset is part of the effort led by Arnd Bergmann to clean up
> warnings in the kernel. This and following patchsets will focus on 
> "-Wunused-but-set-variable" as it among the noisier ones. These were
> found compiling with W=1.
> 
> -- 
> Changes in v2:
> - Made the following changes suggested by Arnd in all 3 patches
>   1. Each patch has a unique subject line.
>   2. Add the commit that introduced/led to the warning with the "Fixes:" line.
>   3. cc linux-wireles, cc commit authors.
> 
> Kirtika Ruchandani (3):
>   iwlwifi: mvm: rs: Remove unused 'mvmvif'/'mvmsta' variables
>   iwlwifi: mvm: rs: Remove unused 'mcs' variable
>   iwlwifi: pcie: trans: Remove unused 'shift_param'
> 
>  drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 10 +-
>  drivers/net/wireless/intel/iwlwifi/pcie/trans.c |  3 ---
>  2 files changed, 1 insertion(+), 12 deletions(-)

Thanks, Kirtika! I'll push these patches via our internal tree and
include them in my next pull request to Kalle.

-- 
Cheers,
Luca.

Re: [PATCH 09/10] iwlwifi: mvm: operate in dqa mode

[Coelho, Luciano]

Hi Kalle,

On Wed, 2016-10-26 at 09:32 +0300, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > From: Liad Kaufman 
> > 
> > Run DQA flows by default, as long as the FW supports it.
> > 
> > Signed-off-by: Liad Kaufman 
> > Signed-off-by: Luca Coelho 
> 
> What's this DQA mode? A short summary would have been nice why it's
> useful.

DQA is Dynamic Queue Allocation.  We have been working on this for
quite a while now (since v4.7) and have many patches mentioning that
are already merged.  The first patch is this:

commit 24afba7690e4 ("iwlwifi: mvm: support bss dynamic alloc/dealloc
of queues")

Its commit message has some explanation and we also have a DOC section
in our driver that explains it in further details:

https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/sta.h#n82

Since we've been working on this for so long and already use the term
DQA broadly, we thought it wouldn't be necessary to explain more when
we are finally enabling it by default.  But of course I can change that
if you prefer.

--
Cheers,
Luca.

Re: [PATCH 1/6] iwlwifi: mvm: don't restart HW if suspend fails with unified image

[Coelho, Luciano]

On Fri, 2017-01-13 at 16:13 +0200, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > From: Luca Coelho 
> > 
> > For unified images, we shouldn't restart the HW if suspend fails.  The
> > only reason for restarting the HW with non-unified images is to go
> > back to the D0 image.
> > 
> > Fixes: commit 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on 
> > suspend")
> 
> s/commit // :)

Argh, did I do it again? I was specifically trying to remember this, but
I guess it was not enough...

--
Luca.

Re: [RESEND PATCH 4.11] iwlwifi: mvm: cleanup pending frames in DQA mode

[Coelho, Luciano]

On Wed, 2017-03-15 at 11:52 +0200, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > From: Sara Sharon 
> > 
> > When a station is asleep, the fw will set it as "asleep".
> > All queues that are used only by one station will be stopped by
> > the fw.
> > 
> > In pre-DQA mode this was relevant for aggregation queues. However,
> > in DQA mode a queue is owned by one station only, so all queues
> > will be stopped.
> > As a result, we don't expect to get filtered frames back to
> > mac80211 and don't have to maintain the entire pending_frames
> > state logic, the same way as we do in aggregations.
> > 
> > The correct behavior is to align DQA behavior with the aggregation
> > queue behaviour pre-DQA:
> > - Don't count pending frames.
> > - Let mac80211 know we have frames in these queues so that it can
> > properly handle trigger frames.
> > 
> > When a trigger frame is received, mac80211 tells the driver to send
> > frames from the queues using release_buffered_frames.
> > The driver will tell the fw to let frames out even if the station
> > is asleep. This is done by iwl_mvm_sta_modify_sleep_tx_count.
> > 
> > Reported-and-tested-by: Jens Axboe 
> > Reported-by: Linus Torvalds 
> > Signed-off-by: Sara Sharon 
> > Signed-off-by: Luca Coelho 
> > ---
> > 
> > Just resending with reported and tested by tags.
> 
> Sorry, my usual rant follows :) But never ever use "RESEND", it's so
> much simpler to have v2, v3 and so on to figure out what's the (latest)
> version I should take.

Okay, sorry.  I just used RESEND, because I didn't change the patch
itself, but only added the reported-by tags...

--
Luca.

Re: [PATCH] iwlwifi: pcie: missing unlock on error path

[Coelho, Luciano]

Hi Dan

On Fri, 2017-04-21 at 13:39 +0300, Dan Carpenter wrote:
> We should unlock before returning.
> 
> Fixes: eda50cde58de ("iwlwifi: pcie: add context information support")
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c 
> b/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
> index 302310dfef9e..4e84955d55c8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
> @@ -212,8 +212,10 @@ int iwl_trans_pcie_gen2_start_fw(struct iwl_trans *trans,
>   goto out;
>   }
>  
> - if (iwl_pcie_ctxt_info_init(trans, fw))
> - return -ENOMEM;
> + if (iwl_pcie_ctxt_info_init(trans, fw)) {
> + ret = -ENOMEM;
> + goto out;
> + }
>  
>   /* re-check RF-Kill state since we may have missed the interrupt */
>   hw_rfkill = iwl_trans_check_hw_rf_kill(trans);

Thanks for your patch! But this has already been fixed by Johannes in
the patch I sent out yesterday[1].

[1] https://www.spinics.net/lists/linux-wireless/msg161582.html

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: pcie: off by one in iwl_trans_pcie_dyn_txq_alloc()

[Coelho, Luciano]

On Fri, 2017-04-21 at 23:12 +0300, Dan Carpenter wrote:
> Heh.  I raced this one through to see if I could beat you to the punch.

Yeah, this patch is in our internal tree.  But maybe we can take Dan's
for upstream for his efforts? :)

--
Cheers,
Luca.

Re: WARNING: CPU: 1 PID: 23668 at drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1539 iwl_mvm_rm_sta+0x3ce/0x450

[Coelho, Luciano]

On Fri, 2017-03-10 at 14:01 +0200, Luca Coelho wrote:
> Hi Jens,
> 
> On Thu, 2017-03-09 at 21:41 -0700, Jens Axboe wrote:
> > On 03/01/2017 09:10 PM, Jens Axboe wrote:
> > > On 03/01/2017 08:33 PM, Luca Coelho wrote:
> > > > Hi Jens,
> > > > 
> > > > On Mar 1, 2017 20:25, Jens Axboe  wrote:
> > > > 
> > > > Not that folks have been jumping all over this, but in case someone 
> > > > is
> > > > curious, it triggered twice here today. For those two times, the 
> > > > value
> > > > of mvm->pending_frames[sta_id] was 80 and 39, respectively.
> > > > 
> > > > Sorry for the delay, I'm on vacation now with limited internet access.
> > > > But we'll take a look into this early next week at the latest.
> > > > 
> > > > Thanks a lot for the detailed report!
> > > 
> > > No worries, thanks for responding. I just wanted to ensure this wasn't
> > > dropped on the floor.
> > > 
> > > BTW, a few more values of ->pending_frames[sta_id]:
> > > 
> > > $ dmesg | grep "ret="
> > > [ 2334.308254] ret=39
> > > [ 7915.311828] ret=80
> > > [31602.317204] ret=41
> > > [32139.510993] ret=54
> > > [33292.917759] ret=96
> > > 
> > > it seems to often happen around resume.
> > 
> > This is still happening all the time in current -git.
> 
> Could you collect traces with trace-cmd, as explained in our wiki[1]?
> This will probably help point out the problem.  I know it's a bit
> difficult because it appears to happen randomly for you, but it's worth
> trying.

And of course, the link I wanted to give you is this:

[1] https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/debugging#tracing


--
Luca.

Re: [PATCH V5 1/2] firmware: add more flexible request_firmware_async function

[Coelho, Luciano]

On Thu, 2017-08-03 at 08:23 +0300, Kalle Valo wrote:
> "Luis R. Rodriguez"  writes:
> 
> > > +int request_firmware_nowait(struct module *module, bool uevent,
> > > + const char *name, struct device *device, gfp_t gfp,
> > > + void *context,
> > > + void (*cont)(const struct firmware *fw, void 
> > > *context))
> > > +{
> > > + unsigned int opt_flags = FW_OPT_FALLBACK |
> > > + (uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
> > > +
> > > + return __request_firmware_nowait(module, opt_flags, name, device, gfp,
> > > +  context, cont);
> > > +}
> > >  EXPORT_SYMBOL(request_firmware_nowait);
> > >  
> > > +int __request_firmware_async(struct module *module, const char *name,
> > > +  struct firmware_opts *fw_opts, struct device *dev,
> > > +  void *context,
> > > +  void (*cont)(const struct firmware *fw, void 
> > > *context))
> > > +{
> > > + unsigned int opt_flags = FW_OPT_UEVENT;
> > 
> > This exposes a long issue. Think -- why do we want this enabled by default? 
> > Its
> > actually because even though the fallback stuff is optional and can be, the 
> > uevent
> > internal flag *also* provides caching support as a side consequence only. We
> > don't want to add a new API without first cleaning up that mess.
> > 
> > This is a slipery slope and best to clean that up before adding any new API.
> > 
> > That and also Greg recently stated he would like to see at least 3 users of
> > a feature before adding it. Although I think that's pretty arbitrary, and
> > considering that request_firmware_into_buf() only has *one* user -- its what
> > he wishes.
> 
> ath10k at least needs a way to silence the warning for missing firmware
> and I think iwlwifi also.

Yes, iwlwifi needs to silence the warning.  It the feature (only one,
really) that I've been waiting for.

--
Luca.

Re: [linuxwifi] x86/thermal: AB-BA dependency between mvm->mutex and tz->lock

[Coelho, Luciano]

On Thu, 2017-08-03 at 13:02 +0300, Kalle Valo wrote:
> "Coelho, Luciano" <luciano.coe...@intel.com> writes:
> 
> > On Thu, 2017-08-03 at 11:10 +0200, Jiri Kosina wrote:
> > > On Mon, 31 Jul 2017, Jiri Kosina wrote:
> > > 
> > > > Hi,
> > > > 
> > > > booting current Linus' tree, I'm seeing lockdep splat (see the end of 
> > > > this 
> > > > mail).
> > > > 
> > > > Apparently, there is AB-BA between tz->lock and mvm->mutex through the 
> > > > CPU 
> > > > hotplug lock.
> > > > 
> > > > The obivous depency is: thermal_zone_get_temp() acquires tz->lock, and 
> > > > then calls iwl_mvm_tzone_get_temp() (through tz->ops->get_temp() 
> > > > callback), which acquires mvm->mutex
> > > > 
> > > > The less obvious dependency is primarily caused by 
> > > > iwl_op_mode_mvm_start() 
> > > > allocating workqueue (#2 stacktrace) while holding mvm->mutex (which is 
> > > > broken, because that mutex is being taken also from CPU hotplug 
> > > > callback 
> > > > path, hence the AB-BA).
> > > 
> > > As the "central" part of the dependency is being added by iwlwifi driver 
> > > (_iwl_pcie_rx_init() allocating workqueue while holding 
> > > trans_pcie->mutex), I'm adding iwlwifi folks as well to CC.

[...]

> > > >  -> #2 (cpu_hotplug_lock.rw_sem){++}:
> > > > lock_acquire+0xbd/0x220
> > > > cpus_read_lock+0x46/0x90
> > > > apply_workqueue_attrs+0x17/0x50
> > > > __alloc_workqueue_key+0x195/0x4d0
> > > > _iwl_pcie_rx_init+0x384/0x390 [iwlwifi]
> > > > iwl_pcie_rx_init+0x1e/0x380 [iwlwifi]
> > > > iwl_trans_pcie_start_fw+0x295/0x6f0 [iwlwifi]
> > > > iwl_mvm_load_ucode_wait_alive+0xe7/0x390 [iwlmvm]
> > > > iwl_run_init_mvm_ucode+0x84/0x320 [iwlmvm]
> > > > iwl_op_mode_mvm_start+0x964/0xd30 [iwlmvm]
> > > > _iwl_op_mode_start.isra.9+0x47/0xa0 [iwlwifi]
> > > > iwl_opmode_register+0xaa/0xd0 [iwlwifi]
> > > > iwl_mvm_init+0x37/0x1000 [iwlmvm]
> > > > do_one_initcall+0x51/0x1a9
> > > > do_init_module+0x60/0x20e
> > > > load_module+0x203f/0x2b50
> > > > SYSC_finit_module+0x96/0xd0
> > > > SyS_finit_module+0xe/0x10
> > > > entry_SYSCALL_64_fastpath+0x23/0xc2

Okay, so as I understand it the problem has been there for a long time,
but the splat is only coming up now because of Thomas' patch that adds
the lockdep map[1], right?

I see the workqueue allocation you mentioned.  I'll try to move this
allocation out of the mutex and see how it goes.

[1] http://lkml.kernel.org/r/20170524081549.709375...@linutronix.de

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: pcie: Fix error code in iwl_trans_pcie_alloc()

[Coelho, Luciano]

Hi Dan,

On Wed, 2017-07-12 at 10:53 +0300, Dan Carpenter wrote:
> We accidentally forgot to propogate the error code on this path.  It
> means we return ERR_PTR(0) which is NULL and it results in a NULL
> dereference in the caller.
> 
> Fixes: 2e5d4a8f61dc ("iwlwifi: pcie: Add new configuration to enable MSIX")
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c 
> b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> index 92b3a55d0fbc..f95eec52508e 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -3150,7 +3150,8 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev 
> *pdev,
>   init_waitqueue_head(_pcie->d0i3_waitq);
>  
>   if (trans_pcie->msix_enabled) {
> - if (iwl_pcie_init_msix_handler(pdev, trans_pcie))
> + ret = iwl_pcie_init_msix_handler(pdev, trans_pcie);
> + if (ret)
>   goto out_no_pci;
>} else {
>   ret = iwl_pcie_alloc_ict(trans);

You already sent an equivalent patch:

https://patchwork.kernel.org/patch/9825895/

And I've already sent it out to be applied in 4.13-rc*.  I'll send a
pull-req for -fixes later this week.

Thanks anyway. :)

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: pcie: Fix error code in iwl_trans_pcie_alloc()

[Coelho, Luciano]

On Wed, 2017-07-19 at 11:21 +0300, Dan Carpenter wrote:
> Oops, heh, sorry.  I switched computers so I first one wasn't in my
> outbox.

No worries, better safe than sorry. :)

--
Luca.

Re: [PATCH] iwlwifi: Demote messages about fw flags size to info

[Coelho, Luciano]

On Fri, 2017-07-21 at 07:51 -0700, João Paulo Rechi Vita wrote:
> These messages are not reporting a real error, just the fact that the
> firmware knows about more flags then the driver.
> 
> Currently these messages are presented to the user during boot if there
> is no bootsplash covering the console, sometimes even if the boot splash
> is enabled but has not started yet by the time this message is shown.
> 
> Demoting it to the info level helps having a clean boot process.
> 
> Signed-off-by: João Paulo Rechi Vita 
> ---

The idea with this error is that if the firmware is too new and includes
a TLV that we are not aware of, there can be unexpected issues.  For
instance, sometimes the FW API changes some of its structures and we use
TLVs to know which one to use.  If a new struct is in use by the
firmware but not by the driver, problems will occur.

Recently we accidentally omitted one TLV from the driver and released a
new firmware that had it set... That's the error you are currently
seeing.  We have a bugzilla entry[1] and it is fixed in our internal
tree.  The fix will be sent upstream in the next -fixes round we send
out.

This specific case is harmless, but I'd rather keep this message as an
error, because in other situations it could lead to unexpected
behavioir, so I prefer to keep it very visible.


[1] https://bugzilla.kernel.org/show_bug.cgi?id=196195

--
Cheers,
Luca.

Re: [PATCH 00/31] iwlwifi: updates intended for v4.13 2017-06-28

[Coelho, Luciano]

On Wed, 2017-06-28 at 23:12 +0300, Luca Coelho wrote:
> From: Luca Coelho 
> 
> Hi Kalle,
> 
> Here is one more batch that I'd like to get into v4.13, hopefully it's
> not too late.  I'm also going to send some more tomorrow.  The changes
> are:
> 
> * Some important fixes for 9000 HW;
> * FW API changes for the upcoming -30 ucode release;
> * A few new PCI IDs for 9000 series;
> * Reorganization of common files;
> * Some more fixes and improvements here and there
> 
> Thanks!

Forgot to say that these patches already passed kbuildbot test.

--
Cheers,
Luca.

Re: [PATCH 21/26] iwlwifi: mvm: print base HW address during init

[Coelho, Luciano]

On Tue, 2017-06-27 at 18:19 +, Coelho, Luciano wrote:
> On Tue, 2017-06-27 at 20:36 +0300, Kalle Valo wrote:
> > Fabio Estevam <feste...@gmail.com> writes:
> > 
> > > On Tue, Jun 27, 2017 at 2:06 PM, Kalle Valo <kv...@codeaurora.org> wrote:
> > > 
> > > > "crouded sniffer logs", what's that?
> > > 
> > > I think Luca meant 'crowded'.
> > 
> > Ah, makes sense. Thanks.
> 
> Huh, obviously a typo, sorry! Shall I fix it before my pull-req?

And, of course, what I mean here is that sometimes you get sniffer logs
from very crowded areas with gazillions of different MAC addresses. 
This way we can match the logs with the sniffer a bit more easily.

--
Cheers,
Luca.

Re: [PATCH 21/26] iwlwifi: mvm: print base HW address during init

[Coelho, Luciano]

On Tue, 2017-06-27 at 20:36 +0300, Kalle Valo wrote:
> Fabio Estevam  writes:
> 
> > On Tue, Jun 27, 2017 at 2:06 PM, Kalle Valo  wrote:
> > 
> > > "crouded sniffer logs", what's that?
> > 
> > I think Luca meant 'crowded'.
> 
> Ah, makes sense. Thanks.

Huh, obviously a typo, sorry! Shall I fix it before my pull-req?

--
Cheers,
Luca.

Re: [PATCH v3] iwlwifi: Demote messages about fw flags size to info

[Coelho, Luciano]

On Thu, 2017-08-03 at 07:47 -0700, João Paulo Rechi Vita wrote:
> These messages are not reporting a real error, just the fact that the
> firmware knows about more flags than the driver.
> 
> Currently these messages are presented to the user during boot if there
> is no bootsplash covering the console, even when booting the kernel with
> "quiet".
> 
> Demoting it to the warn level helps having a clean boot process.
> 
> Signed-off-by: João Paulo Rechi Vita 
> ---
> 
> v2 changes:
>  - Set to warn level instead of info
> 
> v3 changes:
>  - Fix commit message typo

Thanks, João Paulo! I pushed this to our internal tree and it will
eventually reach the mainline following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH 00/24] iwlwifi: updates intended for v4.13 2017-07-28

[Coelho, Luciano]

As Emmanuel pointed out, the subject is obviously wrong.  These are
intended for v4.14 as my text correctly says below.

Sorry if this caused confusion.

--
Luca.

On Fri, 2017-07-28 at 17:23 +0300, Luca Coelho wrote:
> From: Luca Coelho 
> 
> Hi,
> 
> Here's my first set of patches for v4.14.  These are the changes:
> 
> * Reorganization of the code into separate directories continues;
> * A couple of new minor features;
> * Fixes and cleanups here and there.
> 
> As usual, I'm pushing this to a pending branch, for kbuild bot, and
> will send a pull-request later.
> 
> Please review.
> 
> Cheers,
> Luca.
> 
> 
> Emmanuel Grumbach (4):
>   iwlwifi: mvm: fix the FIFO numbers in A000 devices
>   iwlwifi: pcie: fix A-MSDU on gen2 devices
>   iwlwifi: mvm: fix TCP CSUM offload with WEP and A000 series
>   iwlwifi: mvm: don't retake the pointer to skb's CB
> 
> Johannes Berg (13):
>   iwlwifi: refactor out paging code
>   iwlwifi: refactor shared mem parsing
>   iwlwifi: track current firmware image in common code
>   iwlwifi: refactor firmware debug code
>   iwlwifi: reorganize firmware API
>   iwlwifi: fw api: fix various kernel-doc warnings
>   iwlwifi: mvm: add and use iwl_mvm_has_unified_ucode()
>   iwlwifi: mvm: check family instead of new TX API for workarounds
>   iwlwifi: mvm: byte-swap constant instead of variable
>   iwlwifi: pcie: rename iwl_trans_check_hw_rf_kill() to pcie
>   iwlwifi: mvm: require AP_LINK_PS for TVQM
>   iwlwifi: mvm: simplify bufferable MMPDU check
>   iwlwifi: mvm: remove non-DQA mode
> 
> Liad Kaufman (1):
>   iwlwifi: mvm: support fw reading empty OTP
> 
> Luca Coelho (2):
>   iwlwifi: mvm: refactor beacon template command code
>   iwlwifi: mvm: rename p2p-specific sta functions to include p2p in the
> names
> 
> Mordechai Goodstein (1):
>   iwlwifi: implement fseq version mismatch warning
> 
> Seraphime Kirkovski (1):
>   iwlwifi: dvm: remove unused defines
> 
> Sharon Dvir (1):
>   iwlwifi: mvm: fix uninitialized var while waiting for queues to empty
> 
> Zamir, Roee (1):
>   iwlwifi: mvm: add compile-time option to disable EBS
> 
>  drivers/net/wireless/intel/iwlwifi/Makefile|2 +
>  drivers/net/wireless/intel/iwlwifi/dvm/commands.h  |   16 -
>  drivers/net/wireless/intel/iwlwifi/fw/api/alive.h  |  206 ++
>  .../net/wireless/intel/iwlwifi/fw/api/binding.h|  144 +
>  .../intel/iwlwifi/fw/{api.h => api/cmdhdr.h}   |   78 +-
>  .../iwlwifi/{mvm/fw-api-coex.h => fw/api/coex.h}   |   11 +-
>  .../net/wireless/intel/iwlwifi/fw/api/commands.h   |  664 +
>  drivers/net/wireless/intel/iwlwifi/fw/api/config.h |  192 ++
>  .../net/wireless/intel/iwlwifi/fw/api/context.h|   94 +
>  .../intel/iwlwifi/{mvm/fw-api-d3.h => fw/api/d3.h} |   11 +-
>  .../net/wireless/intel/iwlwifi/fw/api/datapath.h   |  127 +
>  drivers/net/wireless/intel/iwlwifi/fw/api/debug.h  |  345 +++
>  drivers/net/wireless/intel/iwlwifi/fw/api/filter.h |  183 ++
>  .../net/wireless/intel/iwlwifi/fw/api/mac-cfg.h|  152 ++
>  .../iwlwifi/{mvm/fw-api-mac.h => fw/api/mac.h} |   33 +-
>  .../net/wireless/intel/iwlwifi/fw/api/nvm-reg.h|  386 +++
>  .../net/wireless/intel/iwlwifi/fw/api/offload.h|  101 +
>  drivers/net/wireless/intel/iwlwifi/fw/api/paging.h |  108 +
>  .../net/wireless/intel/iwlwifi/fw/api/phy-ctxt.h   |  164 ++
>  drivers/net/wireless/intel/iwlwifi/fw/api/phy.h|  258 ++
>  .../iwlwifi/{mvm/fw-api-power.h => fw/api/power.h} |   13 +-
>  .../intel/iwlwifi/{mvm/fw-api-rs.h => fw/api/rs.h} |   13 +-
>  .../intel/iwlwifi/{mvm/fw-api-rx.h => fw/api/rx.h} |   31 +-
>  .../iwlwifi/{mvm/fw-api-scan.h => fw/api/scan.h}   |   11 +-
>  drivers/net/wireless/intel/iwlwifi/fw/api/sf.h |  138 +
>  .../iwlwifi/{mvm/fw-api-sta.h => fw/api/sta.h} |   15 +-
>  .../iwlwifi/{mvm/fw-api-stats.h => fw/api/stats.h} |   13 +-
>  drivers/net/wireless/intel/iwlwifi/fw/api/tdls.h   |  208 ++
>  .../net/wireless/intel/iwlwifi/fw/api/time-event.h |  386 +++
>  .../iwlwifi/{mvm/fw-api-tof.h => fw/api/tof.h} |9 +-
>  .../intel/iwlwifi/{mvm/fw-api-tx.h => fw/api/tx.h} |   42 +-
>  drivers/net/wireless/intel/iwlwifi/fw/api/txq.h|  163 ++
>  drivers/net/wireless/intel/iwlwifi/fw/common_rx.c  |   88 +
>  .../intel/iwlwifi/{mvm/fw-dbg.c => fw/dbg.c}   |  438 +--
>  .../intel/iwlwifi/{mvm/fw-dbg.h => fw/dbg.h}   |  125 +-
>  drivers/net/wireless/intel/iwlwifi/fw/init.c   |   75 +
>  drivers/net/wireless/intel/iwlwifi/fw/paging.c |  414 +++
>  drivers/net/wireless/intel/iwlwifi/fw/runtime.h|  156 ++
>  drivers/net/wireless/intel/iwlwifi/fw/smem.c   |  152 ++
>  drivers/net/wireless/intel/iwlwifi/iwl-trans.h |3 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/Makefile|2 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/coex.c  |2 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/constants.h |2 +-
>  .../net/wireless/intel/iwlwifi/mvm/debugfs-vif.c   |2 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c   | 

Re: [PATCH 19/24] iwlwifi: mvm: fix TCP CSUM offload with WEP and A000 series

[Coelho, Luciano]

On Fri, 2017-07-28 at 17:23 +0300, Luca Coelho wrote:
> From: Emmanuel Grumbach 
> 
> When we enabled TCP checksum offload, we need to tell the
> firmware where the IP header starts. If we have an IV, then
> we need to adapt that value since the IV is placed before
> the SNAP header. This is true only for cases where the
> driver adds the IV, not the WEP case in which the IV is
> added by the firmware itself.
> 
> On A000 devices series, the IV is always added by the
> device.
> 
> Fix this.
> 
> Fixes: 5e6a98dc4863 ("iwlwifi: mvm: enable TCP/UDP checksum support for 9000 
> family")
> Signed-off-by: Emmanuel Grumbach 
> Signed-off-by: Luca Coelho 
> ---

Emmanuel asked me to send this for the rc-series, since it fixes our
newly supported 9000 family.

--
Luca.

Re: [linuxwifi] x86/thermal: AB-BA dependency between mvm->mutex and tz->lock

[Coelho, Luciano]

On Thu, 2017-08-17 at 15:38 +0200, Jiri Kosina wrote:
> Hi,
> 
> anything new on this front please?
> 
> The splat (and therefore deadlock potential) is still there with current 
> Linus' tree.

Sorry, haven't had more time to spend on it.  I'll do it this evening.

But, just to clarify, the deadlock potential has been there for a while,
right? The only difference is that now we get the splat.

Not saying we shouldn't fix it though. ;)

--
Luca.

Re: new warning at net/wireless/util.c:1236

[Coelho, Luciano]

On Thu, 2017-05-04 at 07:35 +0300, Kalle Valo wrote:
> Linus Torvalds  writes:
> 
> > So my Dell XPS 13 seems to have grown a new warning as of the
> > networking merge yesterday.
> > 
> > Things still work, but when it starts warning, it generates a *lot* of
> > noise (I got 36 of these within about ten minutes).
> > 
> > I have no idea what triggered it, because when I rebooted (not because
> > of this issue, but just to reboot into a newer kernel) I don't see it
> > again.
> > 
> > This is all pretty regular wireless - it's intel 8260 wireless in a
> > fairly normal laptop.
> > 
> > Things still seem to *work* ok, so the only problem here is the overly
> > verbose and useless WARN_ON. It doesn't even print out *which* rate it
> > is warning about, it just does that stupid unconditional WARN_ON()
> > without ever shutting up about it..
> > 
> > The WARN_ON() seems to be old, but my logs don't seem to have any
> > mention of this until today, so there's something that has changed
> > that now triggers it.
> > 
> > Ideas?
> > 
> >   Linus
> > 
> > ---
> > 
> > WARNING: CPU: 3 PID: 1138 at net/wireless/util.c:1236
> > cfg80211_calculate_bitrate+0x139/0x170 [cfg80211]
> 
> As this is with iwlwifi adding also Luca. There were some rate handling
> changes in iwlwifi, like commit 77e409455f41, but don't know if that
> could cause this.

Thanks Kalle.  I don't see anything in the iwlwifi driver that could be
causing this.  Johannes suspects some RX rate changes he made in
mac80211...

--
Luca.

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2017-09-06 at 21:57 -0700, Linus Torvalds wrote:
> On Wed, Sep 6, 2017 at 9:11 PM, Coelho, Luciano
> <luciano.coe...@intel.com> wrote:
> > 
> > This seems to be a problem with backwards-compatibility with FW version
> > 27.  We are now in version 31[1] and upgrading will probably fix that.
> 
> I can confirm that fw version 31 works.

Great, so I know for sure that this is a backwards-compatibility issue
with the FW API.


> > But obviously the driver should not fail miserably like this with
> > version 27, because it claims to support it still.
> 
> Not just "claims to support it", but if it's what is shipped with a
> fairly recent distro like an up-to-date version of F26, I would really
> hope that the driver can still work with it.

I totally agree, we support a bunch of older versions for that exact
reason.  We just don't really test all the supported versions very
often.  We should probably change that.

I'll make sure it still works with version 27.

--
Cheers,
Luca.

Re: [GIT] Networking

[Coelho, Luciano]

On Wed, 2017-09-06 at 16:27 -0700, Linus Torvalds wrote:
> This pull request completely breaks Intel wireless for me.
> 
> This is my trusty old XPS 13 (9350), using Intel Wireless 8260 (rev 3a).
> 
> That remains a very standard Intel machine with absolutely zero odd
> things going on.
> 
> The firmware is iwlwifi-8000C-28.ucode from
> iwl7260-firmware-25.30.13.0-75.fc26.noarch, and the kernel reports
> 
>   iwlwifi :3a:00.0: loaded firmware version 27.455470.0 op_mode iwlmvm
> 
> the thing starts acting badly with this:
> 
>   iwlwifi :3a:00.0: FW Error notification: type 0x cmd_id 0x04
>   iwlwifi :3a:00.0: FW Error notification: seq 0x service 0x0004
>   iwlwifi :3a:00.0: FW Error notification: timestamp 0x5D84
>   iwlwifi :3a:00.0: Microcode SW error detected.  Restarting 0x200.
>   iwlwifi :3a:00.0: Start IWL Error Log Dump:
>   iwlwifi :3a:00.0: Status: 0x0100, count: 6
>   iwlwifi :3a:00.0: Loaded firmware version: 27.455470.0
>   iwlwifi :3a:00.0: 0x0038 | BAD_COMMAND
>   iwlwifi :3a:00.0: 0x00A002F0 | trm_hw_status0
>   ...
>   iwlwifi :3a:00.0: 0x | isr status reg
>   ieee80211 phy0: Hardware restart was requested
>   iwlwifi :3a:00.0: FW error in SYNC CMD MAC_CONTEXT_CMD

This seems to be a problem with backwards-compatibility with FW version
27.  We are now in version 31[1] and upgrading will probably fix that.

But obviously the driver should not fail miserably like this with
version 27, because it claims to support it still.

I'm looking into this now and will provide a fix asap.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/iwlwifi-8000C-31.ucode


--
Cheers,
Luca.

Re: iwlwifi firmware load broken in current -git

[Coelho, Luciano]

Hi Jens,

On Tue, 2017-09-12 at 09:48 -0600, Jens Axboe wrote:
> Hi,
> 
> I have no wifi in current git (8fac2f96ab8), it simply fails with:
> 
> [4.363481] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-34.ucode failed with error -2
> [4.363733] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-33.ucode failed with error -2
> [4.363744] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-32.ucode failed with error -2
> [4.368077] iwlwifi :04:00.0: loaded firmware version 31.532993.0 
> op_mode iwlmvm
> [4.461753] iwlwifi :04:00.0: Detected Intel(R) Dual Band Wireless AC 
> 8260, REV=0x208
> [ ... ]
> [9.510570] iwlwifi :04:00.0: Failed to load firmware chunk!   
>   
> [9.513903] iwlwifi :04:00.0: Could not load the [0] uCode section 
>   
> [9.516201] iwlwifi :04:00.0: Failed to start INIT ucode: -110 
>   
> [9.530842] iwlwifi :04:00.0: Failed to run INIT ucode: -110   
>   
> 
> I get the same thing with -27 of the firmware:
> 
> [   60.990831] Intel(R) Wireless WiFi driver for Linux
>   
> [   60.990833] Copyright(c) 2003- 2015 Intel Corporation  
>   
> [   60.991936] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-34.ucode failed with error -2
> [   60.991941] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-33.ucode failed with error -2
> [   60.991946] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-32.ucode failed with error -2
> [   60.991957] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-31.ucode failed with error -2
> [   60.991964] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-30.ucode failed with error -2
> [   60.991969] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-29.ucode failed with error -2
> [   60.991975] iwlwifi :04:00.0: Direct firmware load for 
> iwlwifi-8000C-28.ucode failed with error -2
> [   61.029852] iwlwifi :04:00.0: loaded firmware version 27.541033.0 
> op_mode iwlmvm
> [   61.043660] iwlwifi :04:00.0: Detected Intel(R) Dual Band Wireless AC 
> 8260, REV=0x208
> [   66.070135] iwlwifi :04:00.0: Failed to load firmware chunk!   
>   
> [   66.070149] iwlwifi :04:00.0: Could not load the [0] uCode section 
>   
> [   66.070165] iwlwifi :04:00.0: Failed to start INIT ucode: -110 
>   
> [   66.083462] iwlwifi :04:00.0: Failed to run INIT ucode: -110   
>   
> 
> Things work fine in 4.13-rc7+, which was the last kernel I ran on my laptop.

This is strange, Linus has been running this same combination with -31
(with -27 we had a regression, but it was fixed recently and the fix is
in the current master).  I have also tried this combination with both
-27 and -31 after my fix and it works fine.

There are only a couple of other patches that may affect iwlwifi since
the previous net-next.git pull request...

I'll try this combination on my machine and see if I can reproduce the
problem.

--
Cheers,
Luca.

Re: [PATCH] wireless: iwlwifi: fix minor code style issues

[Coelho, Luciano]

On Sat, 2017-09-23 at 12:31 +0200, Christoph Böhmwalder wrote:
> Fixes three trivial issues as reported by checkpatch.pl, namely two
> switch/case indentation issues and one alignment issue in a multiline comment.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---

Thanks, applied to our internal tree.  It will eventually reach the
mainline, following our usual upstreaming process.

--
Cheers,
Luca.

Re: [PATCH v2] iwlwifi: mvm: allow monitor mode capture in STA mode

[Coelho, Luciano]

Hi Gavin,

On Thu, 2017-09-14 at 22:11 -0700, gavi...@thegavinli.com wrote:
> From: Gavin Li 
>
> Open up the filter if there is a monitor interface configured; this
> allows all packets on the channel to be captured even if the device is
> in STA mode and associated to a BSS.
> 
> Signed-off-by: Gavin Li 
> ---

Sorry for the delay in responding, we were discussing this patch
internally.

Unfortunately this is not something that our firmware officially
supports, so we cannot allow it in the driver.  The firmware doesn't
*block* this from working, but there are no guarantees that it will work
and that there will not be any side-effects.

If this works for you, feel free to use it (that's one of the beauties
of open source), but I won't apply it in the mainline driver.


--
Cheers,
Luca.

Re: [PATCH RESEND] wireless: iwlwifi: fix minor code style issues

[Coelho, Luciano]

On Mon, 2017-09-25 at 13:37 +0200, Christoph Böhmwalder wrote:
> Fixes three trivial issues as reported by checkpatch.pl, namely two
switch/case indentation issues and one alignment issue in a multiline
comment.

Signed-off-by: Christoph Böhmwalder 
---

Why are you already resending this? You sent the first email 2 days ago,
you can't expect that a non-critical patch be merged in such a short
time (especially during the weekend).

--
Cheers,
Luca

Re: [bug report] iwlwifi: mvm: add station before allocating a queue

[Coelho, Luciano]

On Fri, 2017-09-01 at 11:30 +0300, Dan Carpenter wrote:
> Hello Shaul Triebitz,
> 
> The patch 732d06e9d9cf: "iwlwifi: mvm: add station before allocating
> a queue" from Jul 10, 2017, leads to the following static checker
> warning:
> 
>   drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1312 
> iwl_mvm_add_int_sta_common()
>   error: uninitialized symbol 'status'.
> 
> drivers/net/wireless/intel/iwlwifi/mvm/sta.c
>   1281  static int iwl_mvm_add_int_sta_common(struct iwl_mvm *mvm,
>   1282struct iwl_mvm_int_sta *sta,
>   1283const u8 *addr,
>   1284u16 mac_id, u16 color)
>   1285  {
>   1286  struct iwl_mvm_add_sta_cmd cmd;
>   1287  int ret;
>   1288  u32 status;
> ^^
>   1289  
>   1290  lockdep_assert_held(>mutex);
>   1291  
>   1292  memset(, 0, sizeof(cmd));
>   1293  cmd.sta_id = sta->sta_id;
>   1294  cmd.mac_id_n_color = cpu_to_le32(FW_CMD_ID_AND_COLOR(mac_id,
>   1295   color));
>   1296  if (fw_has_api(>fw->ucode_capa, 
> IWL_UCODE_TLV_API_STA_TYPE))
>   1297  cmd.station_type = sta->type;
>   1298  
>   1299  if (!iwl_mvm_has_new_tx_api(mvm))
>   1300  cmd.tfd_queue_msk = cpu_to_le32(sta->tfd_queue_msk);
>   1301  cmd.tid_disable_tx = cpu_to_le16(0x);
>   1302  
>   1303  if (addr)
>   1304  memcpy(cmd.addr, addr, ETH_ALEN);
>   1305  
>   1306  ret = iwl_mvm_send_cmd_pdu_status(mvm, ADD_STA,
>   1307
> iwl_mvm_add_sta_cmd_size(mvm),
>   1308, );
>  ^^
>   1309  if (ret)
>   1310  return ret;
>   1311  
>   1312  switch (status & IWL_ADD_STA_STATUS_MASK) {
> 
>   1313  case ADD_STA_SUCCESS:
>   1314  IWL_DEBUG_INFO(mvm, "Internal station added.\n");
>   1315  return 0;
>   1316  default:
>   1317  ret = -EIO;
>   1318  IWL_ERR(mvm, "Add internal station failed, 
> status=0x%x\n",
>   1319  status);
>   1320  break;
>   1321  }
>   1322  return ret;
>   1323  }
> 
> The problem here is this code from iwl_mvm_send_cmd_status()
> 
> drivers/net/wireless/intel/iwlwifi/mvm/utils.c
>157  cmd->flags |= CMD_WANT_SKB;
>158  
>159  ret = iwl_trans_send_cmd(mvm->trans, cmd);
>160  if (ret == -ERFKILL) {
>161  /*
>162   * The command failed because of RFKILL, don't update
>163   * the status, leave it as success and return 0.
>164   */
>165  return 0;
> 
> We return zero without setting "status".  Shouldn't we set *status = 0;?
> 
>166  } else if (ret) {
>167  return ret;
>168  }
>169  
>170  pkt = cmd->resp_pkt;

The caller should set the status to "success" before calling this
function.  In some cases success is not zero (e.g. we use
ADD_STA_SUCCESS, which is 1, in several places).

I'll send a fix specific for this patch and an additional one for other
places where we also call this function without initializing status.

Thanks for reporting!

--
Cheers,
Luca.

Re: [PATCH v2] iwlwifi: mvm: allow monitor mode capture in STA mode

[Coelho, Luciano]

On Tue, 2017-09-26 at 10:29 -0700, Gavin Li wrote:
> Hi Luciano,

Hi,

> Thanks for the update. I was hoping that it would be supported since I
> remember that iwldvm had no problem with it in the past.

As I said, it *may* work for you as it did with iwldvm, but we cannot
guarantee that it will work and we cannot support it officially.  Thus I
can't apply this to the official driver, but you can use it at your own
risk. ;)

--
Cheers,
Luca.