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: <d5725c87-ff96-4a25-995a-d4c3cbcc13a9@alliedtelesis.co.nz>
Date: Thu, 28 Aug 2025 21:22:05 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Guenter Roeck <linux@...ck-us.net>, "jdelvare@...e.com"
	<jdelvare@...e.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>
CC: "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] hwmon: (ina238) Add support for INA780


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

> 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 :-( ] 

No problem. Same thing for me.