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