|
| From: | Jean-Christophe DUBOIS |
| Subject: | Re: [Qemu-devel] [PATCH v3 2/4] Add i.MX I2C controller emulator |
| Date: | Mon, 06 May 2013 08:26:04 +0200 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130404 Thunderbird/17.0.5 |
On 05/06/2013 04:19 AM, Peter Crosthwaite wrote:
Hi JC, On Mon, May 6, 2013 at 12:28 AM, Jean-Christophe DUBOIS <address@hidden> wrote:The slave mode is not implemented. Changes since v1: * use QOM cast * run checkpatch on code * added restrictin on MemoryRegionOps * use DeviceClass::realise as init function Changes since v2: * use CamelCase for state type * use extrac32() for bit manipilation. * improve QOM cast * separate regs definition in its own file (to be reused by qtest)Per-patch change logs need to go below the line ...Signed-off-by: Jean-Christophe DUBOIS <address@hidden> ---... here. Otherwise it will go into the mainline git log on merge. The edit can be done by hand edit before the send-email. But I use a scripted approach personally to move my changelogs out of commit message pre send (and my working tree has commits formatted similar to what you have sent here). Others using the manual flow tend to just use cover letter change logs I think.
OK
hw/i2c/Makefile.objs | 1 + hw/i2c/imx_i2c.c | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++ hw/i2c/imx_i2c_regs.h | 63 +++++++++ 3 files changed, 416 insertions(+) create mode 100644 hw/i2c/imx_i2c.c create mode 100644 hw/i2c/imx_i2c_regs.h diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index 648278e..d27bbaa 100644 --- a/hw/i2c/Makefile.objs +++ b/hw/i2c/Makefile.objs @@ -4,4 +4,5 @@ common-obj-$(CONFIG_ACPI) += smbus_ich9.o common-obj-$(CONFIG_APM) += pm_smbus.o common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o +common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o obj-$(CONFIG_OMAP) += omap_i2c.o diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c new file mode 100644 index 0000000..9debcce --- /dev/null +++ b/hw/i2c/imx_i2c.c @@ -0,0 +1,352 @@ +/* + * i.MX I2C Bus Serial Interface Emulation + * + * Copyright (C) 2013 Jean-Christophe Dubois. + * + * 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/bitops.h" + +#include "hw/sysbus.h" +#include "hw/i2c/i2c.h" + +#include "hw/i2c/imx_i2c_regs.h" + +#ifndef IMX_I2C_DEBUG +#define IMX_I2C_DEBUG 0 +#endif + +#if IMX_I2C_DEBUG +#define DPRINT(fmt, ...) \ + do { fprintf(stderr, "imx_i2c[%s]: " fmt, __func__, ## __VA_ARGS__); \ + } while (0) + +static const char *imx_i2c_get_regname(unsigned offset) +{ + switch (offset) { + case IADR_ADDR: + return "IADR"; + case IFDR_ADDR: + return "IFDR"; + case I2CR_ADDR: + return "I2CR"; + case I2SR_ADDR: + return "I2SR"; + case I2DR_ADDR: + return "I2DR"; + default: + return "[?]"; + } +} +#else +#define DPRINT(fmt, args...) do { } while (0) +#endif + +#define TYPE_IMX_I2C "imx.i2c" +#define IMX_I2C(obj) \ + OBJECT_CHECK(IMXI2CState, (obj), TYPE_IMX_I2C) + +typedef struct IMXI2CState { + SysBusDevice parent_obj; + + MemoryRegion iomem; + i2c_bus *bus; + 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; +} IMXI2CState; + +static inline bool imx_i2c_is_enabled(IMXI2CState *s) +{ + return s->i2cr & I2CR_IEN; +} + +static inline bool imx_i2c_interrupt_is_enabled(IMXI2CState *s) +{ + return s->i2cr & I2CR_IIEN; +} + +static inline bool imx_i2c_is_master(IMXI2CState *s) +{ + return s->i2cr & I2CR_MSTA; +} + +static inline bool imx_i2c_direction_is_tx(IMXI2CState *s) +{ + return s->i2cr & I2CR_MTX; +} + +static void imx_i2c_reset(DeviceState *dev) +{ + IMXI2CState *s = IMX_I2C(dev); + + if (s->address != ADDR_RESET) { + i2c_end_transfer(s->bus); + }I don't think this is right, unless your device has actual logic that cleans up the I2C bus on hard reset (which would be very strange). The I2C bus should be responsible for its own reset as a reset (if needed at all). As an I2C bus is stateless, cleanup of an inflight transaction should happen naturally when a reset happens both master and slave side. Did this manifest for you as a bug at any stage? If so I think its a bug in the I2C framework and worth RFCing.
No, I was trying to be extra cautious and leave a clean state. I'll remove the i2c_end_transfer().
+ + s->address = ADDR_RESET; + s->iadr = IADR_RESET; + s->ifdr = IFDR_RESET; + s->i2cr = I2CR_RESET; + s->i2sr = I2SR_RESET; + s->i2dr_read = I2DR_RESET; + s->i2dr_write = I2DR_RESET; +} + +static inline void imx_i2c_raise_interrupt(IMXI2CState *s) +{ + /* + * raise an interrupt if the device is enabled and it is configured + * to generate some interrupts. + */ + if (imx_i2c_is_enabled(s) && imx_i2c_interrupt_is_enabled(s)) { + s->i2sr |= I2SR_IIF; + qemu_irq_raise(s->irq); + } +} + +static uint64_t imx_i2c_read(void *opaque, hwaddr offset, + unsigned size) +{ + uint16_t value; + IMXI2CState *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");This DPRINT and the one below it looks guest accessible through SW error and should be qemu_log_mask(LOG_GUEST_ERROR. Printfery should generally be classified into three buckets. qemu_log_mask(LOG_GUEST_ERROR, for the ones where bad software can send you down obviously bad code paths like this. LOG_UNIMP for stuff that we didn't bother with in QEMU. Lastly DPRINT (or just plain qemu_log) is for extra verbosity that the debugging QEMU developer needs to develop the device model.
OK
+ } 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), + offset, value); + + return (uint64_t)value; +} + +static void imx_i2c_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + IMXI2CState *s = IMX_I2C(opaque); + + DPRINT("write %s [0x%02x] <- 0x%02x\n", imx_i2c_get_regname(offset), + offset, (int)value); + + value &= 0xff; + + switch (offset) { + case IADR_ADDR: + s->iadr = value & IADR_MASK; + /* i2c_set_slave_address(s->bus, (uint8_t)s->iadr); */ + break; + case IFDR_ADDR: + s->ifdr = value & IFDR_MASK; + break; + case I2CR_ADDR: + if (imx_i2c_is_enabled(s) && ((value & I2CR_IEN) == 0)) { + /* This is a soft reset. IADR is preserved during soft resets */ + uint16_t iadr = s->iadr; + imx_i2c_reset(&s->parent_obj.qdev);QOM cast needed. And I think this may be in other parts of the series. Replace all derefs of .qdev with DEVICE() casts. imx_i2c_reset(DEVICE(s));
OK
Regards, Peter+ s->iadr = iadr; + } else { /* normal write */ + s->i2cr = value & I2CR_MASK; + + if (imx_i2c_is_master(s)) { /* master mode */ + /* set the bus to busy */ + s->i2sr |= I2SR_IBB; + } else { /* slave mode */ + /* bus is not busy anymore */ + s->i2sr &= ~I2SR_IBB; + + /* + *if we unset the master mode then it ends the ongoing + * transfer if any + */ + if (s->address != ADDR_RESET) { + i2c_end_transfer(s->bus); + s->address = ADDR_RESET; + } + } + + if (s->i2cr & I2CR_RSTA) { /* Restart */ + /* if this is a restart then it ends the ongoing transfer */ + if (s->address != ADDR_RESET) { + i2c_end_transfer(s->bus); + s->address = ADDR_RESET; + s->i2cr &= ~I2CR_RSTA; + } + } + } + break; + case I2SR_ADDR: + /* + * if the user writes 0 to IIF then lower the interrupt and + * reset the bit + */ + if ((s->i2sr & I2SR_IIF) && !(value & I2SR_IIF)) { + s->i2sr &= ~I2SR_IIF; + qemu_irq_lower(s->irq); + } + + /* + * if the user writes 0 to IAL, reset the bit + */ + if ((s->i2sr & I2SR_IAL) && !(value & I2SR_IAL)) { + s->i2sr &= ~I2SR_IAL; + } + + break; + case I2DR_ADDR: + /* if the device is not enabled, nothing to do */ + if (!imx_i2c_is_enabled(s)) { + break; + } + + s->i2dr_write = value & I2DR_MASK; + + if (imx_i2c_is_master(s)) { /* master mode */ + /* If this is the first write cycle then it is the slave addr */ + if (s->address == ADDR_RESET) { + if (i2c_start_transfer(s->bus, extract32(s->i2dr_write, 1, 7), + extract32(s->i2dr_write, 0, 1))) { + /* if non zero is returned, the adress is not valid */ + s->i2sr |= I2SR_RXAK; + } else { + s->address = s->i2dr_write; + s->i2sr &= ~I2SR_RXAK; + imx_i2c_raise_interrupt(s); + } + } else { /* This is a normal data write */ + if (i2c_send(s->bus, s->i2dr_write)) { + /* if the target return non zero then end the transfer */ + s->i2sr |= I2SR_RXAK; + s->address = ADDR_RESET; + i2c_end_transfer(s->bus); + } else { + s->i2sr &= ~I2SR_RXAK; + imx_i2c_raise_interrupt(s); + } + } + } + break; + default: + hw_error("%s: Bad address 0x%x\n", __func__, (int)offset); + break; + } +} + +static const MemoryRegionOps imx_i2c_ops = { + .read = imx_i2c_read, + .write = imx_i2c_write, + .valid.min_access_size = 1, + .valid.max_access_size = 2, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static const VMStateDescription imx_i2c_vmstate = { + .name = TYPE_IMX_I2C, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT16(address, IMXI2CState), + VMSTATE_UINT16(iadr, IMXI2CState), + VMSTATE_UINT16(ifdr, IMXI2CState), + VMSTATE_UINT16(i2cr, IMXI2CState), + VMSTATE_UINT16(i2sr, IMXI2CState), + VMSTATE_UINT16(i2dr_read, IMXI2CState), + VMSTATE_UINT16(i2dr_write, IMXI2CState), + VMSTATE_END_OF_LIST() + } +}; + +static void imx_i2c_realize(DeviceState *dev, Error **errp) +{ + IMXI2CState *s = IMX_I2C(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + + memory_region_init_io(&s->iomem, &imx_i2c_ops, s, TYPE_IMX_I2C, + IMX_I2C_MEM_SIZE); + sysbus_init_mmio(sbd, &s->iomem); + sysbus_init_irq(sbd, &s->irq); + s->bus = i2c_init_bus(DEVICE(dev), "i2c"); +} + +static void imx_i2c_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->vmsd = &imx_i2c_vmstate; + dc->reset = imx_i2c_reset; + dc->realize = imx_i2c_realize; +} + +static const TypeInfo imx_i2c_type_info = { + .name = TYPE_IMX_I2C, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(IMXI2CState), + .class_init = imx_i2c_class_init, +}; + +static void imx_i2c_register_types(void) +{ + type_register_static(&imx_i2c_type_info); +} + +type_init(imx_i2c_register_types) diff --git a/hw/i2c/imx_i2c_regs.h b/hw/i2c/imx_i2c_regs.h new file mode 100644 index 0000000..95c7cf5 --- /dev/null +++ b/hw/i2c/imx_i2c_regs.h @@ -0,0 +1,63 @@ +/* + * i.MX I2C Bus Serial Interface registers definition + * + * Copyright (C) 2013 Jean-Christophe Dubois. + * + * 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/>. + * + */ + +#ifndef __IMX_I2C_REGS_H_ +#define __IMX_I2C_REGS_H_ + +#define IMX_I2C_MEM_SIZE 0x14 + +/* i.MX I2C memory map */ +#define IADR_ADDR 0x00 /* address register */ +#define IFDR_ADDR 0x04 /* frequency divider register */ +#define I2CR_ADDR 0x08 /* control register */ +#define I2SR_ADDR 0x0c /* status register */ +#define I2DR_ADDR 0x10 /* data register */ + +#define IADR_MASK 0xFE +#define IADR_RESET 0 + +#define IFDR_MASK 0x3F +#define IFDR_RESET 0 + +#define I2CR_IEN (1 << 7) +#define I2CR_IIEN (1 << 6) +#define I2CR_MSTA (1 << 5) +#define I2CR_MTX (1 << 4) +#define I2CR_TXAK (1 << 3) +#define I2CR_RSTA (1 << 2) +#define I2CR_MASK 0xFC +#define I2CR_RESET 0 + +#define I2SR_ICF (1 << 7) +#define I2SR_IAAF (1 << 6) +#define I2SR_IBB (1 << 5) +#define I2SR_IAL (1 << 4) +#define I2SR_SRW (1 << 2) +#define I2SR_IIF (1 << 1) +#define I2SR_RXAK (1 << 0) +#define I2SR_MASK 0xE9 +#define I2SR_RESET 0x81 + +#define I2DR_MASK 0xFF +#define I2DR_RESET 0 + +#define ADDR_RESET 0xFF00 + +#endif /* __IMX_I2C_REGS_H_ */ -- 1.8.1.2
| [Prev in Thread] | Current Thread | [Next in Thread] |