LinuxLists.cc - [PATCH] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

2018-05-31 18:56:48

Subject: [PATCH] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

This patch fixes following checkpatch.pl issues:

WARNING : line over 80 characters
ERROR : code indent should use tabs where possible
WARNING : no spaces at the start of a line
ERROR : switch and case should be at the same indent
ERROR : space required before the open parenthesis
WARNING : braces {} are not necessary for single statement blocks

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 37f299080410..cd94f5f569df 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -55,7 +55,8 @@
#define MT7621_CPOL BIT(4)
#define MT7621_LSB_FIRST BIT(3)

-#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_CS_HIGH)
+#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | \
+ SPI_LSB_FIRST | SPI_CS_HIGH)

struct mt7621_spi;

@@ -104,7 +105,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
int cs = spi->chip_select;
u32 polar = 0;

- mt7621_spi_reset(rs, cs);
+ mt7621_spi_reset(rs, cs);
if (enable)
polar = BIT(cs);
mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
@@ -137,18 +138,18 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
reg |= MT7621_LSB_FIRST;

reg &= ~(MT7621_CPHA | MT7621_CPOL);
- switch(spi->mode & (SPI_CPOL | SPI_CPHA)) {
- case SPI_MODE_0:
- break;
- case SPI_MODE_1:
- reg |= MT7621_CPHA;
- break;
- case SPI_MODE_2:
- reg |= MT7621_CPOL;
- break;
- case SPI_MODE_3:
- reg |= MT7621_CPOL | MT7621_CPHA;
- break;
+ switch (spi->mode & (SPI_CPOL | SPI_CPHA)) {
+ case SPI_MODE_0:
+ break;
+ case SPI_MODE_1:
+ reg |= MT7621_CPHA;
+ break;
+ case SPI_MODE_2:
+ reg |= MT7621_CPOL;
+ break;
+ case SPI_MODE_3:
+ reg |= MT7621_CPOL | MT7621_CPHA;
+ break;
}
mt7621_spi_write(rs, MT7621_SPI_MASTER, reg);

@@ -164,9 +165,8 @@ static inline int mt7621_spi_wait_till_ready(struct spi_device *spi)
u32 status;

status = mt7621_spi_read(rs, MT7621_SPI_TRANS);
- if ((status & SPITRANS_BUSY) == 0) {
+ if ((status & SPITRANS_BUSY) == 0)
return 0;
- }
cpu_relax();
udelay(1);
}
--
2.11.0

2018-05-31 21:06:15

by NeilBrown

[permalink] [raw]

Subject: Re: [PATCH] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

On Fri, Jun 01 2018, Sankalp Negi wrote:

> This patch fixes following checkpatch.pl issues:
>
> WARNING : line over 80 characters
> ERROR : code indent should use tabs where possible
> WARNING : no spaces at the start of a line
> ERROR : switch and case should be at the same indent
> ERROR : space required before the open parenthesis
> WARNING : braces {} are not necessary for single statement blocks
>
> Signed-off-by: Sankalp Negi <[email protected]>
> ---
> drivers/staging/mt7621-spi/spi-mt7621.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
> index 37f299080410..cd94f5f569df 100644
> --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> @@ -55,7 +55,8 @@
> #define MT7621_CPOL BIT(4)
> #define MT7621_LSB_FIRST BIT(3)
>
> -#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_CS_HIGH)
> +#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | \
> + SPI_LSB_FIRST | SPI_CS_HIGH)
>

Thanks for this. It all looks good except that above. I'm a bit picky
about indentation, and more picky about making the code easy to read.
The above breaks an indentation rule and (I think) hurts readability.
It was only just over 80 columns so it wasn't all that bad as it was -
let's be sure to make it better.
Some options:

#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_CS_HIGH)

i.e. replace the tab with a space. Now it doesn't line up with the
previous lines, but I'm not sure that matters much.

#define RT2880_SPI_MODE_BITS \
(SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_CS_HIGH)

This keeps all the content together on one line.

#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | \
SPI_LSB_FIRST | SPI_CS_HIGH)

