[U-Boot] [PATCH] cmd_i2c: Provide option for bulk 'i2c write' in one transaction

[Lubomir Popov]

Hi Simon,

> Hi,
>
> On 24 November 2014 at 09:00, Lubomir Popov <lpopov at mm-sol.com> wrote:
>> I2C chips do exist that require a write of some multi-byte data to occur in
>> a single bus transaction (aka atomic transfer), otherwise either the write
>> does not come into effect at all, or normal operation of internal circuitry
>> cannot be guaranteed. The current implementation of the 'i2c write' command
>> (transfer of multiple bytes from a memory buffer) in fact performs a separate
>> transaction for each byte to be written and thus cannot support such types of
>> I2C slave devices.
>>
>> This patch provides an alternative by allowing 'i2c write' to execute the
>> write transfer of the given number of bytes in a single bus transaction if
>> CONFIG_SYS_I2C_BULK_WRITE is defined in the board header (otherwise the old
>> method shall compile).
>>
>> Signed-off-by: Lubomir Popov <l-popov at ti.com>
>> ---
>>  common/cmd_i2c.c |   15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
>> index 3a75f94..7116458 100644
>> --- a/common/cmd_i2c.c
>> +++ b/common/cmd_i2c.c
>> @@ -280,10 +280,22 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
>>                 return cmd_usage(cmdtp);
>>
>>         /*
>> -        * Length is the number of objects, not number of bytes.
>> +        * Length is the number of bytes.
>>          */
>>         length = simple_strtoul(argv[4], NULL, 16);
>>
>> +#if defined(CONFIG_SYS_I2C_BULK_WRITE)
>> +       /*
>> +        * Write all bytes in a single I2C transaction. If the target
>> +        * device is an EEPROM, it is your responsibility to not cross
>> +        * a page bounady.
>> +        */
>> +       if (i2c_write(chip, devaddr, alen, memaddr, length) != 0) {
>> +               puts("Error writing to the chip.\n");
>> +               return 1;
>> +       }
>> +#else
>> +       /* Perform <length> separate write transactions of one byte each */
>>         while (length-- > 0) {
>>                 if (i2c_write(chip, devaddr++, alen, memaddr++, 1) != 0) {
>>                         puts("Error writing to the chip.\n");
>> @@ -296,6 +308,7 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
>>                 udelay(11000);
>>  #endif
>
> This should really be an option/flag on the command. We might want to
> do different things depending on the situation.
Agree. I thought about this initially, even about adding a separate command,
but then decided that the human device using 'i2c write' with or without the
config option would be intelligent enough to know what is being needed and
done. From a chip compatibility perspective this is not an issue, since for
byte-wise transfers the 'i2c mw' and 'i2c mm' commands still do exist. So I
went the easy way :-).
>
> In fact with driver model's I2C implementation, currently in review,
> this logic of splitting things up can be handled in the uclass layer:
>
> http://patchwork.ozlabs.org/patch/414066/
Right, in the DM context a command line option would be the better solution.
I shall see to revising the patch in this respect.
>
> Regards,
> Simon
>
Best regards,
Lubo