Hi Bin, On 21 January 2015 at 03:45, Masahiro Yamada <yamad...@jp.panasonic.com> wrote: > Hi Simon, > > > > On Mon, 19 Jan 2015 20:12:30 -0700 > Simon Glass <s...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
