| Message ID | 20200417114020.31291-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3,v3] iio: magnetometer: ak8974: Correct realbits | expand |
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
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
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;
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(-)