This is close to what you had, but doesn't break the indenting rule:
everything inside brackets must be to the right of the opening bracket
unless that opening bracket is at the end of a line.

Any of these would be acceptable - my personal preference is the second
one.

Thanks,
NeilBrown

> struct mt7621_spi;
>
> @@ -104,7 +105,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
> int cs = spi->chip_select;
> u32 polar = 0;
>
> - mt7621_spi_reset(rs, cs);
> + mt7621_spi_reset(rs, cs);
> if (enable)
> polar = BIT(cs);
> mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> @@ -137,18 +138,18 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
> reg |= MT7621_LSB_FIRST;
>
> reg &= ~(MT7621_CPHA | MT7621_CPOL);
> - switch(spi->mode & (SPI_CPOL | SPI_CPHA)) {
> - case SPI_MODE_0:
> - break;
> - case SPI_MODE_1:
> - reg |= MT7621_CPHA;
> - break;
> - case SPI_MODE_2:
> - reg |= MT7621_CPOL;
> - break;
> - case SPI_MODE_3:
> - reg |= MT7621_CPOL | MT7621_CPHA;
> - break;
> + switch (spi->mode & (SPI_CPOL | SPI_CPHA)) {
> + case SPI_MODE_0:
> + break;
> + case SPI_MODE_1:
> + reg |= MT7621_CPHA;
> + break;
> + case SPI_MODE_2:
> + reg |= MT7621_CPOL;
> + break;
> + case SPI_MODE_3:
> + reg |= MT7621_CPOL | MT7621_CPHA;
> + break;
> }
> mt7621_spi_write(rs, MT7621_SPI_MASTER, reg);
>
> @@ -164,9 +165,8 @@ static inline int mt7621_spi_wait_till_ready(struct spi_device *spi)
> u32 status;
>
> status = mt7621_spi_read(rs, MT7621_SPI_TRANS);
> - if ((status & SPITRANS_BUSY) == 0) {
> + if ((status & SPITRANS_BUSY) == 0)
> return 0;
> - }
> cpu_relax();
> udelay(1);
> }
> --
> 2.11.0

Attachments:

signature.asc (847.00 B)

2018-05-31 22:16:07

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

On Fri, 2018-06-01 at 00:25 +0530, Sankalp Negi wrote:
> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
[]
> @@ -164,9 +165,8 @@ static inline int mt7621_spi_wait_till_ready(struct spi_device *spi)
> u32 status;
>
> status = mt7621_spi_read(rs, MT7621_SPI_TRANS);
> - if ((status & SPITRANS_BUSY) == 0) {
> + if ((status & SPITRANS_BUSY) == 0)
> return 0;
> - }
> cpu_relax();
> udelay(1);
> }

It might also be more readable to avoid a status temporary
and use a direct compare instead:

for (i = 0; i < RALINK_SPI_WAIT_MAX_LOOP; i++) {
if (!(mt7621_spi_read(rs, MT7621_SPI_TRANS) & SPITRANS_BUSY))
return 0;
cpu_relax();
udelay(1);
}

return -ETIMEDOUT;
}

2018-06-01 09:17:19

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

On Fri, Jun 01, 2018 at 12:25:43AM +0530, Sankalp Negi wrote:
> This patch fixes following checkpatch.pl issues:
>
> WARNING : line over 80 characters
> ERROR : code indent should use tabs where possible
> WARNING : no spaces at the start of a line
> ERROR : switch and case should be at the same indent
> ERROR : space required before the open parenthesis
> WARNING : braces {} are not necessary for single statement blocks

Those are all different types of coding style cleanups and should be
done as a patch series, only fixing up one "type" of thing at a time.

thanks,

greg k-h

2018-06-02 14:27:32

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 0/5] staging: mt7621-spi: Fix Coding style issues.

This patch series lists different changes for coding style fixes
as reported by checkpatch.pl, changes can be broken down as :

1. Indent switch and case labels at the same level
2. Fix a line over 80 columns
3. Place tabs for indentation instead of spaces
4. Add a space before open paranthesis in switch
5. Remove unnecessary braces {} from single statement if

