From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYxDE-0005An-Dk for qemu-devel@nongnu.org; Sun, 05 May 2013 07:34:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UYxDD-00079f-4R for qemu-devel@nongnu.org; Sun, 05 May 2013 07:34:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:37810 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYxDC-00079T-RO for qemu-devel@nongnu.org; Sun, 05 May 2013 07:34:39 -0400 Message-ID: <518643C8.7010207@suse.de> Date: Sun, 05 May 2013 13:34:32 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/4] Add i.MX I2C controller driver. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , Jean-Christophe DUBOIS Cc: peter.maydell@linaro.org, peter.chubb@nicta.com.au, qemu-devel@nongnu.org Am 05.05.2013 05:14, schrieb Peter Crosthwaite: > On Sun, May 5, 2013 at 12:09 AM, Jean-Christophe DUBOIS > wrote: >> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c >> new file mode 100644 >> index 0000000..5b0d046 >> --- /dev/null >> +++ b/hw/i2c/imx_i2c.c [...] >> +typedef struct imx_i2c_state { >=20 > types should be in CamelCase IMXI2CState >=20 >> + SysBusDevice parent_obj; While at it, please add a white line here. Background is that this parent field will pretty likely go away once we switch to a better object-orientation framework - if it were in a header we would annotate it as private and thus hidden from documentation. >> + MemoryRegion iomem; >> + i2c_bus *bus; Please rather use i2c_bus bus; and qbus_create_inline() in instance_init. >> + qemu_irq irq; >> + >> + uint16_t address; >> + >> + uint16_t iadr; >> + uint16_t ifdr; >> + uint16_t i2cr; >> + uint16_t i2sr; >> + uint16_t i2dr_read; >> + uint16_t i2dr_write; >> +} imx_i2c_state; [...] >> +static uint64_t imx_i2c_read(void *opaque, hwaddr offset, >> + unsigned size) >> +{ >> + uint16_t value; >> + imx_i2c_state *s =3D IMX_I2C(opaque); >> + >> + switch (offset) { >> + case IADR_ADDR: >> + value =3D s->iadr; >> + break; >> + case IFDR_ADDR: >> + value =3D s->ifdr; >> + break; >> + case I2CR_ADDR: >> + value =3D s->i2cr; >> + break; >> + case I2SR_ADDR: >> + value =3D s->i2sr; >> + break; >> + case I2DR_ADDR: >> + value =3D s->i2dr_read; >> + >> + if (imx_i2c_is_master(s)) { /* master mode */ >> + int ret =3D 0xff; >> + >> + if (s->address =3D=3D ADDR_RESET) { >> + /* something is wrong as the address is not set */ >> + DPRINT("Trying to read without specifying the slave a= ddress\n"); >> + } else if (s->i2cr & I2CR_MTX) { >> + DPRINT("Trying to read but MTX is set\n"); >> + } else { >> + /* get the next byte */ >> + ret =3D i2c_recv(s->bus); >> + >> + if (ret >=3D 0) { >> + imx_i2c_raise_interrupt(s); >> + } else { >> + DPRINT("read failed for device 0x%02x\n" s->addre= ss); >> + ret =3D 0xff; >> + } >> + } >> + >> + s->i2dr_read =3D ret; >> + } >> + break; >> + default: >> + hw_error("%s: Bad address 0x%x\n", __func__, (int)offset); >> + break; >> + } >> + >> + DPRINT("read %s [0x%02x] -> 0x%02x\n", imx_i2c_get_regname(offset= ) >=20 > General question for the list, are we encouraging the use or PRIx in > new code to remove the needs for casts from uint_XXt to unsigned in > printfery? > , >> + (unsigned int)offset, value); I believe so, I've been asked to use HWADDR_PRIx macro in ppc patches. >> + >> + return (uint64_t)value; >> +} [snip] Cheers, Andreas --=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