All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] hw/i2c-ddc: Do not fail writes
@ 2018-02-17 14:00 Linus Walleij
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-17 14:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, Linus Walleij

The tx function of the DDC I2C slave emulation was returning 1
on all writes resulting in NACK in the I2C bus. Changing it to
0 makes the DDC I2C work fine with bit-banged I2C such as the
versatile I2C.

I guess it was not affecting whatever I2C controller this was
used with until now, but with the Versatile I2C it surely
does not work.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 hw/i2c/i2c-ddc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c
index 199dac9e41c1..bec0c91e2dd0 100644
--- a/hw/i2c/i2c-ddc.c
+++ b/hw/i2c/i2c-ddc.c
@@ -259,12 +259,12 @@ static int i2c_ddc_tx(I2CSlave *i2c, uint8_t data)
         s->reg = data;
         s->firstbyte = false;
         DPRINTF("[EDID] Written new pointer: %u\n", data);
-        return 1;
+        return 0;
     }
 
     /* Ignore all writes */
     s->reg++;
-    return 1;
+    return 0;
 }
 
 static void i2c_ddc_init(Object *obj)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-17 14:00 [Qemu-devel] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Linus Walleij
@ 2018-02-17 14:00 ` Linus Walleij
  2018-02-17 18:32   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-02-22 14:42   ` Peter Maydell
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation Linus Walleij
  2018-02-22 14:36 ` [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Peter Maydell
  2 siblings, 2 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-17 14:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, Linus Walleij

This adds support for emulating the Silicon Image SII9022 DVI/HDMI
bridge. It's not very clever right now, it just acknowledges
the switch into DDC I2C mode and back. Combining this with the
existing DDC I2C emulation gives the right behavior on the Versatile
Express emulation passing through the QEMU EDID to the emulated
platform.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 hw/display/Makefile.objs |   1 +
 hw/display/sii9022.c     | 185 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+)
 create mode 100644 hw/display/sii9022.c

diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index d3a4cb396eb9..3c7c75b94da5 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
 common-obj-$(CONFIG_G364FB) += g364fb.o
 common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
 common-obj-$(CONFIG_PL110) += pl110.o
+common-obj-$(CONFIG_SII9022) += sii9022.o
 common-obj-$(CONFIG_SSD0303) += ssd0303.o
 common-obj-$(CONFIG_SSD0323) += ssd0323.o
 common-obj-$(CONFIG_XEN) += xenfb.o
diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c
new file mode 100644
index 000000000000..d6f3cdc04293
--- /dev/null
+++ b/hw/display/sii9022.c
@@ -0,0 +1,185 @@
+/*
+ * Silicon Image SiI9022
+ *
+ * This is a pretty hollow emulation: all we do is acknowledge that we
+ * exist (chip ID) and confirm that we get switched over into DDC mode
+ * so the emulated host can proceed to read out EDID data. All subsequent
+ * set-up of connectors etc will be acknowledged and ignored.
+ *
+ * Copyright (c) 2018 Linus Walleij
+ *
+ * This code is licensed under the GNU GPL v2.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/i2c/i2c.h"
+
+#define DEBUG_SII9022 0
+
+#define DPRINTF(fmt, ...) \
+    do { \
+        if (DEBUG_SII9022) { \
+            printf("sii9022: " fmt, ## __VA_ARGS__); \
+        } \
+    } while (0)
+
+#define SII9022_SYS_CTRL_DATA 0x1a
+#define SII9022_SYS_CTRL_PWR_DWN 0x10
+#define SII9022_SYS_CTRL_AV_MUTE 0x08
+#define SII9022_SYS_CTRL_DDC_BUS_REQ 0x04
+#define SII9022_SYS_CTRL_DDC_BUS_GRTD 0x02
+#define SII9022_SYS_CTRL_OUTPUT_MODE 0x01
+#define SII9022_SYS_CTRL_OUTPUT_HDMI 1
+#define SII9022_SYS_CTRL_OUTPUT_DVI 0
+#define SII9022_REG_CHIPID 0x1b
+#define SII9022_INT_ENABLE 0x3c
+#define SII9022_INT_STATUS 0x3d
+#define SII9022_INT_STATUS_HOTPLUG 0x01;
+#define SII9022_INT_STATUS_PLUGGED 0x04;
+
+#define TYPE_SII9022 "sii9022"
+#define SII9022(obj) OBJECT_CHECK(sii9022_state, (obj), TYPE_SII9022)
+
+typedef struct sii9022_state {
+    I2CSlave parent_obj;
+    uint8_t ptr;
+    bool addr_byte;
+    bool ddc_req;
+    bool ddc_skip_finish;
+    bool ddc;
+} sii9022_state;
+
+static const VMStateDescription vmstate_sii9022 = {
+    .name = "sii9022",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_I2C_SLAVE(parent_obj, sii9022_state),
+        VMSTATE_UINT8(ptr, sii9022_state),
+        VMSTATE_BOOL(addr_byte, sii9022_state),
+        VMSTATE_BOOL(ddc_req, sii9022_state),
+        VMSTATE_BOOL(ddc_skip_finish, sii9022_state),
+        VMSTATE_BOOL(ddc, sii9022_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int sii9022_event(I2CSlave *i2c, enum i2c_event event)
+{
+    sii9022_state *s = SII9022(i2c);
+
+    switch (event) {
+    case I2C_START_SEND:
+        s->addr_byte = true;
+        break;
+    case I2C_START_RECV:
+        break;
+    case I2C_FINISH:
+        break;
+    case I2C_NACK:
+        break;
+    }
+
+    return 0;
+}
+
+static int sii9022_rx(I2CSlave *i2c)
+{
+    sii9022_state *s = SII9022(i2c);
+    uint8_t res = 0x00;
+
+    switch (s->ptr) {
+    case SII9022_SYS_CTRL_DATA:
+        if (s->ddc_req) {
+            /* Acknowledge DDC bus request */
+            res = SII9022_SYS_CTRL_DDC_BUS_GRTD | SII9022_SYS_CTRL_DDC_BUS_REQ;
+        }
+        break;
+    case SII9022_REG_CHIPID:
+        res = 0xb0;
+        break;
+    case SII9022_INT_STATUS:
+        /* Something is cold-plugged in, no interrupts */
+        res = SII9022_INT_STATUS_PLUGGED;
+        break;
+    default:
+        break;
+    }
+    DPRINTF("%02x read from %02x\n", res, s->ptr);
+    s->ptr++;
+
+    return res;
+}
+
+static int sii9022_tx(I2CSlave *i2c, uint8_t data)
+{
+    sii9022_state *s = SII9022(i2c);
+
+    if (s->addr_byte) {
+        s->ptr = data;
+        s->addr_byte = false;
+        return 0;
+    }
+
+    switch (s->ptr) {
+    case SII9022_SYS_CTRL_DATA:
+        if (data & SII9022_SYS_CTRL_DDC_BUS_REQ) {
+            s->ddc_req = true;
+            if (data & SII9022_SYS_CTRL_DDC_BUS_GRTD) {
+                s->ddc = true;
+                /* Skip this finish since we just switched to DDC */
+                s->ddc_skip_finish = true;
+                DPRINTF("switched to DDC mode\n");
+            }
+        } else {
+            s->ddc_req = false;
+            s->ddc = false;
+        }
+        break;
+    default:
+        break;
+    }
+
+    DPRINTF("%02x written to %02x\n", data, s->ptr);
+    s->ptr++;
+
+    return 0;
+}
+
+static void sii9022_reset(DeviceState *dev)
+{
+    sii9022_state *s = SII9022(dev);
+
+    s->ptr = 0;
+    s->addr_byte = false;
+}
+
+static void sii9022_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+
+    k->event = sii9022_event;
+    k->recv = sii9022_rx;
+    k->send = sii9022_tx;
+    dc->reset = sii9022_reset;
+    dc->vmsd = &vmstate_sii9022;
+}
+
+static const TypeInfo sii9022_info = {
+    .name          = TYPE_SII9022,
+    .parent        = TYPE_I2C_SLAVE,
+    .instance_size = sizeof(sii9022_state),
+    .class_init    = sii9022_class_init,
+};
+
+static void sii9022_register_types(void)
+{
+    type_register_static(&sii9022_info);
+}
+
+type_init(sii9022_register_types)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation
  2018-02-17 14:00 [Qemu-devel] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Linus Walleij
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
@ 2018-02-17 14:00 ` Linus Walleij
  2018-02-17 18:28   ` Philippe Mathieu-Daudé
  2018-02-22 14:36 ` [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Peter Maydell
  2 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2018-02-17 14:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, Linus Walleij

This adds the SiI9022 and EDID I2C devices to the ARM Versatile
Express machine, and selects the two I2C devices necessary in the
arm-softmmy.mak configuration so everything will build smoothly.

I am implementing proper handling of the graphics in the Linux
kernel and adding proper emulation of SiI9022 and EDID makes the
driver probe as nicely as before, retrieveing the resolutions
supported by the "QEMU monitor" and overall just working nice.

The assignment of the SiI9022 at address 0x39 and the EDID
DDC I2C at address 0x50 is not strictly correct: the DDC I2C
is there all the time but in the actual component it only
appears once activated inside the SiI9022, so ideally it should
be added and removed to the bus by the SiI9022. However for this
purpose it works fine to just have it around.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 default-configs/arm-softmmu.mak | 2 ++
 hw/arm/vexpress.c               | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ca34cf446242..54f855d07206 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -21,6 +21,8 @@ CONFIG_STELLARIS_INPUT=y
 CONFIG_STELLARIS_ENET=y
 CONFIG_SSD0303=y
 CONFIG_SSD0323=y
+CONFIG_DDC=y
+CONFIG_SII9022=y
 CONFIG_ADS7846=y
 CONFIG_MAX111X=y
 CONFIG_SSI=y
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index dc5928ae1ab5..d6c912c97684 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -29,6 +29,7 @@
 #include "hw/arm/arm.h"
 #include "hw/arm/primecell.h"
 #include "hw/devices.h"
+#include "hw/i2c/i2c.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
@@ -537,6 +538,7 @@ static void vexpress_common_init(MachineState *machine)
     uint32_t sys_id;
     DriveInfo *dinfo;
     pflash_t *pflash0;
+    I2CBus *i2c;
     ram_addr_t vram_size, sram_size;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *vram = g_new(MemoryRegion, 1);
@@ -628,7 +630,10 @@ static void vexpress_common_init(MachineState *machine)
     sysbus_create_simple("sp804", map[VE_TIMER01], pic[2]);
     sysbus_create_simple("sp804", map[VE_TIMER23], pic[3]);
 
-    /* VE_SERIALDVI: not modelled */
+    dev = sysbus_create_simple("versatile_i2c", map[VE_SERIALDVI], NULL);
+    i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
+    i2c_create_slave(i2c, "sii9022", 0x39);
+    i2c_create_slave(i2c, "i2c-ddc", 0x50);
 
     sysbus_create_simple("pl031", map[VE_RTC], pic[4]); /* RTC */
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation Linus Walleij
@ 2018-02-17 18:28   ` Philippe Mathieu-Daudé
  2018-02-19 14:10     ` Linus Walleij
  0 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-17 18:28 UTC (permalink / raw)
  To: Linus Walleij, qemu-devel; +Cc: qemu-arm

Hi Linus,

On 02/17/2018 11:00 AM, Linus Walleij wrote:
> This adds the SiI9022 and EDID I2C devices to the ARM Versatile
> Express machine, and selects the two I2C devices necessary in the
> arm-softmmy.mak configuration so everything will build smoothly.
> 
> I am implementing proper handling of the graphics in the Linux
> kernel and adding proper emulation of SiI9022 and EDID makes the
> driver probe as nicely as before, retrieveing the resolutions
> supported by the "QEMU monitor" and overall just working nice.
> 
> The assignment of the SiI9022 at address 0x39 and the EDID
> DDC I2C at address 0x50 is not strictly correct: the DDC I2C
> is there all the time but in the actual component it only
> appears once activated inside the SiI9022, so ideally it should
> be added and removed to the bus by the SiI9022. However for this
> purpose it works fine to just have it around.

This seems easier to just do it now rather than postpone :)

In your patch #2:

static void sii9022_realize(DeviceState *dev, Error **errp)
{
    I2CBus *bus;

    bus = I2C_BUS(qdev_get_parent_bus(dev));
    i2c_create_slave(bus, TYPE_I2CDDC, 0x50);
}

static void sii9022_class_init(ObjectClass *klass, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(klass);

...
    dc->realize = sii9022_realize;
}

> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  default-configs/arm-softmmu.mak | 2 ++
>  hw/arm/vexpress.c               | 7 ++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index ca34cf446242..54f855d07206 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -21,6 +21,8 @@ CONFIG_STELLARIS_INPUT=y
>  CONFIG_STELLARIS_ENET=y
>  CONFIG_SSD0303=y
>  CONFIG_SSD0323=y
> +CONFIG_DDC=y
> +CONFIG_SII9022=y
>  CONFIG_ADS7846=y
>  CONFIG_MAX111X=y
>  CONFIG_SSI=y
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index dc5928ae1ab5..d6c912c97684 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -29,6 +29,7 @@
>  #include "hw/arm/arm.h"
>  #include "hw/arm/primecell.h"
>  #include "hw/devices.h"
> +#include "hw/i2c/i2c.h"
>  #include "net/net.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
> @@ -537,6 +538,7 @@ static void vexpress_common_init(MachineState *machine)
>      uint32_t sys_id;
>      DriveInfo *dinfo;
>      pflash_t *pflash0;
> +    I2CBus *i2c;
>      ram_addr_t vram_size, sram_size;
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *vram = g_new(MemoryRegion, 1);
> @@ -628,7 +630,10 @@ static void vexpress_common_init(MachineState *machine)
>      sysbus_create_simple("sp804", map[VE_TIMER01], pic[2]);
>      sysbus_create_simple("sp804", map[VE_TIMER23], pic[3]);
>  
> -    /* VE_SERIALDVI: not modelled */
> +    dev = sysbus_create_simple("versatile_i2c", map[VE_SERIALDVI], NULL);
> +    i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
> +    i2c_create_slave(i2c, "sii9022", 0x39);
> +    i2c_create_slave(i2c, "i2c-ddc", 0x50);
>  
>      sysbus_create_simple("pl031", map[VE_RTC], pic[4]); /* RTC */
>  
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
@ 2018-02-17 18:32   ` Philippe Mathieu-Daudé
  2018-02-27  7:41     ` Linus Walleij
  2018-02-22 14:42   ` Peter Maydell
  1 sibling, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-17 18:32 UTC (permalink / raw)
  To: Linus Walleij, qemu-devel; +Cc: qemu-arm

Hi Linus,

On 02/17/2018 11:00 AM, Linus Walleij wrote:
> This adds support for emulating the Silicon Image SII9022 DVI/HDMI
> bridge. It's not very clever right now, it just acknowledges
> the switch into DDC I2C mode and back. Combining this with the
> existing DDC I2C emulation gives the right behavior on the Versatile
> Express emulation passing through the QEMU EDID to the emulated
> platform.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  hw/display/Makefile.objs |   1 +
>  hw/display/sii9022.c     | 185 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 186 insertions(+)
>  create mode 100644 hw/display/sii9022.c
> 
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index d3a4cb396eb9..3c7c75b94da5 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
>  common-obj-$(CONFIG_G364FB) += g364fb.o
>  common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
>  common-obj-$(CONFIG_PL110) += pl110.o
> +common-obj-$(CONFIG_SII9022) += sii9022.o
>  common-obj-$(CONFIG_SSD0303) += ssd0303.o
>  common-obj-$(CONFIG_SSD0323) += ssd0323.o
>  common-obj-$(CONFIG_XEN) += xenfb.o
> diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c
> new file mode 100644
> index 000000000000..d6f3cdc04293
> --- /dev/null
> +++ b/hw/display/sii9022.c
> @@ -0,0 +1,185 @@
> +/*
> + * Silicon Image SiI9022
> + *
> + * This is a pretty hollow emulation: all we do is acknowledge that we
> + * exist (chip ID) and confirm that we get switched over into DDC mode
> + * so the emulated host can proceed to read out EDID data. All subsequent
> + * set-up of connectors etc will be acknowledged and ignored.
> + *
> + * Copyright (c) 2018 Linus Walleij
> + *
> + * This code is licensed under the GNU GPL v2.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "hw/i2c/i2c.h"
> +
> +#define DEBUG_SII9022 0
> +
> +#define DPRINTF(fmt, ...) \
> +    do { \
> +        if (DEBUG_SII9022) { \
> +            printf("sii9022: " fmt, ## __VA_ARGS__); \
> +        } \
> +    } while (0)

Can you replace DPRINTF() by trace events?

Except that, patch looks fine.

Regards,

Phil.

> +
> +#define SII9022_SYS_CTRL_DATA 0x1a
> +#define SII9022_SYS_CTRL_PWR_DWN 0x10
> +#define SII9022_SYS_CTRL_AV_MUTE 0x08
> +#define SII9022_SYS_CTRL_DDC_BUS_REQ 0x04
> +#define SII9022_SYS_CTRL_DDC_BUS_GRTD 0x02
> +#define SII9022_SYS_CTRL_OUTPUT_MODE 0x01
> +#define SII9022_SYS_CTRL_OUTPUT_HDMI 1
> +#define SII9022_SYS_CTRL_OUTPUT_DVI 0
> +#define SII9022_REG_CHIPID 0x1b
> +#define SII9022_INT_ENABLE 0x3c
> +#define SII9022_INT_STATUS 0x3d
> +#define SII9022_INT_STATUS_HOTPLUG 0x01;
> +#define SII9022_INT_STATUS_PLUGGED 0x04;
> +
> +#define TYPE_SII9022 "sii9022"
> +#define SII9022(obj) OBJECT_CHECK(sii9022_state, (obj), TYPE_SII9022)
> +
> +typedef struct sii9022_state {
> +    I2CSlave parent_obj;
> +    uint8_t ptr;
> +    bool addr_byte;
> +    bool ddc_req;
> +    bool ddc_skip_finish;
> +    bool ddc;
> +} sii9022_state;
> +
> +static const VMStateDescription vmstate_sii9022 = {
> +    .name = "sii9022",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_I2C_SLAVE(parent_obj, sii9022_state),
> +        VMSTATE_UINT8(ptr, sii9022_state),
> +        VMSTATE_BOOL(addr_byte, sii9022_state),
> +        VMSTATE_BOOL(ddc_req, sii9022_state),
> +        VMSTATE_BOOL(ddc_skip_finish, sii9022_state),
> +        VMSTATE_BOOL(ddc, sii9022_state),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static int sii9022_event(I2CSlave *i2c, enum i2c_event event)
> +{
> +    sii9022_state *s = SII9022(i2c);
> +
> +    switch (event) {
> +    case I2C_START_SEND:
> +        s->addr_byte = true;
> +        break;
> +    case I2C_START_RECV:
> +        break;
> +    case I2C_FINISH:
> +        break;
> +    case I2C_NACK:
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
> +static int sii9022_rx(I2CSlave *i2c)
> +{
> +    sii9022_state *s = SII9022(i2c);
> +    uint8_t res = 0x00;
> +
> +    switch (s->ptr) {
> +    case SII9022_SYS_CTRL_DATA:
> +        if (s->ddc_req) {
> +            /* Acknowledge DDC bus request */
> +            res = SII9022_SYS_CTRL_DDC_BUS_GRTD | SII9022_SYS_CTRL_DDC_BUS_REQ;
> +        }
> +        break;
> +    case SII9022_REG_CHIPID:
> +        res = 0xb0;
> +        break;
> +    case SII9022_INT_STATUS:
> +        /* Something is cold-plugged in, no interrupts */
> +        res = SII9022_INT_STATUS_PLUGGED;
> +        break;
> +    default:
> +        break;
> +    }
> +    DPRINTF("%02x read from %02x\n", res, s->ptr);
> +    s->ptr++;
> +
> +    return res;
> +}
> +
> +static int sii9022_tx(I2CSlave *i2c, uint8_t data)
> +{
> +    sii9022_state *s = SII9022(i2c);
> +
> +    if (s->addr_byte) {
> +        s->ptr = data;
> +        s->addr_byte = false;
> +        return 0;
> +    }
> +
> +    switch (s->ptr) {
> +    case SII9022_SYS_CTRL_DATA:
> +        if (data & SII9022_SYS_CTRL_DDC_BUS_REQ) {
> +            s->ddc_req = true;
> +            if (data & SII9022_SYS_CTRL_DDC_BUS_GRTD) {
> +                s->ddc = true;
> +                /* Skip this finish since we just switched to DDC */
> +                s->ddc_skip_finish = true;
> +                DPRINTF("switched to DDC mode\n");
> +            }
> +        } else {
> +            s->ddc_req = false;
> +            s->ddc = false;
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    DPRINTF("%02x written to %02x\n", data, s->ptr);
> +    s->ptr++;
> +
> +    return 0;
> +}
> +
> +static void sii9022_reset(DeviceState *dev)
> +{
> +    sii9022_state *s = SII9022(dev);
> +
> +    s->ptr = 0;
> +    s->addr_byte = false;
> +}
> +
> +static void sii9022_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> +    k->event = sii9022_event;
> +    k->recv = sii9022_rx;
> +    k->send = sii9022_tx;
> +    dc->reset = sii9022_reset;
> +    dc->vmsd = &vmstate_sii9022;
> +}
> +
> +static const TypeInfo sii9022_info = {
> +    .name          = TYPE_SII9022,
> +    .parent        = TYPE_I2C_SLAVE,
> +    .instance_size = sizeof(sii9022_state),
> +    .class_init    = sii9022_class_init,
> +};
> +
> +static void sii9022_register_types(void)
> +{
> +    type_register_static(&sii9022_info);
> +}
> +
> +type_init(sii9022_register_types)
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation
  2018-02-17 18:28   ` Philippe Mathieu-Daudé
