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

2017-09-25 Thread 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: [GIT] Networking

2017-09-06 Thread 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

2017-09-06 Thread 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: [PATCH v3] iwlwifi: Demote messages about fw flags size to info

2017-08-08 Thread 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 V5 1/2] firmware: add more flexible request_firmware_async function

2017-08-02 Thread 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: [PATCH] iwlwifi: Demote messages about fw flags size to info

2017-07-24 Thread 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: new warning at net/wireless/util.c:1236

2017-05-04 Thread 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: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded

2016-07-11 Thread 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-wirel...@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---

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

--
Luca.

Re: [PATCH] iwlwifi: mvm: avoid harmless -Wmaybe-uninialized warning

2016-05-30 Thread Coelho, Luciano
On Fri, 2016-05-27 at 15:07 +0200, Arnd Bergmann wrote:
> gcc is apparently unablel to track the state of the local 'resp_v2'
> variable across the kzalloc() function, and warns about the response
> variable being used without an initialization:
> 
> drivers/net/wireless/intel/iwlwifi/mvm/nvm.c: In function
> ‘iwl_mvm_update_mcc’:
> drivers/net/wireless/intel/iwlwifi/mvm/nvm.c:727:36: warning:
> ‘mcc_resp_v1’ may be used uninitialized in this function [-Wmaybe-
> uninitialized]
>    resp_cp->n_channels = mcc_resp_v1->n_channels;
> drivers/net/wireless/intel/iwlwifi/mvm/nvm.c:721:3: warning:
> ‘mcc_resp’ may be used uninitialized in this function [-Wmaybe-
> uninitialized]
>    memcpy(resp_cp, mcc_resp, resp_len);
> 
> The warning showed up in x86 allmodconfig after my patch to
> unhide -Wmaybe-uninitialized warnings by default was merged,
> though it always existed in randconfig builds. I did not
> catch the warning earlier because I was testing on ARM, which
> never produced the warning.
> 
> This rearranges the code in a way that improves readability for
> both humans and the compiler, and that avoids the warning.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 6fa52430f0b3 ("iwlwifi: mvm: change mcc update API")
> ---

Thanks, Arnd! I queued this via our internal tree.

--
Cheers,
Luca.

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

2016-05-19 Thread 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.

Re: [GIT] Networking

2016-05-18 Thread 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.

Re: [GIT] Networking

2016-05-18 Thread 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.

Re: [GIT] Networking

2016-05-18 Thread 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.

Re: [GIT] Networking

2016-05-18 Thread 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.

Re: [GIT] Networking

2016-05-18 Thread 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: pull-request: wireless-drivers-next 2016-05-13

2016-05-16 Thread 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: linux-next: manual merge of the wireless-drivers-next tree with the net-next tree

2016-05-16 Thread 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.

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

2015-10-08 Thread 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.