[PATCH] regmap-i2c: add 16 bits register width support

[PATCH] regmap-i2c: add 16 bits register width support

[AceLan Kao]

This allows to access data with 16 bits register width
via i2c smbus block functions.

The implementation is inspired by below commit
https://patchwork.ozlabs.org/patch/545292/

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/base/regmap/regmap-i2c.c | 61 ++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 008f8da69d97..62b95a9212ae 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -246,6 +246,63 @@ static const struct regmap_bus regmap_i2c_smbus_i2c_block = {
 	.max_raw_write = I2C_SMBUS_BLOCK_MAX,
 };
 
+static int regmap_i2c_smbus_i2c_write_reg16(void *context, const void *data,
+				      size_t count)
+{
+	struct device *dev = context;
+	struct i2c_client *i2c = to_i2c_client(dev);
+
+	if (count < 2)
+		return -EINVAL;
+
+	count--;
+	return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
+					      (u8 *)data + 1);
+}
+
+static int regmap_i2c_smbus_i2c_read_reg16(void *context, const void *reg,
+				     size_t reg_size, void *val,
+				     size_t val_size)
+{
+	struct device *dev = context;
+	struct i2c_client *i2c = to_i2c_client(dev);
+	int ret, count, len = val_size;
+
+	if (reg_size != 2)
+		return -EINVAL;
+
+	ret = i2c_smbus_write_byte_data(i2c, ((u16 *)reg)[0] & 0xff,
+					((u16 *)reg)[0] >> 8);
+	if (ret < 0)
+		return ret;
+
+	count = 0;
+	do {
+		/* Current Address Read */
+		ret = i2c_smbus_read_byte(i2c);
+		if (ret < 0)
+			break;
+
+		*((u8 *)val++) = ret;
+		count++;
+		len--;
+	} while (len > 0);
+
+	if (count == val_size)
+		return 0;
+	else if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static const struct regmap_bus regmap_i2c_smbus_i2c_block_reg16 = {
+	.write = regmap_i2c_smbus_i2c_write_reg16,
+	.read = regmap_i2c_smbus_i2c_read_reg16,
+	.max_raw_read = I2C_SMBUS_BLOCK_MAX,
+	.max_raw_write = I2C_SMBUS_BLOCK_MAX,
+};
+
 static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
 					const struct regmap_config *config)
 {
@@ -255,6 +312,10 @@ static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
 		 i2c_check_functionality(i2c->adapter,
 					 I2C_FUNC_SMBUS_I2C_BLOCK))
 		return &regmap_i2c_smbus_i2c_block;
+	else if (config->val_bits == 8 && config->reg_bits == 16 &&
+		i2c_check_functionality(i2c->adapter,
+					I2C_FUNC_SMBUS_I2C_BLOCK))
+		return &regmap_i2c_smbus_i2c_block_reg16;
 	else if (config->val_bits == 16 && config->reg_bits == 8 &&
 		 i2c_check_functionality(i2c->adapter,
 					 I2C_FUNC_SMBUS_WORD_DATA))
-- 
2.25.1

Re: [PATCH] regmap-i2c: add 16 bits register width support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On Thu, Apr 23, 2020 at 04:58:57PM +0800, AceLan Kao wrote:
> This allows to access data with 16 bits register width
> via i2c smbus block functions.

> The implementation is inspired by below commit
> https://patchwork.ozlabs.org/patch/545292/

Do you actually have a system that needs this or is it just being
implemented for completeness?  The patch you link to mentions that there
are correctness issues with this implementation.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

> @@ -255,6 +312,10 @@ static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
>  		 i2c_check_functionality(i2c->adapter,
>  					 I2C_FUNC_SMBUS_I2C_BLOCK))
>  		return &regmap_i2c_smbus_i2c_block;
> +	else if (config->val_bits == 8 && config->reg_bits == 16 &&
> +		i2c_check_functionality(i2c->adapter,
> +					I2C_FUNC_SMBUS_I2C_BLOCK))
> +		return &regmap_i2c_smbus_i2c_block_reg16;

OTOH we'll only use it if the device wouldn't otherwise work so I'm not
sure that it's any worse than just hoping the bus is uncontested,
hopefully system designers have taken this into account when building
systems without real I2C controllers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap-i2c: add 16 bits register width support

[AceLan Kao]

Mark Brown <broonie@kernel.org> 於 2020年4月23日 週四 下午10:45寫道：
>
> On Thu, Apr 23, 2020 at 04:58:57PM +0800, AceLan Kao wrote:
> > This allows to access data with 16 bits register width
> > via i2c smbus block functions.
>
> > The implementation is inspired by below commit
> > https://patchwork.ozlabs.org/patch/545292/
>
> Do you actually have a system that needs this or is it just being
> implemented for completeness?  The patch you link to mentions that there
> are correctness issues with this implementation.
Yes, I'm working on an Eurotech's new platform which comes with an
EEPROM requires that patch
https://www.eurotech.com/en/news/cpu-162-24-a-new-rugged-com-express-basic-type-6
I'd like to upstream the commit, so that we don't have to maintain the
commit in our kernel,
so I migrate the code to the latest upstream driver.

> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.
I'll submit v2 soon and add more description.

> > @@ -255,6 +312,10 @@ static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
> >                i2c_check_functionality(i2c->adapter,
> >                                        I2C_FUNC_SMBUS_I2C_BLOCK))
> >               return &regmap_i2c_smbus_i2c_block;
> > +     else if (config->val_bits == 8 && config->reg_bits == 16 &&
> > +             i2c_check_functionality(i2c->adapter,
> > +                                     I2C_FUNC_SMBUS_I2C_BLOCK))
> > +             return &regmap_i2c_smbus_i2c_block_reg16;
>
> OTOH we'll only use it if the device wouldn't otherwise work so I'm not
> sure that it's any worse than just hoping the bus is uncontested,
> hopefully system designers have taken this into account when building
> systems without real I2C controllers.
I'm not an expert in this field, please let me know if there is any
better way to archive this.

Re: [PATCH] regmap-i2c: add 16 bits register width support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Fri, Apr 24, 2020 at 08:24:53PM +0800, AceLan Kao wrote:

> I'm not an expert in this field, please let me know if there is any
> better way to archive this.

I think you're limited by the decisions of the hardware people here
sadly and there are no really good options.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]