@ 2018-02-19 14:10     ` Linus Walleij
  2018-02-19 15:16       ` Corey Minyard
  2018-02-19 15:20       ` [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h minyard
  0 siblings, 2 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-19 14:10 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-arm

On Sat, Feb 17, 2018 at 7:28 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/17/2018 11:00 AM, Linus Walleij wrote:

>> The assignment of the SiI9022 at address 0x39 and the EDID
>> DDC I2C at address 0x50 is not strictly correct: the DDC I2C
>> is there all the time but in the actual component it only
>> appears once activated inside the SiI9022, so ideally it should
>> be added and removed to the bus by the SiI9022. However for this
>> purpose it works fine to just have it around.
>
> This seems easier to just do it now rather than postpone :)
>
> In your patch #2:
>
> static void sii9022_realize(DeviceState *dev, Error **errp)
> {
>     I2CBus *bus;
>
>     bus = I2C_BUS(qdev_get_parent_bus(dev));
>     i2c_create_slave(bus, TYPE_I2CDDC, 0x50);
> }

I tried an approach like this but the I2C_BUS() macro was not
publicly visible in any header file and trying to pry it out just
brought along a lot of refactoring in the i2c core.c file that
scared me away...

Has someone else fixed it already?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation
  2018-02-19 14:10     ` Linus Walleij
@ 2018-02-19 15:16       ` Corey Minyard
  2018-02-19 15:20       ` [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h minyard
  1 sibling, 0 replies; 25+ messages in thread
From: Corey Minyard @ 2018-02-19 15:16 UTC (permalink / raw)
  To: Linus Walleij, Philippe Mathieu-Daudé; +Cc: qemu-arm, qemu-devel

On 02/19/2018 08:10 AM, Linus Walleij wrote:
> On Sat, Feb 17, 2018 at 7:28 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> On 02/17/2018 11:00 AM, Linus Walleij wrote:
>>> The assignment of the SiI9022 at address 0x39 and the EDID
>>> DDC I2C at address 0x50 is not strictly correct: the DDC I2C
>>> is there all the time but in the actual component it only
>>> appears once activated inside the SiI9022, so ideally it should
>>> be added and removed to the bus by the SiI9022. However for this
>>> purpose it works fine to just have it around.
>> This seems easier to just do it now rather than postpone :)
>>
>> In your patch #2:
>>
>> static void sii9022_realize(DeviceState *dev, Error **errp)
>> {
>>      I2CBus *bus;
>>
>>      bus = I2C_BUS(qdev_get_parent_bus(dev));
>>      i2c_create_slave(bus, TYPE_I2CDDC, 0x50);
>> }
> I tried an approach like this but the I2C_BUS() macro was not
> publicly visible in any header file and trying to pry it out just
> brought along a lot of refactoring in the i2c core.c file that
> scared me away...
>
> Has someone else fixed it already?

