Hi Robert, On Thu, 17 Oct 2019 at 08:35, Robert Beckett <bob.beck...@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.beck...@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.beck...@collabora.com> > > > Signed-off-by: Ian Ray <ian....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
