[lkml]   [2014]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC PATCH v2] regmap: smbus: add support for regmap over SMBus

Hash: SHA1

On 15/04/2014 12:09, Mark Brown wrote:
> On Tue, Apr 15, 2014 at 09:36:13AM +0200, Boris BREZILLON wrote:
>> On 14/04/2014 23:04, Mark Brown wrote:
>>> 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.
> OK, so if this a realistic issue then it seems like it's better to
> implement three different buses - there is not really any common code
> between the various paths.

Okay, I'll create 4 different busses (one for each access type).
BTW, should I keep these implementations in the same source file
(regmap-smbus.c) ?
And, should I keep one method to register an smbus regmap or should I
provide one method per access type and get rid of the
regmap_smbus_transfer_type enum ?

> This would also mean that you avoid having
> gather write when it can't be implemented.

Correct me if I'm wrong, but they all support gather write.

> The code is also not validating the lengths for two byte values.

I'm not sure I get this one.
Do you mean I should check that val_size is a 2 byte multiple ?
If this is what you meant, then I should also check it for block 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.
> If you need to cast away from void at all something is going wrong (and
> we appear to be always passing in write buffers as const too, I can't
> remember which operation this was though).

You're right, removing the cast when passing the val argument to the i2c
block transfer functions works.

Best Regards,


- --
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird -


 \ /
  Last update: 2014-04-15 14:41    [W:0.066 / U:7.296 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site