linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe
@ 2019-02-21  2:06 Gustavo A. R. Silva
  2019-02-21  7:00 ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-21  2:06 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, Kees Cook
  Cc: linux-iio, linux-kernel, Gustavo A. R. Silva, Gwendal Grignou

Refactor some code in order to fix both the technical implementation
and the following warnings:

drivers/iio/accel/cros_ec_accel_legacy.c: In function ‘cros_ec_accel_legacy_probe’:
drivers/iio/accel/cros_ec_accel_legacy.c:387:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
    ec_accel_channels[X].scan_index = Y;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
drivers/iio/accel/cros_ec_accel_legacy.c:388:3: note: here
   case Y:
   ^~~~
drivers/iio/accel/cros_ec_accel_legacy.c:389:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
    ec_accel_channels[Y].scan_index = X;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
drivers/iio/accel/cros_ec_accel_legacy.c:390:3: note: here
   case Z:
   ^~~~

Notice that neither the for loop nor the switch statement is needed.
Also, "state->sign[Y] = 1" should be unconditional.

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/iio/accel/cros_ec_accel_legacy.c | 27 +++++++++++-------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
index 063e89eff791..021f9f5cd3bb 100644
--- a/drivers/iio/accel/cros_ec_accel_legacy.c
+++ b/drivers/iio/accel/cros_ec_accel_legacy.c
@@ -353,7 +353,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
 	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
 	struct iio_dev *indio_dev;
 	struct cros_ec_accel_legacy_state *state;
