Subject: [RFC PATCH v2] regmap: smbus: add support for regmap over SMBus

From: [email-masked] (Boris BREZILLON)
Date: Tue, 15 Apr 2014 09:36:13 +0200

On 14/04/2014 23:04, Mark Brown wrote:
> On Mon, Apr 14, 2014 at 03:08:05PM +0200, Boris BREZILLON wrote:
>> SMBus is a subset of the I2C protocol, oftenly used to access
registers on
>> external devices.
> 
> This is basically fine.  However...
> 
>> +    switch (ctx->transfer_type) { +    case
>> REGMAP_SMBUS_BYTE_TRANSFER: +        while (count > 0 && !ret) { 
>> +            ret = i2c_smbus_write_byte_data(ctx->i2c, reg++, +
>> *(u8 *)data++); + +            count--; +        } +
>> break;
> 
> The transfer type gets set once per device at init time so why not
> just parameterise based on val_bytes?

Actually, you may want to transfer 1 byte registers using the block
method (if your device only support block transfers). This depends on
the device being accessed and what it supports, but I'm not sure we can
assume 1 byte registers will always be transferred using SMBUS byte
transfers.

> 
> 
>> +    case REGMAP_SMBUS_I2C_BLOCK_TRANSFER: +        while (count
>> > 0 && !ret) { +            ret =
>> i2c_smbus_write_i2c_block_data(ctx->i2c, +
>> reg, +                                 ctx->val_bytes, +
>> (const u8 *)data);
> 
> Fix the const correctness of the API rather than casting.

The API is correct because the i2c_smbus_write_i2c_block does not modify
the data pointer.
Just removing the const keyword in the cast should be enough, because
you can safely cast a non const pointer to a const one.

> 
> 
>> +            reg += ctx->val_bytes; +            count -=
>> ctx->val_bytes; +            data += ctx->val_bytes; +        }
> 
> I'm assuming this will only be used if val_bytes isn't 1 or 2?

As explained earlier, this depends on the device being accessed, but it
might be used for smaller transfers (1 or 2 bytes).

Thanks for the review.

Best Regards,

Boris

- -- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com