On Fri, 17 Apr 2020 16:05:21 +0200
Michał Mirosław <mirq-linux@xxxxxxxxxxxx> 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
