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

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

On 8/28/25 05: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.
> 
> 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.
> 
> 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. For the ina_2xx
> driver I was able to resolve that quite easily; we should do the same
> for the ina238 driver. Maybe I have an evaluation board somewhere;
> I'll need to check.
> 
> [ Sorry for being so late with this; I am being swamped at work :-( ]
> 

Follow-up: I ordered evaluation boards for INA228, INA237, INA238, and INA780A.
I'll want to see support added for current limits on all chips, using a similar
approach as the one in the ina2xx driver. After all, the shunt voltage limits
are really current limits in disguise. With that and appropriate chip specific
parameters the differences between the chips should become relatively minor.

This should also simplify adding support for INA700 which seems similar to INA780A.

Guenter