[U-Boot] [U-Boot, v3, 4/4] cmd_eeprom: bug fix for i2c read/write

[Alexey Brodkin]

On Mon, 2013-12-09 at 12:21 +0100, Heiko Schocher wrote:
> I thought the v3 patch just rolls things back as patch comment states:
> ------------------------
>   Changes for v3:
>    - It turns out that what we did before 2013-11-13
>      (i.e., cmd_eeprom: fix i2c_{read|write} usage if env is in I2C EEPROM)
>      is still the best one, this patch simply rollback to it with coding
>      style fix.
> ------------------------
> 
> Without this roll back, I think, some boards are broken now in current
> mainline... so first we should go this step back.

I'd say it's very questionable. For example if you look at I2C driver
that Kuo-Jung refers to ("soft_i2c") you'll find that with my patch
applied (the one you've just rolled back) it will work flawlessly
because:
1. "chip" address there has double applied MSB bits of offset (the first
time it gets applied in "cmd_eeprom")
2. Only LSB byte of offset gets written in I2C device if addr_len = 1.

This one makes IMHO much more sense because it excludes an extra "chip"
address modification - so it's clear that it will be done by particular
I2C driver so people won't be confused with their expectations.

>  > BTW what I cannot understand is why "soft_i2c_read" has this "chip"
>  > modification part while "soft_i2c_write" doesn't? Is it done on purpose?
>  > And how it actually works at all then?
> 
> Good question ... maybe currently only used on i2c reads ?

Frankly I have only 1 supposition regarding this strange state of things
in "soft_i2c_write". Because without any doubts the same modification of
"chip" address is applicable to both "read" and "write" because it is an
integral part of data addressing.

But due to discussed a lot in this thread "double chip address
modification" (application of MSB bits of offset) proper support of
"chip" address was never implemented in "soft_i2c_write". As you
correctly mentioned - "real ancient code" works and nobody has problems
with it. Well, just because we have current implementation of
"eeprom_write" that does "chip" address modification "soft_i2c" may not
have this feature in both "read" and "write".

So I'd prefer to go with previous version of patch sent by Kuo-Jung
http://patchwork.ozlabs.org/patch/294733/

And indeed this will "break" functionality of currently incomplete
implementation of "soft_i2c_write".

Regards,
Alexey