Re: [PATCH v9 3/3] staging: rtl8723bs: refactor queue priority initialization

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, May 28, 2026 at 08:07:57PM +0200, artur.ugnivenko@gmx.de wrote:
> From: Artur Ugnivenko <artur.ugnivenko@gmx.de>
> 
> Pack individual priority values into an enum-indexed array
> to simplify the initialization code and prevent passing around
> six loose values as function parameters.
> 
> Signed-off-by: Artur Ugnivenko <artur.ugnivenko@gmx.de>
> ---
> Changes in v9:
> - don't declare variables in the middle of functions
> - remove unnecessary whitespace
> Changes in v8:
> - Remove the struct from v7, use an enum-indexed array instead
> - Remove other lines ending in parentheses fixes to keep the
>   commit small
> Changes in v7:
> - Add changelog.
> - Add transmit_queues struct in include/drv_types.h to improve
>   style of _InitNormalChipRegPriority
> Changes in v6: Make the patch apply to gregkh/staging-testing.
> Changes in v5: No changes in this patch.
> Changes in v4: No changes in this patch.
> Changes in v3: Split the patch into multiple patches.
> Changes in v2: Make patch apply to gregkh/staging-testing.
> 
>  drivers/staging/rtl8723bs/hal/sdio_halinit.c  | 105 +++++++++---------
>  drivers/staging/rtl8723bs/include/drv_types.h |  10 ++
>  2 files changed, 62 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> index 6f2aea984b30..1246c5fdfc32 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> @@ -188,25 +188,18 @@ static void _InitTxBufferBoundary(struct adapter *padapter)
>  	rtw_write8(padapter, REG_TDECTRL + 1, txpktbuf_bndy);
>  }
>  
> -static void _InitNormalChipRegPriority(
> -	struct adapter *Adapter,
> -	u16 beQ,
> -	u16 bkQ,
> -	u16 viQ,
> -	u16 voQ,
> -	u16 mgtQ,
> -	u16 hiQ
> -)
> +static void _InitNormalChipRegPriority(struct adapter *Adapter,
> +				       u16 queues[TX_Q_MAX])

Wow, that's an odd thing to be passing by value in a function, do you
really mean to do this?  Yes, the compiler will let you, but it's not a
good idea to do that at all in the kernel as it messes with the stack,
and all sorts of "odd" things can happen.

Please always pass structures by reference.

Also, you are "open coding" the structure size here, which is not a good
idea as you have to manually keep everything in sync.  What's wrong with
that original structure you used a few versions before?

thanks,

greg k-h