On 06/06/2018 10:31 AM, BALATON Zoltan wrote: > Make it more readable by converting register indexes to decimal > (avoids lot of superfluous 0x0) and distinguish errors caused by > accessing non-existent vs. unimplemented registers. > No functional change. > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > --- > hw/i2c/ppc4xx_i2c.c | 94 > +++++++++++++++++++++++++++++------------------------ > 1 file changed, 51 insertions(+), 43 deletions(-) > > diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c > index ab64d19..d1936db 100644 > --- a/hw/i2c/ppc4xx_i2c.c > +++ b/hw/i2c/ppc4xx_i2c.c > @@ -31,7 +31,7 @@ > #include "hw/hw.h" > #include "hw/i2c/ppc4xx_i2c.h" > > -#define PPC4xx_I2C_MEM_SIZE 0x12 > +#define PPC4xx_I2C_MEM_SIZE 18
This looks weird, it seems all memory range sizes are in hex in other QEMU devices. > > #define IIC_CNTL_PT (1 << 0) > #define IIC_CNTL_READ (1 << 1) > @@ -70,7 +70,7 @@ static void ppc4xx_i2c_reset(DeviceState *s) > i2c->intrmsk = 0; > i2c->xfrcnt = 0; > i2c->xtcntlss = 0; > - i2c->directcntl = 0x0f; > + i2c->directcntl = 0xf; > i2c->intr = 0; > } > > @@ -85,7 +85,7 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr, > unsigned int size) > uint64_t ret; > > switch (addr) { > - case 0x00: > + case 0: > ret = i2c->mdata; > if (ppc4xx_i2c_is_master(i2c)) { > ret = 0xff; > @@ -139,58 +139,62 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr > addr, unsigned int size) > TYPE_PPC4xx_I2C, __func__); > } > break; > - case 0x02: > + case 2: > ret = i2c->sdata; > break; > - case 0x04: > + case 4: > ret = i2c->lmadr; > break; > - case 0x05: > + case 5: > ret = i2c->hmadr; > break; > - case 0x06: > + case 6: > ret = i2c->cntl; > break; > - case 0x07: > + case 7: > ret = i2c->mdcntl; > break; > - case 0x08: > + case 8: > ret = i2c->sts; > break; > - case 0x09: > + case 9: > ret = i2c->extsts; > break; > - case 0x0A: > + case 10: > ret = i2c->lsadr; > break; > - case 0x0B: > + case 11: > ret = i2c->hsadr; > break; > - case 0x0C: > + case 12: > ret = i2c->clkdiv; > break; > - case 0x0D: > + case 13: > ret = i2c->intrmsk; > break; > - case 0x0E: > + case 14: > ret = i2c->xfrcnt; > break; > - case 0x0F: > + case 15: > ret = i2c->xtcntlss; > break; > - case 0x10: > + case 16: > ret = i2c->directcntl; > break; > - case 0x11: > + case 17: > ret = i2c->intr; > break; > default: > - qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%" > - HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr); > + if (addr < PPC4xx_I2C_MEM_SIZE) { > + qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%" > + HWADDR_PRIx "\n", __func__, addr); > + } else { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address 0x%" > + HWADDR_PRIx "\n", __func__, addr); > + } > ret = 0; > break; > } > - > return ret; > } > > @@ -200,7 +204,7 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, > uint64_t value, > PPC4xxI2CState *i2c = opaque; > > switch (addr) { > - case 0x00: > + case 0: > i2c->mdata = value; > if (!i2c_bus_busy(i2c->bus)) { > /* assume we start a write transfer */ > @@ -225,19 +229,19 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr > addr, uint64_t value, > } > } > break; > - case 0x02: > + case 2: > i2c->sdata = value; > break; > - case 0x04: > + case 4: > i2c->lmadr = value; > if (i2c_bus_busy(i2c->bus)) { > i2c_end_transfer(i2c->bus); > } > break; > - case 0x05: > + case 5: > i2c->hmadr = value; > break; > - case 0x06: > + case 6: > i2c->cntl = value; > if (i2c->cntl & IIC_CNTL_PT) { > if (i2c->cntl & IIC_CNTL_READ) { > @@ -263,32 +267,31 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr > addr, uint64_t value, > } > } > break; > - case 0x07: > - i2c->mdcntl = value & 0xDF; > + case 7: > + i2c->mdcntl = value & 0xdf; > break; > - case 0x08: > - i2c->sts &= ~(value & 0x0A); > + case 8: > + i2c->sts &= ~(value & 0xa); 'value & 0x0a' implicitly denotes than 'value' is a 8-bit register. Matter of taste... > break; > - case 0x09: > - i2c->extsts &= ~(value & 0x8F); > + case 9: > + i2c->extsts &= ~(value & 0x8f); > break; > - case 0x0A: > + case 10: > i2c->lsadr = value; > - /*i2c_set_slave_address(i2c->bus, i2c->lsadr);*/ > break; > - case 0x0B: > + case 11: > i2c->hsadr = value; > break; > - case 0x0C: > + case 12: > i2c->clkdiv = value; > break; > - case 0x0D: > + case 13: > i2c->intrmsk = value; > break; > - case 0x0E: > + case 14: > i2c->xfrcnt = value & 0x77; > break; > - case 0x0F: > + case 15: > if (value & IIC_XTCNTLSS_SRST) { > /* Is it actually a full reset? U-Boot sets some regs before */ > ppc4xx_i2c_reset(DEVICE(i2c)); > @@ -296,15 +299,20 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr > addr, uint64_t value, > } > i2c->xtcntlss = value; > break; > - case 0x10: > + case 16: > i2c->directcntl = value & 0x7; > break; > - case 0x11: > + case 17: > i2c->intr = value; > break; > default: > - qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%" > - HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr); > + if (addr < PPC4xx_I2C_MEM_SIZE) { > + qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%" > + HWADDR_PRIx "\n", __func__, addr); > + } else { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address 0x%" > + HWADDR_PRIx "\n", __func__, addr); > + } > break; > } > } >