Re: [PATCH v5 0/2] Add support for Texas Instruments INA4230 power monitor

[Guenter Roeck]

On 4/1/26 07:03, Rob Herring wrote:
> On Tue, Mar 31, 2026 at 11:46 AM Alexey Charkov <alchark@xxxxxxxxxxx> wrote:
> > On Tue, Mar 31, 2026 at 8:10 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > > On 3/31/26 08:52, Rob Herring wrote:
> > > > On Mon, Mar 30, 2026 at 09:07:32AM -0700, Guenter Roeck wrote:
> > > > > On 3/30/26 08:14, Alexey Charkov wrote:
> > > > > > TI INA4230 is a 4-channel power monitor with I2C interface, similar in
> > > > > > operation to INA3221 (3-channel) and INA219 (single-channel) but with
> > > > > > a different register layout, different alerting mechanism and slightly
> > > > > > different support for directly reading calculated current/power/energy
> > > > > > values (pre-multiplied by the device itself and needing only to be scaled
> > > > > > by the driver depending on its selected LSB unit values).
> > > > > >
> > > > > > In this initial implementation, the driver supports reading voltage,
> > > > > > current, power and energy values, but does not yet support alerts, which
> > > > > > can be added separately if needed. Also the overflows during hardware
> > > > > > calculations are not yet handled, nor is the support for the device's
> > > > > > internal 32-bit energy counter reset.
> > > > > >
> > > > > > An example device tree using this binding and driver is available at [1]
> > > > > > (not currently upstreamed, as the device in question is in engineering
> > > > > > phase and not yet publicly available)
> > > > > >
> > > > > > [1] https://github.com/flipperdevices/flipper-linux-kernel/blob/flipper-devel/arch/arm64/boot/dts/rockchip/rk3576-flipper-one-rev-f0b0c1.dts
> > > > > >
> > > > > > Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
> > > > > > ---
> > > > > > Changes in v5:
> > > > > > - Reworded per-channel subnodes description in the binding for clarity (Sashiko)
> > > > > > - NB: Sashiko's suggestion to allow interrupts in the binding sounds premature,
> > > > > >      as the alerts mechanism is not implemented yet and there are no known users
> > > > > >      to test it. If anyone has hardware with the alert pins wired to an interrupt
> > > > > >      line - please shout and we can test/extend it together
> > > > >
> > > > > The bindings are supposed to be complete, even if not implemented, so I am not sure
> > > > > if the DT maintainers will agree here. We'll see.
> > > >
> > > > Given ti,alert-polarity-active-high is added seems like the interrupt
> > > > should be too. And the interrupt can specify the polarity, so is that
> > > > property really needed? There's alway the possibility that you have some
> > > > inverter on the board too and the interrupt polarity is not enough, but
> > > > solve that problem when it actually exists.
> > >
> > > The alert pin can be attached to a board interrupt, or (more likely) it can
> > > be attached to the I2C controller's alert pin. In the latter case there is
> > > no interrupt property.
> >
> > Alright, I will add the interrupt property and keep the dedicated flag
> > for alert polarity.
> >
> > Following the logic of binding completeness, should I add a flag for
> > the single-shot mode too, even though I dropped that functionality
> > from the driver in one of the prior iterations?
>
> I don't remember what that was exactly, but that sounds like a user
> selection which would be some sysfs or other runtime control rather
> than in DT. Unless the h/w design dictates what mode should be used.

I agree. I can not imagine that to be a hardware design constraint.

Thanks,
Guenter