On Wed, Apr 13, 2022 at 09:26:45AM +0200, Jérôme Pouiller wrote: > Hello Jaehee, > > On Tuesday 12 April 2022 06:12:18 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 macro > > 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 in v3: > > - Made edits to the commit message. > > - Shortened the macro name from wvif_to_vif to to_vif. > > hmm... wvif_to_vif() was fine for me (while to_vif() is a bit too > generic). > > > - For functions that had more than one instance of vif, defined one > > reference vif at the beginning of the function and used that instead. > > I suggest to declare a new variable even if there is only one instance > of vif. So: > - the code is unified > - it avoids the syntax foo(var)->bar > > > - Broke the if-statements that ran long into two lines. > > (There are 3 lines that exceed 80 by less than 4 characters. Two of > > those lines of code could be shorted but it involved defining two more > > variables, and could potentially make the code less readable.) > > > > Note: I will mail this patch to the wireless-next tree after testing. > > > > > > drivers/staging/wfx/wfx.h | 2 +- > > drivers/staging/wfx/data_rx.c | 5 ++-- > > drivers/staging/wfx/data_tx.c | 2 +- > > drivers/staging/wfx/key.c | 2 +- > > drivers/staging/wfx/queue.c | 2 +- > > drivers/staging/wfx/scan.c | 10 +++++--- > > drivers/staging/wfx/sta.c | 46 +++++++++++++++++++---------------- > > 7 files changed, 38 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h > > index 78f2a416fe4f..a07baadc5e70 100644 > > --- a/drivers/staging/wfx/wfx.h > > +++ b/drivers/staging/wfx/wfx.h > > @@ -25,7 +25,7 @@ > > #define USEC_PER_TXOP 32 /* see struct ieee80211_tx_queue_params */ > > #define USEC_PER_TU 1024 > > > > -#define wvif_to_vif(ptr)(container_of((void *)ptr, struct ieee80211_vif, drv_priv)) > > +#define to_vif(wvif)container_of((void *)wvif, struct ieee80211_vif, drv_priv) > > It seems you have stacked up this v3 on your v2. So this v3 does not apply > on any public tree. You have to merge your v2 and your v3. > > > > > struct wfx_hwbus_ops; > > > > diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c > > index 98c2223089b8..3677b3431467 100644 > > --- a/drivers/staging/wfx/data_rx.c > > +++ b/drivers/staging/wfx/data_rx.c > > The wfx driver has moved, you have to rebase your work. > > [...] > > -- > Jérôme Pouiller > > > Hi Jérôme, Sorry about the delay! I have sent a revised patch to the wireless-next mailing list for your review. Thanks, Jaehee
