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

[Neil Armstrong <neil.armstrong@linaro.org>]

On 11/8/26 00:09, Jonathan Cameron wrote:
> On Wed, 4 Nov 2026 09:45:02 +0100
> Neil Armstrong <neil.armstrong@linaro.org> 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