Re: [Qemu-devel] [PATCH v14 3/8] i.MX: Add I2C controller emulator

2015-08-29 Thread Peter Crosthwaite 
On Mon, Aug 10, 2015 at 3:02 PM, Jean-Christophe Dubois
j...@tribudubois.net wrote:
 The slave mode is not implemented.

 Signed-off-by: Jean-Christophe Dubois j...@tribudubois.net
 Reviewed-by: Peter Crosthwaite crosthwaite.pe...@gmail.com

Have you changed my email from the original RB? You should generally
keep this the same to preserve the affiliations which are part of the
review. If you are worried about bouncing auto-cc's use
--supress-cc-all to git send-email and go manual on the to/cc list.

Few minor style changes since I gave this review, comments below.

 ---

 Changes since v1:
 * none

 Changes since v2:
 * use QOM cast
 * reworked debug printf
 * use CamelCase for state type
 * warn with qemu_log_mask(LOG_GUEST_ERROR) or qemu_log_mask(LOG_UNIMP)
 * move to dma_memory_read/write API
 * rework interrupt handling
 * use qemu_flush_queued_packets() in rx_enable()

 Changes since v3:
 * use realise for device initialization
 * More QOM cast
 * reworked debug printf some more
 * standardise GPL header
 * use CamelCase for buffer descriptor type

 Changes since v4:
 * none

 Changes since v5:
 * replace hw_error() with qemu_log_mask(LOG_GUEST_ERROR, ...)
 * remove reformating of imx.h header file.
 * remove unnecessary spaces.

 Changes since v6:
 * port to new memory API

 Change since v7:
 * refactor emulator to be used by SOC

 Changes since v8:
 * no change

 Changes since v9:
 * no change

 Changes since v10:
 * no change.

 Changes since v11:
 * no change.

 Changes since v12:
 * no change.

 Changes since v13:
 * no change.

  default-configs/arm-softmmu.mak |   2 +
  hw/i2c/Makefile.objs|   1 +
  hw/i2c/imx_i2c.c| 339 
 
  include/hw/i2c/imx_i2c.h|  85 ++
  4 files changed, 427 insertions(+)
  create mode 100644 hw/i2c/imx_i2c.c
  create mode 100644 include/hw/i2c/imx_i2c.h

 diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
 index 3f86e7e..47390db 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -100,6 +100,8 @@ CONFIG_ALLWINNER_A10=y

  CONFIG_FSL_IMX31=y

 +CONFIG_IMX_I2C=y
 +
  CONFIG_XIO3130=y
  CONFIG_IOH3420=y
  CONFIG_I82801B11=y
 diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
 index 0f13060..aeb8f38 100644
 --- a/hw/i2c/Makefile.objs
 +++ b/hw/i2c/Makefile.objs
 @@ -4,4 +4,5 @@ common-obj-$(CONFIG_ACPI_X86) += 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 000..468712b
 --- /dev/null
 +++ b/hw/i2c/imx_i2c.c
 @@ -0,0 +1,339 @@
 +/*
 + *  i.MX I2C Bus Serial Interface Emulation
 + *
 + *  Copyright (C) 2013 Jean-Christophe Dubois. j...@tribudubois.net
 + *
 + *  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 hw/i2c/imx_i2c.h
 +#include hw/i2c/i2c.h
 +
 +#ifndef IMX_I2C_DEBUG
 +#define IMX_I2C_DEBUG 0
 +#endif
 +
 +#if IMX_I2C_DEBUG
 +#define DPRINT(fmt, args...)  \
 +do { fprintf(stderr, %s: fmt, __func__, ## 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
 +
 +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) {

[Qemu-devel] [PATCH v14 3/8] i.MX: Add I2C controller emulator

2015-08-10 Thread Jean-Christophe Dubois 
The slave mode is not implemented.

Signed-off-by: Jean-Christophe Dubois j...@tribudubois.net
Reviewed-by: Peter Crosthwaite crosthwaite.pe...@gmail.com
---

Changes since v1:
* none

Changes since v2:
* use QOM cast
* reworked debug printf
* use CamelCase for state type
* warn with qemu_log_mask(LOG_GUEST_ERROR) or qemu_log_mask(LOG_UNIMP)
* move to dma_memory_read/write API
* rework interrupt handling
* use qemu_flush_queued_packets() in rx_enable()

Changes since v3:
* use realise for device initialization
* More QOM cast
* reworked debug printf some more
* standardise GPL header
* use CamelCase for buffer descriptor type

Changes since v4:
* none

Changes since v5:
* replace hw_error() with qemu_log_mask(LOG_GUEST_ERROR, ...)
* remove reformating of imx.h header file.
* remove unnecessary spaces.

Changes since v6:
* port to new memory API

Change since v7:
* refactor emulator to be used by SOC

Changes since v8:
* no change

Changes since v9:
* no change

Changes since v10:
* no change.

Changes since v11:
* no change.

Changes since v12:
* no change.

Changes since v13:
* no change.

 default-configs/arm-softmmu.mak |   2 +
 hw/i2c/Makefile.objs|   1 +
 hw/i2c/imx_i2c.c| 339 
 include/hw/i2c/imx_i2c.h|  85 ++
 4 files changed, 427 insertions(+)
 create mode 100644 hw/i2c/imx_i2c.c
 create mode 100644 include/hw/i2c/imx_i2c.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 3f86e7e..47390db 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -100,6 +100,8 @@ CONFIG_ALLWINNER_A10=y
 
 CONFIG_FSL_IMX31=y
 
+CONFIG_IMX_I2C=y
+
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
index 0f13060..aeb8f38 100644
--- a/hw/i2c/Makefile.objs
+++ b/hw/i2c/Makefile.objs
@@ -4,4 +4,5 @@ common-obj-$(CONFIG_ACPI_X86) += 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 000..468712b
--- /dev/null
+++ b/hw/i2c/imx_i2c.c
@@ -0,0 +1,339 @@
+/*
+ *  i.MX I2C Bus Serial Interface Emulation
+ *
+ *  Copyright (C) 2013 Jean-Christophe Dubois. j...@tribudubois.net
+ *
+ *  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 hw/i2c/imx_i2c.h
+#include hw/i2c/i2c.h
+
+#ifndef IMX_I2C_DEBUG
+#define IMX_I2C_DEBUG 0
+#endif
+
+#if IMX_I2C_DEBUG
+#define DPRINT(fmt, args...)  \
+do { fprintf(stderr, %s: fmt, __func__, ## 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
+
+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);
+}
+
+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;
+