From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCTRH-0001Br-5l for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:24:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCTR9-0004JF-Km for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:24:47 -0500 Received: from cantor2.suse.de ([195.135.220.15]:52426 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCTR9-0004J9-B2 for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:24:39 -0500 Message-ID: <52F77381.9020109@suse.de> Date: Sun, 09 Feb 2014 13:24:33 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1391178886-17277-1-git-send-email-afaerber@suse.de> <1391178886-17277-3-git-send-email-afaerber@suse.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 02/11] pxa2xx: QOM'ify I2C slave List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , "qemu-devel@nongnu.org Developers" Am 09.02.2014 02:35, schrieb Peter Crosthwaite: > On Sat, Feb 1, 2014 at 12:34 AM, Andreas F=E4rber wr= ote: >> Replace usages of FROM_I2C_SLAVE() with QOM cast macro and rename pare= nt >> field to assure we caught all. >> >> Signed-off-by: Andreas F=E4rber >> --- >> hw/arm/pxa2xx.c | 38 +++++++++++++++++++++++++------------- >> 1 file changed, 25 insertions(+), 13 deletions(-) >> >> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c >> index daf60e8..e5f1e10 100644 >> --- a/hw/arm/pxa2xx.c >> +++ b/hw/arm/pxa2xx.c >> @@ -1222,8 +1222,14 @@ static const TypeInfo pxa2xx_rtc_sysbus_info =3D= { >> }; >> >> /* I2C Interface */ >> -typedef struct { >> - I2CSlave i2c; >> + >> +#define TYPE_PXA2XX_I2C_SLAVE "pxa2xx-i2c-slave" >> +#define PXA2XX_I2C_SLAVE(obj) \ >> + OBJECT_CHECK(PXA2xxI2CSlaveState, (obj), TYPE_PXA2XX_I2C_SLAVE) >> + >> +typedef struct PXA2xxI2CSlaveState { >> + I2CSlave parent_obj; >> + >> PXA2xxI2CState *host; >> } PXA2xxI2CSlaveState; >> >> @@ -1268,7 +1274,7 @@ static void pxa2xx_i2c_update(PXA2xxI2CState *s) >> /* These are only stubs now. */ >> static void pxa2xx_i2c_event(I2CSlave *i2c, enum i2c_event event) >> { >> - PXA2xxI2CSlaveState *slave =3D FROM_I2C_SLAVE(PXA2xxI2CSlaveState= , i2c); >> + PXA2xxI2CSlaveState *slave =3D PXA2XX_I2C_SLAVE(i2c); >> PXA2xxI2CState *s =3D slave->host; >> >> switch (event) { >> @@ -1292,10 +1298,12 @@ static void pxa2xx_i2c_event(I2CSlave *i2c, en= um i2c_event event) >> >> static int pxa2xx_i2c_rx(I2CSlave *i2c) >> { >> - PXA2xxI2CSlaveState *slave =3D FROM_I2C_SLAVE(PXA2xxI2CSlaveState= , i2c); >> + PXA2xxI2CSlaveState *slave =3D PXA2XX_I2C_SLAVE(i2c); >> PXA2xxI2CState *s =3D slave->host; >> - if ((s->control & (1 << 14)) || !(s->control & (1 << 6))) >> + >> + if ((s->control & (1 << 14)) || !(s->control & (1 << 6))) { >> return 0; >> + } >=20 > This will look funny when git-blamed. Should out-of-scope trivials be > separate patch so anyone git-blame will at least see "Fix coding > style" rather than then misleading "QOM'ify I2C"?. We've had the policy of fixing Coding Style on lines touched or adjacent to those touched, to gradually get rid of them - this one was within 3 lines of context. I'm sure PXA code will have many more Coding Style faults, so placing 2 out of X in their own patch seems silly. Should I rather drop them? > With the multiple > instances of this cleanup it should at least feature in the commit > message. You're right that I should've mentioned it. Same for replacing parent field accesses. Fixed. Regards, Andreas >> >> if (s->status & (1 << 0)) { /* RWM */ >> s->status |=3D 1 << 6; /* set ITE */ >> @@ -1307,10 +1315,12 @@ static int pxa2xx_i2c_rx(I2CSlave *i2c) >> >> static int pxa2xx_i2c_tx(I2CSlave *i2c, uint8_t data) >> { >> - PXA2xxI2CSlaveState *slave =3D FROM_I2C_SLAVE(PXA2xxI2CSlaveState= , i2c); >> + PXA2xxI2CSlaveState *slave =3D PXA2XX_I2C_SLAVE(i2c); >> PXA2xxI2CState *s =3D slave->host; >> - if ((s->control & (1 << 14)) || !(s->control & (1 << 6))) >> + >> + if ((s->control & (1 << 14)) || !(s->control & (1 << 6))) { >> return 1; >> + } >> >> if (!(s->status & (1 << 0))) { /* RWM */ >> s->status |=3D 1 << 7; /* set IRF */ >> @@ -1325,6 +1335,7 @@ static uint64_t pxa2xx_i2c_read(void *opaque, hw= addr addr, >> unsigned size) >> { >> PXA2xxI2CState *s =3D (PXA2xxI2CState *) opaque; >> + I2CSlave *slave; >> >> addr -=3D s->offset; >> switch (addr) { >> @@ -1333,7 +1344,8 @@ static uint64_t pxa2xx_i2c_read(void *opaque, hw= addr addr, >> case ISR: >> return s->status | (i2c_bus_busy(s->bus) << 2); >> case ISAR: >> - return s->slave->i2c.address; >> + slave =3D I2C_SLAVE(s->slave); >> + return slave->address; >> case IDBR: >> return s->data; >> case IBMR: >> @@ -1408,7 +1420,7 @@ static void pxa2xx_i2c_write(void *opaque, hwadd= r addr, >> break; >> >> case ISAR: >> - i2c_set_slave_address(&s->slave->i2c, value & 0x7f); >> + i2c_set_slave_address(I2C_SLAVE(s->slave), value & 0x7f); >> break; >> >> case IDBR: >> @@ -1432,7 +1444,7 @@ static const VMStateDescription vmstate_pxa2xx_i= 2c_slave =3D { >> .minimum_version_id =3D 1, >> .minimum_version_id_old =3D 1, >> .fields =3D (VMStateField []) { >> - VMSTATE_I2C_SLAVE(i2c, PXA2xxI2CSlaveState), >> + VMSTATE_I2C_SLAVE(parent_obj, PXA2xxI2CSlaveState), >> VMSTATE_END_OF_LIST() >> } >> }; >> @@ -1470,7 +1482,7 @@ static void pxa2xx_i2c_slave_class_init(ObjectCl= ass *klass, void *data) >> } >> >> static const TypeInfo pxa2xx_i2c_slave_info =3D { >> - .name =3D "pxa2xx-i2c-slave", >> + .name =3D TYPE_PXA2XX_I2C_SLAVE, >> .parent =3D TYPE_I2C_SLAVE, >> .instance_size =3D sizeof(PXA2xxI2CSlaveState), >> .class_init =3D pxa2xx_i2c_slave_class_init, >> @@ -1496,8 +1508,8 @@ PXA2xxI2CState *pxa2xx_i2c_init(hwaddr base, >> s =3D PXA2XX_I2C(i2c_dev); >> /* FIXME: Should the slave device really be on a separate bus? *= / >> i2cbus =3D i2c_init_bus(dev, "dummy"); >> - dev =3D i2c_create_slave(i2cbus, "pxa2xx-i2c-slave", 0); >> - s->slave =3D FROM_I2C_SLAVE(PXA2xxI2CSlaveState, I2C_SLAVE(dev)); >> + dev =3D i2c_create_slave(i2cbus, TYPE_PXA2XX_I2C_SLAVE, 0); >> + s->slave =3D PXA2XX_I2C_SLAVE(dev); >> s->slave->host =3D s; >> >> return s; >> -- >> 1.8.4.5 >> >> --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg