diff mbox series

[v2,5/9] wifi: wfx: add placeholders for remain_on_channel feature

Message ID 20230927163257.568496-6-jerome.pouiller@silabs.com
State New
Headers
Series wfx: implement Remain On Channel |

Commit Message

Jérôme Pouiller Sept. 27, 2023, 4:32 p.m. UTC 
  First step to implement remain_on_channel.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/main.c |  3 +++
 drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
 drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
 3 files changed, 20 insertions(+)

Comments

Kalle Valo Oct. 4, 2023, 10:23 a.m. UTC | #1 
Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> First step to implement remain_on_channel.
>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/net/wireless/silabs/wfx/main.c |  3 +++
>  drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
>  drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
>  3 files changed, 20 insertions(+)
>
> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index ede822d771aaf..31f6e0d3dc089 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c
> @@ -151,6 +151,8 @@ static const struct ieee80211_ops wfx_ops = {
>  	.change_chanctx          = wfx_change_chanctx,
>  	.assign_vif_chanctx      = wfx_assign_vif_chanctx,
>  	.unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
> +	.remain_on_channel       = wfx_remain_on_channel,
> +	.cancel_remain_on_channel = wfx_cancel_remain_on_channel,
>  };
>  
>  bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
> @@ -288,6 +290,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
>  	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
>  	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
>  	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
> +	hw->wiphy->max_remain_on_channel_duration = 5000;
>  	hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
>  	hw->wiphy->max_scan_ssids = 2;
>  	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
> diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
> index 16f619ed22e00..51338fd43ae4f 100644
> --- a/drivers/net/wireless/silabs/wfx/scan.c
> +++ b/drivers/net/wireless/silabs/wfx/scan.c
> @@ -145,3 +145,15 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
>  	wvif->scan_nb_chan_done = nb_chan_done;
>  	complete(&wvif->scan_complete);
>  }
> +
> +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> +			  struct ieee80211_channel *chan, int duration,
> +			  enum ieee80211_roc_type type)
> +{
> +	return 0;
> +}
> +
> +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
> +{
> +	return 0;
> +}
> diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
> index 78e3b984f375c..2f8361769303e 100644
> --- a/drivers/net/wireless/silabs/wfx/scan.h
> +++ b/drivers/net/wireless/silabs/wfx/scan.h
> @@ -19,4 +19,9 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>  void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
>  void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
>  
> +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> +			  struct ieee80211_channel *chan, int duration,
> +			  enum ieee80211_roc_type type);
> +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> +
>  #endif

I'm not really seeing the point of this patch. I would expect that once
.remain_on_channel is assign the feature will work without issues, for
example otherwise git bisect will not work correctly.

What about folding patches 5 and 6 into one patch? And then moving that
patch as the last to make sure that the feature is enabled on the driver
only after it works correctly?
Jérôme Pouiller Oct. 4, 2023, 10:40 a.m. UTC | #2 
On Wednesday 4 October 2023 12:23:30 CEST Kalle Valo wrote:
> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
> 
> > First step to implement remain_on_channel.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/net/wireless/silabs/wfx/main.c |  3 +++
> >  drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
> >  drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
> >  3 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> > index ede822d771aaf..31f6e0d3dc089 100644
> > --- a/drivers/net/wireless/silabs/wfx/main.c
> > +++ b/drivers/net/wireless/silabs/wfx/main.c
> > @@ -151,6 +151,8 @@ static const struct ieee80211_ops wfx_ops = {
> >       .change_chanctx          = wfx_change_chanctx,
> >       .assign_vif_chanctx      = wfx_assign_vif_chanctx,
> >       .unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
> > +     .remain_on_channel       = wfx_remain_on_channel,
> > +     .cancel_remain_on_channel = wfx_cancel_remain_on_channel,
> >  };
> >
> >  bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
> > @@ -288,6 +290,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
> >       hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
> >       hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
> >       hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
> > +     hw->wiphy->max_remain_on_channel_duration = 5000;
> >       hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
> >       hw->wiphy->max_scan_ssids = 2;
> >       hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
> > diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
> > index 16f619ed22e00..51338fd43ae4f 100644
> > --- a/drivers/net/wireless/silabs/wfx/scan.c
> > +++ b/drivers/net/wireless/silabs/wfx/scan.c
> > @@ -145,3 +145,15 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
> >       wvif->scan_nb_chan_done = nb_chan_done;
> >       complete(&wvif->scan_complete);
> >  }
> > +
> > +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > +                       struct ieee80211_channel *chan, int duration,
> > +                       enum ieee80211_roc_type type)
> > +{
> > +     return 0;
> > +}
> > +
> > +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
> > +{
> > +     return 0;
> > +}
> > diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
> > index 78e3b984f375c..2f8361769303e 100644
> > --- a/drivers/net/wireless/silabs/wfx/scan.h
> > +++ b/drivers/net/wireless/silabs/wfx/scan.h
> > @@ -19,4 +19,9 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >  void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> >  void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
> >
> > +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > +                       struct ieee80211_channel *chan, int duration,
> > +                       enum ieee80211_roc_type type);
> > +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> > +
> >  #endif
> 
> I'm not really seeing the point of this patch. I would expect that once
> .remain_on_channel is assign the feature will work without issues, for
> example otherwise git bisect will not work correctly.
> 
> What about folding patches 5 and 6 into one patch? And then moving that
> patch as the last to make sure that the feature is enabled on the driver
> only after it works correctly?

Ok.

I think I will have to reword a bit the commit logs but the reordering should
not be difficult.
diff mbox series

Patch

diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index ede822d771aaf..31f6e0d3dc089 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -151,6 +151,8 @@  static const struct ieee80211_ops wfx_ops = {
 	.change_chanctx          = wfx_change_chanctx,
 	.assign_vif_chanctx      = wfx_assign_vif_chanctx,
 	.unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
+	.remain_on_channel       = wfx_remain_on_channel,
+	.cancel_remain_on_channel = wfx_cancel_remain_on_channel,
 };
 
 bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
@@ -288,6 +290,7 @@  struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
 	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
+	hw->wiphy->max_remain_on_channel_duration = 5000;
 	hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
 	hw->wiphy->max_scan_ssids = 2;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
index 16f619ed22e00..51338fd43ae4f 100644
--- a/drivers/net/wireless/silabs/wfx/scan.c
+++ b/drivers/net/wireless/silabs/wfx/scan.c
@@ -145,3 +145,15 @@  void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
 	wvif->scan_nb_chan_done = nb_chan_done;
 	complete(&wvif->scan_complete);
 }
+
+int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			  struct ieee80211_channel *chan, int duration,
+			  enum ieee80211_roc_type type)
+{
+	return 0;
+}
+
+int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
+{
+	return 0;
+}
diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
index 78e3b984f375c..2f8361769303e 100644
--- a/drivers/net/wireless/silabs/wfx/scan.h
+++ b/drivers/net/wireless/silabs/wfx/scan.h
@@ -19,4 +19,9 @@  int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
 
+int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			  struct ieee80211_channel *chan, int duration,
+			  enum ieee80211_roc_type type);
+int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
+
 #endif