[U-Boot] [PATCH v2 01/26] dm: i2c: Provide an offset length parameter where needed

[Simon Glass]

Hi Bin,

On 21 January 2015 at 03:45, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Mon, 19 Jan 2015 20:12:30 -0700
> Simon Glass <sjg at chromium.org> wrote:
>
>
>> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
>> index 1e500fb..7c3ad00 100644
>> --- a/common/cmd_i2c.c
>> +++ b/common/cmd_i2c.c
>> @@ -168,7 +168,7 @@ static int i2c_get_cur_bus_chip(uint chip_addr, struct udevice **devp)
>>       if (ret)
>>               return ret;
>>
>> -     return i2c_get_chip(bus, chip_addr, devp);
>> +     return i2c_get_chip(bus, chip_addr, 1, devp);
>>  }
>
> The i2c command calls
> [1] i2c_get_cur_bus_chip()   = set the offset len to 1
> [2] i2c_set_chip_offset_len  = change the offset
>
>
> Now we can do [1] and [2] at the same time, right?
>
>
> If we set the offset address when we get the device,
> we won't need i2c_set_chip_offset_len(), I think.
>
> The offset_len for each device does not change.
> Chainging it on the way makes no sense.

Except that I inserted this patch into the series I sent a week ago,
which I want to apply today/tomorrow. So I wanted to limit the scope
of the change. I was not able to convince myself that there would be
no side-effects with the cmd_i2c.c change. But if you have had a look
too then perhaps we are OK.

I'll send a follow-up patch to clean this up (still will be in this
merge window).

>
>
>
>
>>  }
>>
>> -int i2c_get_chip(struct udevice *bus, uint chip_addr, struct udevice **devp)
>> +int i2c_get_chip(struct udevice *bus, uint chip_addr, uint offset_len,
>> +              struct udevice **devp)
>
>
> If the device tree for the child device is not found
> (i.e. i2c_bind_driver() is called), the new generic chip
> will be given with the offset_len.
>
> On the other hand, if the device tree is found,
> offset_len is default to 1 because
> i2c_chip_ofdata_to_platdata() always set chip->offset_len to 1.
>
> It is a pity.
>
> I wonder if it would not be possible to
> get the default offset_len from the device tree node of the child device.
>
>
> For example, the EEPROM on my board expects chip->offset_len == 2.
>
> It would be nice if we could have the offset property for the EEPROM device node.
>
> I dug into Documentation/devicetree/bindings/, but I could not find the one.

I have a local patch which adds a 'u-boot,i2c-offset-length' property,
but in the case of cros_ec I just changed the value in the probe
function. So it wasn't essential.

I would prefer to add device tree support though. Since it seems you
agree I will go ahead with this. Any preference on property naming?

Regards,
Simon