On Thu, Nov 04, 2021 at 07:44:22AM -0700, Joe Perches wrote: > 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. Obviously we all agree it has no effect on runtime at all. I just think that people get into trouble getting too creative with their types. There was some discussion earlier about where the casting was required (it's not required at all). It seems like there was some confusion about how types work. Declaring it as "unsigned short" has some documentation value, I guess, but you avoid signedness bugs when you declare things as int. I don't care too much, but I also try to tell people not to be fancy. regards, dan carpenter
