Re: [PATCH v2 2/8] staging: vt6655: Use incrementation in `idx`

[Joe Perches]

On Thu, 2021-11-04 at 16:00 +0300, Dan Carpenter wrote:
> On Mon, Nov 01, 2021 at 02:32:00PM +0000, Karolina Drobnik wrote:
> > Increment `idx` in a loop instead of adding the loop counter
> > `i` to do so. Thanks to this change, the cast to unsigned short
> > can be removed.
> > 
> > Signed-off-by: Karolina Drobnik <karolinadrobnik@xxxxxxxxx>
> > ---
> >  drivers/staging/vt6655/rf.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
[]
> > @@ -700,11 +700,11 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char rf_type,
> >  			return false;
> >  
> >  		for (i = 0; i < CB_AL2230_INIT_SEQ; i++)
> > -			MACvSetMISCFifo(priv, (unsigned short)(idx + i), al2230_init_table[i]);
> > +			MACvSetMISCFifo(priv, idx++, al2230_init_table[i]);
> 
> Of course, idx is no longer a constant so declaring it as a variable
> makes sense here.  But maybe just do it in the same patch because the
> patch 1/1 doesn't make sense as a stand alone patch.
> 
> Also don't declare idx as an unsigned short.  It's better to declare it
> as a int so it just works like a normal number and you don't have to
> think about signedness bugs and wrapping and edge cases.

No, IMO it really should be a u16.
Look at the iowrite16() within the call to MACvSetMISCFifo.