On Thu, Jun 14, 2018 at 10:18:33AM +0200, BALATON Zoltan wrote: > On Thu, 14 Jun 2018, David Gibson wrote: > > On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > > > > But.. they are implemented. Albeit as an entirely software controlled > > register. > > They were implemented only in that they could be written and read but did > not implement any of the functionality they have in real hardware so in that > sense they were not really implemented. > > > I'm guessing that's not what they're supposed to do, which is why > > you're removing them, but that needs to be explained in the commit > > message. > > Maybe adding this explanation: > > We dont emulate slave mode so related registers are not needed. [lh]sadr are > only retained to avoid too many warnings and simplify debugging but sdata is > not even correct because device has a 4 byte FIFO instead so just remove > this unimplemented register for now. > > The intr register is also not implemented correctly, it is for diagnostics > and normally not even visible on device without explicitly enabling it. As > no guests are known to need this remove it as well.
That sounds reasonable. > > Regards, > BALATON Zoltan > > > As a general rule cases where a one line commit message is acceptable > > are *very* rare. > > > > > --- > > > hw/i2c/ppc4xx_i2c.c | 16 +--------------- > > > include/hw/i2c/ppc4xx_i2c.h | 4 +--- > > > 2 files changed, 2 insertions(+), 18 deletions(-) > > > > > > diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c > > > index d1936db..4e0aaae 100644 > > > --- a/hw/i2c/ppc4xx_i2c.c > > > +++ b/hw/i2c/ppc4xx_i2c.c > > > @@ -3,7 +3,7 @@ > > > * > > > * Copyright (c) 2007 Jocelyn Mayer > > > * Copyright (c) 2012 François Revol > > > - * Copyright (c) 2016 BALATON Zoltan > > > + * Copyright (c) 2016-2018 BALATON Zoltan > > > * > > > * Permission is hereby granted, free of charge, to any person obtaining > > > a copy > > > * of this software and associated documentation files (the "Software"), > > > to deal > > > @@ -63,7 +63,6 @@ static void ppc4xx_i2c_reset(DeviceState *s) > > > i2c->mdcntl = 0; > > > i2c->sts = 0; > > > i2c->extsts = 0x8f; > > > - i2c->sdata = 0; > > > i2c->lsadr = 0; > > > i2c->hsadr = 0; > > > i2c->clkdiv = 0; > > > @@ -71,7 +70,6 @@ static void ppc4xx_i2c_reset(DeviceState *s) > > > i2c->xfrcnt = 0; > > > i2c->xtcntlss = 0; > > > i2c->directcntl = 0xf; > > > - i2c->intr = 0; > > > } > > > > > > static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c) > > > @@ -139,9 +137,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr > > > addr, unsigned int size) > > > TYPE_PPC4xx_I2C, __func__); > > > } > > > break; > > > - case 2: > > > - ret = i2c->sdata; > > > - break; > > > case 4: > > > ret = i2c->lmadr; > > > break; > > > @@ -181,9 +176,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr > > > addr, unsigned int size) > > > case 16: > > > ret = i2c->directcntl; > > > break; > > > - case 17: > > > - ret = i2c->intr; > > > - break; > > > default: > > > if (addr < PPC4xx_I2C_MEM_SIZE) { > > > qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%" > > > @@ -229,9 +221,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr > > > addr, uint64_t value, > > > } > > > } > > > break; > > > - case 2: > > > - i2c->sdata = value; > > > - break; > > > case 4: > > > i2c->lmadr = value; > > > if (i2c_bus_busy(i2c->bus)) { > > > @@ -302,9 +291,6 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr > > > addr, uint64_t value, > > > case 16: > > > i2c->directcntl = value & 0x7; > > > break; > > > - case 17: > > > - i2c->intr = value; > > > - break; > > > default: > > > if (addr < PPC4xx_I2C_MEM_SIZE) { > > > qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%" > > > diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h > > > index 3c60307..e4b6ded 100644 > > > --- a/include/hw/i2c/ppc4xx_i2c.h > > > +++ b/include/hw/i2c/ppc4xx_i2c.h > > > @@ -3,7 +3,7 @@ > > > * > > > * Copyright (c) 2007 Jocelyn Mayer > > > * Copyright (c) 2012 François Revol > > > - * Copyright (c) 2016 BALATON Zoltan > > > + * Copyright (c) 2016-2018 BALATON Zoltan > > > * > > > * Permission is hereby granted, free of charge, to any person obtaining > > > a copy > > > * of this software and associated documentation files (the "Software"), > > > to deal > > > @@ -49,7 +49,6 @@ typedef struct PPC4xxI2CState { > > > uint8_t mdcntl; > > > uint8_t sts; > > > uint8_t extsts; > > > - uint8_t sdata; > > > uint8_t lsadr; > > > uint8_t hsadr; > > > uint8_t clkdiv; > > > @@ -57,7 +56,6 @@ typedef struct PPC4xxI2CState { > > > uint8_t xfrcnt; > > > uint8_t xtcntlss; > > > uint8_t directcntl; > > > - uint8_t intr; > > > } PPC4xxI2CState; > > > > > > #endif /* PPC4XX_I2C_H */ > > > > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature