[PATCH 1/3 v3] iio: magnetometer: ak8974: Correct realbits

[PATCH 1/3 v3] iio: magnetometer: ak8974: Correct realbits

[Linus Walleij]

The original AK8974 has 16 bits of actual value, while the
HSCDTD008A has 15 bits and the AMI305 and AMI306 has 12 bits.
Correct this by providing an extra parameter to the channel
macro and define a separate set of channels for each variant
of the chip. The resolution is the actual resolution of the
internal ADC of the chip.

The values are stored in a S16 in 2's complement so all 16
bits are used for storing (no shifting needed).

The AMI305, AMI306 and HSCDTD008A valid bits are picked from
respective datasheet.

My best educated guess is that AK8974 is also 12 bits. The
AK8973 is an 8 bit and earlier version, and the sibling
drivers AMI305 and AMI306 are 12 bits, so it makes sense
to assume that the AK8974 is also 12 bits.

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->v3:
- New patch in the v3 patch set.
---
 drivers/iio/magnetometer/ak8974.c | 53 +++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index ade4ed8f67d2..f22b40ef5661 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -651,7 +651,7 @@ static const struct iio_chan_spec_ext_info ak8974_ext_info[] = {
 	{ },
 };
 
-#define AK8974_AXIS_CHANNEL(axis, index)				\
+#define AK8974_AXIS_CHANNEL(axis, index, bits)				\
 	{								\
 		.type = IIO_MAGN,					\
 		.modified = 1,						\
@@ -662,16 +662,42 @@ static const struct iio_chan_spec_ext_info ak8974_ext_info[] = {
 		.scan_index = index,					\
 		.scan_type = {						\
 			.sign = 's',					\
-			.realbits = 16,					\
+			.realbits = bits,				\
 			.storagebits = 16,				\
 			.endianness = IIO_LE				\
 		},							\
 	}
 
+/*
+ * We have no datasheet for the AK8974 but we guess that its
+ * ADC is 12 bits.
+ */
 static const struct iio_chan_spec ak8974_channels[] = {
-	AK8974_AXIS_CHANNEL(X, 0),
-	AK8974_AXIS_CHANNEL(Y, 1),
-	AK8974_AXIS_CHANNEL(Z, 2),
+	AK8974_AXIS_CHANNEL(X, 0, 12),
+	AK8974_AXIS_CHANNEL(Y, 1, 12),
+	AK8974_AXIS_CHANNEL(Z, 2, 12),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+/*
+ * The AMI305 and AMI306 have 12 bit ADC resolution according to
+ * datasheets.
+ */
+static const struct iio_chan_spec ami30x_channels[] = {
+	AK8974_AXIS_CHANNEL(X, 0, 12),
+	AK8974_AXIS_CHANNEL(Y, 1, 12),
+	AK8974_AXIS_CHANNEL(Z, 2, 12),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+/*
+ * The HSCDTD008A has 15 bits resolution the way we set it up
+ * in CTRL4.
+ */
+static const struct iio_chan_spec hscdtd008a_channels[] = {
+	AK8974_AXIS_CHANNEL(X, 0, 15),
+	AK8974_AXIS_CHANNEL(Y, 1, 15),
+	AK8974_AXIS_CHANNEL(Z, 2, 15),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
@@ -815,8 +841,21 @@ static int ak8974_probe(struct i2c_client *i2c,
 	pm_runtime_put(&i2c->dev);
 
 	indio_dev->dev.parent = &i2c->dev;
-	indio_dev->channels = ak8974_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ak8974_channels);
+	switch (ak8974->variant) {
+	case AK8974_WHOAMI_VALUE_AMI306:
+	case AK8974_WHOAMI_VALUE_AMI305:
+		indio_dev->channels = ami30x_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ami30x_channels);
+		break;
+	case AK8974_WHOAMI_VALUE_HSCDTD008A:
+		indio_dev->channels = hscdtd008a_channels;
+		indio_dev->num_channels = ARRAY_SIZE(hscdtd008a_channels);
+		break;
+	default:
+		indio_dev->channels = ak8974_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ak8974_channels);
+		break;
+	}
 	indio_dev->info = &ak8974_info;
 	indio_dev->available_scan_masks = ak8974_scan_masks;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-- 
2.21.1

[PATCH 2/3 v3] iio: magnetometer: ak8974: Break out measurement

[Linus Walleij]

This breaks out the measurement code to its own function
so we can handle this without swirling it up with the
big switch() statement inside ak8974_read_raw().

Keep a local s16 helper variable for the signed value
coming out of the measurement before assigning it to the
integer *val. The local variable makes the code easier
to read and the compiler will optimize it if possible.

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 v2->v3:
- Return directly from the raw read function, we
  need no goto:s as we got rid of the lock.
- Change the measurement function to return an int *
  after measurement and just pass *val
  to the function saving a local variable.
- Insert a comment explaining the explicit cast to
  (s16).
- Rename function as ak8974_measure_channel() so the
  name states exactly what is going on.
- Break out as a separate patch.
---
 drivers/iio/magnetometer/ak8974.c | 68 +++++++++++++++++++------------
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index f22b40ef5661..b8dbea119a67 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -554,47 +554,63 @@ static int ak8974_detect(struct ak8974 *ak8974)
 	return 0;
 }
 
+static int ak8974_measure_channel(struct ak8974 *ak8974, unsigned long address,
+				  int *val)
+{
+	__le16 hw_values[3];
+	int ret;
+	s16 outval;
+
+	pm_runtime_get_sync(&ak8974->i2c->dev);
+	mutex_lock(&ak8974->lock);
+
+	/*
+	 * We read all axes and discard all but one, for optimized
+	 * reading, use the triggered buffer.
+	 */
+	ret = ak8974_trigmeas(ak8974);
+	if (ret)
+		goto out_unlock;
+	ret = ak8974_getresult(ak8974, hw_values);
+	if (ret)
+		goto out_unlock;
+	/*
+	 * This explicit cast to (s16) is necessary as the measurement
+	 * is done in 2's complement with positive and negative values.
+	 * The follwing assignment to *val will then convert the signed
+	 * s16 value to a signed int value.
+	 */
+	outval = (s16)le16_to_cpu(hw_values[address]);
+	*val = outval;
+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);
+	int ret;
 
 	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;
+			return -EIO;
 		}
-		ret = ak8974_trigmeas(ak8974);
+		ret = ak8974_measure_channel(ak8974, chan->address, val);
 		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 = IIO_VAL_INT;
+			return ret;
+		return IIO_VAL_INT;
 	}
 
- out_unlock:
-	mutex_unlock(&ak8974->lock);
-	pm_runtime_mark_last_busy(&ak8974->i2c->dev);
-	pm_runtime_put_autosuspend(&ak8974->i2c->dev);
-
-	return ret;
+	return -EINVAL;
 }
 
 static void ak8974_fill_buffer(struct iio_dev *indio_dev)
-- 
2.21.1

Re: [PATCH 2/3 v3] iio: magnetometer: ak8974: Break out measurement

[Andy Shevchenko]

On Fri, Apr 17, 2020 at 2:42 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This breaks out the measurement code to its own function
> so we can handle this without swirling it up with the
> big switch() statement inside ak8974_read_raw().
>
> Keep a local s16 helper variable for the signed value
> coming out of the measurement before assigning it to the
> integer *val. The local variable makes the code easier
> to read and the compiler will optimize it if possible.

> +       /*
> +        * This explicit cast to (s16) is necessary as the measurement
> +        * is done in 2's complement with positive and negative values.
> +        * The follwing assignment to *val will then convert the signed
> +        * s16 value to a signed int value.
> +        */
> +       outval = (s16)le16_to_cpu(hw_values[address]);
> +       *val = outval;

I'm wondering if you may use sign_extend32() here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3 v3] iio: magnetometer: ak8974: Break out measurement

[Michał Mirosław]

On Fri, Apr 17, 2020 at 01:40:19PM +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
> big switch() statement inside ak8974_read_raw().
> 
> Keep a local s16 helper variable for the signed value
> coming out of the measurement before assigning it to the
> integer *val. The local variable makes the code easier
> to read and the compiler will optimize it if possible.
[...]
> +static int ak8974_measure_channel(struct ak8974 *ak8974, unsigned long address,
> +				  int *val)
> +{
> +	__le16 hw_values[3];
> +	int ret;
> +	s16 outval;
[...]
> +	/*
> +	 * This explicit cast to (s16) is necessary as the measurement
> +	 * is done in 2's complement with positive and negative values.
> +	 * The follwing assignment to *val will then convert the signed
> +	 * s16 value to a signed int value.
> +	 */
> +	outval = (s16)le16_to_cpu(hw_values[address]);
> +	*val = outval;

The intermediate 'outval' is not needed. What you describe in the
comment is a normal C integer promotion rule, so I would leave the
comment out, too. IOW, this is equivalent to:

*val = (s16)le16_to_cpu(...);

Otherwise:
Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

[PATCH 3/3 v3] iio: magnetometer: ak8974: Provide scaling

[Linus Walleij]

The manual for the HSCDTD008A gives us a scaling for the
three axis as +/- 2.4mT (24 Gauss) per axis.

The manual for the AMI305 and AMI306 gives us a scaling
for the three axis as +/- 12 Gauss per axis.

Tests with the HSCDTD008A sensor, cat the raw values:
$ cat in_magn_*_raw
raw
45
189
-19

The scaling factor in in_magn_*_scale is 0.001464843,
which gives:
0.065 Gauss
0.277 Gauss
-0.027 Gauss

The earths magnetic field is in the range of 0.25 to 0.65
Gauss on the surface according to Wikipedia, so these
seem like reasonable values.

Again we are guessing that the AK8974 has a 12 bit ADC,
based on the similarity with AMI305 and AMI306.

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 v2->v3:
- Scale the 2.4mT/24Gauss to 15 bits for the HSCDTD008A.
- Scale the 12 Gauss to 12 bits for the AMI305/AMI306
- Use 12 bits for the other variants.
- Return directly in the raw read function.
ChangeLog v1->v2:
- Split out the measurement refactoring.
---
 drivers/iio/magnetometer/ak8974.c | 45 ++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index b8dbea119a67..f8410ac34316 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -608,6 +608,48 @@ static int ak8974_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		switch (ak8974->variant) {
+		case AK8974_WHOAMI_VALUE_AMI306:
+		case AK8974_WHOAMI_VALUE_AMI305:
+			/*
+			 * The datasheet for AMI305 and AMI306, page 6
+			 * specifies the range of the sensor to be
+			 * +/- 12 Gauss.
+			 */
+			*val = 12 * 2;
+			/*
+			 * 12 bits are used
+			 * [ -2048 .. 2047 ] (manual page 20)
+			 * [ 0xf800 .. 0x07ff ]
+			 */
+			*val2 = 4096;
+			return IIO_VAL_FRACTIONAL;
+		case AK8974_WHOAMI_VALUE_HSCDTD008A:
+			/*
+			 * 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.
+			 */
+			*val = 24 * 2;
+			/*
+			 * 15 bits are used (set up in CTRL4)
+			 * [ -16384 .. 16383 ] (manual page 24)
+			 * [ 0xc000 .. 0x3fff ]
+			 */
+			*val2 = 32768;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			/* GUESSING +/- 12 Gauss */
+			*val = 12 * 2;
+			/* GUESSING 12 bits ADC */
+			*val2 = 4096;
+			return IIO_VAL_FRACTIONAL;
+		}
+		break;
+	default:
+		/* Unknown request */
+		break;
 	}
 
 	return -EINVAL;
@@ -672,7 +714,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,					\
-- 
2.21.1

Re: [PATCH 3/3 v3] iio: magnetometer: ak8974: Provide scaling

[Michał Mirosław]

On Fri, Apr 17, 2020 at 01:40:20PM +0200, Linus Walleij wrote:
> The manual for the HSCDTD008A gives us a scaling for the
> three axis as +/- 2.4mT (24 Gauss) per axis.
> 
> The manual for the AMI305 and AMI306 gives us a scaling
> for the three axis as +/- 12 Gauss per axis.
> 
> Tests with the HSCDTD008A sensor, cat the raw values:
> $ cat in_magn_*_raw
> raw
> 45
> 189
> -19
> 
> The scaling factor in in_magn_*_scale is 0.001464843,
> which gives:
> 0.065 Gauss
> 0.277 Gauss
> -0.027 Gauss
> 
> The earths magnetic field is in the range of 0.25 to 0.65
> Gauss on the surface according to Wikipedia, so these
> seem like reasonable values.
> 
> Again we are guessing that the AK8974 has a 12 bit ADC,
> based on the similarity with AMI305 and AMI306.
> 
> 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 v2->v3:
> - Scale the 2.4mT/24Gauss to 15 bits for the HSCDTD008A.
> - Scale the 12 Gauss to 12 bits for the AMI305/AMI306
> - Use 12 bits for the other variants.
> - Return directly in the raw read function.
> ChangeLog v1->v2:
> - Split out the measurement refactoring.
> ---
>  drivers/iio/magnetometer/ak8974.c | 45 ++++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index b8dbea119a67..f8410ac34316 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -608,6 +608,48 @@ static int ak8974_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (ak8974->variant) {
> +		case AK8974_WHOAMI_VALUE_AMI306:
> +		case AK8974_WHOAMI_VALUE_AMI305:
> +			/*
> +			 * The datasheet for AMI305 and AMI306, page 6
> +			 * specifies the range of the sensor to be
> +			 * +/- 12 Gauss.
> +			 */
> +			*val = 12 * 2;
> +			/*
> +			 * 12 bits are used
> +			 * [ -2048 .. 2047 ] (manual page 20)
> +			 * [ 0xf800 .. 0x07ff ]
> +			 */
> +			*val2 = 4096;
> +			return IIO_VAL_FRACTIONAL;

You can leave '* 2' out from both values. And actually, since *val2 is 2^n,
you can return IIO_VAL_FRACTIONAL_LOG2 (with *val2 = 11) and save a division
in the caller.

Best Regards,
Michał Mirosław

Re: [PATCH 1/3 v3] iio: magnetometer: ak8974: Correct realbits

[Michał Mirosław]

On Fri, Apr 17, 2020 at 01:40:18PM +0200, Linus Walleij wrote:
> The original AK8974 has 16 bits of actual value, while the
> HSCDTD008A has 15 bits and the AMI305 and AMI306 has 12 bits.
> Correct this by providing an extra parameter to the channel
> macro and define a separate set of channels for each variant
> of the chip. The resolution is the actual resolution of the
> internal ADC of the chip.
> 
> The values are stored in a S16 in 2's complement so all 16
> bits are used for storing (no shifting needed).
> 
> The AMI305, AMI306 and HSCDTD008A valid bits are picked from
> respective datasheet.
> 
> My best educated guess is that AK8974 is also 12 bits. The
> AK8973 is an 8 bit and earlier version, and the sibling
> drivers AMI305 and AMI306 are 12 bits, so it makes sense
> to assume that the AK8974 is also 12 bits.
[...]
> -#define AK8974_AXIS_CHANNEL(axis, index)				\
> +#define AK8974_AXIS_CHANNEL(axis, index, bits)				\
>  	{								\
>  		.type = IIO_MAGN,					\
>  		.modified = 1,						\
> @@ -662,16 +662,42 @@ static const struct iio_chan_spec_ext_info ak8974_ext_info[] = {
>  		.scan_index = index,					\
>  		.scan_type = {						\
>  			.sign = 's',					\
> -			.realbits = 16,					\
> +			.realbits = bits,				\
>  			.storagebits = 16,				\
>  			.endianness = IIO_LE				\
>  		},							\
>  	}
>  
> +/*
> + * We have no datasheet for the AK8974 but we guess that its
> + * ADC is 12 bits.
> + */
>  static const struct iio_chan_spec ak8974_channels[] = {
> -	AK8974_AXIS_CHANNEL(X, 0),
> -	AK8974_AXIS_CHANNEL(Y, 1),
> -	AK8974_AXIS_CHANNEL(Z, 2),
> +	AK8974_AXIS_CHANNEL(X, 0, 12),
> +	AK8974_AXIS_CHANNEL(Y, 1, 12),
> +	AK8974_AXIS_CHANNEL(Z, 2, 12),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +/*
> + * The AMI305 and AMI306 have 12 bit ADC resolution according to
> + * datasheets.
> + */
> +static const struct iio_chan_spec ami30x_channels[] = {
> +	AK8974_AXIS_CHANNEL(X, 0, 12),
> +	AK8974_AXIS_CHANNEL(Y, 1, 12),
> +	AK8974_AXIS_CHANNEL(Z, 2, 12),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};

Maybe call it channels_12bit[] and then you wouldn't need to make
am exact duplicate for ak8974?

Best Regards,
Michał Mirosław

Re: [PATCH 1/3 v3] iio: magnetometer: ak8974: Correct realbits

[Jonathan Cameron]

On Fri, 17 Apr 2020 16:05:21 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Fri, Apr 17, 2020 at 01:40:18PM +0200, Linus Walleij wrote:
> > The original AK8974 has 16 bits of actual value, while the
> > HSCDTD008A has 15 bits and the AMI305 and AMI306 has 12 bits.
> > Correct this by providing an extra parameter to the channel
> > macro and define a separate set of channels for each variant
> > of the chip. The resolution is the actual resolution of the
> > internal ADC of the chip.
> > 
> > The values are stored in a S16 in 2's complement so all 16
> > bits are used for storing (no shifting needed).
> > 
> > The AMI305, AMI306 and HSCDTD008A valid bits are picked from
> > respective datasheet.
> > 
> > My best educated guess is that AK8974 is also 12 bits. The
> > AK8973 is an 8 bit and earlier version, and the sibling
> > drivers AMI305 and AMI306 are 12 bits, so it makes sense
> > to assume that the AK8974 is also 12 bits.  
> [...]
> > -#define AK8974_AXIS_CHANNEL(axis, index)				\
> > +#define AK8974_AXIS_CHANNEL(axis, index, bits)				\
> >  	{								\
> >  		.type = IIO_MAGN,					\
> >  		.modified = 1,						\
> > @@ -662,16 +662,42 @@ static const struct iio_chan_spec_ext_info ak8974_ext_info[] = {
> >  		.scan_index = index,					\
> >  		.scan_type = {						\
> >  			.sign = 's',					\
> > -			.realbits = 16,					\
> > +			.realbits = bits,				\
> >  			.storagebits = 16,				\
> >  			.endianness = IIO_LE				\
> >  		},							\
> >  	}
> >  
> > +/*
> > + * We have no datasheet for the AK8974 but we guess that its
> > + * ADC is 12 bits.
> > + */
> >  static const struct iio_chan_spec ak8974_channels[] = {
> > -	AK8974_AXIS_CHANNEL(X, 0),
> > -	AK8974_AXIS_CHANNEL(Y, 1),
> > -	AK8974_AXIS_CHANNEL(Z, 2),
> > +	AK8974_AXIS_CHANNEL(X, 0, 12),
> > +	AK8974_AXIS_CHANNEL(Y, 1, 12),
> > +	AK8974_AXIS_CHANNEL(Z, 2, 12),
> > +	IIO_CHAN_SOFT_TIMESTAMP(3),
> > +};
> > +
> > +/*
> > + * The AMI305 and AMI306 have 12 bit ADC resolution according to
> > + * datasheets.
> > + */
> > +static const struct iio_chan_spec ami30x_channels[] = {
> > +	AK8974_AXIS_CHANNEL(X, 0, 12),
> > +	AK8974_AXIS_CHANNEL(Y, 1, 12),
> > +	AK8974_AXIS_CHANNEL(Z, 2, 12),
> > +	IIO_CHAN_SOFT_TIMESTAMP(3),
> > +};  
> 
> Maybe call it channels_12bit[] and then you wouldn't need to make
> am exact duplicate for ak8974?

Agreed they should be combined.  I've not problem with just picking
one device name though on a first come first named basis...

Up to you.

Jonathan



> 
> Best Regards,
> Michał Mirosław