Re: [PATCH v4 2/3] hwmon: ltc4283: Add support for the LTC4283 Swap Controller

["Nuno Sá" <noname.nuno@gmail.com>]

On Fri, 2025-12-12 at 09:07 -0800, Guenter Roeck wrote:
> On 12/12/25 08:50, Nuno Sá wrote:
> > On Thu, 2025-12-11 at 09:56 -0800, Guenter Roeck wrote:
> > > On 12/4/25 08:15, Nuno Sá via B4 Relay wrote:
> > > > From: Nuno Sá <nuno.sa@analog.com>
> > > > 
> > > > Support the LTC4283 How Swap Controller. The device features programmable
> > > > current limit with foldback and independently adjustable inrush current to
> > > > optimize the MOSFET safe operating area (SOA). The SOA timer limits MOSFET
> > > > temperature rise for reliable protection against overstresses.
> > > > 
> > > > An I2C interface and onboard ADC allow monitoring of board current,
> > > > voltage, power, energy, and fault status.
> > > > 
> > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > > > ---
> > > ...
> > > > diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c
> > > > new file mode 100644
> > > > index 000000000000..d79432678b84
> > > > --- /dev/null
> > > > +++ b/drivers/hwmon/ltc4283.c
> > > ...
> > > > +
> > > > +static int ltc4283_read_voltage_word(const struct ltc4283_hwmon *st,
> > > > +				     u32 reg, u32 fs, long *val)
> > > > +{
> > > > +	__be16 in;
> > > > +	int ret;
> > > > +
> > > > +	ret = regmap_bulk_read(st->map, reg, &in, sizeof(in));
> > > 
> > > I had a look into the regmap code. In its current implementation,
> > > that will work as long as
> > > 1) regmap is configured to not cache anything
> > > 2) the I2C controller supports I2C_FUNC_SMBUS_I2C_BLOCK
> > > 
> > 
> > Possibly dumb question... what would be the issue about doing multiple
> > regmap reads if we can't do the bulk? That would be my naive approach in
> > the driver if I'm not going to use the bulk API.
> > 
> 
> You mean on the same address ? I don't have the chip to check, but I am quite
> sure that it would return the 1st byte again.
> 
> FWIW, in the lm92 driver I used a regmap_bus to solve pretty much the same
> problem.
> 
> 

Ok, I know understand the issue. We do have real word registers (though the energy
spans 6 registers IIRC) in which case the regmap bulk fallback would fail. So I do
agree with you that relying on regmap implementation details is questionable so
I'll add my own bus.

But the above made me start thinking about how we should handle
i2c_check_functionality()? For this device, I2C_FUNC_SMBUS_WORD_DATA and
I2C_FUNC_SMBUS_BYTE_DATA (can we have word without byte??) are the minimum the
controller needs to support. So I could add the below:

if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
				    | I2C_FUNC_SMBUS_WORD_DATA))
		return -ENODEV;

But can't we have I2C_FUNC_I2C (or even I2C_FUNC_SMBUS_I2C_BLOCK) without
supporting SMBUS?

I mean, I can so something similar to what's being done in regmap-i2c but maybe
that's too much? I'm leaning in that direction though...


This made me wonder, again, about ltc4282 but in there word registers are in
fact reg and reg + 1.

- Nuno Sá