Let me add my two cents here. I tested this patch slightly and didn't find any problems.
On 2012-03-15 11:35, Igor Mitsyanko wrote: > Create 9 exynos4210 i2c interfaces. > > Signed-off-by: Igor Mitsyanko <i.mitsya...@samsung.com> > --- > Makefile.target | 1 + > hw/exynos4210.c | 26 +++ > hw/exynos4210.h | 3 + > hw/exynos4210_i2c.c | 469 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 499 insertions(+), 0 deletions(-) > create mode 100644 hw/exynos4210_i2c.c > > diff --git a/Makefile.target b/Makefile.target > index eb25941..5e8a1d4 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -357,6 +357,7 @@ obj-arm-y += realview_gic.o realview.o arm_sysctl.o > arm11mpcore.o a9mpcore.o > obj-arm-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o > obj-arm-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o > obj-arm-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o > +obj-arm-y += exynos4210_i2c.o > obj-arm-y += arm_l2x0.o > obj-arm-y += arm_mptimer.o a15mpcore.o > obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o > diff --git a/hw/exynos4210.c b/hw/exynos4210.c > index f904370..464f157 100644 > --- a/hw/exynos4210.c > +++ b/hw/exynos4210.c > @@ -35,6 +35,13 @@ > /* MCT */ > #define EXYNOS4210_MCT_BASE_ADDR 0x10050000 > > +/* I2C */ > +#define EXYNOS4210_I2C_SHIFT 0x00010000 > +#define EXYNOS4210_I2C_BASE_ADDR 0x13860000 > +/* Interrupt Group of External Interrupt Combiner for I2C */ > +#define EXYNOS4210_I2C_INTG 27 > +#define EXYNOS4210_HDMI_INTG 16 > + > /* UART's definitions */ > #define EXYNOS4210_UART0_BASE_ADDR 0x13800000 > #define EXYNOS4210_UART1_BASE_ADDR 0x13810000 > @@ -242,6 +249,25 @@ Exynos4210State *exynos4210_init(MemoryRegion > *system_mem, > s->irq_table[exynos4210_get_irq(35, 3)]); > sysbus_mmio_map(busdev, 0, EXYNOS4210_MCT_BASE_ADDR); > > + /*** I2C ***/ > + for (n = 0; n < EXYNOS4210_I2C_NUMBER; n++) { > + uint32_t addr = EXYNOS4210_I2C_BASE_ADDR + EXYNOS4210_I2C_SHIFT * n; > + qemu_irq i2c_irq; > + > + if (n < 8) { > + i2c_irq = s->irq_table[exynos4210_get_irq(EXYNOS4210_I2C_INTG, > n)]; > + } else { > + i2c_irq = s->irq_table[exynos4210_get_irq(EXYNOS4210_HDMI_INTG, > 1)]; > + } > + > + dev = qdev_create(NULL, "exynos4210.i2c"); > + qdev_init_nofail(dev); > + busdev = sysbus_from_qdev(dev); > + sysbus_connect_irq(busdev, 0, i2c_irq); > + sysbus_mmio_map(busdev, 0, addr); > + s->i2c_if[n] = (i2c_bus *)qdev_get_child_bus(dev, "i2c"); > + } > + > /*** UARTs ***/ > exynos4210_uart_create(EXYNOS4210_UART0_BASE_ADDR, > EXYNOS4210_UART0_FIFO_SIZE, 0, NULL, > diff --git a/hw/exynos4210.h b/hw/exynos4210.h > index c112e03..1205fb5 100644 > --- a/hw/exynos4210.h > +++ b/hw/exynos4210.h > @@ -74,6 +74,8 @@ > #define EXYNOS4210_EXT_GIC_NIRQ (160-32) > #define EXYNOS4210_INT_GIC_NIRQ 64 > > +#define EXYNOS4210_I2C_NUMBER 9 > + > typedef struct Exynos4210Irq { > qemu_irq int_combiner_irq[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ]; > qemu_irq ext_combiner_irq[EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ]; > @@ -95,6 +97,7 @@ typedef struct Exynos4210State { > MemoryRegion dram1_mem; > MemoryRegion boot_secondary; > MemoryRegion bootreg_mem; > + i2c_bus *i2c_if[EXYNOS4210_I2C_NUMBER]; > } Exynos4210State; > > Exynos4210State *exynos4210_init(MemoryRegion *system_mem, > diff --git a/hw/exynos4210_i2c.c b/hw/exynos4210_i2c.c > new file mode 100644 > index 0000000..42f770d > --- /dev/null > +++ b/hw/exynos4210_i2c.c > @@ -0,0 +1,469 @@ > +/* > + * Exynos4210 I2C Bus Serial Interface Emulation > + * > + * Copyright (C) 2012 Samsung Electronics Co Ltd. > + * Maksim Kozlov, <m.koz...@samsung.com> > + * Igor Mitsyanko, <i.mitsya...@samsung.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > + * for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see <http://www.gnu.org/licenses/>. > + * > + */ > + > +#include "qemu-timer.h" > +#include "sysbus.h" > +#include "i2c.h" > + > +#ifndef EXYNOS4_I2C_DEBUG > +#define EXYNOS4_I2C_DEBUG 0 > +#endif > + > +#define TYPE_EXYNOS4_I2C "exynos4210.i2c" > +#define TYPE_EXYNOS4_I2C_SLAVE "exynos4210.i2c-slave" > +#define EXYNOS4_I2C(obj) \ > + OBJECT_CHECK(Exynos4210I2CState, (obj), TYPE_EXYNOS4_I2C) > +#define EXYNOS4_I2C_SLAVE(obj) \ > + OBJECT_CHECK(Exynos4210I2CSlave, (obj), TYPE_EXYNOS4_I2C_SLAVE) > + > +/* Exynos4210 I2C memory map */ > +#define EXYNOS4_I2C_MEM_SIZE 0x14 > +#define I2CCON_ADDR 0x00 /* control register */ > +#define I2CSTAT_ADDR 0x04 /* control/status register */ > +#define I2CADD_ADDR 0x08 /* address register */ > +#define I2CDS_ADDR 0x0c /* data shift register */ > +#define I2CLC_ADDR 0x10 /* line control register */ > + > +#define I2CCON_INTERRUPTS_EN (1 << 5) > +#define I2CCON_INTERRUPT_PEND (1 << 4) > + > +#define EXYNOS4_I2C_MODE(reg) (((reg) >> 6) & 3) > +#define I2C_IN_MASTER_MODE(reg) (((reg) >> 6) & 2) > +#define I2CSTAT_MODE_SLAVE_Rx 0x0 > +#define I2CSTAT_MODE_SLAVE_Tx 0x1 > +#define I2CMODE_MASTER_Rx 0x2 > +#define I2CMODE_MASTER_Tx 0x3 > +#define I2CSTAT_LAST_BIT (1 << 0) > +#define I2CSTAT_SLAVE_ADDR_ZERO (1 << 1) > +#define I2CSTAT_SLAVE_ADDR_MATCH (1 << 2) > +#define I2CSTAT_ARBITR_STATUS (1 << 3) > +#define I2CSTAT_OUTPUT_EN (1 << 4) > +#define I2CSTAT_START_BUSY (1 << 5) > + > + > +#if EXYNOS4_I2C_DEBUG > +#define DPRINT(fmt, args...) \ > + do {fprintf(stderr, "QEMU I2C: "fmt, ## args); } while (0) > + > +static const char *exynos4_i2c_regs_names[] = { > + [I2CCON_ADDR >> 2] = "I2CCON", > + [I2CSTAT_ADDR >> 2] = "I2CSTAT", > + [I2CADD_ADDR >> 2] = "I2CADD", > + [I2CDS_ADDR >> 2] = "I2CDS", > + [I2CLC_ADDR >> 2] = "I2CLC", > +}; > + > +#else > +#define DPRINT(fmt, args...) do { } while (0) > +#endif > + > +typedef struct Exynos4210I2CSlave Exynos4210I2CSlave; > + > +typedef struct Exynos4210I2CState { > + SysBusDevice busdev; > + MemoryRegion iomem; > + i2c_bus *bus; > + Exynos4210I2CSlave *slave; > + qemu_irq irq; > + QEMUTimer *receive_timer; > + QEMUTimer *send_timer; > + > + uint8_t i2ccon; > + uint8_t i2cstat; > + uint8_t i2cadd; > + uint8_t i2cds; > + uint8_t i2clc; > +} Exynos4210I2CState; > + > +enum { > + idle, > + start_bit_received, > + sending_data, > + receiving_data > +}; > + > +struct Exynos4210I2CSlave { > + I2CSlave i2c; > + Exynos4210I2CState *host; > + uint32_t status; > +}; > + > + > +static inline void exynos4210_i2c_set_irq(Exynos4210I2CState *s) > +{ > + if (s->i2ccon & I2CCON_INTERRUPTS_EN) { > + s->i2ccon |= I2CCON_INTERRUPT_PEND; > + qemu_irq_raise(s->irq); > + } > +} > + > +static void exynos4210_i2c_data_received(void *opaque) > +{ > + Exynos4210I2CState *s = (Exynos4210I2CState *)opaque; > + int ret; > + > + ret = i2c_recv(s->bus); > + if (ret < 0) { > + s->i2cstat |= I2CSTAT_LAST_BIT; /* Data is not acknowledged */ > + } else { > + s->i2cds = ret; > + s->i2cstat &= ~I2CSTAT_LAST_BIT; /* Data is acknowledged */ > + } > + exynos4210_i2c_set_irq(s); > +} > + > +static void exynos4210_i2c_data_sent(void *opaque) > +{ > + Exynos4210I2CState *s = (Exynos4210I2CState *)opaque; > + > + if (i2c_send(s->bus, s->i2cds) < 0) { > + s->i2cstat |= I2CSTAT_LAST_BIT; /* Data is not acknowledged */ > + } else { > + s->i2cstat &= ~I2CSTAT_LAST_BIT; /* Data is acknowledged */ > + } > + exynos4210_i2c_set_irq(s); > +} > + > +static uint64_t exynos4210_i2c_read(void *opaque, target_phys_addr_t offset, > + unsigned size) > +{ > + Exynos4210I2CState *s = (Exynos4210I2CState *)opaque; > + uint8_t value; > + > + switch (offset) { > + case I2CCON_ADDR: > + value = s->i2ccon; > + break; > + case I2CSTAT_ADDR: > + value = s->i2cstat; > + break; > + case I2CADD_ADDR: > + value = s->i2cadd; > + break; > + case I2CDS_ADDR: > + value = s->i2cds; > + break; > + case I2CLC_ADDR: > + value = s->i2clc; > + break; > + default: > + value = 0; > + DPRINT("ERROR: Bad read offset 0x%x\n", (unsigned int)offset); > + break; > + } > + > + DPRINT("read %s [0x%02x] -> 0x%02x\n", exynos4_i2c_regs_names[offset >> > 2], > + (unsigned int)offset, value); Just for security it's better to check offset to not go out of the array borders. > + return value; > +} > + > +static void exynos4210_i2c_write(void *opaque, target_phys_addr_t offset, > + uint64_t value, unsigned size) > +{ > + Exynos4210I2CState *s = (Exynos4210I2CState *)opaque; > + uint8_t v = value & 0xff; > + > + DPRINT("write %s [0x%02x] <- 0x%02x\n", exynos4_i2c_regs_names[offset >> > 2], > + (unsigned int)offset, v); > + > + switch (offset) { > + case I2CCON_ADDR: > + if ((s->i2ccon & I2CCON_INTERRUPT_PEND) && > + !(v & I2CCON_INTERRUPT_PEND)) { > + s->i2ccon &= ~I2CCON_INTERRUPT_PEND; > + if (!(s->i2ccon & I2CCON_INTERRUPTS_EN)) { > + s->i2cstat &= ~I2CSTAT_START_BUSY; > + } > + qemu_irq_lower(s->irq); > + > + if (!(s->i2cstat & I2CSTAT_OUTPUT_EN) || > + !(s->i2cstat & I2CSTAT_START_BUSY)) { > + s->i2cstat &= ~I2CSTAT_START_BUSY; /* Line is not busy */ > + break; > + } > + /* Continue operation if transfer is ongoing */ > + if (EXYNOS4_I2C_MODE(s->i2cstat) == I2CMODE_MASTER_Rx) { > + qemu_mod_timer(s->receive_timer, > qemu_get_clock_ns(vm_clock)+1); > + } else if (EXYNOS4_I2C_MODE(s->i2cstat) == I2CMODE_MASTER_Tx) { > + qemu_mod_timer(s->send_timer, qemu_get_clock_ns(vm_clock) + > 1); Why do you need to use timers here? Why not just call the corresponding functions directly? I can guess because when sending new message Linux driver first enables the interrupt and then writes the stat register. Am I right? > + } > + } > + > + /* bit 4 can't be written */ > + s->i2ccon = (v & ~I2CCON_INTERRUPT_PEND) | > + (s->i2ccon & I2CCON_INTERRUPT_PEND); > + break; > + case I2CSTAT_ADDR: > + s->i2cstat = > + (s->i2cstat & I2CSTAT_START_BUSY) | (v & > ~I2CSTAT_START_BUSY); > + > + if (!(s->i2cstat & I2CSTAT_OUTPUT_EN)) { > + s->i2cstat &= ~I2CSTAT_START_BUSY; > + qemu_del_timer(s->receive_timer); > + qemu_del_timer(s->send_timer); > + break; > + } > + > + /* Nothing to do if in i2c slave mode */ > + if (!I2C_IN_MASTER_MODE(s->i2cstat)) { > + break; > + } > + > + if (v & I2CSTAT_START_BUSY) { > + s->i2cstat |= I2CSTAT_START_BUSY; /* Line is busy */ > + /* Generate start bit and send slave address */ > + i2c_start_transfer(s->bus, s->i2cds >> 1, s->i2cds & 0x1); > + exynos4210_i2c_data_sent(s); > + } else { > + i2c_end_transfer(s->bus); > + /* Linux driver may start next i2c transaction before it > + * acknowledges an interrupt request from previous transaction. > + * This may result in timeout waiting for I2C bus idle and > + * corrupted transfer. Releasing I2C bus only after interrupt is > + * accepted prevents this corruption. */ > + if (!(s->i2ccon & I2CCON_INTERRUPT_PEND)) { > + s->i2cstat &= ~I2CSTAT_START_BUSY; > + } > + } > + break; > + case I2CADD_ADDR: > + if ((s->i2cstat & I2CSTAT_OUTPUT_EN) == 0) { > + s->i2cadd = v; > + i2c_set_slave_address(&s->slave->i2c, v); > + } > + break; > + case I2CDS_ADDR: > + if (s->i2cstat & I2CSTAT_OUTPUT_EN) { > + s->i2cds = v; > + } > + break; > + case I2CLC_ADDR: > + s->i2clc = v; > + break; > + default: > + DPRINT("ERROR: Bad write offset 0x%x\n", (unsigned int)offset); > + break; > + } > +} > + > +static const MemoryRegionOps exynos4210_i2c_ops = { > + .read = exynos4210_i2c_read, > + .write = exynos4210_i2c_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static const VMStateDescription exynos4210_i2c_slave_vmstate = { > + .name = TYPE_EXYNOS4_I2C_SLAVE, > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_I2C_SLAVE(i2c, Exynos4210I2CSlave), > + VMSTATE_UINT32(status, Exynos4210I2CSlave), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription exynos4210_i2c_vmstate = { > + .name = TYPE_EXYNOS4_I2C, > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(i2ccon, Exynos4210I2CState), > + VMSTATE_UINT8(i2cstat, Exynos4210I2CState), > + VMSTATE_UINT8(i2cds, Exynos4210I2CState), > + VMSTATE_UINT8(i2clc, Exynos4210I2CState), Should i2cadd also be saved here? > + VMSTATE_TIMER(receive_timer, Exynos4210I2CState), > + VMSTATE_TIMER(send_timer, Exynos4210I2CState), > + VMSTATE_STRUCT_POINTER(slave, Exynos4210I2CState, > + exynos4210_i2c_slave_vmstate, Exynos4210I2CSlave *), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void exynos4210_i2c_reset(DeviceState *d) > +{ > + Exynos4210I2CState *s = EXYNOS4_I2C(d); > + > + s->i2ccon = 0x0f; /* 0x0X */ > + s->i2cstat = 0x00; > + s->i2cds = 0xff; /* 0xXX */ > + s->i2clc = 0x00; > + s->i2cadd = 0xff; /* 0xXX */ > + s->slave->status = idle; > + qemu_del_timer(s->receive_timer); > + qemu_del_timer(s->send_timer); > +} > + > +static int exynos4210_i2c_init(SysBusDevice *dev) > +{ > + Exynos4210I2CState *s = EXYNOS4_I2C(dev); > + DeviceState *slave; > + > + memory_region_init_io(&s->iomem, &exynos4210_i2c_ops, s, > TYPE_EXYNOS4_I2C, > + EXYNOS4_I2C_MEM_SIZE); > + sysbus_init_mmio(dev, &s->iomem); > + sysbus_init_irq(dev, &s->irq); > + s->bus = i2c_init_bus(&dev->qdev, "i2c"); > + s->receive_timer = > + qemu_new_timer_ns(vm_clock, exynos4210_i2c_data_received, s); > + s->send_timer = qemu_new_timer_ns(vm_clock, exynos4210_i2c_data_sent, s); > + > + /* i2c slave initialization */ > + slave = i2c_create_slave(s->bus, TYPE_EXYNOS4_I2C_SLAVE, 0xff); > + s->slave = EXYNOS4_I2C_SLAVE(slave); > + s->slave->host = s; > + > + return 0; > +} > + > +static void exynos4210_i2c_class_init(ObjectClass *class, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(class); > + SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(class); > + > + dc->vmsd = &exynos4210_i2c_vmstate; > + dc->reset = exynos4210_i2c_reset; > + sbc->init = exynos4210_i2c_init; > +} > + > +static TypeInfo exynos4210_i2c_info = { > + .name = TYPE_EXYNOS4_I2C, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(Exynos4210I2CState), > + .class_init = exynos4210_i2c_class_init, > +}; > + > +static int exynos4210_i2c_slave_init(I2CSlave *i2c) > +{ > + Exynos4210I2CSlave *s = EXYNOS4_I2C_SLAVE(i2c); > + > + s->status = idle; > + return 0; > +} > + > +static void exynos4210_i2c_slave_event(I2CSlave *i2c, enum i2c_event event) > +{ > + Exynos4210I2CSlave *s = EXYNOS4_I2C_SLAVE(i2c); > + Exynos4210I2CState *host = s->host; > + > + if (!(host->i2cstat & I2CSTAT_OUTPUT_EN) || > + I2C_IN_MASTER_MODE(host->i2cstat)) { > + return; > + } > + > + switch (event) { > + case I2C_START_RECV: > + case I2C_START_SEND: > + host->i2cstat &= ~(I2CSTAT_SLAVE_ADDR_ZERO | > I2CSTAT_SLAVE_ADDR_MATCH); > + s->status = start_bit_received; > + host->i2cstat |= I2CSTAT_START_BUSY; /* Line is busy */ > + break; > + case I2C_FINISH: > + s->status = idle; > + host->i2cstat &= ~(I2CSTAT_SLAVE_ADDR_ZERO | > I2CSTAT_SLAVE_ADDR_MATCH | > + I2CSTAT_START_BUSY); > + break; > + default: > + break; > + } > +} > + > +static int exynos4210_i2c_slave_recv(I2CSlave *i2c) > +{ > + Exynos4210I2CSlave *s = EXYNOS4_I2C_SLAVE(i2c); > + Exynos4210I2CState *host = s->host; > + > + if (!(host->i2cstat & I2CSTAT_OUTPUT_EN) || > + I2C_IN_MASTER_MODE(host->i2cstat)) { > + return -1; > + } > + > + if ((s->status != sending_data) || (EXYNOS4_I2C_MODE(host->i2cstat) != > + I2CSTAT_MODE_SLAVE_Tx)) { > + return -1; > + } > + > + exynos4210_i2c_set_irq(host); > + return host->i2cds; > +} > + > +static int exynos4210_i2c_slave_send(I2CSlave *i2c, uint8_t data) > +{ > + Exynos4210I2CSlave *s = EXYNOS4_I2C_SLAVE(i2c); > + Exynos4210I2CState *host = s->host; > + > + if (!(host->i2cstat & I2CSTAT_OUTPUT_EN) || > + I2C_IN_MASTER_MODE(host->i2cstat)) { > + return -1; > + } > + > + if (s->status == start_bit_received) { > + host->i2cds = data; > + s->status = idle; > + if ((data & 0xFE) == 0) { > + host->i2cstat |= I2CSTAT_SLAVE_ADDR_ZERO; > + return -1; > + } > + if ((host->i2cadd & 0xFE) != (data & 0xFE)) { > + return -1; > + } > + host->i2cstat |= I2CSTAT_SLAVE_ADDR_MATCH; > + (data & 0x1) ? (s->status = receiving_data) : > + (s->status = sending_data); > + exynos4210_i2c_set_irq(host); > + return 0; > + } > + > + if ((s->status != receiving_data) || (EXYNOS4_I2C_MODE(host->i2cstat) != > + I2CSTAT_MODE_SLAVE_Rx)) { > + return -1; > + } > + > + host->i2cds = data; > + exynos4210_i2c_set_irq(host); > + return 0; > +} > + > +static void exynos4210_i2cslave_class_init(ObjectClass *klass, void *data) > +{ > + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); > + > + k->init = exynos4210_i2c_slave_init; > + k->event = exynos4210_i2c_slave_event; > + k->recv = exynos4210_i2c_slave_recv; > + k->send = exynos4210_i2c_slave_send; > +} > + > +static TypeInfo exynos4210_i2c_slave = { > + .name = TYPE_EXYNOS4_I2C_SLAVE, > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(Exynos4210I2CSlave), > + .class_init = exynos4210_i2cslave_class_init, > +}; > + > +static void exynos4210_i2c_register_types(void) > +{ > + type_register_static(&exynos4210_i2c_slave); > + type_register_static(&exynos4210_i2c_info); > +} > + > +type_init(exynos4210_i2c_register_types) PS: This I2C implementation seems to be rather complex. Did you check what we wrote for S5PC110 (ah, I also learnt that Samsung renamed it to Exynos3210) almost three years ago (https://gitorious.org/samsung-linux-platform/qemu/blobs/s5pc110-stable/hw/s5pc1xx_i2c.c)? When I compared documentation and kernel driver sources for both devices I couldn't find any significant difference. Generally speaking, is it worth redoing everything from scratch instead of updating?