ACK: [PATCH v2 1/1][SRU][Focal] regmap-i2c: add 16-bit width registers support

[Kamal Mostafa]

This patch (for focal) looks okay to me.

Acked-by: Kamal Mostafa <kamal at canonical.com>

 -Kamal

On Thu, May 14, 2020 at 06:39:56PM +0800, AceLan Kao wrote:
> BugLink: https://bugs.launchpad.net/bugs/1876699
> 
> This allows to access data with 16-bit width of registers
> via i2c SMBus block functions.
> 
> The multi-command sequence of the reading function is not safe
> and may read the wrong data from other address if other commands
> are sent in-between the SMBus commands in the read function.
> 
> Read performance:
>    32768 bytes (33 kB, 32 KiB) copied, 11.4869 s, 2.9 kB/s
> Write performance(with 1-byte page):
>    32768 bytes (33 kB, 32 KiB) copied, 129.591 s, 0.3 kB/s
> 
> The implementation is inspired by below commit
> https://patchwork.ozlabs.org/patch/545292/
> 
> v2: add more descriptions about the issue that maybe introduced
>     by this commit
> 
> Signed-off-by: AceLan Kao <acelan.kao at canonical.com>
> Link: https://lore.kernel.org/r/20200424123358.144850-1-acelan.kao@canonical.com
> Signed-off-by: Mark Brown <broonie at kernel.org>
> (cherry picked from commit 82f25bd73c0bee4d29df47007a4f7290695b7db7
> linux-next)
> Signed-off-by: AceLan Kao <acelan.kao at 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
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team