| Message ID | 20240830080719.826142-1-yu-hao.lin@nxp.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: mwifiex: fix firmware crash for AP DFS mode | expand |
On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > Firmware crashes when AP works on a DFS channel and radar detection occurs. > This patch fixes the issue, also add "fake_radar_detect" entry to mimic > radar detection for testing purpose. Is this issue generic or specific to some firmware version or Wi-Fi device? Francesco
+Sascha On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > Firmware crashes when AP works on a DFS channel and radar detection occurs. > This patch fixes the issue, also add "fake_radar_detect" entry to mimic > radar detection for testing purpose. > > Signed-off-by: David Lin <yu-hao.lin@nxp.com> Sascha: FYI, given you are working on this driver lately. Francesco
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Friday, August 30, 2024 4:11 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete > Hsieh <tsung-hsien.hsieh@nxp.com> > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS mode > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > > Firmware crashes when AP works on a DFS channel and radar detection > occurs. > > This patch fixes the issue, also add "fake_radar_detect" entry to > > mimic radar detection for testing purpose. > > Is this issue generic or specific to some firmware version or Wi-Fi device? > > Francesco > This is generic issue. Mwifiex is too old to have the correct code to handle DFS. David
+Lucas (in case he missed this patch) On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > Firmware crashes when AP works on a DFS channel and radar detection occurs. > This patch fixes the issue, also add "fake_radar_detect" entry to mimic > radar detection for testing purpose. Do we want such kind of "fake" code in the driver? I do not agree that we mix an actual bug fix with additional testing code, and if I understand correctly the commit message this is what we are doing here. BTW, I think you should elaborate more in the commit message "This patch fixes the issue" to allow this patch to be reviewed. With that said I had a quick look at the patch, I think that those points need to be clarified before I can look more into it. > > Signed-off-by: David Lin <yu-hao.lin@nxp.com> > --- > drivers/net/wireless/marvell/mwifiex/11h.c | 56 +++++++++++++++---- > .../net/wireless/marvell/mwifiex/cfg80211.c | 50 ++++++++--------- > .../net/wireless/marvell/mwifiex/cfg80211.h | 4 +- > .../net/wireless/marvell/mwifiex/debugfs.c | 42 ++++++++++++++ > drivers/net/wireless/marvell/mwifiex/decl.h | 1 + > drivers/net/wireless/marvell/mwifiex/main.h | 1 + > 6 files changed, 115 insertions(+), 39 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/11h.c b/drivers/net/wireless/marvell/mwifiex/11h.c > index b90f922f1cdc..e7e7a154831f 100644 > --- a/drivers/net/wireless/marvell/mwifiex/11h.c > +++ b/drivers/net/wireless/marvell/mwifiex/11h.c > @@ -7,7 +7,7 @@ > > #include "main.h" > #include "fw.h" > - > +#include "cfg80211.h" > > void mwifiex_init_11h_params(struct mwifiex_private *priv) > { > @@ -220,8 +220,11 @@ int mwifiex_11h_handle_chanrpt_ready(struct mwifiex_private *priv, > cancel_delayed_work_sync(&priv->dfs_cac_work); > cfg80211_cac_event(priv->netdev, > &priv->dfs_chandef, > - NL80211_RADAR_DETECTED, > + NL80211_RADAR_CAC_ABORTED, > GFP_KERNEL); > + cfg80211_radar_event(priv->adapter->wiphy, > + &priv->dfs_chandef, > + GFP_KERNEL); > } > break; > default: > @@ -244,9 +247,16 @@ int mwifiex_11h_handle_radar_detected(struct mwifiex_private *priv, > > mwifiex_dbg(priv->adapter, MSG, > "radar detected; indicating kernel\n"); > - if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) > - mwifiex_dbg(priv->adapter, ERROR, > - "Failed to stop CAC in FW\n"); > + > + if (priv->wdev.cac_started) { > + if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) > + mwifiex_dbg(priv->adapter, ERROR, > + "Failed to stop CAC in FW\n"); > + cancel_delayed_work_sync(&priv->dfs_cac_work); > + cfg80211_cac_event(priv->netdev, &priv->dfs_chandef, > + NL80211_RADAR_CAC_ABORTED, GFP_KERNEL); > + } > + > cfg80211_radar_event(priv->adapter->wiphy, &priv->dfs_chandef, > GFP_KERNEL); > mwifiex_dbg(priv->adapter, MSG, "regdomain: %d\n", > @@ -267,27 +277,53 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct *work) > struct mwifiex_uap_bss_param *bss_cfg; > struct delayed_work *delayed_work = to_delayed_work(work); > struct mwifiex_private *priv = > - container_of(delayed_work, struct mwifiex_private, > - dfs_chan_sw_work); > + container_of(delayed_work, struct mwifiex_private, > + dfs_chan_sw_work); > + struct mwifiex_adapter *adapter = priv->adapter; > + > + if (mwifiex_del_mgmt_ies(priv)) > + mwifiex_dbg(adapter, ERROR, > + "Failed to delete mgmt IEs!\n"); > > bss_cfg = &priv->bss_cfg; > if (!bss_cfg->beacon_period) { > - mwifiex_dbg(priv->adapter, ERROR, > + mwifiex_dbg(adapter, ERROR, > "channel switch: AP already stopped\n"); This change of adding `struct mwifiex_adapter *adapter` and refactoring the related code is 100% fine, but mixing it here is just making the review work more complex. > + > + if (priv->uap_stop_tx) { > + if (!netif_carrier_ok(priv->netdev)) is this if needed? why? can't you just call netif_carrier_on() in every case? > + netif_carrier_on(priv->netdev); > + mwifiex_wake_up_net_dev_queue(priv->netdev, adapter); > + priv->uap_stop_tx = false; > + } > } > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c > index 722ead51e912..c5e8f12da0ae 100644 > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c > @@ -1892,6 +1882,20 @@ static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, > return 0; > } > > +/* cfg80211 operation handler for change_beacon. > + * Function retrieves and sets modified management IEs to FW. > + */ > +static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, > + struct net_device *dev, > + struct cfg80211_ap_update *params) > +{ > + int ret; > + > + ret = mwifiex_cfg80211_change_beacon_data(wiphy, dev, ¶ms->beacon); > + > + return ret; just return mwifiex_cfg80211_change_beacon_data(wiphy, dev, ¶ms->beacon);
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Wednesday, September 11, 2024 5:33 PM > To: David Lin <yu-hao.lin@nxp.com>; l.stach@pengutronix.de > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete > Hsieh <tsung-hsien.hsieh@nxp.com> > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS mode > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > +Lucas (in case he missed this patch) > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > > Firmware crashes when AP works on a DFS channel and radar detection > occurs. > > This patch fixes the issue, also add "fake_radar_detect" entry to > > mimic radar detection for testing purpose. > > Do we want such kind of "fake" code in the driver? > > I do not agree that we mix an actual bug fix with additional testing code, and if > I understand correctly the commit message this is what we are doing here. > This file can be used to test this patch on other chips without really radar detection from HW. We only test this patch with IW416. > BTW, I think you should elaborate more in the commit message "This patch > fixes the issue" to allow this patch to be reviewed. > O.K. > With that said I had a quick look at the patch, I think that those points need to > be clarified before I can look more into it. > O.K. > > > > Signed-off-by: David Lin <yu-hao.lin@nxp.com> > > --- > > drivers/net/wireless/marvell/mwifiex/11h.c | 56 +++++++++++++++---- > > .../net/wireless/marvell/mwifiex/cfg80211.c | 50 ++++++++--------- > > .../net/wireless/marvell/mwifiex/cfg80211.h | 4 +- > > .../net/wireless/marvell/mwifiex/debugfs.c | 42 ++++++++++++++ > > drivers/net/wireless/marvell/mwifiex/decl.h | 1 + > > drivers/net/wireless/marvell/mwifiex/main.h | 1 + > > 6 files changed, 115 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/11h.c > > b/drivers/net/wireless/marvell/mwifiex/11h.c > > index b90f922f1cdc..e7e7a154831f 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/11h.c > > +++ b/drivers/net/wireless/marvell/mwifiex/11h.c > > @@ -7,7 +7,7 @@ > > > > #include "main.h" > > #include "fw.h" > > - > > +#include "cfg80211.h" > > > > void mwifiex_init_11h_params(struct mwifiex_private *priv) { @@ > > -220,8 +220,11 @@ int mwifiex_11h_handle_chanrpt_ready(struct > mwifiex_private *priv, > > > cancel_delayed_work_sync(&priv->dfs_cac_work); > > cfg80211_cac_event(priv->netdev, > > > &priv->dfs_chandef, > > - > NL80211_RADAR_DETECTED, > > + > > + NL80211_RADAR_CAC_ABORTED, > > GFP_KERNEL); > > + > cfg80211_radar_event(priv->adapter->wiphy, > > + > &priv->dfs_chandef, > > + GFP_KERNEL); > > } > > break; > > default: > > @@ -244,9 +247,16 @@ int mwifiex_11h_handle_radar_detected(struct > > mwifiex_private *priv, > > > > mwifiex_dbg(priv->adapter, MSG, > > "radar detected; indicating kernel\n"); > > - if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) > > - mwifiex_dbg(priv->adapter, ERROR, > > - "Failed to stop CAC in FW\n"); > > + > > + if (priv->wdev.cac_started) { > > + if (mwifiex_stop_radar_detection(priv, > &priv->dfs_chandef)) > > + mwifiex_dbg(priv->adapter, ERROR, > > + "Failed to stop CAC in FW\n"); > > + cancel_delayed_work_sync(&priv->dfs_cac_work); > > + cfg80211_cac_event(priv->netdev, &priv->dfs_chandef, > > + NL80211_RADAR_CAC_ABORTED, > GFP_KERNEL); > > + } > > + > > cfg80211_radar_event(priv->adapter->wiphy, &priv->dfs_chandef, > > GFP_KERNEL); > > mwifiex_dbg(priv->adapter, MSG, "regdomain: %d\n", @@ -267,27 > > +277,53 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct > *work) > > struct mwifiex_uap_bss_param *bss_cfg; > > struct delayed_work *delayed_work = to_delayed_work(work); > > struct mwifiex_private *priv = > > - container_of(delayed_work, struct > mwifiex_private, > > - dfs_chan_sw_work); > > + container_of(delayed_work, struct mwifiex_private, > > + dfs_chan_sw_work); > > + struct mwifiex_adapter *adapter = priv->adapter; > > + > > + if (mwifiex_del_mgmt_ies(priv)) > > + mwifiex_dbg(adapter, ERROR, > > + "Failed to delete mgmt IEs!\n"); > > > > bss_cfg = &priv->bss_cfg; > > if (!bss_cfg->beacon_period) { > > - mwifiex_dbg(priv->adapter, ERROR, > > + mwifiex_dbg(adapter, ERROR, > > "channel switch: AP already stopped\n"); > This change of adding `struct mwifiex_adapter *adapter` and refactoring the > related code is 100% fine, but mixing it here is just making the review work > more complex. > O.K. I will remove it. > > + > > + if (priv->uap_stop_tx) { > > + if (!netif_carrier_ok(priv->netdev)) > > is this if needed? why? can't you just call netif_carrier_on() in every case? > If netif_carrier_ok(), there is no need to call netif_carrier_on(). > > + netif_carrier_on(priv->netdev); > > > > + mwifiex_wake_up_net_dev_queue(priv->netdev, adapter); > > + priv->uap_stop_tx = false; > > + } > > } > > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c > > b/drivers/net/wireless/marvell/mwifiex/cfg80211.c > > index 722ead51e912..c5e8f12da0ae 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c > > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c > > @@ -1892,6 +1882,20 @@ static int > mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, > > return 0; > > } > > > > +/* cfg80211 operation handler for change_beacon. > > + * Function retrieves and sets modified management IEs to FW. > > + */ > > +static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, > > + struct net_device *dev, > > + struct cfg80211_ap_update > > +*params) { > > + int ret; > > + > > + ret = mwifiex_cfg80211_change_beacon_data(wiphy, dev, > > + ¶ms->beacon); > > + > > + return ret; > > just return mwifiex_cfg80211_change_beacon_data(wiphy, dev, > ¶ms->beacon); O.K. Will change it in patch v2.
On Thu, Sep 12, 2024 at 02:22:09AM +0000, David Lin wrote: > > From: Francesco Dolcini <francesco@dolcini.it> > > Sent: Wednesday, September 11, 2024 5:33 PM > > To: David Lin <yu-hao.lin@nxp.com>; l.stach@pengutronix.de > > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete > > Hsieh <tsung-hsien.hsieh@nxp.com> > > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS mode > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > +Lucas (in case he missed this patch) > > > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > > > Firmware crashes when AP works on a DFS channel and radar detection > > occurs. > > > This patch fixes the issue, also add "fake_radar_detect" entry to > > > mimic radar detection for testing purpose. > > > > Do we want such kind of "fake" code in the driver? > > > > I do not agree that we mix an actual bug fix with additional testing code, and if > > I understand correctly the commit message this is what we are doing here. > > > > This file can be used to test this patch on other chips without really radar > detection from HW. please move the fake test code to a separate patch so that it can be discussed separetely from the actual fix > > > --- a/drivers/net/wireless/marvell/mwifiex/11h.c > > > +++ b/drivers/net/wireless/marvell/mwifiex/11h.c ... > > > + > > > + if (priv->uap_stop_tx) { > > > + if (!netif_carrier_ok(priv->netdev)) > > > > is this if needed? why? can't you just call netif_carrier_on() in every case? > > If netif_carrier_ok(), there is no need to call netif_carrier_on(). yes, ok. this I know. But it seems not needed, and one line less of code is better than having one additional useless line of code. My question is, is it required to have it? for what reason? My undestanding is that you should just remove it, but maybe I am missing something. > > > + netif_carrier_on(priv->netdev); > > > > > > > + mwifiex_wake_up_net_dev_queue(priv->netdev, adapter); > > > + priv->uap_stop_tx = false; > > > + } > > > }
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Thursday, September 12, 2024 3:56 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; l.stach@pengutronix.de; > linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; kvalo@kernel.org; Pete Hsieh > <tsung-hsien.hsieh@nxp.com> > Subject: Re: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS > mode > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Thu, Sep 12, 2024 at 02:22:09AM +0000, David Lin wrote: > > > From: Francesco Dolcini <francesco@dolcini.it> > > > Sent: Wednesday, September 11, 2024 5:33 PM > > > To: David Lin <yu-hao.lin@nxp.com>; l.stach@pengutronix.de > > > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > > > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; > > > Pete Hsieh <tsung-hsien.hsieh@nxp.com> > > > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP > > > DFS mode > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > +Lucas (in case he missed this patch) > > > > > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > > > > Firmware crashes when AP works on a DFS channel and radar > > > > detection > > > occurs. > > > > This patch fixes the issue, also add "fake_radar_detect" entry to > > > > mimic radar detection for testing purpose. > > > > > > Do we want such kind of "fake" code in the driver? > > > > > > I do not agree that we mix an actual bug fix with additional testing > > > code, and if I understand correctly the commit message this is what we are > doing here. > > > > > > > This file can be used to test this patch on other chips without really > > radar detection from HW. > > please move the fake test code to a separate patch so that it can be discussed > separetely from the actual fix > O.K. I will remove this debugfs file for patch v2. > > > > --- a/drivers/net/wireless/marvell/mwifiex/11h.c > > > > +++ b/drivers/net/wireless/marvell/mwifiex/11h.c > > ... > > > > > + > > > > + if (priv->uap_stop_tx) { > > > > + if (!netif_carrier_ok(priv->netdev)) > > > > > > is this if needed? why? can't you just call netif_carrier_on() in every case? > > > > If netif_carrier_ok(), there is no need to call netif_carrier_on(). > > yes, ok. this I know. But it seems not needed, and one line less of code is better > than having one additional useless line of code. > > My question is, is it required to have it? for what reason? My undestanding is > that you should just remove it, but maybe I am missing something. > I check netif_carrier_on(), it will check "test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state)" before turning on the carrier. I will remove this check for patch v2 . I will also remove all the checks for nxpwifi, but for original code of mwifiex, there should be no plan to remove this kind of check. David
Francesco Dolcini <francesco@dolcini.it> writes: > +Lucas (in case he missed this patch) > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: >> Firmware crashes when AP works on a DFS channel and radar detection occurs. >> This patch fixes the issue, also add "fake_radar_detect" entry to mimic >> radar detection for testing purpose. > > Do we want such kind of "fake" code in the driver? BTW in ath11k we have dfs_simulate_radar debugfs file for testing DFS, I assume this is something similar. So there are benefits from having it. > I do not agree that we mix an actual bug fix with additional testing code, > and if I understand correctly the commit message this is what we are doing > here. Yeah, we have a rule "one logical change per patch". So the debugfs addition needs to be in a separate patch.
> From: Kalle Valo <kvalo@kernel.org> > Sent: Thursday, September 19, 2024 1:58 AM > To: Francesco Dolcini <francesco@dolcini.it> > Cc: David Lin <yu-hao.lin@nxp.com>; l.stach@pengutronix.de; > linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; Pete Hsieh <tsung-hsien.hsieh@nxp.com> > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS mode > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Francesco Dolcini <francesco@dolcini.it> writes: > > > +Lucas (in case he missed this patch) > > > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > >> Firmware crashes when AP works on a DFS channel and radar detection > occurs. > >> This patch fixes the issue, also add "fake_radar_detect" entry to > >> mimic radar detection for testing purpose. > > > > Do we want such kind of "fake" code in the driver? > > BTW in ath11k we have dfs_simulate_radar debugfs file for testing DFS, I > assume this is something similar. So there are benefits from having it. > Nice to know it. > > I do not agree that we mix an actual bug fix with additional testing > > code, and if I understand correctly the commit message this is what we > > are doing here. > > Yeah, we have a rule "one logical change per patch". So the debugfs addition > needs to be in a separate patch. > O.K. I will create another patch to add this debugfs file for radar detection test. David
diff --git a/drivers/net/wireless/marvell/mwifiex/11h.c b/drivers/net/wireless/marvell/mwifiex/11h.c index b90f922f1cdc..e7e7a154831f 100644 --- a/drivers/net/wireless/marvell/mwifiex/11h.c +++ b/drivers/net/wireless/marvell/mwifiex/11h.c @@ -7,7 +7,7 @@ #include "main.h" #include "fw.h" - +#include "cfg80211.h" void mwifiex_init_11h_params(struct mwifiex_private *priv) { @@ -220,8 +220,11 @@ int mwifiex_11h_handle_chanrpt_ready(struct mwifiex_private *priv, cancel_delayed_work_sync(&priv->dfs_cac_work); cfg80211_cac_event(priv->netdev, &priv->dfs_chandef, - NL80211_RADAR_DETECTED, + NL80211_RADAR_CAC_ABORTED, GFP_KERNEL); + cfg80211_radar_event(priv->adapter->wiphy, + &priv->dfs_chandef, + GFP_KERNEL); } break; default: @@ -244,9 +247,16 @@ int mwifiex_11h_handle_radar_detected(struct mwifiex_private *priv, mwifiex_dbg(priv->adapter, MSG, "radar detected; indicating kernel\n"); - if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) - mwifiex_dbg(priv->adapter, ERROR, - "Failed to stop CAC in FW\n"); + + if (priv->wdev.cac_started) { + if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) + mwifiex_dbg(priv->adapter, ERROR, + "Failed to stop CAC in FW\n"); + cancel_delayed_work_sync(&priv->dfs_cac_work); + cfg80211_cac_event(priv->netdev, &priv->dfs_chandef, + NL80211_RADAR_CAC_ABORTED, GFP_KERNEL); + } + cfg80211_radar_event(priv->adapter->wiphy, &priv->dfs_chandef, GFP_KERNEL); mwifiex_dbg(priv->adapter, MSG, "regdomain: %d\n", @@ -267,27 +277,53 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct *work) struct mwifiex_uap_bss_param *bss_cfg; struct delayed_work *delayed_work = to_delayed_work(work); struct mwifiex_private *priv = - container_of(delayed_work, struct mwifiex_private, - dfs_chan_sw_work); + container_of(delayed_work, struct mwifiex_private, + dfs_chan_sw_work); + struct mwifiex_adapter *adapter = priv->adapter; + + if (mwifiex_del_mgmt_ies(priv)) + mwifiex_dbg(adapter, ERROR, + "Failed to delete mgmt IEs!\n"); bss_cfg = &priv->bss_cfg; if (!bss_cfg->beacon_period) { - mwifiex_dbg(priv->adapter, ERROR, + mwifiex_dbg(adapter, ERROR, "channel switch: AP already stopped\n"); return; } + if (mwifiex_send_cmd(priv, HostCmd_CMD_UAP_BSS_STOP, + HostCmd_ACT_GEN_SET, 0, NULL, true)) { + mwifiex_dbg(adapter, ERROR, + "channel switch: Failed to stop the BSS\n"); + return; + } + + if (mwifiex_cfg80211_change_beacon_data(adapter->wiphy, priv->netdev, + &priv->beacon_after)) { + mwifiex_dbg(adapter, ERROR, + "channel switch: Failed to set beacon\n"); + return; + } + mwifiex_uap_set_channel(priv, bss_cfg, priv->dfs_chandef); if (mwifiex_config_start_uap(priv, bss_cfg)) { - mwifiex_dbg(priv->adapter, ERROR, + mwifiex_dbg(adapter, ERROR, "Failed to start AP after channel switch\n"); return; } - mwifiex_dbg(priv->adapter, MSG, + mwifiex_dbg(adapter, MSG, "indicating channel switch completion to kernel\n"); wiphy_lock(priv->wdev.wiphy); cfg80211_ch_switch_notify(priv->netdev, &priv->dfs_chandef, 0); wiphy_unlock(priv->wdev.wiphy); + + if (priv->uap_stop_tx) { + if (!netif_carrier_ok(priv->netdev)) + netif_carrier_on(priv->netdev); + mwifiex_wake_up_net_dev_queue(priv->netdev, adapter); + priv->uap_stop_tx = false; + } } diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c index 722ead51e912..c5e8f12da0ae 100644 --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c @@ -1858,16 +1858,12 @@ static int mwifiex_cfg80211_set_cqm_rssi_config(struct wiphy *wiphy, return 0; } -/* cfg80211 operation handler for change_beacon. - * Function retrieves and sets modified management IEs to FW. - */ -static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, - struct net_device *dev, - struct cfg80211_ap_update *params) +int mwifiex_cfg80211_change_beacon_data(struct wiphy *wiphy, + struct net_device *dev, + struct cfg80211_beacon_data *data) { struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); struct mwifiex_adapter *adapter = priv->adapter; - struct cfg80211_beacon_data *data = ¶ms->beacon; mwifiex_cancel_scan(adapter); @@ -1877,12 +1873,6 @@ static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, return -EINVAL; } - if (!priv->bss_started) { - mwifiex_dbg(priv->adapter, ERROR, - "%s: bss not started\n", __func__); - return -EINVAL; - } - if (mwifiex_set_mgmt_ies(priv, data)) { mwifiex_dbg(priv->adapter, ERROR, "%s: setting mgmt ies failed\n", __func__); @@ -1892,6 +1882,20 @@ static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, return 0; } +/* cfg80211 operation handler for change_beacon. + * Function retrieves and sets modified management IEs to FW. + */ +static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy, + struct net_device *dev, + struct cfg80211_ap_update *params) +{ + int ret; + + ret = mwifiex_cfg80211_change_beacon_data(wiphy, dev, ¶ms->beacon); + + return ret; +} + /* cfg80211 operation handler for del_station. * Function deauthenticates station which value is provided in mac parameter. * If mac is NULL/broadcast, all stations in associated station list are @@ -4027,10 +4031,8 @@ static int mwifiex_cfg80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, struct cfg80211_csa_settings *params) { - struct ieee_types_header *chsw_ie; - struct ieee80211_channel_sw_ie *channel_sw; - int chsw_msec; struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); + int chsw_msec; if (priv->adapter->scan_processing) { mwifiex_dbg(priv->adapter, ERROR, @@ -4045,20 +4047,11 @@ mwifiex_cfg80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, &priv->dfs_chandef)) return -EINVAL; - chsw_ie = (void *)cfg80211_find_ie(WLAN_EID_CHANNEL_SWITCH, - params->beacon_csa.tail, - params->beacon_csa.tail_len); - if (!chsw_ie) { - mwifiex_dbg(priv->adapter, ERROR, - "Could not parse channel switch announcement IE\n"); - return -EINVAL; - } - - channel_sw = (void *)(chsw_ie + 1); - if (channel_sw->mode) { + if (params->block_tx) { if (netif_carrier_ok(priv->netdev)) netif_carrier_off(priv->netdev); mwifiex_stop_net_dev_queue(priv->netdev, priv->adapter); + priv->uap_stop_tx = true; } if (mwifiex_del_mgmt_ies(priv)) @@ -4075,7 +4068,7 @@ mwifiex_cfg80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, memcpy(&priv->beacon_after, ¶ms->beacon_after, sizeof(priv->beacon_after)); - chsw_msec = max(channel_sw->count * priv->bss_cfg.beacon_period, 100); + chsw_msec = max(params->count * priv->bss_cfg.beacon_period, 100); queue_delayed_work(priv->dfs_chan_sw_workqueue, &priv->dfs_chan_sw_work, msecs_to_jiffies(chsw_msec)); return 0; @@ -4794,6 +4787,7 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter) WIPHY_FLAG_HAS_CHANNEL_SWITCH | WIPHY_FLAG_NETNS_OK | WIPHY_FLAG_PS_ON_BY_DEFAULT; + wiphy->max_num_csa_counters = MWIFIEX_MAX_CSA_COUNTERS; if (adapter->host_mlme_enabled) wiphy->flags |= WIPHY_FLAG_REPORTS_OBSS; diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.h b/drivers/net/wireless/marvell/mwifiex/cfg80211.h index 50f7001f5ef0..0a12437f89f2 100644 --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.h +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.h @@ -13,5 +13,7 @@ #include "main.h" int mwifiex_register_cfg80211(struct mwifiex_adapter *); - +int mwifiex_cfg80211_change_beacon_data(struct wiphy *wiphy, + struct net_device *dev, + struct cfg80211_beacon_data *data); #endif diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c index 9deaf59dcb62..5d575d2918fe 100644 --- a/drivers/net/wireless/marvell/mwifiex/debugfs.c +++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c @@ -909,6 +909,46 @@ mwifiex_reset_write(struct file *file, return count; } +static ssize_t +mwifiex_fake_radar_detect_write(struct file *file, + const char __user *ubuf, + size_t count, loff_t *ppos) +{ + struct mwifiex_private *priv = file->private_data; + struct mwifiex_adapter *adapter = priv->adapter; + bool result; + int rc; + + rc = kstrtobool_from_user(ubuf, count, &result); + if (rc) + return rc; + + if (!result) + return -EINVAL; + + if (priv->wdev.cac_started) { + mwifiex_dbg(adapter, MSG, + "Generate fake radar detected during CAC\n"); + if (mwifiex_stop_radar_detection(priv, &priv->dfs_chandef)) + mwifiex_dbg(adapter, ERROR, + "Failed to stop CAC in FW\n"); + cancel_delayed_work_sync(&priv->dfs_cac_work); + cfg80211_cac_event(priv->netdev, &priv->dfs_chandef, + NL80211_RADAR_CAC_ABORTED, GFP_KERNEL); + cfg80211_radar_event(adapter->wiphy, &priv->dfs_chandef, + GFP_KERNEL); + } else { + if (priv->bss_chandef.chan->dfs_cac_ms) { + mwifiex_dbg(adapter, MSG, + "Generate fake radar detected\n"); + cfg80211_radar_event(adapter->wiphy, + &priv->dfs_chandef, + GFP_KERNEL); + } + } + + return count; +} #define MWIFIEX_DFS_ADD_FILE(name) do { \ debugfs_create_file(#name, 0644, priv->dfs_dev_dir, priv, \ &mwifiex_dfs_##name##_fops); \ @@ -945,6 +985,7 @@ MWIFIEX_DFS_FILE_OPS(histogram); MWIFIEX_DFS_FILE_OPS(debug_mask); MWIFIEX_DFS_FILE_OPS(timeshare_coex); MWIFIEX_DFS_FILE_WRITE_OPS(reset); +MWIFIEX_DFS_FILE_WRITE_OPS(fake_radar_detect); MWIFIEX_DFS_FILE_OPS(verext); /* @@ -971,6 +1012,7 @@ mwifiex_dev_debugfs_init(struct mwifiex_private *priv) MWIFIEX_DFS_ADD_FILE(debug_mask); MWIFIEX_DFS_ADD_FILE(timeshare_coex); MWIFIEX_DFS_ADD_FILE(reset); + MWIFIEX_DFS_ADD_FILE(fake_radar_detect); MWIFIEX_DFS_ADD_FILE(verext); } diff --git a/drivers/net/wireless/marvell/mwifiex/decl.h b/drivers/net/wireless/marvell/mwifiex/decl.h index 84603f1e7f6e..9ece61743b9c 100644 --- a/drivers/net/wireless/marvell/mwifiex/decl.h +++ b/drivers/net/wireless/marvell/mwifiex/decl.h @@ -19,6 +19,7 @@ #define MWIFIEX_BSS_COEX_COUNT 2 #define MWIFIEX_MAX_BSS_NUM (3) +#define MWIFIEX_MAX_CSA_COUNTERS 5 #define MWIFIEX_DMA_ALIGN_SZ 64 #define MWIFIEX_RX_HEADROOM 64 diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h index 9024bb944e6a..92bc25bf550f 100644 --- a/drivers/net/wireless/marvell/mwifiex/main.h +++ b/drivers/net/wireless/marvell/mwifiex/main.h @@ -678,6 +678,7 @@ struct mwifiex_private { struct delayed_work dfs_cac_work; struct workqueue_struct *dfs_chan_sw_workqueue; struct delayed_work dfs_chan_sw_work; + bool uap_stop_tx; struct cfg80211_beacon_data beacon_after; struct mwifiex_11h_intf_state state_11h; struct mwifiex_ds_mem_rw mem_rw;
Firmware crashes when AP works on a DFS channel and radar detection occurs. This patch fixes the issue, also add "fake_radar_detect" entry to mimic radar detection for testing purpose. Signed-off-by: David Lin <yu-hao.lin@nxp.com> --- drivers/net/wireless/marvell/mwifiex/11h.c | 56 +++++++++++++++---- .../net/wireless/marvell/mwifiex/cfg80211.c | 50 ++++++++--------- .../net/wireless/marvell/mwifiex/cfg80211.h | 4 +- .../net/wireless/marvell/mwifiex/debugfs.c | 42 ++++++++++++++ drivers/net/wireless/marvell/mwifiex/decl.h | 1 + drivers/net/wireless/marvell/mwifiex/main.h | 1 + 6 files changed, 115 insertions(+), 39 deletions(-) base-commit: ae98f5c9fd8ba84cd408b41faa77e65bf1b4cdfa