Am 05.05.2013 05:14, schrieb Peter Crosthwaite: > On Sun, May 5, 2013 at 12:09 AM, Jean-Christophe DUBOIS > <j...@tribudubois.net> 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 { > > types should be in CamelCase IMXI2CState > >> + 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 = IMX_I2C(opaque); >> + >> + switch (offset) { >> + case IADR_ADDR: >> + value = s->iadr; >> + break; >> + case IFDR_ADDR: >> + value = s->ifdr; >> + break; >> + case I2CR_ADDR: >> + value = s->i2cr; >> + break; >> + case I2SR_ADDR: >> + value = s->i2sr; >> + break; >> + case I2DR_ADDR: >> + value = s->i2dr_read; >> + >> + if (imx_i2c_is_master(s)) { /* master mode */ >> + int ret = 0xff; >> + >> + if (s->address == ADDR_RESET) { >> + /* something is wrong as the address is not set */ >> + DPRINT("Trying to read without specifying the slave >> address\n"); >> + } else if (s->i2cr & I2CR_MTX) { >> + DPRINT("Trying to read but MTX is set\n"); >> + } else { >> + /* get the next byte */ >> + ret = i2c_recv(s->bus); >> + >> + if (ret >= 0) { >> + imx_i2c_raise_interrupt(s); >> + } else { >> + DPRINT("read failed for device 0x%02x\n" s->address); >> + ret = 0xff; >> + } >> + } >> + >> + s->i2dr_read = 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) > > 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 -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg