On Sun, Nov 13, 2022 at 12:53 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > On Sat, Nov 12, 2022 at 6:15 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > > Was it possible for these scales to differ before this change?
> > 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 you 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
>
> Ok. That doesn't work as a standard interface because userspace code wants to pick say
> 0.00062500 which appears twice.
I don't know how I missed that. It's clear to me now that this patch is wrong.
> > > If not, then why was the previous patch a fix rather than simply a precursor
> > > to this change (where it now matters).
> > I wanted to separate a bug fix from improvements, if these were rejected for
> > for some reason.
>
> 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 == the
> value set and the code works as expected.
> So the previous patch is only necessary once this one is applied. Hence no
> bug, just a rework that is useful to enabling this feature.
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 = iio_priv(iio);
int err;
+ u8 req_channel = channel->channel;
u8 sample_rate = MCP3422_SAMPLE_RATE(adc->config);
- u8 pga = MCP3422_PGA(adc->config); /* <- this uses the "current" config
which changes depending on the last read channel */
+ u8 pga = adc->pga[req_channel]; /* this now returns the PGA for the
selected channel */
switch (mask) {
case IIO_CHAN_INFO_RAW:
----
I hope this clarifies the bugfix.
Thanks for in depth look at this and sorry for wasting your time with this
flawed patch.
Kind regards,
Mitja
