Hi Robert,
On Thu, 17 Oct 2019 at 08:35, Robert Beckett bob.beckett@collabora.com wrote:
On Tue, 2019-10-15 at 21:40 -0600, Simon Glass wrote:
Hi Robert,
On Tue, 15 Oct 2019 at 09:55, Robert Beckett < bob.beckett@collabora.com> wrote:
Some devices (2 wire eeproms for example) use some bits from the chip address to represent the high bits of the offset instead of or as well as using multiple bytes for the offset, effectively stealing chip addresses on the bus.
Add a chip offset mask that can be set for any i2c chip which gets filled with the offset overflow during offset setup.
Signed-off-by: Robert Beckett bob.beckett@collabora.com Signed-off-by: Ian Ray ian.ray@ge.com
drivers/i2c/i2c-uclass.c | 32 ++++++++++++++++++++++++++------ include/i2c.h | 24 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-)
Please can you update the i2c tests to cover this new feature?
Sure, will do. Having looked at the i2c testing available in test/dm/i2c.c, I'm struggling to see how the current tests are valid. The changes in offset leng for 3 and 4 byte offsets don't set the correct offset length in dm_test_i2c_offset.
Also, there is no visibility of the effect under test to verify the correct behaviour. It seems to be relying on predictable offset mapping in to an assumed 256 byte storage in drivers/misc/i2c_eeprom_emul.c, but as long as the mapping is consistent between read and write, it could be all sorts of wrong and the test would not show any issue.
Also there appears to be a bug in the loop for offset mapping, I think it should be ">=", rather than ">". Currently wouldn't it overflow the storage if an offset of 256 is used?
Am I missing something in how it is meant to work? If not, I can modify the tests while I'm adding a new one for the new addressing mode, I just wanted to check first.
I think the best approach is to modify the tests in a separate patch, then add your new patch.
Thanks for the review, Ill fix the rest up for v2.
Regards, Simon