On Tue, Apr 14, 2020 at 11:17:17PM +0200, Linus Walleij wrote:
> The manual for the HSCDTD008A gives us a scaling for the
> three axis as +/- 2.4mT per axis.
>
> When I implement this the biggest axis indicates 0.59 Gauss
> which is a reasonable measurement for the earths magnetic
> which is in the range of 0.25 to 0.65 Gauss on the surface
> according to Wikipedia.
>
> Since the raw read function is now also used for scaling
> we need to break out a function that takes the locks and
> runtime PM so we don't get too hairy goto:s.
>
> Cc: Nick Reitemeyer <nick.reitemeyer@xxxxxx>
> Cc: Stephan Gerhold <stephan@xxxxxxxxxxx>
> Cc: Michał Mirosław <mirq-linux@xxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> This patch is based on top of Nick's patches for the
> HSCDTD008A support.
> ---
> drivers/iio/magnetometer/ak8974.c | 66 +++++++++++++++++++++----------
> 1 file changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index ade4ed8f67d2..effcdd93e650 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -554,46 +554,69 @@ 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]);
You could pass a pointer to int, and avoid later copy in
ak8974_read_raw().
> +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;
[...]
This can be just return -EIO since you've pushed the locks into separate
function.
Can you split the patch into one extracting the code for
ak8974_measure() and second for adding the scale?
Best Regards,
Michał Mirosław
