Re: pull-request: wireless-drivers-next 2015-05-21
David Miller writes: > From: Kalle Valo > 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
From: Kalle Valo 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
On 22 May 2015 at 14:27, Kalle Valo wrote: > Michal Kazior writes: >> On 21 May 2015 at 15:39, Kalle Valo 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
Michal Kazior writes: > On 21 May 2015 at 15:39, Kalle Valo 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
Re: pull-request: wireless-drivers-next 2015-05-21
On 21 May 2015 at 15:39, Kalle Valo 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; > +