Re: pull-request: wireless-drivers-next 2015-05-21

2015-05-25 Thread Kalle Valo
David Miller da...@davemloft.net writes:

 From: Kalle Valo kv...@codeaurora.org
 Date: Thu, 21 May 2015 16:39:04 +0300

 here's a wireless-drivers pull request for 4.2. This time please pay
 extra attention to this pull as there are two problems:
 
 First of all as you can see the diffstat from git-pull-request in the
 end is just weird. I was long and hard trying to check everything and to
 my understanding all the merges look ok and I cannot explain the reason
 for the diffstat, but of course I might be missing something. Maybe
 git-request-pull is just buggy? At least with gitk everything looks to
 be ok and the patch list below also looks valid.

 The diffstat doesn't look anything like that for me.  It contained only
 your wireless changes.

 It may have helped that I merged 'net' into 'net-next' right before I
 pulled this.

Good to hear.

 Secondly there's a non-trivial conflict in
 drivers/net/wireless/ath/ath10k/mac.c which is due to removal of
 FIF_PROMISC_IN_BSS in commit df1404650c. You need to remove more code
 than just the obvious conflicts shown by git. In the end of this mail I
 added a git diff output after I fixed the conflict, hopefully that helps
 you to fix it. The main points are that you remove
 ath10k_mac_should_disable_promisc() and the last ath10k_monitor_recalc()
 call from ath10k_vdev_start_restart() along with the obvious conflict
 fixes git points out.
 
 There's also a patch from Michal which will also help to fix the
 resolution. Michal, please double check the resolution proposal below so
 that I didn't miss anything.
 
 https://patchwork.kernel.org/patch/6387631/

 Thanks, I think I got the conflict resolution correct, please have a look.

Looks good, thanks for fixing it.

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


Re: pull-request: wireless-drivers-next 2015-05-21

2015-05-24 Thread David Miller
From: Kalle Valo kv...@codeaurora.org
Date: Thu, 21 May 2015 16:39:04 +0300

 here's a wireless-drivers pull request for 4.2. This time please pay
 extra attention to this pull as there are two problems:
 
 First of all as you can see the diffstat from git-pull-request in the
 end is just weird. I was long and hard trying to check everything and to
 my understanding all the merges look ok and I cannot explain the reason
 for the diffstat, but of course I might be missing something. Maybe
 git-request-pull is just buggy? At least with gitk everything looks to
 be ok and the patch list below also looks valid.

The diffstat doesn't look anything like that for me.  It contained only
your wireless changes.

It may have helped that I merged 'net' into 'net-next' right before I
pulled this.

 Secondly there's a non-trivial conflict in
 drivers/net/wireless/ath/ath10k/mac.c which is due to removal of
 FIF_PROMISC_IN_BSS in commit df1404650c. You need to remove more code
 than just the obvious conflicts shown by git. In the end of this mail I
 added a git diff output after I fixed the conflict, hopefully that helps
 you to fix it. The main points are that you remove
 ath10k_mac_should_disable_promisc() and the last ath10k_monitor_recalc()
 call from ath10k_vdev_start_restart() along with the obvious conflict
 fixes git points out.
 
 There's also a patch from Michal which will also help to fix the
 resolution. Michal, please double check the resolution proposal below so
 that I didn't miss anything.
 
 https://patchwork.kernel.org/patch/6387631/

Thanks, I think I got the conflict resolution correct, please have a look.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: pull-request: wireless-drivers-next 2015-05-21

2015-05-22 Thread Michal Kazior
On 21 May 2015 at 15:39, Kalle Valo kv...@codeaurora.org wrote:
 Hi Dave,

 here's a wireless-drivers pull request for 4.2. This time please pay
 extra attention to this pull as there are two problems:

 First of all as you can see the diffstat from git-pull-request in the
 end is just weird. I was long and hard trying to check everything and to
 my understanding all the merges look ok and I cannot explain the reason
 for the diffstat, but of course I might be missing something. Maybe
 git-request-pull is just buggy? At least with gitk everything looks to
 be ok and the patch list below also looks valid.

 Secondly there's a non-trivial conflict in
 drivers/net/wireless/ath/ath10k/mac.c which is due to removal of
 FIF_PROMISC_IN_BSS in commit df1404650c. You need to remove more code
 than just the obvious conflicts shown by git. In the end of this mail I
 added a git diff output after I fixed the conflict, hopefully that helps
 you to fix it. The main points are that you remove
 ath10k_mac_should_disable_promisc() and the last ath10k_monitor_recalc()
 call from ath10k_vdev_start_restart() along with the obvious conflict
 fixes git points out.

 There's also a patch from Michal which will also help to fix the
 resolution. Michal, please double check the resolution proposal below so
 that I didn't miss anything.

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

