diff mbox series

iio: magnetometer: ak8974: Provide scaling

Message ID 20200414211717.11472-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show
Series iio: magnetometer: ak8974: Provide scaling | expand

Commit Message

Linus Walleij April 14, 2020, 9:17 p.m. UTC
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@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>
---
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(-)

Comments

Michał Mirosław April 15, 2020, 3:58 p.m. UTC | #1
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@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>
> ---
> 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
diff mbox series

Patch

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]);
+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;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * The datasheet for HSCDTF008A, page 3 specifies the
+		 * range of the sensor as +/- 2.4 mT per axis, which corresponds
+		 * to +/- 2400 uT = +/- 24 Gauss. So 0x7fff is 24 Gauss and
+		 * 0xffff is -24 Gauss. To account for the one missing value if
+		 * we multiply by 1/S16_MAX, instead multiply with 2/U16_MAX.
+		 */
+		*val = 24 * 2;
+		*val2 = U16_MAX;
+		ret = IIO_VAL_FRACTIONAL;
+		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;
 }
 
@@ -656,7 +679,8 @@  static const struct iio_chan_spec_ext_info ak8974_ext_info[] = {
 		.type = IIO_MAGN,					\
 		.modified = 1,						\
 		.channel2 = IIO_MOD_##axis,				\
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			BIT(IIO_CHAN_INFO_SCALE),			\
 		.ext_info = ak8974_ext_info,				\
 		.address = index,					\
 		.scan_index = index,					\