Re: [PATCH v5 0/3] iio: adc: add support for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong]

On 11/8/26 00:09, Jonathan Cameron wrote:
> On Wed, 4 Nov 2026 09:45:02 +0100
> Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:

<snip>

> > But for the other reports:
> > - [Medium] iio: adc: max34417: shared accumulators clobbered on CAM power/energy read
> >
> > This is a tough one, today if either we read power or energy, we erase the accumulated
> > power+count in the context, the context accumulator is only here to make sure we don't
> > loose data from other channels, like if we had a single HW channel.
> > This simulates this, if we had a single HW channel, at each UPDATE we would get back
> > to 0 in hardware anyway if we did read the power or energy.
>
>
> I'm only half understanding this.  Let me see...
>
> I'd expect userspace to see accumulated values since we enabled the channel
> rather than since last read.  I think that's the final suggestion from
> sashiko.

Oh this changes quite a lot of things.
How do we handle overflow in this case ? we disable the channel and enable again to reset the state ?

> > - [Medium] iio: adc: max34417: missing 1 ms delay between UPDATE commands
> >
> > It's unclear if we need to wait 1ms in CAM mode, I think we should but should we block
> > from the read if we are under the 1ms ?
>
> I'll go with probably?

Ack

> > - [Medium] iio: adc: max34417: scale truncation and mJ unit in max34417_read_raw()
> >
> > Ok this one may be real, should we return Joules or milliJoules ? If so we could
> > fix our precision issue.
>
> Units in the ABI are joules, so if you want to return in millijoules you will need
> scale to incorporate the extra factor of 1000.  Hmm. The comments suggest
> that today it is using milijoules?  If so that needs fixing.
>
> Our units are a bit messy as many years ago I started with what hwmon had before
> concluding they were tricky to keep insistent. So what the base units are tends
> to be dependent on when the channel type was added.  More recent ones are the SI
> unit in most cases.

Ack

> > - [Medium] iio: adc: max34417: missing UPDATE before CAM in max34417_setup_cam_mode()
> >
> > It's highly improbable, but I'll fix it.
> >
> > - [Low] iio: adc: max34417: reuse max34417_reset_smm() in max34417_trigger_smm()
> >
> > I'll check if it's worth it
> >
> > Thanks,
> > Neil

Thanks,
Neil