I have a patch that does this.  Well, it's a small cleanup patch and another
patch, but it's really no that bad.  I'll send it.

-corey

>
> Yours,
> Linus Walleij
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h
  2018-02-19 14:10     ` Linus Walleij
  2018-02-19 15:16       ` Corey Minyard
@ 2018-02-19 15:20       ` minyard
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues minyard
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h minyard
  1 sibling, 2 replies; 25+ messages in thread
From: minyard @ 2018-02-19 15:20 UTC (permalink / raw)
  To: Linus Walleij, Philippe Mathieu-Daudé; +Cc: qemu-arm, qemu-devel

This is a couple of changes I have because I have a bus mux emulator
and a multi-master bus mux emulator.  Others need this change, too,
so get it out there.

-corey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues
  2018-02-19 15:20       ` [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h minyard
@ 2018-02-19 15:20         ` minyard
  2018-02-19 15:24           ` Peter Maydell
  2018-02-22 15:37           ` Linus Walleij
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h minyard
  1 sibling, 2 replies; 25+ messages in thread
From: minyard @ 2018-02-19 15:20 UTC (permalink / raw)
  To: Linus Walleij, Philippe Mathieu-Daudé
  Cc: qemu-arm, qemu-devel, Corey Minyard

From: Corey Minyard <cminyard@mvista.com>

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/i2c/core.c        | 3 +--
 include/hw/i2c/i2c.h | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 59068f1..9a54b61 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -19,8 +19,7 @@ struct I2CNode {
 
 #define I2C_BROADCAST 0x00
 
-struct I2CBus
-{
+struct I2CBus {
     BusState qbus;
     QLIST_HEAD(, I2CNode) current_devs;
     uint8_t saved_address;
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 24e95d0..8fd449f 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -25,8 +25,7 @@ typedef struct I2CSlave I2CSlave;
 #define I2C_SLAVE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(I2CSlaveClass, (obj), TYPE_I2C_SLAVE)
 
-typedef struct I2CSlaveClass
-{
+typedef struct I2CSlaveClass {
     DeviceClass parent_class;
 
     /* Callbacks provided by the device.  */
@@ -50,8 +49,7 @@ typedef struct I2CSlaveClass
     int (*event)(I2CSlave *s, enum i2c_event event);
 } I2CSlaveClass;
 
-struct I2CSlave
-{
+struct I2CSlave {
     DeviceState qdev;
 
     /* Remaining fields for internal use by the I2C code.  */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-19 15:20       ` [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h minyard
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues minyard
@ 2018-02-19 15:20         ` minyard
  2018-02-19 15:25           ` Peter Maydell
  2018-02-22 15:38           ` Linus Walleij
  1 sibling, 2 replies; 25+ messages in thread
From: minyard @ 2018-02-19 15:20 UTC (permalink / raw)
  To: Linus Walleij, Philippe Mathieu-Daudé
  Cc: qemu-arm, qemu-devel, Corey Minyard

From: Corey Minyard <cminyard@mvista.com>

Some devices need access to it.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/i2c/core.c        | 17 -----------------
 include/hw/i2c/i2c.h | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 9a54b61..cfccefc 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -10,30 +10,13 @@
 #include "qemu/osdep.h"
 #include "hw/i2c/i2c.h"
 
-typedef struct I2CNode I2CNode;
-
-struct I2CNode {
-    I2CSlave *elt;
-    QLIST_ENTRY(I2CNode) next;
-};
-
 #define I2C_BROADCAST 0x00
 
-struct I2CBus {
-    BusState qbus;
-    QLIST_HEAD(, I2CNode) current_devs;
-    uint8_t saved_address;
-    bool broadcast;
-};
-
 static Property i2c_props[] = {
     DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-#define TYPE_I2C_BUS "i2c-bus"
-#define I2C_BUS(obj) OBJECT_CHECK(I2CBus, (obj), TYPE_I2C_BUS)
-
 static const TypeInfo i2c_bus_info = {
     .name = TYPE_I2C_BUS,
     .parent = TYPE_BUS,
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 8fd449f..d727379 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -56,6 +56,23 @@ struct I2CSlave {
     uint8_t address;
 };
 
+#define TYPE_I2C_BUS "i2c-bus"
+#define I2C_BUS(obj) OBJECT_CHECK(I2CBus, (obj), TYPE_I2C_BUS)
+
+typedef struct I2CNode I2CNode;
+
+struct I2CNode {
+    I2CSlave *elt;
+    QLIST_ENTRY(I2CNode) next;
+};
+
+struct I2CBus {
+    BusState qbus;
+    QLIST_HEAD(, I2CNode) current_devs;
+    uint8_t saved_address;
+    bool broadcast;
+};
+
 I2CBus *i2c_init_bus(DeviceState *parent, const char *name);
 void i2c_set_slave_address(I2CSlave *dev, uint8_t address);
 int i2c_bus_busy(I2CBus *bus);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues minyard
@ 2018-02-19 15:24           ` Peter Maydell
  2018-02-22 15:37           ` Linus Walleij
  1 sibling, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2018-02-19 15:24 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Linus Walleij, Philippe Mathieu-Daudé,
	Corey Minyard, qemu-arm, QEMU Developers

On 19 February 2018 at 15:20,  <minyard@acm.org> wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h minyard
@ 2018-02-19 15:25           ` Peter Maydell
  2018-02-20 13:06             ` Corey Minyard
  2018-02-22 15:38           ` Linus Walleij
  1 sibling, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2018-02-19 15:25 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Linus Walleij, Philippe Mathieu-Daudé,
	Corey Minyard, qemu-arm, QEMU Developers

On 19 February 2018 at 15:20,  <minyard@acm.org> wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Some devices need access to it.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  hw/i2c/core.c        | 17 -----------------
>  include/hw/i2c/i2c.h | 17 +++++++++++++++++
>  2 files changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-19 15:25           ` Peter Maydell
@ 2018-02-20 13:06             ` Corey Minyard
  2018-02-22 15:39               ` Linus Walleij
  0 siblings, 1 reply; 25+ messages in thread
From: Corey Minyard @ 2018-02-20 13:06 UTC (permalink / raw)
  To: Linus Walleij, Philippe Mathieu-Daudé
  Cc: Peter Maydell, Corey Minyard, qemu-arm, QEMU Developers

On 02/19/2018 09:25 AM, Peter Maydell wrote:
> On 19 February 2018 at 15:20,  <minyard@acm.org> wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> Some devices need access to it.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>>   hw/i2c/core.c        | 17 -----------------
>>   include/hw/i2c/i2c.h | 17 +++++++++++++++++
>>   2 files changed, 17 insertions(+), 17 deletions(-)
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Linus, Philippe, do you want me to submit this, or do you want
to take it?  You can pull it from:

https://github.com/cminyard/qemu.git tags/i2c-bus-move

-corey

> thanks
> -- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/i2c-ddc: Do not fail writes
  2018-02-17 14:00 [Qemu-devel] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Linus Walleij
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation Linus Walleij
@ 2018-02-22 14:36 ` Peter Maydell
  2 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2018-02-22 14:36 UTC (permalink / raw)
  To: Linus Walleij; +Cc: QEMU Developers, qemu-arm

On 17 February 2018 at 14:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> The tx function of the DDC I2C slave emulation was returning 1
> on all writes resulting in NACK in the I2C bus. Changing it to
> 0 makes the DDC I2C work fine with bit-banged I2C such as the
> versatile I2C.
>
> I guess it was not affecting whatever I2C controller this was
> used with until now, but with the Versatile I2C it surely
> does not work.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  hw/i2c/i2c-ddc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c
> index 199dac9e41c1..bec0c91e2dd0 100644
> --- a/hw/i2c/i2c-ddc.c
> +++ b/hw/i2c/i2c-ddc.c
> @@ -259,12 +259,12 @@ static int i2c_ddc_tx(I2CSlave *i2c, uint8_t data)
>          s->reg = data;
>          s->firstbyte = false;
>          DPRINTF("[EDID] Written new pointer: %u\n", data);
> -        return 1;
> +        return 0;
>      }
>
>      /* Ignore all writes */
>      s->reg++;
> -    return 1;
> +    return 0;
>  }
>
>  static void i2c_ddc_init(Object *obj)
> --
> 2.14.3

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Does the guest attempting writes mean that we're missing functionality
that the hardware has? Is it interesting?

