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

From: Kalle Valo
Date: Wed Oct 04 2023 - 06:21:18 EST


Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx> writes:

> First step to implement remain_on_channel.
>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>
> ---
> 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?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches