On Wed, 2021-10-20 at 06:44 -0700, Joe Perches wrote: > trivial suggestion: > > > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c > [] > > @@ -33,7 +33,7 @@ > > #define SWITCH_CHANNEL_DELAY_AL7230 200 /* us */ > > #define AL7230_PWR_IDX_LEN 64 > > > > -static const unsigned long dwAL2230InitTable[CB_AL2230_INIT_SEQ] = { > > +static const unsigned long al2230_init_table[CB_AL2230_INIT_SEQ] = { > > 0x03F79000 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW, > > 0x03333100 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW, > > 0x01A00200 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW, > > In this file there are more than 100 uses of > > (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW > > Maybe add a define for it and substitute the uses for the define. Look at the code too. It looks as if every use of IFRFbWriteEmbedded() has this added to the 2nd argument and that the 2nd argument isn't used anywhere else. Maybe remove it altogether and add it to IFRFbWriteEmbedded(). And it looks as if the + uses for these should logically be | Something like: diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c index 0dae593c6944f..26803f6f9a27b 100644 --- a/drivers/staging/vt6655/rf.c +++ b/drivers/staging/vt6655/rf.c @@ -498,7 +498,8 @@ bool IFRFbWriteEmbedded(struct vnt_private *priv, unsigned long dwData) unsigned short ww; unsigned long dwValue; - VNSvOutPortD(iobase + MAC_REG_IFREGCTL, dwData); + VNSvOutPortD(iobase + MAC_REG_IFREGCTL, + dwData | (BY_AL2230_REG_LEN << 3) | IFREGCTL_REGW); /* W_MAX_TIMEOUT is the timeout period */ for (ww = 0; ww < W_MAX_TIMEOUT; ww++) {