On Tue, Feb 14, 2023 at 03:29:33PM +0100, Cédric Le Goater wrote: > On 2/10/23 07:20, ~ssinprem wrote: > > From: Sittisak Sinprem <ssinp...@celestca.com> > > > > You will need to add a Signed-off-by tag > > Thanks, > > C.
Oh, yeah this is a pretty good change: I mean, at first I had no idea what's going on here, so it would be nice if we could leave a comment or refactor it to be simpler. Maybe instead of if-statements for > 256 or <= 256, we could do an address size attribute and compute the 256 bound from the address size. Anyways, this is a really small change to fix behavior, we can do a refactoring like that later (If Cedric is ok with it). Reviewed-by: Peter Delevoryas <pe...@pjd.dev> > > > --- > > hw/nvram/eeprom_at24c.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c > > index 2d4d8b952f..693212b661 100644 > > --- a/hw/nvram/eeprom_at24c.c > > +++ b/hw/nvram/eeprom_at24c.c > > @@ -87,7 +87,7 @@ uint8_t at24c_eeprom_recv(I2CSlave *s) > > EEPROMState *ee = AT24C_EE(s); > > uint8_t ret; > > - if (ee->haveaddr == 1) { > > + if (ee->rsize > 256 && ee->haveaddr == 1) { > > return 0xff; > > } > > @@ -104,11 +104,13 @@ int at24c_eeprom_send(I2CSlave *s, uint8_t data) > > { > > EEPROMState *ee = AT24C_EE(s); > > - if (ee->haveaddr < 2) { > > + if ((ee->rsize > 256 && ee->haveaddr < 2) || > > + (ee->rsize <= 256 && ee->haveaddr < 1)) { > > ee->cur <<= 8; > > ee->cur |= data; > > ee->haveaddr++; > > - if (ee->haveaddr == 2) { > > + if ((ee->rsize > 256 && ee->haveaddr == 2) || > > + (ee->rsize <= 256 && ee->haveaddr == 1)) { > > ee->cur %= ee->rsize; > > DPRINTK("Set pointer %04x\n", ee->cur); > > } >