Re: [Qemu-devel] [PATCH 3/5] exynos4210: add Exynos4210 i2c implementation

2012-03-06 Thread Peter Maydell
On 2 March 2012 11:35, Igor Mitsyanko i.mitsya...@samsung.com wrote:
 Create 9 exynos4210 i2c interfaces.

 Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com

 +#define EXYNOS4_I2C(obj)                  \
 +OBJECT_CHECK(Exynos4210I2CState, (obj), TYPE_EXYNOS4_I2C)
 +#define EXYNOS4_I2C_SLAVE(obj)            \
 +OBJECT_CHECK(Exynos4210I2CSlave, (obj), TYPE_EXYNOS4_I2C_SLAVE)

Can you indent the second and subsequent lines of multi-line
macro definitions, please?

 +enum {
 +    idle,
 +    start_bit_received,
 +    sending_data,
 +    receiving_data
 +};
 +
 +struct Exynos4210I2CSlave {
 +    I2CSlave i2c;
 +    Exynos4210I2CState *host;
 +    uint32_t status;
 +};

I'm confused about the purpose of this class. My (possibly naive)
model of i2c is that there's a controller (in this case TYPE_EXYNOS4_I2C
, with its state in the struct Exynos4210I2CState) which is a subclass
of SysBus or whatever, and then there are i2c slaves, which are
subclasses of TYPE_I2C_SLAVE and communicate with the controller
strictly via the methods provided by the I2C_SLAVE class. But
this seems to be an I2CSlave subclass which has a direct pointer to
the internal state of the controller and very little state of its own.
What's it for?

 +static const VMStateDescription exynos4210_i2c_vmstate = {
 +    .name = TYPE_EXYNOS4_I2C,
 +    .version_id = 1,
 +    .minimum_version_id = 1,
 +    .fields      = (VMStateField[]) {

Why the big space?

-- PMM



Re: [Qemu-devel] [PATCH 3/5] exynos4210: add Exynos4210 i2c implementation

2012-03-06 Thread Igor Mitsyanko

On 03/06/2012 10:49 PM, Peter Maydell wrote:

On 2 March 2012 11:35, Igor Mitsyankoi.mitsya...@samsung.com  wrote:

Create 9 exynos4210 i2c interfaces.

Signed-off-by: Igor Mitsyankoi.mitsya...@samsung.com



+#define EXYNOS4_I2C(obj)  \
+OBJECT_CHECK(Exynos4210I2CState, (obj), TYPE_EXYNOS4_I2C)
+#define EXYNOS4_I2C_SLAVE(obj)\
+OBJECT_CHECK(Exynos4210I2CSlave, (obj), TYPE_EXYNOS4_I2C_SLAVE)


Can you indent the second and subsequent lines of multi-line
macro definitions, please?


OK


+enum {
+idle,
+start_bit_received,
+sending_data,
+receiving_data
+};
+
+struct Exynos4210I2CSlave {
+I2CSlave i2c;
+Exynos4210I2CState *host;
+uint32_t status;
+};


I'm confused about the purpose of this class. My (possibly naive)
model of i2c is that there's a controller (in this case TYPE_EXYNOS4_I2C
, with its state in the struct Exynos4210I2CState) which is a subclass
of SysBus or whatever, and then there are i2c slaves, which are
subclasses of TYPE_I2C_SLAVE and communicate with the controller
strictly via the methods provided by the I2C_SLAVE class. But
this seems to be an I2CSlave subclass which has a direct pointer to
the internal state of the controller and very little state of its own.
What's it for?



The reasoning behind this is that Exynos i2c controller can operate as 
i2c master or i2c slave (not simultaneously), so it must be a 
SysBusDevice and I2CSlave at the same time. There shouldn't be any 
communications of controller and some abstract I2C_SLAVE because 
controller is in fact I2C_SLAVE by itself, internal state of 
TYPE_EXYNOS4_I2C and TYPE_EXYNOS4_I2C_SLAVE is the same set of registers 
(hence the pointer).

I took this approach from pxa2xx i2c implementation.


+static const VMStateDescription exynos4210_i2c_vmstate = {
+.name = TYPE_EXYNOS4_I2C,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields  = (VMStateField[]) {


Why the big space?



I don't know :)


--
Mitsyanko Igor
ASWG, Moscow RD center, Samsung Electronics
email: i.mitsya...@samsung.com




[Qemu-devel] [PATCH 3/5] exynos4210: add Exynos4210 i2c implementation

2012-03-02 Thread Igor Mitsyanko
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 68a5641..3b309d9 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -348,6 +348,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   0x1005
 
+/* I2C */
+#define EXYNOS4210_I2C_SHIFT   0x0001
+#define EXYNOS4210_I2C_BASE_ADDR   0x1386
+/* Interrupt Group of External Interrupt Combiner for I2C */
+#define EXYNOS4210_I2C_INTG27
+#define EXYNOS4210_HDMI_INTG   16
+
 /* UART's definitions */
 #define EXYNOS4210_UART0_BASE_ADDR 0x1380
 #define EXYNOS4210_UART1_BASE_ADDR 0x1381
@@ -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 e7522f8..07cc579 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 000..c0beb48
--- /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_SLAVEexynos4210.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  /*