| Message ID | 20200416140917.8087-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2,v2] iio: magnetometer: ak8974: Break out measurement | expand |
On Thu, Apr 16, 2020 at 04:09:16PM +0200, Linus Walleij wrote: > This breaks out the measurement code to its own function > so we can handle this without swirling it up with the > bis switch() statement inside ak8974_read_raw(). > > Use an intermediary s16* variable since we read s16 but > the external API required an int* so this way we get > explicit casting. > > Cc: Nick Reitemeyer <nick.reitemeyer@web.de> > Cc: Stephan Gerhold <stephan@gerhold.net> > Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Break out as a separate patch. > --- > drivers/iio/magnetometer/ak8974.c | 51 +++++++++++++++++++------------ > 1 file changed, 31 insertions(+), 20 deletions(-) > > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c > index ade4ed8f67d2..5361647b9054 100644 > --- a/drivers/iio/magnetometer/ak8974.c > +++ b/drivers/iio/magnetometer/ak8974.c > @@ -554,46 +554,57 @@ static int ak8974_detect(struct ak8974 *ak8974) > return 0; > } > > +static int ak8974_measure(struct ak8974 *ak8974, unsigned long address, s16 *val) > +{ > + __le16 hw_values[3]; > + int ret; > + > + pm_runtime_get_sync(&ak8974->i2c->dev); > + mutex_lock(&ak8974->lock); > + > + ret = ak8974_trigmeas(ak8974); > + if (ret) > + goto out_unlock; > + ret = ak8974_getresult(ak8974, hw_values); > + if (ret) > + goto out_unlock; > + *val = (s16)le16_to_cpu(hw_values[address]); This will still work if the 'val' argument was 'int *'. And it would make 'outval' in the caller redundant. > +out_unlock: > + mutex_unlock(&ak8974->lock); > + pm_runtime_mark_last_busy(&ak8974->i2c->dev); > + pm_runtime_put_autosuspend(&ak8974->i2c->dev); > + > + return ret; > +} > + > static int ak8974_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, > long mask) > { > struct ak8974 *ak8974 = iio_priv(indio_dev); > - __le16 hw_values[3]; > int ret = -EINVAL; > - > - pm_runtime_get_sync(&ak8974->i2c->dev); > - mutex_lock(&ak8974->lock); > + s16 outval; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > if (chan->address > 2) { > dev_err(&ak8974->i2c->dev, "faulty channel address\n"); > ret = -EIO; > - goto out_unlock; > + goto out_err_read; You could get rid of the gotos to shorten and clear up the code. > } > - ret = ak8974_trigmeas(ak8974); > - if (ret) > - goto out_unlock; > - ret = ak8974_getresult(ak8974, hw_values); > - if (ret) > - goto out_unlock; > - > /* > * We read all axes and discard all but one, for optimized > * reading, use the triggered buffer. > */ > - *val = (s16)le16_to_cpu(hw_values[chan->address]); > - > + ret = ak8974_measure(ak8974, chan->address, &outval); > + if (ret) > + goto out_err_read; > + *val = outval; > ret = IIO_VAL_INT; > + break; > } > - > - out_unlock: > - mutex_unlock(&ak8974->lock); > - pm_runtime_mark_last_busy(&ak8974->i2c->dev); > - pm_runtime_put_autosuspend(&ak8974->i2c->dev); > - > +out_err_read: > return ret; > } [...] Best Regards, Michał Mirosław
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c index ade4ed8f67d2..5361647b9054 100644 --- a/drivers/iio/magnetometer/ak8974.c +++ b/drivers/iio/magnetometer/ak8974.c @@ -554,46 +554,57 @@ static int ak8974_detect(struct ak8974 *ak8974) return 0; } +static int ak8974_measure(struct ak8974 *ak8974, unsigned long address, s16 *val) +{ + __le16 hw_values[3]; + int ret; + + pm_runtime_get_sync(&ak8974->i2c->dev); + mutex_lock(&ak8974->lock); + + ret = ak8974_trigmeas(ak8974); + if (ret) + goto out_unlock; + ret = ak8974_getresult(ak8974, hw_values); + if (ret) + goto out_unlock; + *val = (s16)le16_to_cpu(hw_values[address]); +out_unlock: + mutex_unlock(&ak8974->lock); + pm_runtime_mark_last_busy(&ak8974->i2c->dev); + pm_runtime_put_autosuspend(&ak8974->i2c->dev); + + return ret; +} + static int ak8974_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) { struct ak8974 *ak8974 = iio_priv(indio_dev); - __le16 hw_values[3]; int ret = -EINVAL; - - pm_runtime_get_sync(&ak8974->i2c->dev); - mutex_lock(&ak8974->lock); + s16 outval; switch (mask) { case IIO_CHAN_INFO_RAW: if (chan->address > 2) { dev_err(&ak8974->i2c->dev, "faulty channel address\n"); ret = -EIO; - goto out_unlock; + goto out_err_read; } - ret = ak8974_trigmeas(ak8974); - if (ret) - goto out_unlock; - ret = ak8974_getresult(ak8974, hw_values); - if (ret) - goto out_unlock; - /* * We read all axes and discard all but one, for optimized * reading, use the triggered buffer. */ - *val = (s16)le16_to_cpu(hw_values[chan->address]); - + ret = ak8974_measure(ak8974, chan->address, &outval); + if (ret) + goto out_err_read; + *val = outval; ret = IIO_VAL_INT; + break; } - - out_unlock: - mutex_unlock(&ak8974->lock); - pm_runtime_mark_last_busy(&ak8974->i2c->dev); - pm_runtime_put_autosuspend(&ak8974->i2c->dev); - +out_err_read: return ret; }
This breaks out the measurement code to its own function so we can handle this without swirling it up with the bis switch() statement inside ak8974_read_raw(). Use an intermediary s16* variable since we read s16 but the external API required an int* so this way we get explicit casting. Cc: Nick Reitemeyer <nick.reitemeyer@web.de> Cc: Stephan Gerhold <stephan@gerhold.net> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Break out as a separate patch. --- drivers/iio/magnetometer/ak8974.c | 51 +++++++++++++++++++------------ 1 file changed, 31 insertions(+), 20 deletions(-)