linux-kernel - Re: [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <16472781.hlxOUv9cDv@pc-42>
Date:   Wed, 04 Oct 2023 12:40:13 +0200
From:   Jérôme Pouiller <jerome.pouiller@...abs.com>
To:     Kalle Valo <kvalo@...nel.org>
Cc:     linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature

On Wednesday 4 October 2023 12:23:30 CEST Kalle Valo wrote:
> Jérôme Pouiller <jerome.pouiller@...abs.com> writes:
> 
> > First step to implement remain_on_channel.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.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