On Sun, Oct 27, 2024 at 11:42:28AM +0000, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 19:17:05 +0200
> Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx> wrote:
>
> > When during a measurement two channels are enabled, two measurements are
> > done that are reported sequencially in the DATA register. As the code
> > triggered by reading one of the sysfs properties expects that only one
> > channel is enabled it only reads the first data set which might or might
> > not belong to the intended channel.
> >
> > To prevent this situation disable all channels during probe. This fixes
> > a problem in practise because the reset default for channel 0 is
> > enabled. So all measurements before the first measurement on channel 0
> > (which disables channel 0 at the end) might report wrong values.
> >
> > Fixes: 7b8d045e497a ("iio: adc: ad7124: allow more than 8 channels")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> Makes sense in general, but one comment inline.
>
> > ---
> > drivers/iio/adc/ad7124.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> > index a5d91933f505..912ba6592560 100644
> > --- a/drivers/iio/adc/ad7124.c
> > +++ b/drivers/iio/adc/ad7124.c
> > @@ -917,6 +917,9 @@ static int ad7124_setup(struct ad7124_state *st)
> > * set all channels to this default value.
> > */
> > ad7124_set_channel_odr(st, i, 10);
> > +
> > + /* Disable all channels to prevent unintended conversions. */
> > + ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, 0x0001);
> Why 1? Build that default up from the register definitions rather than a magic
> constant.
I picked 0x0001 because that's the documented reset default values for
the channels > 0. But agreed. Will fix in a v2.
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
