Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 14 Nov 2022 20:18:40 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Mitja =?UTF-8?B?xaBwZXM=?= <mitja@lxnav.com>
Cc:     Lars-Peter Clausen <lars@metafoo.de>,
        Angelo Compagnucci <angelo.compagnucci@gmail.com>,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] iio: adc: mcp3422: allow setting gain and sampling
 per channel
Message-ID: <20221114201840.71a1326c@jic23-huawei>
In-Reply-To: <CACbQKWfzbxS2SKzd3v=h8-3oQw3hRhZJr_fJMaiTKaFwLn-jJg@mail.gmail.com>
References: <20221111112657.1521307-1-mitja@lxnav.com>
        <20221111112657.1521307-3-mitja@lxnav.com>
        <20221112172806.6db090eb@jic23-huawei>
        <CACbQKWfEa64Fv4CmW8BDp2rXw504YyL_s2TWiA_SwH-zCKKvCA@mail.gmail.com>
        <20221113120610.6568f9e6@jic23-huawei>
        <CACbQKWfzbxS2SKzd3v=h8-3oQw3hRhZJr_fJMaiTKaFwLn-jJg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, 13 Nov 2022 14:39:03 +0100
Mitja =C5=A0pes <mitja@lxnav.com> wrote:

> On Sun, Nov 13, 2022 at 12:53 PM Jonathan Cameron <jic23@kernel.org> wrot=
e:
> > > On Sat, Nov 12, 2022 at 6:15 PM Jonathan Cameron <jic23@kernel.org> w=
rote: =20
> > > > Was it possible for these scales to differ before this change? =20
> > > Yes. The difference is that before this change you could only see and=
 set
> > > available scales that were available for specified sampling rate. Now=
 you're
> > > able to set gain and sampling rate via scale. So before the change yo=
u got
> > > these (@240sps):
> > >
> > > 0.001000000 0.000500000 0.000250000 0.000125000
> > >
> > > Now you get the complete set:
> > > /*                 gain x1     gain x2     gain x4     gain x8  */
> > > /* 240 sps    */ 0.001000000 0.000500000 0.000250000 0.000125000
> > > /*  60 sps    */ 0.000250000 0.000125000 0.000062500 0.000031250
> > > /*  15 sps    */ 0.000062500 0.000031250 0.000015625 0.000007812
> > > /*   3.75 sps */ 0.000015625 0.000007812 0.000003906 0.000001953 =20
> >
> > Ok. That doesn't work as a standard interface because userspace code wa=
nts to pick say
> > 0.00062500 which appears twice. =20
> I don't know how I missed that. It's clear to me now that this patch is w=
rong.
>=20
>=20
> > > > If not, then why was the previous patch a fix rather than simply a =
precursor
> > > > to this change (where it now matters). =20
> > > I wanted to separate a bug fix from improvements, if these were rejec=
ted for
> > > for some reason. =20
> >
> > Is it a bug fix?  The way I read it is that, before this patch there is=
 only
> > one scale that is applied to all channels.  As such, the current value =
=3D=3D the
> > value set and the code works as expected.
> > So the previous patch is only necessary once this one is applied.  Henc=
e no
> > bug, just a rework that is useful to enabling this feature. =20
> I'll post the previous snippet here and write the comments inline:
> ----
> @@ -164,8 +164,9 @@ static int mcp3422_read_raw(struct iio_dev *iio,
>   struct mcp3422 *adc =3D iio_priv(iio);
>   int err;
>=20
> + u8 req_channel =3D channel->channel;
>   u8 sample_rate =3D MCP3422_SAMPLE_RATE(adc->config);
> - u8 pga =3D MCP3422_PGA(adc->config);  /* <- this uses the "current" con=
fig
>       which changes depending on the last read channel */
> + u8 pga =3D adc->pga[req_channel];          /* this now returns the PGA =
for the
>       selected channel */
>=20
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
> ----
> I hope this clarifies the bugfix.

Ah I see. The bit that threw me off was the title of this patch.
"allow setting gain ... per channel" which made me think that before this p=
atch
there was one gain for all channels.  I was too lazy to actually check and =
discover
that it has always been per channel on the write side of things.

Jonathan


>=20
>=20
> Thanks for in depth look at this and sorry for wasting your time with this
> flawed patch.
>=20
> Kind regards,
> Mitja