PS: I didn't see a cover letter email with this patchset --
if you can send a cover letter with multi-email patchsets that makes
our automated tooling much happier.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
  2018-02-17 18:32   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-02-22 14:42   ` Peter Maydell
  1 sibling, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2018-02-22 14:42 UTC (permalink / raw)
  To: Linus Walleij; +Cc: QEMU Developers, qemu-arm

On 17 February 2018 at 14:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> This adds support for emulating the Silicon Image SII9022 DVI/HDMI
> bridge. It's not very clever right now, it just acknowledges
> the switch into DDC I2C mode and back. Combining this with the
> existing DDC I2C emulation gives the right behavior on the Versatile
> Express emulation passing through the QEMU EDID to the emulated
> platform.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  hw/display/Makefile.objs |   1 +
>  hw/display/sii9022.c     | 185 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 186 insertions(+)
>  create mode 100644 hw/display/sii9022.c
>
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index d3a4cb396eb9..3c7c75b94da5 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
>  common-obj-$(CONFIG_G364FB) += g364fb.o
>  common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
>  common-obj-$(CONFIG_PL110) += pl110.o
> +common-obj-$(CONFIG_SII9022) += sii9022.o
>  common-obj-$(CONFIG_SSD0303) += ssd0303.o
>  common-obj-$(CONFIG_SSD0323) += ssd0323.o
>  common-obj-$(CONFIG_XEN) += xenfb.o
> diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c
> new file mode 100644
> index 000000000000..d6f3cdc04293
> --- /dev/null
> +++ b/hw/display/sii9022.c
> @@ -0,0 +1,185 @@
> +/*
> + * Silicon Image SiI9022
> + *
> + * This is a pretty hollow emulation: all we do is acknowledge that we
> + * exist (chip ID) and confirm that we get switched over into DDC mode
> + * so the emulated host can proceed to read out EDID data. All subsequent
> + * set-up of connectors etc will be acknowledged and ignored.
> + *
> + * Copyright (c) 2018 Linus Walleij

Sanity check: really copyright you and not Linaro ?

> + *
> + * This code is licensed under the GNU GPL v2.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.

If this is copyright 2018 then what parts of it are not covered by
the "contributions after 2012-01-13" condition? Could we just have
this be GPL-v2-or-later, which is what most new code is ?


> +static void sii9022_reset(DeviceState *dev)
> +{
> +    sii9022_state *s = SII9022(dev);
> +
> +    s->ptr = 0;
> +    s->addr_byte = false;

I think we should reset ddc/ddc_skip_finish/ddc_req
here too. I suspect the state machine logic means we
can't get to a place where their values are used once
we reset ptr and addr_byte, but it's easier to reason
about if we just reset all the device's state.

I agree with Philippe that we should use trace events here rather
than the DPRINTF macro. Otherwise the code looks fine.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues minyard
  2018-02-19 15:24           ` Peter Maydell
@ 2018-02-22 15:37           ` Linus Walleij
  1 sibling, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-22 15:37 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Philippe Mathieu-Daudé, qemu-arm, qemu-devel, Corey Minyard

On Mon, Feb 19, 2018 at 4:20 PM,  <minyard@acm.org> wrote:

> From: Corey Minyard <cminyard@mvista.com>
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-19 15:20         ` [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h minyard
  2018-02-19 15:25           ` Peter Maydell
@ 2018-02-22 15:38           ` Linus Walleij
  1 sibling, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-22 15:38 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Philippe Mathieu-Daudé, qemu-arm, qemu-devel, Corey Minyard

On Mon, Feb 19, 2018 at 4:20 PM,  <minyard@acm.org> wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Some devices need access to it.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-20 13:06             ` Corey Minyard
@ 2018-02-22 15:39               ` Linus Walleij
  2018-02-22 15:44                 ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2018-02-22 15:39 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Philippe Mathieu-Daudé,
	Peter Maydell, Corey Minyard, qemu-arm, QEMU Developers

On Tue, Feb 20, 2018 at 2:06 PM, Corey Minyard <minyard@acm.org> wrote:
> On 02/19/2018 09:25 AM, Peter Maydell wrote:
>>
>> On 19 February 2018 at 15:20,  <minyard@acm.org> wrote:
>>>
>>> From: Corey Minyard <cminyard@mvista.com>
>>>
>>> Some devices need access to it.
>>>
>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>> ---
>>>   hw/i2c/core.c        | 17 -----------------
>>>   include/hw/i2c/i2c.h | 17 +++++++++++++++++
>>>   2 files changed, 17 insertions(+), 17 deletions(-)
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
>
> Linus, Philippe, do you want me to submit this, or do you want
> to take it?  You can pull it from:
>
> https://github.com/cminyard/qemu.git tags/i2c-bus-move

I don't have any commit rights, if you can push this to the
QEMU master I will happily rebase my stuff and resubmit :)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-22 15:39               ` Linus Walleij
@ 2018-02-22 15:44                 ` Peter Maydell
  2018-02-22 15:55                   ` Linus Walleij
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2018-02-22 15:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Corey Minyard, Philippe Mathieu-Daudé,
	Corey Minyard, qemu-arm, QEMU Developers

On 22 February 2018 at 15:39, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Feb 20, 2018 at 2:06 PM, Corey Minyard <minyard@acm.org> wrote:
>> Linus, Philippe, do you want me to submit this, or do you want
>> to take it?  You can pull it from:
>>
>> https://github.com/cminyard/qemu.git tags/i2c-bus-move
>
> I don't have any commit rights, if you can push this to the
> QEMU master I will happily rebase my stuff and resubmit :)

I suggest you take Corey's 2 patches, add them to the front
of your patchset and add your signed-off-by line to them.
That way you don't have to wait for them to go into git master.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h
  2018-02-22 15:44                 ` Peter Maydell
@ 2018-02-22 15:55                   ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2018-02-22 15:55 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Philippe Mathieu-Daudé,
	Corey Minyard, qemu-arm, QEMU Developers

On Thu, Feb 22, 2018 at 4:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 February 2018 at 15:39, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Tue, Feb 20, 2018 at 2:06 PM, Corey Minyard <minyard@acm.org> wrote:
>>> Linus, Philippe, do you want me to submit this, or do you want
>>> to take it?  You can pull it from:
>>>
>>> https://github.com/cminyard/qemu.git tags/i2c-bus-move
>>
>> I don't have any commit rights, if you can push this to the
>> QEMU master I will happily rebase my stuff and resubmit :)
>
> I suggest you take Corey's 2 patches, add them to the front
> of your patchset and add your signed-off-by line to them.
> That way you don't have to wait for them to go into git master.

OK no problem!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-17 18:32   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-02-27  7:41     ` Linus Walleij
  2018-02-27 10:09       ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2018-02-27  7:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: QEMU Developers, qemu-arm

On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> [Me]
>> +#define DEBUG_SII9022 0
>> +
>> +#define DPRINTF(fmt, ...) \
>> +    do { \
>> +        if (DEBUG_SII9022) { \
>> +            printf("sii9022: " fmt, ## __VA_ARGS__); \
>> +        } \
>> +    } while (0)
>
> Can you replace DPRINTF() by trace events?

Absolutely but which ones?

I do not feel senior enough to also invent new trace events
for displays or I2C devices...

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-27  7:41     ` Linus Walleij
@ 2018-02-27 10:09       ` Peter Maydell
  2018-02-27 10:21         ` Linus Walleij
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2018-02-27 10:09 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Philippe Mathieu-Daudé, qemu-arm, QEMU Developers

On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
>> [Me]
>>> +#define DEBUG_SII9022 0
>>> +
>>> +#define DPRINTF(fmt, ...) \
>>> +    do { \
>>> +        if (DEBUG_SII9022) { \
>>> +            printf("sii9022: " fmt, ## __VA_ARGS__); \
>>> +        } \
>>> +    } while (0)
>>
>> Can you replace DPRINTF() by trace events?
>
> Absolutely but which ones?
>
> I do not feel senior enough to also invent new trace events
> for displays or I2C devices...

Just put a trace event where you've put DPRINTF debug statements.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-27 10:09       ` Peter Maydell
@ 2018-02-27 10:21         ` Linus Walleij
  2018-02-27 10:24           ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2018-02-27 10:21 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Philippe Mathieu-Daudé, qemu-arm, QEMU Developers

On Tue, Feb 27, 2018 at 11:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>>> [Me]
>>>> +#define DEBUG_SII9022 0
>>>> +
>>>> +#define DPRINTF(fmt, ...) \
>>>> +    do { \
>>>> +        if (DEBUG_SII9022) { \
>>>> +            printf("sii9022: " fmt, ## __VA_ARGS__); \
>>>> +        } \
>>>> +    } while (0)
>>>
>>> Can you replace DPRINTF() by trace events?
>>
>> Absolutely but which ones?
>>
>> I do not feel senior enough to also invent new trace events
>> for displays or I2C devices...
>
> Just put a trace event where you've put DPRINTF debug statements.

Yeah, hm the question might be silly or something but I don't
know how to do that.

I guess I should include "trace.h".

#include "trace.h" says it is autogenerated from tracetool.

Is there some documentation I should be reading or a good
example commit I can study to get the hang of it?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-27 10:21         ` Linus Walleij
@ 2018-02-27 10:24           ` Peter Maydell
  2018-02-27 10:29             ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2018-02-27 10:24 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Philippe Mathieu-Daudé, qemu-arm, QEMU Developers

On 27 February 2018 at 10:21, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Feb 27, 2018 at 11:09 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>>> [Me]
>>>>> +#define DEBUG_SII9022 0
>>>>> +
>>>>> +#define DPRINTF(fmt, ...) \
>>>>> +    do { \
>>>>> +        if (DEBUG_SII9022) { \
>>>>> +            printf("sii9022: " fmt, ## __VA_ARGS__); \
>>>>> +        } \
>>>>> +    } while (0)
>>>>
>>>> Can you replace DPRINTF() by trace events?
>>>
>>> Absolutely but which ones?
>>>
>>> I do not feel senior enough to also invent new trace events
>>> for displays or I2C devices...
>>
>> Just put a trace event where you've put DPRINTF debug statements.
>
> Yeah, hm the question might be silly or something but I don't
> know how to do that.

docs/devel/tracing.txt describes them, but basically:
 * include "trace.h"
 * define trace events with a line for each in the trace-events
   file for the subdirectory (basically a function prototype-ish
   thing followed by a printf-style format string)
 * call them like normal function calls with a trace_ prefix

commit 1b640aa9292bc00beb441e97d862ba322a7ba18d is a recent one
which converted some DRINTFs in hw/sd/sd.c to trace events.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022
  2018-02-27 10:24           ` Peter Maydell
@ 2018-02-27 10:29             ` Peter Maydell
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2018-02-27 10:29 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Philippe Mathieu-Daudé, qemu-arm, QEMU Developers

On 27 February 2018 at 10:24, Peter Maydell <peter.maydell@linaro.org> wrote:
> docs/devel/tracing.txt describes them, but basically:
>  * include "trace.h"
>  * define trace events with a line for each in the trace-events
>    file for the subdirectory (basically a function prototype-ish
>    thing followed by a printf-style format string)
>  * call them like normal function calls with a trace_ prefix
>
> commit 1b640aa9292bc00beb441e97d862ba322a7ba18d is a recent one
> which converted some DRINTFs in hw/sd/sd.c to trace events.

...and you can test them most conveniently with the QEMU
-d argument: "-d trace:sii9022*" will enable tracing to stderr
of all trace events with names that begin 'sii9022'.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-02-27 10:29 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-17 14:00 [Qemu-devel] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Linus Walleij
2018-02-17 14:00 ` [Qemu-devel] [PATCH 2/3] hw/sii9022: Add support for Silicon Image SII9022 Linus Walleij
2018-02-17 18:32   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-02-27  7:41     ` Linus Walleij
2018-02-27 10:09       ` Peter Maydell
2018-02-27 10:21         ` Linus Walleij
2018-02-27 10:24           ` Peter Maydell
2018-02-27 10:29             ` Peter Maydell
2018-02-22 14:42   ` Peter Maydell
2018-02-17 14:00 ` [Qemu-devel] [PATCH 3/3] arm/vexpress: Add proper display connector emulation Linus Walleij
2018-02-17 18:28   ` Philippe Mathieu-Daudé
2018-02-19 14:10     ` Linus Walleij
2018-02-19 15:16       ` Corey Minyard
2018-02-19 15:20       ` [Qemu-devel] [PATCH 0/2] Move the bus class to i2c.h minyard
2018-02-19 15:20         ` [Qemu-devel] [PATCH 1/2] i2c: Fix some brace style issues minyard
2018-02-19 15:24           ` Peter Maydell
2018-02-22 15:37           ` Linus Walleij
2018-02-19 15:20         ` [Qemu-devel] [PATCH 2/2] i2c: Move the bus class to i2c.h minyard
2018-02-19 15:25           ` Peter Maydell
2018-02-20 13:06             ` Corey Minyard
2018-02-22 15:39               ` Linus Walleij
2018-02-22 15:44                 ` Peter Maydell
2018-02-22 15:55                   ` Linus Walleij
2018-02-22 15:38           ` Linus Walleij
2018-02-22 14:36 ` [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/i2c-ddc: Do not fail writes Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.