[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <84f3c940fedb961e6e7e88d47c3d15e598bc32c3.camel@perches.com>
Date: Mon, 18 Oct 2021 22:56:50 -0700
From: Joe Perches <joe@...ches.com>
To: Greg KH <gregkh@...uxfoundation.org>,
Karolina Drobnik <karolinadrobnik@...il.com>
Cc: outreachy-kernel@...glegroups.com, forest@...ttletooquiet.net,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: vt6655: Fix line wrapping in rf.c file
On Mon, 2021-10-18 at 17:12 +0200, Greg KH wrote:
> On Mon, Oct 18, 2021 at 04:05:26PM +0100, Karolina Drobnik wrote:
> > Fix line length warnings raised by checkpatch.pl in
> > rf.c file for `RFvWriteWakeProgSyn`,`RFbRawSetPower`
> > and `RFbAL7230SelectChannelPostProcess`functions.
> >
> > Signed-off-by: Karolina Drobnik <karolinadrobnik@...il.com>
> > ---
> > drivers/staging/vt6655/rf.c | 66 +++++++++++++++++++++++++++----------
> > 1 file changed, 49 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> > index a6f17162d017..206d34b555bc 100644
> > --- a/drivers/staging/vt6655/rf.c
> > +++ b/drivers/staging/vt6655/rf.c
> > @@ -699,11 +699,17 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
> > return false;
> >
> > for (ii = 0; ii < CB_AL2230_INIT_SEQ; ii++)
> > - MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230InitTable[ii]);
> > + MACvSetMISCFifo(priv,
> > + (unsigned short)(MISCFIFO_SYNDATA_IDX + ii),
> > + dwAL2230InitTable[ii]);
> >
> > - MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230ChannelTable0[uChannel - 1]);
> > + MACvSetMISCFifo(priv,
> > + (unsigned short)(MISCFIFO_SYNDATA_IDX + ii),
> > + dwAL2230ChannelTable0[uChannel - 1]);
> > ii++;
> > - MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230ChannelTable1[uChannel - 1]);
> > + MACvSetMISCFifo(priv,
> > + (unsigned short)(MISCFIFO_SYNDATA_IDX + ii),
> > + dwAL2230ChannelTable1[uChannel - 1]);
> > break;
> >
> > /* Need to check, PLLON need to be low for channel setting */
> > @@ -716,17 +722,28 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
> >
> > if (uChannel <= CB_MAX_CHANNEL_24G) {
> > for (ii = 0; ii < CB_AL7230_INIT_SEQ; ii++)
> > - MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230InitTable[ii]);
> > + MACvSetMISCFifo(priv,
> > + (unsigned short)(MISCFIFO_SYNDATA_IDX
> > + + ii), dwAL7230InitTable[ii]);
>
> You shouldn't put the "+" on the start of a new line.
>
> Also, these are all just fine as-is for now. A better way to make these
> lines smaller is to use better variable and function names that are
> shorter and make sense :)
And maybe use temporaries for the multiply used
MISCFIFO_SYNDATA_IDX + ii and a few other changes.
Maybe some refactoring like:
---
drivers/staging/vt6655/rf.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
index 0dae593c6944f..7beb0cd5a62df 100644
--- a/drivers/staging/vt6655/rf.c
+++ b/drivers/staging/vt6655/rf.c
@@ -680,16 +680,19 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
u16 uChannel)
{
void __iomem *iobase = priv->PortOffset;
- int ii;
+ int i;
+ unsigned short idx = MISCFIFO_SYNDATA_IDX;
unsigned char byInitCount = 0;
unsigned char bySleepCount = 0;
+ const unsigned long *data;
+ uChannel--;
VNSvOutPortW(iobase + MAC_REG_MISCFFNDEX, 0);
+
switch (byRFType) {
case RF_AIROHA:
case RF_AL2230S:
-
- if (uChannel > CB_MAX_CHANNEL_24G)
+ if (uChannel >= CB_MAX_CHANNEL_24G)
return false;
/* Init Reg + Channel Reg (2) */
@@ -698,12 +701,12 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
if (byInitCount > (MISCFIFO_SYNDATASIZE - bySleepCount))
return false;
- for (ii = 0; ii < CB_AL2230_INIT_SEQ; ii++)
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230InitTable[ii]);
+ data = dwAL2230InitTable;
+ for (i = 0; i < CB_AL2230_INIT_SEQ; i++)
+ MACvSetMISCFifo(priv, idx++, *data++);
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230ChannelTable0[uChannel - 1]);
- ii++;
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230ChannelTable1[uChannel - 1]);
+ MACvSetMISCFifo(priv, idx++, dwAL2230ChannelTable0[uChannel]);
+ MACvSetMISCFifo(priv, idx++, dwAL2230ChannelTable1[uChannel]);
break;
/* Need to check, PLLON need to be low for channel setting */
@@ -714,19 +717,14 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
if (byInitCount > (MISCFIFO_SYNDATASIZE - bySleepCount))
return false;
- if (uChannel <= CB_MAX_CHANNEL_24G) {
- for (ii = 0; ii < CB_AL7230_INIT_SEQ; ii++)
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230InitTable[ii]);
- } else {
- for (ii = 0; ii < CB_AL7230_INIT_SEQ; ii++)
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230InitTableAMode[ii]);
- }
+ data = (uChannel < CB_MAX_CHANNEL_24G) ?
+ dwAL7230InitTable : dwAL7230InitTableAMode;
+ for (i = 0; i < CB_AL7230_INIT_SEQ; i++)
+ MACvSetMISCFifo(priv, idx++, *data++);
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230ChannelTable0[uChannel - 1]);
- ii++;
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230ChannelTable1[uChannel - 1]);
- ii++;
- MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL7230ChannelTable2[uChannel - 1]);
+ MACvSetMISCFifo(priv, idx++, dwAL7230ChannelTable0[uChannel]);
+ MACvSetMISCFifo(priv, idx++, dwAL7230ChannelTable1[uChannel]);
+ MACvSetMISCFifo(priv, idx++, dwAL7230ChannelTable2[uChannel]);
break;
case RF_NOTHING:
Powered by blists - more mailing lists