Hi Simon,
2015-01-22 1:12 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Bin,
On 21 January 2015 at 03:45, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Mon, 19 Jan 2015 20:12:30 -0700 Simon Glass sjg@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).
Sorry, I noticed a side-effect.
If you mistype the offset length on the first run of the I2C command, a generic chip is created and bound with the wrong offset length. You cannot correct this because on the next run, i2c_get_chip() find the chip from the bound-devices list.
We still need i2c_set_chip_offset_len to set the offset length explicitly.
I take back my former comment. Sorry.
}
-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?
I checked Documentation/devicetree/bindings/eeprom.txt, but nothing is mentioned about the offset length.
So, I am OK with your idea.