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

Jérôme Pouiller
2023-10-04T10:40:23Z

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.

--
Jérôme Pouiller