Hi Patrick, On 12/20/21 01:32, Patrick Venture wrote: > The at24 eeproms are 2 byte devices that return 0xff when they are read > from with a partial (1-byte) address written. This distinction was > found comparing model behavior to real hardware testing. > > Tested: `i2ctransfer -f -y 45 w1@85 0 r1` returns 0xff instead of next > byte > > Signed-off-by: Patrick Venture <vent...@google.com> > --- > hw/nvram/eeprom_at24c.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c > index a9e3702b00..184fac9702 100644 > --- a/hw/nvram/eeprom_at24c.c > +++ b/hw/nvram/eeprom_at24c.c > @@ -62,7 +62,9 @@ int at24c_eeprom_event(I2CSlave *s, enum i2c_event event) > case I2C_START_SEND: > case I2C_START_RECV: > case I2C_FINISH: > - ee->haveaddr = 0; > + if (event != I2C_START_RECV) { > + ee->haveaddr = 0; > + }
Alternatively (matter of taste, I find it easier to read): case I2C_START_SEND: case I2C_FINISH: ee->haveaddr = 0; /* fallthrough */ case I2C_START_RECV: > DPRINTK("clear\n"); > if (ee->blk && ee->changed) { > int len = blk_pwrite(ee->blk, 0, ee->mem, ee->rsize, 0); > @@ -86,6 +88,10 @@ uint8_t at24c_eeprom_recv(I2CSlave *s) > EEPROMState *ee = AT24C_EE(s); > uint8_t ret; > > + if (ee->haveaddr == 1) { > + return 0xff; Don't we need to increase ee->haveaddr? > + } > + > ret = ee->mem[ee->cur]; > > ee->cur = (ee->cur + 1u) % ee->rsize; Here for parity with send, what about: if (ee->haveaddr < 2) { ret = 0xff; ee->haveaddr++; } else { ret = ee->mem[ee->cur]; ee->cur = (ee->cur + 1u) % ee->rsize; } ?