Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()

[Ajay Singh]

On Fri, 16 Feb 2018 20:15:53 +0200
Claudiu Beznea  wrote:

> On 14.02.2018 13:10, Ajay Singh wrote:
> > Refactor wilc_spi_read_int() to fix the line over 80 char issues reported
> > by checkpatch.pl script.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/wilc_spi.c | 57 
> > +++--
> >  1 file changed, 29 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/wilc_spi.c 
> > b/drivers/staging/wilc1000/wilc_spi.c
> > index fddc0db..7c58beb8 100644
> > --- a/drivers/staging/wilc1000/wilc_spi.c
> > +++ b/drivers/staging/wilc1000/wilc_spi.c
> > @@ -939,45 +939,46 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 
> > *int_status)
> > int happened, j;
> > u32 unknown_mask;
> > u32 irq_flags;
> > +   int k = IRG_FLAGS_OFFSET + 5;
> >  
> > if (g_spi.has_thrpt_enh) {
> > ret = spi_internal_read(wilc, 0xe840 - WILC_SPI_REG_BASE,
> > int_status);
> > -   } else {
> > -   ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE,
> > -   _cnt);
> > -   if (!ret) {
> > -   dev_err(>dev,
> > -   "Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> > -   goto _fail_;
> > -   }
> > -   tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> > +   return ret;
> > +   }
> > +   ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE, _cnt);
> > +   if (!ret) {
> > +   dev_err(>dev,
> > +   "Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> > +   goto _fail_;
> > +   }
> > +   tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> >  
> > -   j = 0;
> > -   do {
> > -   happened = 0;
> > +   j = 0;
> > +   do {
> > +   happened = 0;  
> You could remove this happen
> >  

Yes, we don't need "happened" variable,it can be removed. As the patch
was only to remove the 80 char checkpatch.pl warning. Will remove the
use of this variable in separate patch.

> > -   wilc_spi_read_reg(wilc, 0x1a90, _flags);
> > -   tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> > +   wilc_spi_read_reg(wilc, 0x1a90, _flags);
> > +   tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> >  
> > -   if (g_spi.nint > 5) {
> > -   wilc_spi_read_reg(wilc, 0x1a94,
> > - _flags);
> > -   tmp |= (((irq_flags >> 0) & 0x7) << 
> > (IRG_FLAGS_OFFSET + 5));
> > -   }
> > +   if (g_spi.nint > 5) {
> > +   wilc_spi_read_reg(wilc, 0x1a94, _flags);
> > +   tmp |= (((irq_flags >> 0) & 0x7) << k);
> > +   }
> >  
> > -   unknown_mask = ~((1ul << g_spi.nint) - 1);
> > +   unknown_mask = ~((1ul << g_spi.nint) - 1);  
> You could use GENMASK(g_spi.nint - 1, 0) instead of ~((1ul << g_spi.nint) - 1)
> >  

Will do the modification to make use of GENMASK in future patch.

> > -   if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> > -   dev_err(>dev, "Unexpected interrupt (2): 
> > j=%d, tmp=%x, mask=%x\n", j, tmp, unknown_mask);
> > -   happened = 1;
> > -   }
> > +   if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> > +   dev_err(>dev,
> > +   "Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
> > +   j, tmp, unknown_mask);
> > +   happened = 1;  
> And here just break;
> > +   }
> >  
> > -   j++;
> > -   } while (happened);  
> And here use while (true);
> > +   j++;
> > +   } while (happened);
> >  
> > -   *int_status = tmp;
> > -   }
> > +   *int_status = tmp;
> >  
> >  _fail_:
> > return ret;
> >   



regards,
ajay

Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()

[Claudiu Beznea]

On 14.02.2018 13:10, Ajay Singh wrote:
> Refactor wilc_spi_read_int() to fix the line over 80 char issues reported
> by checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 57 
> +++--
>  1 file changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c 
> b/drivers/staging/wilc1000/wilc_spi.c
> index fddc0db..7c58beb8 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -939,45 +939,46 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 
> *int_status)
>   int happened, j;
>   u32 unknown_mask;
>   u32 irq_flags;
> + int k = IRG_FLAGS_OFFSET + 5;
>  
>   if (g_spi.has_thrpt_enh) {
>   ret = spi_internal_read(wilc, 0xe840 - WILC_SPI_REG_BASE,
>   int_status);
> - } else {
> - ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE,
> - _cnt);
> - if (!ret) {
> - dev_err(>dev,
> - "Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> - goto _fail_;
> - }
> - tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> + return ret;
> + }
> + ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE, _cnt);
> + if (!ret) {
> + dev_err(>dev,
> + "Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> + goto _fail_;
> + }
> + tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
>  
> - j = 0;
> - do {
> - happened = 0;
> + j = 0;
> + do {
> + happened = 0;
You could remove this happen
>  
> - wilc_spi_read_reg(wilc, 0x1a90, _flags);
> - tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> + wilc_spi_read_reg(wilc, 0x1a90, _flags);
> + tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
>  
> - if (g_spi.nint > 5) {
> - wilc_spi_read_reg(wilc, 0x1a94,
> -   _flags);
> - tmp |= (((irq_flags >> 0) & 0x7) << 
> (IRG_FLAGS_OFFSET + 5));
> - }
> + if (g_spi.nint > 5) {
> + wilc_spi_read_reg(wilc, 0x1a94, _flags);
> + tmp |= (((irq_flags >> 0) & 0x7) << k);
> + }
>  
> - unknown_mask = ~((1ul << g_spi.nint) - 1);
> + unknown_mask = ~((1ul << g_spi.nint) - 1);
You could use GENMASK(g_spi.nint - 1, 0) instead of ~((1ul << g_spi.nint) - 1)
>  
> - if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> - dev_err(>dev, "Unexpected interrupt (2): 
> j=%d, tmp=%x, mask=%x\n", j, tmp, unknown_mask);
> - happened = 1;
> - }
> + if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> + dev_err(>dev,
> + "Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
> + j, tmp, unknown_mask);
> + happened = 1;
And here just break;
> + }
>  
> - j++;
> - } while (happened);
And here use while (true);
> + j++;
> + } while (happened);
>  
> - *int_status = tmp;
> - }
> + *int_status = tmp;
>  
>  _fail_:
>   return ret;
>