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

[AceLan Kao]

Hi Stefan,

Stefan Bader <stefan.bader at canonical.com> 於 2020年5月13日 週三 下午4:35寫道：
>
> On 04.05.20 13:15, 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>
> > ---
>
> Somehow neither the commit message nor the justification in the bug report are
> explaining what benefit (what is fixed) of this change is. Why would we want to
> pick this up?
On cover letter, the impact section it says
"Larger EEPROM devices that use 16-bit addresses couldn't be accessed."
We need this because we have a project that comes with an EEPROM
which uses 16-bit address to access the data, without this commit, we
can read/write data on it.
Do you need me add some more wordings on SRU justification?

>
> If there is a better reason, this is now at least in linux-next:
>
> commit 82f25bd73c0bee4d29df47007a4f7290695b7db7
>
> so it could be a proper cherry-pick.
Cool, I didn't notice this, I'll prepare v3 and cherry pick from linux-next

>
> -Stefan
> >  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 ac9b31c57967..7f924797be58 100644
> > --- a/drivers/base/regmap/regmap-i2c.c
> > +++ b/drivers/base/regmap/regmap-i2c.c
> > @@ -246,6 +246,63 @@ static 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))
> >
>
>