Re: [Outreachy kernel] [PATCH 4/7] staging: vt6655: Introduce `data` temporary variable

[Fabio M. De Francesco]

On Thursday, October 28, 2021 12:35:34 PM CEST Karolina Drobnik wrote:
> Add a variable to store initialization tables. Use this variable
> in AL2230 initialization.
> 
> Signed-off-by: Karolina Drobnik <karolinadrobnik@xxxxxxxxx>
> ---
>  drivers/staging/vt6655/rf.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> index ea74701917e5..afd202ea3356 100644
> --- a/drivers/staging/vt6655/rf.c
> +++ b/drivers/staging/vt6655/rf.c
> @@ -684,6 +684,7 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, 
unsigned char rf_type,
>  	unsigned short idx = MISCFIFO_SYNDATA_IDX;
>  	unsigned char init_count = 0;
>  	unsigned char sleep_count = 0;
> +	const unsigned long *data;
>  
>  	VNSvOutPortW(iobase + MAC_REG_MISCFFNDEX, 0);
>  	switch (rf_type) {
> @@ -699,8 +700,9 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, 
unsigned char rf_type,
>  		if (init_count > (MISCFIFO_SYNDATASIZE - sleep_count))
>  			return false;
>  
> +		data = al2230_init_table;
>  		for (i = 0; i < CB_AL2230_INIT_SEQ; i++)
> -			MACvSetMISCFifo(priv, idx++, 
al2230_init_table[i]);
> +			MACvSetMISCFifo(priv, idx++, *(data++));

Hi Karolina,

I think you are using redundant parentheses in "* (data ++)" but understand 
that those increments and dereferences are equivalent to "* data ++" 
(according to the C precedence rules).

Some time ago I suggested that you use those redundant parentheses because
Greg Kroah-Hartman had previously explained that he prefers not to see 
"*foo++" because maintainers and reviewers are not required to remember the C 
precedence rules.

I hope my suggestion isn't based on a misunderstanding of what Greg wants 
here and that your patch can be accepted as is.

While we are at it, please notice that Maintainers of different subsystems 
may see this topic from a different point of view and that they might very 
well ask you for removing those redundant parentheses.

I'd suggest to use grep and find out what is preferred in other subsystems, 
when you want to contribute to other parts of Linux.

Thanks,

Fabio

>  		MACvSetMISCFifo(priv, idx++, 
al2230_channel_table0[channel - 1]);
>  		MACvSetMISCFifo(priv, idx++, 
al2230_channel_table1[channel - 1]);
> -- 
> 2.30.2
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
"outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
email to outreachy-kernel+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/
outreachy-kernel/dc72a4c3539aed70569f66396ed3b51818bc2aea.
1635415820.git.karolinadrobnik%40gmail.com.
>