-	int ret, i;
+	int ret;
 
 	if (!ec || !ec->ec_dev) {
 		dev_warn(&pdev->dev, "No EC device found.\n");
@@ -381,20 +381,17 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
 	 * Present the channel using HTML5 standard:
 	 * need to invert X and Y and invert some lid axis.
 	 */
-	for (i = X ; i < MAX_AXIS; i++) {
-		switch (i) {
-		case X:
-			ec_accel_channels[X].scan_index = Y;
-		case Y:
-			ec_accel_channels[Y].scan_index = X;
-		case Z:
-			ec_accel_channels[Z].scan_index = Z;
-		}
-		if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
-			state->sign[i] = -1;
-		else
-			state->sign[i] = 1;
-	}
+	ec_accel_channels[X].scan_index = Y;
+	ec_accel_channels[Y].scan_index = X;
+	ec_accel_channels[Z].scan_index = Z;
+
+	state->sign[Y] = 1;
+
+	if (state->sensor_num == MOTIONSENSE_LOC_LID)
+		state->sign[X] = state->sign[Z] = -1;
+	else
+		state->sign[X] = state->sign[Z] = 1;
+
 	indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->info = &cros_ec_accel_legacy_info;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe
  2019-02-21  2:06 [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe Gustavo A. R. Silva
@ 2019-02-21  7:00 ` Kees Cook
  2019-02-21  8:03   ` Enric Balletbo i Serra
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2019-02-21  7:00 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, linux-iio, LKML, Gwendal Grignou

On Wed, Feb 20, 2019 at 6:06 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Refactor some code in order to fix both the technical implementation
> and the following warnings:
>
> drivers/iio/accel/cros_ec_accel_legacy.c: In function ‘cros_ec_accel_legacy_probe’:
> drivers/iio/accel/cros_ec_accel_legacy.c:387:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     ec_accel_channels[X].scan_index = Y;
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> drivers/iio/accel/cros_ec_accel_legacy.c:388:3: note: here
>    case Y:
>    ^~~~
> drivers/iio/accel/cros_ec_accel_legacy.c:389:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     ec_accel_channels[Y].scan_index = X;
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> drivers/iio/accel/cros_ec_accel_legacy.c:390:3: note: here
>    case Z:
>    ^~~~
>
> Notice that neither the for loop nor the switch statement is needed.
> Also, "state->sign[Y] = 1" should be unconditional.
>
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  drivers/iio/accel/cros_ec_accel_legacy.c | 27 +++++++++++-------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 063e89eff791..021f9f5cd3bb 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -353,7 +353,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>         struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>         struct iio_dev *indio_dev;
>         struct cros_ec_accel_legacy_state *state;
> -       int ret, i;
> +       int ret;
>
>         if (!ec || !ec->ec_dev) {
>                 dev_warn(&pdev->dev, "No EC device found.\n");
> @@ -381,20 +381,17 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>          * Present the channel using HTML5 standard:
>          * need to invert X and Y and invert some lid axis.
>          */
> -       for (i = X ; i < MAX_AXIS; i++) {
> -               switch (i) {
> -               case X:
> -                       ec_accel_channels[X].scan_index = Y;
> -               case Y:
> -                       ec_accel_channels[Y].scan_index = X;
> -               case Z:
> -                       ec_accel_channels[Z].scan_index = Z;
> -               }
> -               if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> -                       state->sign[i] = -1;
> -               else
> -                       state->sign[i] = 1;
> -       }
> +       ec_accel_channels[X].scan_index = Y;
> +       ec_accel_channels[Y].scan_index = X;
> +       ec_accel_channels[Z].scan_index = Z;
> +
> +       state->sign[Y] = 1;
> +
> +       if (state->sensor_num == MOTIONSENSE_LOC_LID)
> +               state->sign[X] = state->sign[Z] = -1;
> +       else
> +               state->sign[X] = state->sign[Z] = 1;
> +
>         indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
>         indio_dev->dev.parent = &pdev->dev;
>         indio_dev->info = &cros_ec_accel_legacy_info;
> --
> 2.20.1
>


-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe
  2019-02-21  7:00 ` Kees Cook
@ 2019-02-21  8:03   ` Enric Balletbo i Serra
  2019-03-03 16:20     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Enric Balletbo i Serra @ 2019-02-21  8:03 UTC (permalink / raw)
  To: Kees Cook, Gustavo A. R. Silva
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Benson Leung, Guenter Roeck, linux-iio,
	LKML, Gwendal Grignou



On 21/2/19 8:00, Kees Cook wrote:
> On Wed, Feb 20, 2019 at 6:06 PM Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>>
>> Refactor some code in order to fix both the technical implementation
>> and the following warnings:
>>
>> drivers/iio/accel/cros_ec_accel_legacy.c: In function ‘cros_ec_accel_legacy_probe’:
>> drivers/iio/accel/cros_ec_accel_legacy.c:387:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>     ec_accel_channels[X].scan_index = Y;
>>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
>> drivers/iio/accel/cros_ec_accel_legacy.c:388:3: note: here
>>    case Y:
>>    ^~~~
>> drivers/iio/accel/cros_ec_accel_legacy.c:389:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>     ec_accel_channels[Y].scan_index = X;
>>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
>> drivers/iio/accel/cros_ec_accel_legacy.c:390:3: note: here
>>    case Z:
>>    ^~~~
>>
>> Notice that neither the for loop nor the switch statement is needed.
>> Also, "state->sign[Y] = 1" should be unconditional.
>>
>> This patch is part of the ongoing efforts to enable
>> -Wimplicit-fallthrough.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> 
> Acked-by: Kees Cook <keescook@chromium.org>
> 

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks,
 Enric

> -Kees
> 
>> ---
>>  drivers/iio/accel/cros_ec_accel_legacy.c | 27 +++++++++++-------------
>>  1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
>> index 063e89eff791..021f9f5cd3bb 100644
>> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
>> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
>> @@ -353,7 +353,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>>         struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>>         struct iio_dev *indio_dev;
>>         struct cros_ec_accel_legacy_state *state;
>> -       int ret, i;
>> +       int ret;
>>
>>         if (!ec || !ec->ec_dev) {
>>                 dev_warn(&pdev->dev, "No EC device found.\n");
>> @@ -381,20 +381,17 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>>          * Present the channel using HTML5 standard:
>>          * need to invert X and Y and invert some lid axis.
>>          */
>> -       for (i = X ; i < MAX_AXIS; i++) {
>> -               switch (i) {
>> -               case X:
>> -                       ec_accel_channels[X].scan_index = Y;
>> -               case Y:
>> -                       ec_accel_channels[Y].scan_index = X;
>> -               case Z:
>> -                       ec_accel_channels[Z].scan_index = Z;
>> -               }
>> -               if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
>> -                       state->sign[i] = -1;
>> -               else
>> -                       state->sign[i] = 1;
>> -       }
>> +       ec_accel_channels[X].scan_index = Y;
>> +       ec_accel_channels[Y].scan_index = X;
>> +       ec_accel_channels[Z].scan_index = Z;
>> +
>> +       state->sign[Y] = 1;
>> +
>> +       if (state->sensor_num == MOTIONSENSE_LOC_LID)
>> +               state->sign[X] = state->sign[Z] = -1;
>> +       else
>> +               state->sign[X] = state->sign[Z] = 1;
>> +
>>         indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
>>         indio_dev->dev.parent = &pdev->dev;
>>         indio_dev->info = &cros_ec_accel_legacy_info;
>> --
>> 2.20.1
>>
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe
  2019-02-21  8:03   ` Enric Balletbo i Serra
@ 2019-03-03 16:20     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2019-03-03 16:20 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Kees Cook, Gustavo A. R. Silva, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Benson Leung,
	Guenter Roeck, linux-iio, LKML, Gwendal Grignou

On Thu, 21 Feb 2019 09:03:39 +0100
Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote:

> On 21/2/19 8:00, Kees Cook wrote:
> > On Wed, Feb 20, 2019 at 6:06 PM Gustavo A. R. Silva
> > <gustavo@embeddedor.com> wrote:  
> >>
> >> Refactor some code in order to fix both the technical implementation
> >> and the following warnings:
> >>
> >> drivers/iio/accel/cros_ec_accel_legacy.c: In function ‘cros_ec_accel_legacy_probe’:
> >> drivers/iio/accel/cros_ec_accel_legacy.c:387:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>     ec_accel_channels[X].scan_index = Y;
> >>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:388:3: note: here
> >>    case Y:
> >>    ^~~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:389:36: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>     ec_accel_channels[Y].scan_index = X;
> >>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:390:3: note: here
> >>    case Z:
> >>    ^~~~
> >>
> >> Notice that neither the for loop nor the switch statement is needed.
> >> Also, "state->sign[Y] = 1" should be unconditional.
> >>
> >> This patch is part of the ongoing efforts to enable
> >> -Wimplicit-fallthrough.
> >>
> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>  
> > 
> > Acked-by: Kees Cook <keescook@chromium.org>
> >   
> 
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

This will probably only make it upstream at the 'next' merge window
given the timing.

Jonathan

> 
> Thanks,
>  Enric
> 
> > -Kees
> >   
> >> ---
> >>  drivers/iio/accel/cros_ec_accel_legacy.c | 27 +++++++++++-------------
> >>  1 file changed, 12 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> index 063e89eff791..021f9f5cd3bb 100644
> >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> @@ -353,7 +353,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> >>         struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
> >>         struct iio_dev *indio_dev;
> >>         struct cros_ec_accel_legacy_state *state;
> >> -       int ret, i;
> >> +       int ret;
> >>
> >>         if (!ec || !ec->ec_dev) {
> >>                 dev_warn(&pdev->dev, "No EC device found.\n");
> >> @@ -381,20 +381,17 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> >>          * Present the channel using HTML5 standard:
> >>          * need to invert X and Y and invert some lid axis.
> >>          */
> >> -       for (i = X ; i < MAX_AXIS; i++) {
> >> -               switch (i) {
> >> -               case X:
> >> -                       ec_accel_channels[X].scan_index = Y;
> >> -               case Y:
> >> -                       ec_accel_channels[Y].scan_index = X;
> >> -               case Z:
> >> -                       ec_accel_channels[Z].scan_index = Z;
> >> -               }
> >> -               if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> >> -                       state->sign[i] = -1;
> >> -               else
> >> -                       state->sign[i] = 1;
> >> -       }
> >> +       ec_accel_channels[X].scan_index = Y;
> >> +       ec_accel_channels[Y].scan_index = X;
> >> +       ec_accel_channels[Z].scan_index = Z;
> >> +
> >> +       state->sign[Y] = 1;
> >> +
> >> +       if (state->sensor_num == MOTIONSENSE_LOC_LID)
> >> +               state->sign[X] = state->sign[Z] = -1;
> >> +       else
> >> +               state->sign[X] = state->sign[Z] = 1;
> >> +
> >>         indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
> >>         indio_dev->dev.parent = &pdev->dev;
> >>         indio_dev->info = &cros_ec_accel_legacy_info;
> >> --
> >> 2.20.1
> >>  
> > 
> >   


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-03-03 16:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21  2:06 [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe Gustavo A. R. Silva
2019-02-21  7:00 ` Kees Cook
2019-02-21  8:03   ` Enric Balletbo i Serra
2019-03-03 16:20     ` Jonathan Cameron

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).