Re: [PATCH 2/3] staging: vt6656: Add comment and change macro to function

Dan Carpenter
2022-02-21T12:34:19Z

On Sun, Feb 20, 2022 at 09:30:36PM +0100, Philipp Hortmann wrote:
> This patch fixes the checkpatch.pl warnings like:
> - CHECK: spinlock_t definition without comment
> - CHECK: Avoid CamelCase: <uVar>
> - CHECK: Macro argument reuse 'uVar' - possible side-effects?
> and moved the only twice used macro to a function in the file
> where the function is used.
>
> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
> ---
> drivers/staging/vt6656/device.h | 11 ++---------
> drivers/staging/vt6656/wcmd.c | 13 +++++++++++--
> 2 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/vt6656/device.h b/drivers/staging/vt6656/device.h
> index 8b6623a751f0..eb84590d5eeb 100644
> --- a/drivers/staging/vt6656/device.h
> +++ b/drivers/staging/vt6656/device.h
> @@ -268,8 +268,8 @@ struct vnt_private {
> u32 rx_buf_sz;
> int mc_list_count;
>
> - spinlock_t lock;
> - struct mutex usb_lock;
> + spinlock_t lock; /*prepare tx USB URB*/
> + struct mutex usb_lock; /*USB control messages*/

This needs to be sent as a separate path. Add a space at the start and
end of the comments. /* prepare tx USB URB */
/* USB control messages */

I'm not 100% sure I understand these comments but I haven't looked
at them in context.

>
> unsigned long flags;
>
> @@ -381,13 +381,6 @@ struct vnt_private {
> struct ieee80211_low_level_stats low_stats;
> };
>
> -#define ADD_ONE_WITH_WRAP_AROUND(uVar, uModulo) { \
> - if ((uVar) >= ((uModulo) - 1)) \
> - (uVar) = 0; \
> - else \
> - (uVar)++; \
> -}
> -
> int vnt_init(struct vnt_private *priv);
>
> #endif
> diff --git a/drivers/staging/vt6656/wcmd.c b/drivers/staging/vt6656/wcmd.c
> index e8ee2fbee76c..77c6d898f660 100644
> --- a/drivers/staging/vt6656/wcmd.c
> +++ b/drivers/staging/vt6656/wcmd.c
> @@ -31,6 +31,15 @@ static void vnt_cmd_timer_wait(struct vnt_private *priv, unsigned long msecs)
> schedule_delayed_work(&priv->run_command_work, msecs_to_jiffies(msecs));
> }
>
> +static u32 add_one_with_wrap_around(u32 u_var, u8 u_modulo)

Please don't name variables with Hungarian notation. Remove the "u_".

regards,
dan carpenter