version 2 changes
- segregated changes in individual commits as per fix category

Sankalp Negi (5):
staging: mt7621-spi: Fix Coding style issues reported by
checkpatch.pl.
staging: mt7621-spi: Fix Coding style issues reported by
checkpatch.pl.
staging: mt7621-spi: Fix Coding style issues reported by
checkpatch.pl.
staging: mt7621-spi: Fix Coding style issues reported by
checkpatch.pl.
staging: mt7621-spi: Fix Coding style issues reported by
checkpatch.pl.

drivers/staging/mt7621-spi/spi-mt7621.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

--
2.11.0

2018-06-02 14:28:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 1/5] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

The patch fixes following checkpatch.pl issue:
ERROR : switch and case should be at the same indent

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 37f299080410..b268084b6cd1 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -138,17 +138,17 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)

reg &= ~(MT7621_CPHA | MT7621_CPOL);
switch(spi->mode & (SPI_CPOL | SPI_CPHA)) {
- case SPI_MODE_0:
- break;
- case SPI_MODE_1:
- reg |= MT7621_CPHA;
- break;
- case SPI_MODE_2:
- reg |= MT7621_CPOL;
- break;
- case SPI_MODE_3:
- reg |= MT7621_CPOL | MT7621_CPHA;
- break;
+ case SPI_MODE_0:
+ break;
+ case SPI_MODE_1:
+ reg |= MT7621_CPHA;
+ break;
+ case SPI_MODE_2:
+ reg |= MT7621_CPOL;
+ break;
+ case SPI_MODE_3:
+ reg |= MT7621_CPOL | MT7621_CPHA;
+ break;
}
mt7621_spi_write(rs, MT7621_SPI_MASTER, reg);

--
2.11.0

2018-06-02 14:28:27

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 2/5] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

The patch fixes following checkpatch.pl issue:
WARNING : line over 80 characters

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index b268084b6cd1..910dbdb8a003 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -55,7 +55,8 @@
#define MT7621_CPOL BIT(4)
#define MT7621_LSB_FIRST BIT(3)

-#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_CS_HIGH)
+#define RT2880_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | \
+ SPI_LSB_FIRST | SPI_CS_HIGH)

struct mt7621_spi;

--
2.11.0

2018-06-02 14:28:36

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 4/5] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

The patch fixes following checkpatch.pl issue:
ERROR : space required before the open parenthesis

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index f7620a988a08..472f72479162 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -138,7 +138,7 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
reg |= MT7621_LSB_FIRST;

reg &= ~(MT7621_CPHA | MT7621_CPOL);
- switch(spi->mode & (SPI_CPOL | SPI_CPHA)) {
+ switch (spi->mode & (SPI_CPOL | SPI_CPHA)) {
case SPI_MODE_0:
break;
case SPI_MODE_1:
--
2.11.0

2018-06-02 14:28:37

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 5/5] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

The patch fixes following checkpatch.pl issue:
WARNING : braces {} are not necessary for single statement blocks

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 472f72479162..cf5b5d7c3cf9 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -165,9 +165,8 @@ static inline int mt7621_spi_wait_till_ready(struct spi_device *spi)
u32 status;

status = mt7621_spi_read(rs, MT7621_SPI_TRANS);
- if ((status & SPITRANS_BUSY) == 0) {
+ if ((status & SPITRANS_BUSY) == 0)
return 0;
- }
cpu_relax();
udelay(1);
}
--
2.11.0

2018-06-02 14:29:28

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v2 3/5] staging: mt7621-spi: Fix Coding style issues reported by checkpatch.pl.

The patch fixes following checkpatch.pl issue:
ERROR : code indent should use tabs where possible

Signed-off-by: Sankalp Negi <[email protected]>
---
drivers/staging/mt7621-spi/spi-mt7621.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 910dbdb8a003..f7620a988a08 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -105,7 +105,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
int cs = spi->chip_select;
u32 polar = 0;

- mt7621_spi_reset(rs, cs);
+ mt7621_spi_reset(rs, cs);
if (enable)
polar = BIT(cs);
mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
--
2.11.0