[...]

 diff --cc drivers/net/wireless/ath/ath10k/mac.c
 index fcd08b2f8d26,eaa0182e001d..
 --- a/drivers/net/wireless/ath/ath10k/mac.c
 +++ b/drivers/net/wireless/ath/ath10k/mac.c
 @@@ -766,9 -1031,68 +1031,48 @@@ static int ath10k_monitor_stop(struct a
 return 0;
   }

  -static bool ath10k_mac_should_disable_promisc(struct ath10k *ar)
  -{
  -  struct ath10k_vif *arvif;
  -
  -  if (!(ar-filter_flags  FIF_PROMISC_IN_BSS))
  -  return true;
  -
  -  if (!ar-num_started_vdevs)
  -  return false;
  -
  -  list_for_each_entry(arvif, ar-arvifs, list)
  -  if (arvif-vdev_type != WMI_VDEV_TYPE_AP)
  -  return false;
  -
  -  ath10k_dbg(ar, ATH10K_DBG_MAC,
  - mac disabling promiscuous mode because vdev is 
 started\n);
  -  return true;
  -}
  -
 + static bool ath10k_mac_monitor_vdev_is_needed(struct ath10k *ar)
 + {
 +   int num_ctx;
 +
 +   /* At least one chanctx is required to derive a channel to start
 +* monitor vdev on.
 +*/
 +   num_ctx = ath10k_mac_num_chanctxs(ar);
 +   if (num_ctx == 0)
 +   return false;
 +
 +   /* If there's already an existing special monitor interface then don't
 +* bother creating another monitor vdev.
 +*/
 +   if (ar-monitor_arvif)
 +   return false;
 +
 +   return ar-monitor ||
  - !ath10k_mac_should_disable_promisc(ar) ||
 +  test_bit(ATH10K_CAC_RUNNING, ar-dev_flags);
 + }
 +
 + static bool ath10k_mac_monitor_vdev_is_allowed(struct ath10k *ar)
 + {
 +   int num_ctx;
 +
 +   num_ctx = ath10k_mac_num_chanctxs(ar);
 +
 +   /* FIXME: Current interface combinations and cfg80211/mac80211 code
 +* shouldn't allow this but make sure to prevent handling the 
 following
 +* case anyway since multi-channel DFS hasn't been tested at all.
 +*/
 +   if (test_bit(ATH10K_CAC_RUNNING, ar-dev_flags)  num_ctx  1)
 +   return false;
 +
 +   return true;
 + }
 +
   static int ath10k_monitor_recalc(struct ath10k *ar)
   {
 -   bool should_start;
 +   bool needed;
 +   bool allowed;
 +   int ret;

 lockdep_assert_held(ar-conf_mutex);


Looks good to me.

There's still some code left which is unnecessary (see 2 last hunks on
patchwork) because due to FIF_PROMISC_IN_BSS removal entire commit
548462133d98 becomes obsolete. Since these 2 hunk leftovers don't
break anything functionally I guess this can be cleaned up in a follow
up patch after the merge. Just my 2cc.


 @@@ -871,12 -1231,46 +1211,46 @@@ static void ath10k_recalc_radar_detecti
 }
   }

 - static int ath10k_vdev_start_restart(struct ath10k_vif *arvif, bool restart)
 + static int ath10k_vdev_stop(struct ath10k_vif *arvif)
 + {
 +   struct ath10k *ar = arvif-ar;
 +   int ret;
 +
 +   lockdep_assert_held(ar-conf_mutex);
 +
 +   reinit_completion(ar-vdev_setup_done);
 +
 +   ret = ath10k_wmi_vdev_stop(ar, arvif-vdev_id);
 +   if (ret) {
 +   ath10k_warn(ar, failed to stop WMI vdev %i: %d\n,
 +   arvif-vdev_id, ret);
 +   return ret;
 +   }
 +
 +   ret = ath10k_vdev_setup_sync(ar);
 +   if (ret) {
 +   ath10k_warn(ar, failed to syncronise setup for vdev %i: 
 %d\n,
 +   arvif-vdev_id, ret);
 +   return ret;
 +   }
 +
 +   WARN_ON(ar-num_started_vdevs == 0);
 +
 +   if (ar-num_started_vdevs != 0) {
 +   ar-num_started_vdevs--;
 + 

Re: pull-request: wireless-drivers-next 2015-05-21

2015-05-22 Thread Michal Kazior
On 22 May 2015 at 14:27, Kalle Valo kv...@codeaurora.org wrote:
 Michal Kazior michal.kaz...@tieto.com writes:
 On 21 May 2015 at 15:39, Kalle Valo kv...@codeaurora.org wrote:
[...]
 + static int ath10k_vdev_start_restart(struct ath10k_vif *arvif,
 +const struct cfg80211_chan_def 
 *chandef,
 +bool restart)
   {
 struct ath10k *ar = arvif-ar;
 -   struct cfg80211_chan_def *chandef = ar-chandef;
 struct wmi_vdev_start_request_arg arg = {};
  -  int ret = 0, ret2;
  +  int ret = 0;

 lockdep_assert_held(ar-conf_mutex);


 Kalle, I'm not seeing this when I merge your pull tag on top of
 net-next/master. Am I missing something?

 You lost me, you are not seeing what?

Your merge resolution diff seems to include stuff regarding
ath10k_vdev_stop(). I didn't see it when I tried the merge and had
only 2 conflicts: the promisc code and at feature flags (XMIT_FAST).


 I think we have a misunderstanding

That's possible.


 here, let me explain how I created that diff:

 git clone --reference ~/linux-2.6/ 
 git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
 cd net-next/
 git pull 
 git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
 tags/wireless-drivers-next-for-davem-2015-05-21
 emacs drivers/net/wireless/ath/ath10k/mac.c
 git diff  resolution.txt

For what it's worth I did:
 git checkout net-next/master
 git merge wireless-drivers-next-for-davem-2015-05-21
 # fix conflicts
 git commit -a
 git show

Merge should essentially yield the same result as your pull.


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


Re: pull-request: wireless-drivers-next 2015-05-21

2015-05-22 Thread Kalle Valo
Michal Kazior michal.kaz...@tieto.com writes:

 On 21 May 2015 at 15:39, Kalle Valo kv...@codeaurora.org wrote:

 Secondly there's a non-trivial conflict in
 drivers/net/wireless/ath/ath10k/mac.c which is due to removal of
 FIF_PROMISC_IN_BSS in commit df1404650c. You need to remove more code
 than just the obvious conflicts shown by git. In the end of this mail I
 added a git diff output after I fixed the conflict, hopefully that helps
 you to fix it. The main points are that you remove
 ath10k_mac_should_disable_promisc() and the last ath10k_monitor_recalc()
 call from ath10k_vdev_start_restart() along with the obvious conflict
 fixes git points out.

 There's also a patch from Michal which will also help to fix the
 resolution. Michal, please double check the resolution proposal below so
 that I didn't miss anything.

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

[...]

 Looks good to me.

Thanks for checking.

 There's still some code left which is unnecessary (see 2 last hunks on
 patchwork) because due to FIF_PROMISC_IN_BSS removal entire commit
 548462133d98 becomes obsolete. Since these 2 hunk leftovers don't
 break anything functionally I guess this can be cleaned up in a follow
 up patch after the merge. Just my 2cc.

Just to avoid any confusion, these are the hunks you mean (copypaste
from patchwork, whitespace damage likely):

@@ -1267,7 +1250,7 @@  static int ath10k_vdev_start_restart(struct ath10k_vif 
*arvif,
 {
struct ath10k *ar = arvif-ar;
struct wmi_vdev_start_request_arg arg = {};
-   int ret = 0, ret2;
+   int ret = 0;
 
lockdep_assert_held(ar-conf_mutex);
 
@@ -1326,16 +1309,6 @@  static int ath10k_vdev_start_restart(struct ath10k_vif 
*arvif,
ar-num_started_vdevs++;
ath10k_recalc_radar_detection(ar);
 
-   ret = ath10k_monitor_recalc(ar);
-   if (ret) {
-   ath10k_warn(ar, mac failed to recalc monitor for vdev %i 
restart %d: %d\n,
-   arg.vdev_id, restart, ret);
-   ret2 = ath10k_vdev_stop(arvif);
-   if (ret2)
-   ath10k_warn(ar, mac failed to stop vdev %i restart %d: 
%d\n,
-   arg.vdev_id, restart, ret2);
-   }
-
return ret;
 }

Yeah, these needs to be removed. For some reason 'git diff' doesn't show
these hunks after the conflicts are fixed.

 + static int ath10k_vdev_start_restart(struct ath10k_vif *arvif,
 +const struct cfg80211_chan_def *chandef,
 +bool restart)
   {
 struct ath10k *ar = arvif-ar;
 -   struct cfg80211_chan_def *chandef = ar-chandef;
 struct wmi_vdev_start_request_arg arg = {};
  -  int ret = 0, ret2;
  +  int ret = 0;

 lockdep_assert_held(ar-conf_mutex);


 Kalle, I'm not seeing this when I merge your pull tag on top of
 net-next/master. Am I missing something?

You lost me, you are not seeing what? I think we have a misunderstanding
here, let me explain how I created that diff:

git clone --reference ~/linux-2.6/ 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
cd net-next/
git pull 
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
tags/wireless-drivers-next-for-davem-2015-05-21
emacs drivers/net/wireless/ath/ath10k/mac.c
git diff  resolution.txt

 Anyway FYI ath10k_vdev_stop() was moved up in the code to avoid
 forward declaration in 822b7e0b633b.

Didn't understand this either.

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