Geliang Tang <geliangtang@xxxxxxx> writes:
> Use list_for_each_entry*() instead of list_for_each*() to simplify
> the code.
>
> Signed-off-by: Geliang Tang <geliangtang@xxxxxxx>
> ---
> Changes in v3:
> - split it into three patches.
> Changes in v2:
> - drop the coding style fixing in v1.
> ---
> drivers/staging/rtl8723au/core/rtw_ap.c | 55 ++++++++-----------
> drivers/staging/rtl8723au/core/rtw_mlme.c | 26 ++++-----
> drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 10 ++--
> drivers/staging/rtl8723au/core/rtw_recv.c | 22 ++++----
> drivers/staging/rtl8723au/core/rtw_sta_mgt.c | 25 ++++-----
> drivers/staging/rtl8723au/core/rtw_xmit.c | 64 ++++++++++-------------
> drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 14 +++--
> drivers/staging/rtl8723au/os_dep/usb_ops_linux.c | 9 ++--
> 8 files changed, 96 insertions(+), 129 deletions(-)
This one is mostly OK, however when you do multi patch sets, always
include a cover letter describing the overall changes of the set.
A few nit picks:
> diff --git a/drivers/staging/rtl8723au/core/rtw_recv.c b/drivers/staging/rtl8723au/core/rtw_recv.c
> index 404b618..18b7d03 100644
> --- a/drivers/staging/rtl8723au/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723au/core/rtw_recv.c
> @@ -88,13 +88,13 @@ int _rtw_init_recv_priv23a(struct recv_priv *precvpriv,
> void _rtw_free_recv_priv23a (struct recv_priv *precvpriv)
> {
> struct rtw_adapter *padapter = precvpriv->adapter;
> - struct recv_frame *precvframe;
> - struct list_head *plist, *ptmp;
> + struct recv_frame *precvframe, *ptmp;
>
> rtw_free_uc_swdec_pending_queue23a(padapter);
>
> - list_for_each_safe(plist, ptmp, &precvpriv->free_recv_queue.queue) {
> - precvframe = container_of(plist, struct recv_frame, list);
> + list_for_each_entry_safe(precvframe, ptmp,
> + &precvpriv->free_recv_queue.queue,
> + list) {
> list_del_init(&precvframe->list);
> kfree(precvframe);
> }
Too aggressive line breaking, the 'list' fits within 80 characters on
the line above.
> @@ -195,16 +195,15 @@ using spinlock to protect
>
> static void rtw_free_recvframe23a_queue(struct rtw_queue *pframequeue)
> {
> - struct recv_frame *hdr;
> - struct list_head *plist, *phead, *ptmp;
> + struct recv_frame *hdr, *ptmp;
> + struct list_head *phead;
>
> spin_lock(&pframequeue->lock);
>
> phead = get_list_head(pframequeue);
> plist = phead->next;
>
> - list_for_each_safe(plist, ptmp, phead) {
> - hdr = container_of(plist, struct recv_frame, list);
> + list_for_each_entry_safe(hdr, ptmp, phead, list) {
> rtw_free_recvframe23a(hdr);
> }
>
You could remove the brackets here, since you are fixing that specific
line. I am fine with this as is, some of the checkpatch police force
might bite over it.
On overall this patch is a lot better than the first version.
All set and done, I am thinking of removing this driver once Kalle pulls
in my currently posted set of changes for rtl8xxxu, plus the next one I
have lined up. I seems to me rtl8xxxu can replace rtl8723au at this
point.
I will probably mark rtl8723au deprecated after 4.6 comes out, and
remove the driver around 4.7.
Cheers,
Jes
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
