* [PATCH] IIO: accel: fixed coding style issues
@ 2022-03-28 15:05 Paul Lemmermann
2022-03-28 17:06 ` Jonathan Cameron
2022-03-28 20:32 ` Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Paul Lemmermann @ 2022-03-28 15:05 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, linux-kernel
Fixed case statement issues and spacing issues.
Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] IIO: accel: fixed coding style issues
2022-03-28 15:05 [PATCH] IIO: accel: fixed coding style issues Paul Lemmermann
@ 2022-03-28 17:06 ` Jonathan Cameron
2022-03-28 19:17 ` Paul Lemmermann
2022-03-28 20:32 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2022-03-28 17:06 UTC (permalink / raw)
To: Paul Lemmermann; +Cc: linux-iio, linux-kernel
On Mon, 28 Mar 2022 10:05:08 -0500
Paul Lemmermann <thepaulodoom@thepaulodoom.com> wrote:
> Fixed case statement issues and spacing issues.
>
> Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com>
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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IIO: accel: fixed coding style issues
2022-03-28 17:06 ` Jonathan Cameron
@ 2022-03-28 19:17 ` Paul Lemmermann
2022-04-02 16:35 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Paul Lemmermann @ 2022-03-28 19:17 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, linux-kernel
On Mon, Mar 28, 2022 at 06:06:23PM +0100, Jonathan Cameron wrote:
> On Mon, 28 Mar 2022 10:05:08 -0500
> Paul Lemmermann <thepaulodoom@thepaulodoom.com> wrote:
>
> > Fixed case statement issues and spacing issues.
> >
> > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com>
> 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);
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IIO: accel: fixed coding style issues
2022-03-28 15:05 [PATCH] IIO: accel: fixed coding style issues Paul Lemmermann
2022-03-28 17:06 ` Jonathan Cameron
@ 2022-03-28 20:32 ` Andy Shevchenko
2022-03-28 21:06 ` Paul Lemmermann
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-28 20:32 UTC (permalink / raw)
To: Paul Lemmermann; +Cc: Jonathan Cameron, linux-iio, Linux Kernel Mailing List
On Mon, Mar 28, 2022 at 7:45 PM Paul Lemmermann
<thepaulodoom@thepaulodoom.com> wrote:
>
> Fixed case statement issues and spacing issues.
...
> 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;
> }
What you cited from documentation mostly affects the new code, but
this code is already in the kernel and modifying it, esp. taking into
account 3x LOCs count, looks like an unneeded churn, even if
documentation thinks otherwise.
What I could acknowledge from your proposal is the default case.
Otherwise just leave it to the point if we touch this code for
something else in the future.
...
> static const struct of_device_id kxsd9_of_match[] = {
> - { .compatible = "kionix,kxsd9" },
> - { },
> + { .compatible = "kionix,kxsd9" },
> + { },
I dunno why you touched this line (likely TABs vs. spaces), but please
remove the comma as well here.
> };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IIO: accel: fixed coding style issues
2022-03-28 20:32 ` Andy Shevchenko
@ 2022-03-28 21:06 ` Paul Lemmermann
0 siblings, 0 replies; 6+ messages in thread
From: Paul Lemmermann @ 2022-03-28 21:06 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Jonathan Cameron, linux-iio, Linux Kernel Mailing List
On Mon, Mar 28, 2022 at 11:32:44PM +0300, Andy Shevchenko wrote:
> On Mon, Mar 28, 2022 at 7:45 PM Paul Lemmermann
> <thepaulodoom@thepaulodoom.com> wrote:
> >
> > Fixed case statement issues and spacing issues.
>
> ...
>
> > 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;
> > }
>
> What you cited from documentation mostly affects the new code, but
> this code is already in the kernel and modifying it, esp. taking into
> account 3x LOCs count, looks like an unneeded churn, even if
> documentation thinks otherwise.
>
> What I could acknowledge from your proposal is the default case.
> Otherwise just leave it to the point if we touch this code for
> something else in the future.
>
> ...
>
> > static const struct of_device_id kxsd9_of_match[] = {
> > - { .compatible = "kionix,kxsd9" },
> > - { },
> > + { .compatible = "kionix,kxsd9" },
>
> > + { },
>
> I dunno why you touched this line (likely TABs vs. spaces), but please
> remove the comma as well here.
Yes, that is exactly why this patch exists.
Thank you, I will take these thoughts into account and submit a new
patch.
Thanks!
Paul
>
> > };
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IIO: accel: fixed coding style issues
2022-03-28 19:17 ` Paul Lemmermann
@ 2022-04-02 16:35 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-04-02 16:35 UTC (permalink / raw)
To: Paul Lemmermann; +Cc: linux-iio, linux-kernel
On Mon, 28 Mar 2022 14:17:51 -0500
Paul Lemmermann <thepaulodoom@thepaulodoom.com> 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 <thepaulodoom@thepaulodoom.com> wrote:
> >
> > > Fixed case statement issues and spacing issues.
> > >
> > > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com>
> > 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);
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-02 16:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-28 15:05 [PATCH] IIO: accel: fixed coding style issues Paul Lemmermann
2022-03-28 17:06 ` Jonathan Cameron
2022-03-28 19:17 ` Paul Lemmermann
2022-04-02 16:35 ` Jonathan Cameron
2022-03-28 20:32 ` Andy Shevchenko
2022-03-28 21:06 ` Paul Lemmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).