On Monday 18 April 2022 05:51:10 CEST Jaehee Park wrote:
>
> Currently, upon virtual interface creation, wfx_add_interface() stores
> a reference to the corresponding struct ieee80211_vif in private data,
> for later usage. This is not needed when using the container_of
> construct. This construct already has all the info it needs to retrieve
> the reference to the corresponding struct from the offset that is
> already available, inherent in container_of(), between its type and
> member inputs (struct ieee80211_vif and drv_priv, respectively).
> Remove vif (which was previously storing the reference to the struct
> ieee80211_vif) from the struct wfx_vif, define a function
> wvif_to_vif(wvif) for container_of(), and replace all wvif->vif with
> the newly defined container_of construct.
>
> Signed-off-by: Jaehee Park <jhpark1013@xxxxxxxxx>
> ---
>
> Changes from staging to wireless-next tree
> - changed macro into function and named it back to wvif_to_vif
> - fit all lines in patch to 80 columns
> - decared a reference to vif at the beginning of the functions
>
> NOTE: Jérôme is going to be testing this patch on his hardware
Don't forget to increment the version number of your submission (option
-v of git send-email).
> drivers/net/wireless/silabs/wfx/wfx.h | 6 +-
> drivers/net/wireless/silabs/wfx/data_rx.c | 5 +-
> drivers/net/wireless/silabs/wfx/data_tx.c | 3 +-
> drivers/net/wireless/silabs/wfx/key.c | 4 +-
> drivers/net/wireless/silabs/wfx/queue.c | 3 +-
> drivers/net/wireless/silabs/wfx/scan.c | 9 ++-
> drivers/net/wireless/silabs/wfx/sta.c | 69 ++++++++++++++---------
> 7 files changed, 63 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h
> index 6594cc647c2f..718693a4273d 100644
> --- a/drivers/net/wireless/silabs/wfx/wfx.h
> +++ b/drivers/net/wireless/silabs/wfx/wfx.h
> @@ -61,7 +61,6 @@ struct wfx_dev {
>
> struct wfx_vif {
> struct wfx_dev *wdev;
> - struct ieee80211_vif *vif;
> struct ieee80211_channel *channel;
> int id;
>
> @@ -91,6 +90,11 @@ struct wfx_vif {
> struct completion set_pm_mode_complete;
> };
>
> +static inline struct ieee80211_vif *wvif_to_vif(struct wfx_vif *wvif)
> +{
> + return container_of((void *)wvif, struct ieee80211_vif, drv_priv);
> +}
> +
> static inline struct wfx_vif *wdev_to_wvif(struct wfx_dev *wdev, int vif_id)
> {
> if (vif_id >= ARRAY_SIZE(wdev->vif)) {
> diff --git a/drivers/net/wireless/silabs/wfx/data_rx.c b/drivers/net/wireless/silabs/wfx/data_rx.c
> index a4b5ffe158e4..342b9cd0e74c 100644
> --- a/drivers/net/wireless/silabs/wfx/data_rx.c
> +++ b/drivers/net/wireless/silabs/wfx/data_rx.c
> @@ -16,6 +16,7 @@
> static void wfx_rx_handle_ba(struct wfx_vif *wvif, struct ieee80211_mgmt *mgmt)
> {
> int params, tid;
> + struct ieee80211_vif *vif = wvif_to_vif(wvif);
When you can, try to place the longest declaration first ("reverse
Christmas tree order").
[...]
> diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
> index 3297d73c327a..97fcbad23c94 100644
> --- a/drivers/net/wireless/silabs/wfx/sta.c
> +++ b/drivers/net/wireless/silabs/wfx/sta.c
[...]
> @@ -152,19 +153,28 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
> {
> struct ieee80211_channel *chan0 = NULL, *chan1 = NULL;
> struct ieee80211_conf *conf = &wvif->wdev->hw->conf;
> + struct ieee80211_vif *vif = wvif_to_vif(wvif);
>
> - WARN(!wvif->vif->bss_conf.assoc && enable_ps,
> + WARN(!vif->bss_conf.assoc && enable_ps,
> "enable_ps is reliable only if associated");
> - if (wdev_to_wvif(wvif->wdev, 0))
> - chan0 = wdev_to_wvif(wvif->wdev, 0)->vif->bss_conf.chandef.chan;
> - if (wdev_to_wvif(wvif->wdev, 1))
> - chan1 = wdev_to_wvif(wvif->wdev, 1)->vif->bss_conf.chandef.chan;
> - if (chan0 && chan1 && wvif->vif->type != NL80211_IFTYPE_AP) {
> + if (wdev_to_wvif(wvif->wdev, 0)) {
> + struct wfx_vif *wvif_ch0 = wdev_to_wvif(wvif->wdev, 0);
> + struct ieee80211_vif *vif_ch0 = wvif_to_vif(wvif_ch0);
> +
> + chan0 = vif_ch0->bss_conf.chandef.chan;
> + }
> + if (wdev_to_wvif(wvif->wdev, 1)) {
> + struct wfx_vif *wvif_ch1 = wdev_to_wvif(wvif->wdev, 1);
> + struct ieee80211_vif *vif_ch1 = wvif_to_vif(wvif_ch1);
> +
> + chan1 = vif_ch1->bss_conf.chandef.chan;
> + }
I think this code could be simplified into:
if (wvif->wdev->vif[1])
chan1 = wvif->wdev->vif[1]->bss_conf.chandef.chan;
(If you choose this way, I suggest to place this change in a separate
patch)
[...]
--
Jérôme Pouiller
