| Message ID | 20190221020633.GA4195@embeddedor (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe | expand |
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 >
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 >> > >
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 > >> > > > >
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;
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(-)