2018-06-02 15:32:26

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] staging: mt7621-spi: Fix Coding style issues.

On Sat, 2018-06-02 at 19:56 +0530, Sankalp Negi wrote:
> This patch series lists different changes for coding style fixes
> as reported by checkpatch.pl, changes can be broken down as :
>
> 1. Indent switch and case labels at the same level
> 2. Fix a line over 80 columns
> 3. Place tabs for indentation instead of spaces
> 4. Add a space before open paranthesis in switch
> 5. Remove unnecessary braces {} from single statement if
>
> version 2 changes
> - segregated changes in individual commits as per fix category
>
> Sankalp Negi (5):
> staging: mt7621-spi: Fix Coding style issues reported by
> checkpatch.pl.
> staging: mt7621-spi: Fix Coding style issues reported by
> checkpatch.pl.
> staging: mt7621-spi: Fix Coding style issues reported by
> checkpatch.pl.
> staging: mt7621-spi: Fix Coding style issues reported by
> checkpatch.pl.
> staging: mt7621-spi: Fix Coding style issues reported by
> checkpatch.pl.

Plese use unique subjects for each patch in a series.

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 5/5] staging: mt7621-spi: Remove unnecessary braces {} from single statement if block.

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 2/5] staging: mt7621-spi: Fix line over 80 characters by refactoring.

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 0/5] staging: mt7621-spi: Fix C Coding style issues.

This patch series lists different changes for coding style fixes
as reported by checkpatch.pl, changes can be broken down as :

1. Indent switch and case labels at the same level.
2. Fix a line over 80 columns by refactoring.
3. Use tabs for indentation instead of spaces.
4. Add a space before open paranthesis in switch.
5. Remove unnecessary braces {} from single statement if block.

version 2 changes
- segregated changes in individual commits as per fix category.

version 3 changes
- added unique subjects for each patch in the series.

Sankalp Negi (5):
staging: mt7621-spi: Indent case labels and switch at the same level.
staging: mt7621-spi: Fix line over 80 characters by refactoring.
staging: mt7621-spi: Use tabs for indentation instead of spaces.
staging: mt7621-spi: Add a space before open paranthesis.
staging: mt7621-spi: Remove unnecessary braces {} from single
statement if block.

drivers/staging/mt7621-spi/spi-mt7621.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

--
2.11.0

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 4/5] staging: mt7621-spi: Add a space before open paranthesis.

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 3/5] staging: mt7621-spi: Use tabs for indentation instead of spaces.

2018-06-02 18:40:06

by Sankalp Negi

[permalink] [raw]

Subject: [PATCH v3 1/5] staging: mt7621-spi: Indent case labels and switch at the same level.

2018-06-04 00:19:24

by NeilBrown

[permalink] [raw]

Subject: Re: [PATCH v3 0/5] staging: mt7621-spi: Fix C Coding style issues.

On Sun, Jun 03 2018, Sankalp Negi wrote:

> This patch series lists different changes for coding style fixes
> as reported by checkpatch.pl, changes can be broken down as :
>
> 1. Indent switch and case labels at the same level.
> 2. Fix a line over 80 columns by refactoring.
> 3. Use tabs for indentation instead of spaces.
> 4. Add a space before open paranthesis in switch.
> 5. Remove unnecessary braces {} from single statement if block.
>
> version 2 changes
> - segregated changes in individual commits as per fix category.
>
> version 3 changes
> - added unique subjects for each patch in the series.
>
>
> Sankalp Negi (5):
> staging: mt7621-spi: Indent case labels and switch at the same level.
> staging: mt7621-spi: Fix line over 80 characters by refactoring.
> staging: mt7621-spi: Use tabs for indentation instead of spaces.
> staging: mt7621-spi: Add a space before open paranthesis.
> staging: mt7621-spi: Remove unnecessary braces {} from single
> statement if block.
>

Thanks,
All
Reviewed-by: NeilBrown <[email protected]>

NeilBrown

Attachments:

signature.asc (847.00 B)