Re: [PATCH v4 2/2] power: supply: add support for MAX1720x standalone fuel gauge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Jun 19, 2024 at 08:14:07AM GMT, Dimitri Fedrau wrote:
> Am Wed, Jun 19, 2024 at 01:19:32AM +0200 schrieb Sebastian Reichel:
> > On Mon, Jun 17, 2024 at 08:45:04PM GMT, Dimitri Fedrau wrote:
> > > The MAX17201 monitors a single cell pack. The MAX17205 monitors and
> > > balances a 2S or 3S pack or monitors a multiple-series cell pack. Both
> > > devices use a I2C interface.
> > > 
> > > Signed-off-by: Dimitri Fedrau <dima.fedrau@xxxxxxxxx>
> > > ---
> > 
> > [...]
> > 
> > > +static int max1720x_read_word_data_nvmem(struct i2c_client *ancillary, u8 addr)
> > > +{
> > > +	u8 rx[2];
> > > +	struct i2c_msg msgs[] = {
> > > +		{
> > > +			.addr = ancillary->addr,
> > > +			.flags = 0,
> > > +			.len = 1,
> > > +			.buf = &addr,
> > > +		},
> > > +		{
> > > +			.addr = ancillary->addr,
> > > +			.flags = I2C_M_RD,
> > > +			.len = 2,
> > > +			.buf = &rx[0],
> > > +		},
> > > +	};
> > > +	int ret;
> > > +
> > > +	ret = i2c_transfer(ancillary->adapter, msgs, ARRAY_SIZE(msgs));
> > > +	if (ret != ARRAY_SIZE(msgs))
> > > +		return ret < 0 ? ret : -EIO;
> > > +
> > > +	return get_unaligned_le16(&rx[0]);
> > > +}
> > 
> > Have you tried using i2c_smbus_read_word_data(ancillary, addr)
> > instead of the above?
> >
> Yes, I did and it worked. Used it in V3 and changed it due to review
> comments by Thomas which make sense to me. I don't have any preference on
> this. Should I change it back ?

Using the smbus function makes the driver work with a pure SMBus
adapter and keeps it working with I2C adapters through the SMBus
"emulation". By using the open coded version we have more code and
the driver does not work with pure SMBus adapters.

So I see no reason why the open coded version should be used. Note,
that regmap also uses i2c_smbus_read_word_data() internally for your
configuration of the non-ancillary device.

Greetings,

-- Sebastian

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux