[U-Boot] [PATCH 16/37] i2c: add support for offset overflow in to address

[Simon Glass]

Hi Robert,

On Thu, 17 Oct 2019 at 08:35, Robert Beckett <bob.beckett at 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 at 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 at collabora.com>
> > > Signed-off-by: Ian Ray <ian.ray at 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