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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c280ac26-3ba4-4e00-87a3-42d6584e80e7@roeck-us.net>
Date: Thu, 28 Aug 2025 13:28:46 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>, jdelvare@...e.com,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org
Cc: linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Christian Kahr <christian.kahr@....at>
Subject: Re: [PATCH v2 2/2] hwmon: (ina238) Add support for INA780

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