Re: [PATCH v2 2/2] hwmon: (ina238) Add support for INA780

[Guenter Roeck <linux@roeck-us.net>]

On 8/28/25 14:22, Chris Packham wrote:
> 
> On 29/08/2025 00:09, Guenter Roeck wrote:
>> On 8/7/25 20:05, Chris Packham wrote:
>>> Add support for the TI INA780 Digital Power Monitor. The INA780 uses
>>> EZShunt(tm) technology, which means there are fixed LSB conversions for
>>> a number of fields rather than needing to be calibrated.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>
>> Your patch does not apply, and I can't figure out its baseline. Please
>> reparent on top of the current mainline and resubmit.
> Sure no problem. The ina238 changes were done on top of my initial
> ina780 stuff so the sha1 recorded in the patch will be a local sha1 that
> you don't have. I'll clean things up on top of master without any local
> junk.
>>
>> To simplify review, the patch should be split into preparation patches
>> (such as adding .has_shunt and .temp_max options), followed by the actual
>> added chip support.
> Sure.
>>
>> Other (not a complete review):
>>
>> I don't see the value of adding INA780_COL and INA780_CUL defines;
>> those are really the same as the shunt voltage limits. Actually,
>> the current limits _are_ available for existing chips, only they
>> are expressed as voltage limits on the shunt voltages.
> 
> My main motivation was trying to match the terms used in the INA780
> datasheet. INA780 uses COL/CUL, INA238 uses SOVL/SUVL. I can kind of

Yeah, only those change all the time. Just try to match register names
(or pin names, for that matter) for the chips supported by the lm90 driver.
I'd rather just add a note explaining the differences in cases like this one,
where it isn't entirely obvious.

> squint and see how they are similar the INA238 is just more complicated
> because of the external shunt. I did kind of think it must be possible
> to express the INA780 behaviour with some fixed values but my math
> skills failed me.
> 

That is why I ordered those evaluation boards. Forget the math, just look
at the registers. Fortunately the TI evaluation boards are not that expensive.

Guenter