2022-03-28 21:50:33

by ThePaulODoom

[permalink] [raw]
Subject: [PATCH] IIO: accel: fixed coding style issues

Fixed case statement issues and spacing issues.

Signed-off-by: Paul Lemmermann <[email protected]>
---
drivers/iio/accel/bmc150-accel-core.c | 15 +++++++++++----
drivers/iio/accel/dmard09.c | 2 +-
drivers/iio/accel/kxsd9-spi.c | 4 ++--
3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index d11f66801..f7dd7ec2c 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -432,10 +432,17 @@ static bool bmc150_apply_bosc0200_acpi_orientation(struct device *dev,

for (j = 0; j < 3; j++) {
switch (val[j]) {
- case -1: str = "-1"; break;
- case 0: str = "0"; break;
- case 1: str = "1"; break;
- default: goto unknown_format;
+ case -1:
+ str = "-1";
+ break;
+ case 0:
+ str = "0";
+ break;
+ case 1:
+ str = "1";
+ break;
+ default:
+ goto unknown_format;
}
orientation->rotation[i * 3 + j] = str;
}
diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
index e6e28c964..87bc38d4d 100644
--- a/drivers/iio/accel/dmard09.c
+++ b/drivers/iio/accel/dmard09.c
@@ -24,7 +24,7 @@
#define DMARD09_AXIS_Y 1
#define DMARD09_AXIS_Z 2
#define DMARD09_AXIS_X_OFFSET ((DMARD09_AXIS_X + 1) * 2)
-#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1 )* 2)
+#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1) * 2)
#define DMARD09_AXIS_Z_OFFSET ((DMARD09_AXIS_Z + 1) * 2)

struct dmard09_data {
diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
index 57c451cfb..989f53fb0 100644
--- a/drivers/iio/accel/kxsd9-spi.c
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -44,8 +44,8 @@ static const struct spi_device_id kxsd9_spi_id[] = {
MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);

static const struct of_device_id kxsd9_of_match[] = {
- { .compatible = "kionix,kxsd9" },
- { },
+ { .compatible = "kionix,kxsd9" },
+ { },
};
MODULE_DEVICE_TABLE(of, kxsd9_of_match);

--
2.35.1


2022-03-28 22:02:27

by ThePaulODoom

[permalink] [raw]
Subject: Re: [PATCH] IIO: accel: fixed coding style issues

On Mon, Mar 28, 2022 at 06:06:23PM +0100, Jonathan Cameron wrote:
> On Mon, 28 Mar 2022 10:05:08 -0500
> Paul Lemmermann <[email protected]> wrote:
>
> > Fixed case statement issues and spacing issues.
> >
> > Signed-off-by: Paul Lemmermann <[email protected]>
> Hi Paul,
>
> Thanks for the patch.
>
> These are drivers written / maintained by different authors, so one
> patch per driver preferred. Particularly handy as people might not
> agree with all of them so separate patches would allow me to pick
> and choose which ones to pick up.
>
> Comments inline.
>
> Please state how you identified the changes btw. Script?

I used the checkpatch.pl script to identify these changes.

>
> Jonathan
>
> > ---
> > drivers/iio/accel/bmc150-accel-core.c | 15 +++++++++++----
> > drivers/iio/accel/dmard09.c | 2 +-
> > drivers/iio/accel/kxsd9-spi.c | 4 ++--
> > 3 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index d11f66801..f7dd7ec2c 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -432,10 +432,17 @@ static bool bmc150_apply_bosc0200_acpi_orientation(struct device *dev,
> >
> > for (j = 0; j < 3; j++) {
> > switch (val[j]) {
> > - case -1: str = "-1"; break;
> > - case 0: str = "0"; break;
> > - case 1: str = "1"; break;
> > - default: goto unknown_format;
> > + case -1:
> > + str = "-1";
> > + break;
> > + case 0:
> > + str = "0";
> > + break;
> > + case 1:
> > + str = "1";
> > + break;
> > + default:
> > + goto unknown_format;
>
> I'm not seeing any clear advantage to this change.
>
The reason for this change is that most, if not all, case statements in
the kernel are multiple lines. Also, in the kernel docs (https://docs.kernel.org/process/coding-style.html)
it says, to not put multiple statements on one line.

Thanks,
Paul
>
> > }
> > orientation->rotation[i * 3 + j] = str;
> > }
> > diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
> > index e6e28c964..87bc38d4d 100644
> > --- a/drivers/iio/accel/dmard09.c
> > +++ b/drivers/iio/accel/dmard09.c
> > @@ -24,7 +24,7 @@
> > #define DMARD09_AXIS_Y 1
> > #define DMARD09_AXIS_Z 2
> > #define DMARD09_AXIS_X_OFFSET ((DMARD09_AXIS_X + 1) * 2)
> > -#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1 )* 2)
> > +#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1) * 2)
>
> This is one is good.
>
> > #define DMARD09_AXIS_Z_OFFSET ((DMARD09_AXIS_Z + 1) * 2)
> >
> > struct dmard09_data {
> > diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> > index 57c451cfb..989f53fb0 100644
> > --- a/drivers/iio/accel/kxsd9-spi.c
> > +++ b/drivers/iio/accel/kxsd9-spi.c
> > @@ -44,8 +44,8 @@ static const struct spi_device_id kxsd9_spi_id[] = {
> > MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
> >
> > static const struct of_device_id kxsd9_of_match[] = {
> > - { .compatible = "kionix,kxsd9" },
> > - { },
> > + { .compatible = "kionix,kxsd9" },
>
> Also good to clean up.
>
> Thanks,
>
> Jonathan
>
>
> > + { },
> > };
> > MODULE_DEVICE_TABLE(of, kxsd9_of_match);
> >
>

2022-03-28 22:18:22

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] IIO: accel: fixed coding style issues

On Mon, 28 Mar 2022 10:05:08 -0500
Paul Lemmermann <[email protected]> wrote:

> Fixed case statement issues and spacing issues.
>
> Signed-off-by: Paul Lemmermann <[email protected]>
Hi Paul,

Thanks for the patch.

These are drivers written / maintained by different authors, so one
patch per driver preferred. Particularly handy as people might not
agree with all of them so separate patches would allow me to pick
and choose which ones to pick up.

Comments inline.

Please state how you identified the changes btw. Script?

Jonathan

> ---
> drivers/iio/accel/bmc150-accel-core.c | 15 +++++++++++----
> drivers/iio/accel/dmard09.c | 2 +-
> drivers/iio/accel/kxsd9-spi.c | 4 ++--
> 3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index d11f66801..f7dd7ec2c 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -432,10 +432,17 @@ static bool bmc150_apply_bosc0200_acpi_orientation(struct device *dev,
>
> for (j = 0; j < 3; j++) {
> switch (val[j]) {
> - case -1: str = "-1"; break;
> - case 0: str = "0"; break;
> - case 1: str = "1"; break;
> - default: goto unknown_format;
> + case -1:
> + str = "-1";
> + break;
> + case 0:
> + str = "0";
> + break;
> + case 1:
> + str = "1";
> + break;
> + default:
> + goto unknown_format;

I'm not seeing any clear advantage to this change.


> }
> orientation->rotation[i * 3 + j] = str;
> }
> diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
> index e6e28c964..87bc38d4d 100644
> --- a/drivers/iio/accel/dmard09.c
> +++ b/drivers/iio/accel/dmard09.c
> @@ -24,7 +24,7 @@
> #define DMARD09_AXIS_Y 1
> #define DMARD09_AXIS_Z 2
> #define DMARD09_AXIS_X_OFFSET ((DMARD09_AXIS_X + 1) * 2)
> -#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1 )* 2)
> +#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1) * 2)

This is one is good.

> #define DMARD09_AXIS_Z_OFFSET ((DMARD09_AXIS_Z + 1) * 2)
>
> struct dmard09_data {
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> index 57c451cfb..989f53fb0 100644
> --- a/drivers/iio/accel/kxsd9-spi.c
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -44,8 +44,8 @@ static const struct spi_device_id kxsd9_spi_id[] = {
> MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
>
> static const struct of_device_id kxsd9_of_match[] = {
> - { .compatible = "kionix,kxsd9" },
> - { },
> + { .compatible = "kionix,kxsd9" },

Also good to clean up.

Thanks,

Jonathan


> + { },
> };
> MODULE_DEVICE_TABLE(of, kxsd9_of_match);
>

2022-04-05 02:27:40

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] IIO: accel: fixed coding style issues

On Mon, 28 Mar 2022 14:17:51 -0500
Paul Lemmermann <[email protected]> wrote:

> On Mon, Mar 28, 2022 at 06:06:23PM +0100, Jonathan Cameron wrote:
> > On Mon, 28 Mar 2022 10:05:08 -0500
> > Paul Lemmermann <[email protected]> wrote:
> >
> > > Fixed case statement issues and spacing issues.
> > >
> > > Signed-off-by: Paul Lemmermann <[email protected]>
> > Hi Paul,
> >
> > Thanks for the patch.
> >
> > These are drivers written / maintained by different authors, so one
> > patch per driver preferred. Particularly handy as people might not
> > agree with all of them so separate patches would allow me to pick
> > and choose which ones to pick up.
> >
> > Comments inline.
> >
> > Please state how you identified the changes btw. Script?
>
> I used the checkpatch.pl script to identify these changes.
Please mention that in patch descriptions in future.
It's useful to anyone tracking this sort of information.

I added it to the broken out patches whilst applying.

Thanks,

Jonathan

>
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/accel/bmc150-accel-core.c | 15 +++++++++++----
> > > drivers/iio/accel/dmard09.c | 2 +-
> > > drivers/iio/accel/kxsd9-spi.c | 4 ++--
> > > 3 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > > index d11f66801..f7dd7ec2c 100644
> > > --- a/drivers/iio/accel/bmc150-accel-core.c
> > > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > > @@ -432,10 +432,17 @@ static bool bmc150_apply_bosc0200_acpi_orientation(struct device *dev,
> > >
> > > for (j = 0; j < 3; j++) {
> > > switch (val[j]) {
> > > - case -1: str = "-1"; break;
> > > - case 0: str = "0"; break;
> > > - case 1: str = "1"; break;
> > > - default: goto unknown_format;
> > > + case -1:
> > > + str = "-1";
> > > + break;
> > > + case 0:
> > > + str = "0";
> > > + break;
> > > + case 1:
> > > + str = "1";
> > > + break;
> > > + default:
> > > + goto unknown_format;
> >
> > I'm not seeing any clear advantage to this change.
> >
> The reason for this change is that most, if not all, case statements in
> the kernel are multiple lines. Also, in the kernel docs (https://docs.kernel.org/process/coding-style.html)
> it says, to not put multiple statements on one line.
>
> Thanks,
> Paul
> >
> > > }
> > > orientation->rotation[i * 3 + j] = str;
> > > }
> > > diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
> > > index e6e28c964..87bc38d4d 100644
> > > --- a/drivers/iio/accel/dmard09.c
> > > +++ b/drivers/iio/accel/dmard09.c
> > > @@ -24,7 +24,7 @@
> > > #define DMARD09_AXIS_Y 1
> > > #define DMARD09_AXIS_Z 2
> > > #define DMARD09_AXIS_X_OFFSET ((DMARD09_AXIS_X + 1) * 2)
> > > -#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1 )* 2)
> > > +#define DMARD09_AXIS_Y_OFFSET ((DMARD09_AXIS_Y + 1) * 2)
> >
> > This is one is good.
> >
> > > #define DMARD09_AXIS_Z_OFFSET ((DMARD09_AXIS_Z + 1) * 2)
> > >
> > > struct dmard09_data {
> > > diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> > > index 57c451cfb..989f53fb0 100644
> > > --- a/drivers/iio/accel/kxsd9-spi.c
> > > +++ b/drivers/iio/accel/kxsd9-spi.c
> > > @@ -44,8 +44,8 @@ static const struct spi_device_id kxsd9_spi_id[] = {
> > > MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
> > >
> > > static const struct of_device_id kxsd9_of_match[] = {
> > > - { .compatible = "kionix,kxsd9" },
> > > - { },
> > > + { .compatible = "kionix,kxsd9" },
> >
> > Also good to clean up.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> > > + { },
> > > };
> > > MODULE_DEVICE_TABLE(of, kxsd9_of_match);
> > >
> >