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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 4 Nov 2026 09:45:02 +0100
Neil Armstrong <neil.armstrong@linaro.org> wrote:

> Hi,
> 
> On 11/4/26 03:17, Jonathan Cameron wrote:
> > On Mon, 02 Nov 2026 15:49:03 +0100
> > Neil Armstrong <neil.armstrong@linaro.org> wrote:
> >   
> >> The MAX34417 is a specialized current and voltage monitor used to
> >> determine power consumption of portable systems. The driver supports
> >> getting the channels voltage, average power and accumulated average
> >> power and energy over an I2C/SMBUS serial interface.  
> > https://sashiko.dev/#/patchset/20261102-topic-sm8x50-iio-max34417-adc-v5-0-0ffa5e5f2d5c%40linaro.org
> > 
> > Sashiko had views and the dt one at least checks out.  
> 
> I'll fix the bindings one.
> 

These are a bit tricky to interpret without context.

> 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. 

> 
> - [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?

> 
> - [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.


> 
> - [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