Skip to contentSkip to conversation

i2c-tiny-usb

RFC PATCH In review Tim Sander 5 participants 13 messages Nov 26, 2015 – Dec 17, 2015 1 message not in the archive

lore
Share & tools
lore

Tim Sander

author
#0 lore raw

RFC PATCH 1/1 i2c-tiny-usb

Hi

Below is a patch implementing the i2c-tiny-usb device. I am currently not sure about the i2c semantics. I think incrementing the address on longer reads is wrong? But at least i can read the high byte(?) of the temperature via "i2cget -y 0 0x50".

With this device it should be possible to define i2c busses via command line e.g:
-device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50 have been used for the first test.

Best regards
Tim

---
 default-configs/usb.mak |   1 +
 hw/usb/Makefile.objs    |   1 +
 hw/usb/dev-i2c-tiny.c   | 383 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 385 insertions(+)
 create mode 100644 hw/usb/dev-i2c-tiny.c
default-configs/usb.mak +1 -0
index f4b8568..01d2c9f 100644
@@ -8,3 +8,4 @@ CONFIG_USB_AUDIO=y
 CONFIG_USB_SERIAL=y
 CONFIG_USB_NETWORK=y
 CONFIG_USB_BLUETOOTH=y
+CONFIG_USB_I2C_TINY=y
hw/usb/Makefile.objs +1 -0
index 8f00fbd..3a4c337 100644
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_USB_AUDIO)        += dev-audio.o
 common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
 common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
+common-obj-$(CONFIG_USB_I2C_TINY)     += dev-i2c-tiny.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
hw/usb/dev-i2c-tiny.c new file +383 -0

383 lines not on this page · raw message

Signature · 1 line
1.9.1
#1 lore raw
Quoted 8 lines · Tim Sander

Hi

Below is a patch implementing the i2c-tiny-usb device. I am currently not sure about the i2c semantics. I think incrementing the address on longer reads is wrong? But at least i can read the high byte(?) of the temperature via "i2cget -y 0 0x50".

Quoted 4 lines

With this device it should be possible to define i2c busses via command line e.g:
-device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50 have been used for the first test.

You are missing a s-o-b line and scripts/checkpatch.pl complains about a few things you should fix before your next submission.

Quoted 166 lines

Best regards
Tim

---
 default-configs/usb.mak |   1 +
 hw/usb/Makefile.objs    |   1 +
 hw/usb/dev-i2c-tiny.c   | 383 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 385 insertions(+)
 create mode 100644 hw/usb/dev-i2c-tiny.c
default-configs/usb.mak +1 -0
index f4b8568..01d2c9f 100644
@@ -8,3 +8,4 @@ CONFIG_USB_AUDIO=y
 CONFIG_USB_SERIAL=y
 CONFIG_USB_NETWORK=y
 CONFIG_USB_BLUETOOTH=y
+CONFIG_USB_I2C_TINY=y
hw/usb/Makefile.objs +1 -0
index 8f00fbd..3a4c337 100644
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_USB_AUDIO)        += dev-audio.o
 common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
 common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
+common-obj-$(CONFIG_USB_I2C_TINY)     += dev-i2c-tiny.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
hw/usb/dev-i2c-tiny.c new file +129 -0
new file mode 100644
index 0000000..1dabb36
@@ -0,0 +1,383 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * Copyright (c) 2015 Tim Sander <[email protected]>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <[email protected]>
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* #define DEBUG_USBI2C */
+
+#ifdef DEBUG_USBI2C
+#define DPRINTF(fmt, ...) \
+do { printf("usb-i2c-tiny: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+/* commands from USB, must e.g. match command ids in kernel driver */
+#define CMD_ECHO       0
+#define CMD_GET_FUNC   1
+#define CMD_SET_DELAY  2
+#define CMD_GET_STATUS 3
+
+/* To determine what functionality is present */
+#define I2C_FUNC_I2C                            0x00000001
+#define I2C_FUNC_10BIT_ADDR                     0x00000002
+#define I2C_FUNC_PROTOCOL_MANGLING              0x00000004
+#define I2C_FUNC_SMBUS_HWPEC_CALC               0x00000008 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_READ_WORD_DATA_PEC       0x00000800 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA_PEC      0x00001000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_PROC_CALL_PEC            0x00002000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL_PEC      0x00004000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL          0x00008000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_QUICK                    0x00010000
+#define I2C_FUNC_SMBUS_READ_BYTE                0x00020000
+#define I2C_FUNC_SMBUS_WRITE_BYTE               0x00040000
+#define I2C_FUNC_SMBUS_READ_BYTE_DATA           0x00080000
+#define I2C_FUNC_SMBUS_WRITE_BYTE_DATA          0x00100000
+#define I2C_FUNC_SMBUS_READ_WORD_DATA           0x00200000
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA          0x00400000
+#define I2C_FUNC_SMBUS_PROC_CALL                0x00800000
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA          0x01000000
+#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA         0x02000000
+#define I2C_FUNC_SMBUS_READ_I2C_BLOCK           0x04000000 /*I2C-like blk-xfr */
+#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK          0x08000000 /*1-byte reg. addr.*/
+#define I2C_FUNC_SMBUS_READ_I2C_BLOCK_2         0x10000000 /*I2C-like blk-xfer*/
+#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK_2        0x20000000 /* w/ 2-byte regadr*/
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA_PEC      0x40000000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA_PEC     0x80000000 /* SMBus 2.0 */
+
+#define I2C_FUNC_SMBUS_BYTE (I2C_FUNC_SMBUS_READ_BYTE | \
+    I2C_FUNC_SMBUS_WRITE_BYTE)
+#define I2C_FUNC_SMBUS_BYTE_DATA (I2C_FUNC_SMBUS_READ_BYTE_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BYTE_DATA)
+#define I2C_FUNC_SMBUS_WORD_DATA (I2C_FUNC_SMBUS_READ_WORD_DATA | \
+    I2C_FUNC_SMBUS_WRITE_WORD_DATA)
+#define I2C_FUNC_SMBUS_BLOCK_DATA (I2C_FUNC_SMBUS_READ_BLOCK_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)
+#define I2C_FUNC_SMBUS_I2C_BLOCK (I2C_FUNC_SMBUS_READ_I2C_BLOCK | \
+    I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)
+
+#define I2C_FUNC_SMBUS_EMUL (I2C_FUNC_SMBUS_QUICK | \
+    I2C_FUNC_SMBUS_BYTE | \
+    I2C_FUNC_SMBUS_BYTE_DATA | \
+    I2C_FUNC_SMBUS_WORD_DATA | \
+    I2C_FUNC_SMBUS_PROC_CALL | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA_PEC | \
+    I2C_FUNC_SMBUS_I2C_BLOCK)
+
+#define DeviceOutVendor ((USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
+#define DeviceInVendor  ((USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
+
+typedef struct {
+    USBDevice dev;
+    I2CBus *i2cbus;
+} UsbI2cTinyState;
+
+#define TYPE_USB_I2C_TINY "usb-i2c-dev"
+#define USB_I2C_TINY_DEV(obj) OBJECT_CHECK(UsbI2cTinyState, \
+    (obj), TYPE_USB_I2C_TINY)
+
+enum {
+    STR_MANUFACTURER = 1,
+    STR_PRODUCT_SERIAL,
+    STR_SERIALNUMBER,
+};
+
+static const USBDescStrings desc_strings = {
+    [STR_MANUFACTURER]    = "QEMU",
+    [STR_PRODUCT_SERIAL]  = "QEMU USB I2C Tiny",
+    [STR_SERIALNUMBER]    = "1",
+};
+
+static const USBDescIface desc_iface0 = {
+    .bInterfaceNumber              = 1,
+    .bNumEndpoints                 = 0,
+    .bInterfaceClass               = 0xff,
+    .bInterfaceSubClass            = 0xff,
+    .bInterfaceProtocol            = 0xff,
+    .eps = (USBDescEndpoint[]) {
+    }
+};
+
+static const USBDescDevice desc_device = {
+    .bcdUSB                        = 0x0110,
+    .bMaxPacketSize0               = 8,
+    .bNumConfigurations            = 1,
+    .confs = (USBDescConfig[]) {
+        {
+            .bNumInterfaces        = 1,
+            .bConfigurationValue   = 1,
+            .bmAttributes          = USB_CFG_ATT_ONE,
+            .bMaxPower             = 50,
+            .nif = 1,

hw/usb/dev-i2c-tiny.c line 139

Quoted 10 lines
hw/usb/dev-i2c-tiny.c new file +10 -0
+            .ifs = &desc_iface0,
+        },
+    },
+};
+
+static const USBDesc desc_usb_i2c = {
+    .id = {
+        .idVendor          = 0x0403,
+        .idProduct         = 0xc631,
+        .bcdDevice         = 0x0205,

Where did these magic numbers come from?

Quoted 34 lines
hw/usb/dev-i2c-tiny.c new file +34 -0
+        .iManufacturer     = STR_MANUFACTURER,
+        .iProduct          = STR_PRODUCT_SERIAL,
+        .iSerialNumber     = STR_SERIALNUMBER,
+    },
+    .full = &desc_device,
+    .str  = desc_strings,
+};
+
+static int usb_i2c_read_byte(I2CBus *bus, uint8_t addr)
+{
+    int retval;
+    i2c_start_transfer(bus, addr, 1);
+    retval = i2c_recv(bus);
+    i2c_end_transfer(bus);
+    return retval;
+}
+
+static int usb_i2c_write_byte(I2CBus *bus, uint8_t addr, uint8_t data)
+{
+    int retval;
+    i2c_start_transfer(bus, addr, 0);
+    retval = i2c_send(bus, data);
+    i2c_end_transfer(bus);
+    return retval;
+}
+
+static void usb_i2c_handle_reset(USBDevice *dev)
+{
+    DPRINTF("reset\n");
+}
+
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t *data)
+{

hw/usb/dev-i2c-tiny.c line 183

Quoted 10 lines
hw/usb/dev-i2c-tiny.c new file +10 -0
+    UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+    int ret;
+
+    DPRINTF("got control %x, value %x\n", request, value);
+    DPRINTF("iov_base:%lx pid:%x stream:%x param:%lx status:%x len:%x\n",
+            (uint64_t)(p->iov).iov->iov_base, p->pid, p->stream, p->parameter,
+            p->status, p->actual_length);
+    ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
+    DPRINTF("usb_desc_handle_control return value: %i status: %x\n", ret,
+            p->status);

I get that debug output is useful for debugging but if you want to maintain ability to look at the i2c transactions you might want to consider the tracing infrastructure.

hw/usb/dev-i2c-tiny.c line 191

Quoted 8 lines
hw/usb/dev-i2c-tiny.c new file +8 -0
+    if (ret >= 0) {
+        return;
+    }
+
+    switch (request) {
+    case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+        break;
+

Again where are these magic numbers coming from?

Quoted 182 lines
hw/usb/dev-i2c-tiny.c new file +182 -0
+    case 0x4102:
+        /* set clock, ignore */
+        break;
+
+    case 0x4105:
+        DPRINTF("unknown access 0x4105 %x\n", index);
+        p->actual_length = 1;
+        break;
+
+    case 0x4107:
+        {
+        int i;
+        DPRINTF("write addr:0x%x len:%i\n", index, length);
+        for (i = 0; i < length; i++) {
+            usb_i2c_write_byte(s->i2cbus, index+i, data[i]);
+        }
+        }
+        break;
+
+    case 0xc101:
+        {
+        /* thats what the real thing reports, FIXME: can we do better here? */
+        uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
+        DPRINTF("got functionality read %x, value %x\n", request, value);
+        memcpy(data, &func, sizeof(func));
+        p->actual_length = sizeof(func);
+        }
+        break;
+
+    case 0xc103:
+        DPRINTF("unknown call 0xc103 index: 0x%x\n", index);
+        DPRINTF("v:%x r:%x l:%x i:%x\n", value, request, length, index);
+        p->actual_length = 1;
+        break;
+
+    case 0xc106:
+        DPRINTF("unknown call 0xc106 index:0x%x\n", index);
+        DPRINTF("v:%x r:%x l:%x i:%x\n", value, request, length, index);
+        p->actual_length = 1;
+        break;
+
+    case 0xc107:
+        {
+        int i;
+        DPRINTF("read access addr:0x%x len:0x%x\n", index, length);
+        DPRINTF("v:%x r:%x l:%x i:%x\n", value, request, length, index);
+        for (i = 0; i < length; i++) {
+            data[i] = usb_i2c_read_byte(s->i2cbus, index+i);
+        }
+        p->actual_length = length;
+        }
+        break;
+
+    default:
+        DPRINTF("got unsupported/bogus control %x, value %x\n", request, value);
+        p->status = USB_RET_STALL;
+        break;
+    }
+}
+
+static void usb_i2c_handle_data(USBDevice *dev, USBPacket *p)
+{
+    DPRINTF("unexpected call to usb_i2c_handle_data\n");
+}
+
+static void usb_i2c_realize(USBDevice *dev, Error **errp)
+{
+    UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+    Error *local_err = NULL;
+
+    usb_desc_create_serial(dev);
+    usb_desc_init(dev);
+
+    usb_check_attach(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    s->i2cbus = i2c_init_bus(&dev->qdev, "i2c");
+    usb_i2c_handle_reset(dev);
+}
+
+static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)
+{
+    USBDevice *dev;
+    CharDriverState *cdrv;
+    uint32_t vendorid = 0, productid = 0;
+    char label[32];
+    static int index;
+
+    while (*filename && *filename != ':') {
+        const char *p;
+        char *e;
+        if (strstart(filename, "vendorid=", &p)) {
+            vendorid = strtol(p, &e, 16);
+            if (e == p || (*e && *e != ',' && *e != ':')) {
+                error_report("bogus vendor ID %s", p);
+                return NULL;
+            }
+            filename = e;
+        } else if (strstart(filename, "productid=", &p)) {
+            productid = strtol(p, &e, 16);
+            if (e == p || (*e && *e != ',' && *e != ':')) {
+                error_report("bogus product ID %s", p);
+                return NULL;
+            }
+            filename = e;
+        } else {
+            error_report("unrecognized usbi2c USB option %s", filename);
+            return NULL;
+        }
+        while (*filename == ',') {
+            filename++;
+        }
+    }
+    if (!*filename) {
+        error_report("character device specification needed");
+        return NULL;
+    }
+    filename++;
+
+    snprintf(label, sizeof(label), "usbusbi2c%d", index++);
+    cdrv = qemu_chr_new(label, filename, NULL);
+    if (!cdrv) {
+        return NULL;
+    }
+
+    dev = usb_create(bus, "usb-usbi2c");
+    qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
+    if (vendorid) {
+        qdev_prop_set_uint16(&dev->qdev, "vendorid", vendorid);
+    }
+    if (productid) {
+        qdev_prop_set_uint16(&dev->qdev, "productid", productid);
+    }
+    return dev;
+}
+
+static const VMStateDescription vmstate_usb_i2c = {
+    .name = "usb-i2c-tiny",
+    .unmigratable = 1,
+};
+
+static Property usbi2c_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void usb_i2c_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+    uc->realize        = usb_i2c_realize;
+    uc->handle_reset   = usb_i2c_handle_reset;
+    uc->handle_control = usb_i2c_handle_control;
+    uc->handle_data    = usb_i2c_handle_data;
+    dc->vmsd = &vmstate_usb_i2c;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo usb_i2c_dev_type_info = {
+    .name = TYPE_USB_I2C_TINY,
+    .parent = TYPE_USB_DEVICE,
+    .instance_size = sizeof(UsbI2cTinyState),
+    .abstract = true,
+    .class_init = usb_i2c_dev_class_init,
+};
+
+static void usb_i2c_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+    uc->product_desc   = "QEMU USB I2C Tiny";
+    uc->usb_desc       = &desc_usb_i2c;
+    dc->props = usbi2c_properties;
+}
+
+static const TypeInfo usbi2c_info = {
+    .name          = "usb-i2c-tiny",
+    .parent        = TYPE_USB_I2C_TINY,
+    .class_init    = usb_i2c_class_initfn,

hw/usb/dev-i2c-tiny.c line 383

Quoted 10 lines
hw/usb/dev-i2c-tiny.c new file +10 -0
+};
+
+static void usb_i2c_register_types(void)
+{
+    type_register_static(&usb_i2c_dev_type_info);
+    type_register_static(&usbi2c_info);
+    usb_legacy_register("usb-i2c-tiny", "i2c-bus-tiny", usb_i2c_init);
+}
+
+type_init(usb_i2c_register_types)

Cheers,

--
Alex Bennée

author
#2 lore raw

Hi Alex

Thanks for your feedback, answers as usual inline.

Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée:

Quoted 9 lines
Quoted 8 lines · Tim Sander

Hi

Below is a patch implementing the i2c-tiny-usb device. I am currently not sure about the i2c semantics. I think incrementing the address on longer reads is wrong? But at least i can read the high byte(?) of the temperature via "i2cget -y 0 0x50".

Quoted 7 lines
Quoted 4 lines

With this device it should be possible to define i2c busses via command line e.g:
-device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50 have been used for the first test.

You are missing a s-o-b line and scripts/checkpatch.pl complains about a few things you should fix before your next submission.

I was unsure about the access length so i was just asking for feedback this time, thats why omitted the signed of by line.
It seems as if i grabbed an older version of checkpatch as these errors didn't come up with the slightly older script. These new occurences will be fixed.

Quoted 183 lines
Quoted 183 lines

Best regards
Tim
---

 default-configs/usb.mak |   1 +
 hw/usb/Makefile.objs    |   1 +
 hw/usb/dev-i2c-tiny.c   | 383
 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 385
 insertions(+)
 create mode 100644 hw/usb/dev-i2c-tiny.c
default-configs/usb.mak +1 -0
index f4b8568..01d2c9f 100644
@@ -8,3 +8,4 @@ CONFIG_USB_AUDIO=y
 
 CONFIG_USB_SERIAL=y
 CONFIG_USB_NETWORK=y
 CONFIG_USB_BLUETOOTH=y
 
+CONFIG_USB_I2C_TINY=y
hw/usb/Makefile.objs +1 -0
index 8f00fbd..3a4c337 100644
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_USB_AUDIO)        += dev-audio.o
 
 common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
 common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
 
+common-obj-$(CONFIG_USB_I2C_TINY)     += dev-i2c-tiny.o
 

ifeq ($(CONFIG_USB_SMARTCARD),y)
common-obj-y += dev-smartcard-reader.o

hw/usb/dev-i2c-tiny.c new file +42 -0
new file mode 100644
index 0000000..1dabb36
@@ -0,0 +1,383 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * Copyright (c) 2015 Tim Sander <[email protected]>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <[email protected]>
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* #define DEBUG_USBI2C */
+
+#ifdef DEBUG_USBI2C
+#define DPRINTF(fmt, ...) \
+do { printf("usb-i2c-tiny: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+/* commands from USB, must e.g. match command ids in kernel driver */
+#define CMD_ECHO       0
+#define CMD_GET_FUNC   1
+#define CMD_SET_DELAY  2
+#define CMD_GET_STATUS 3
+
+/* To determine what functionality is present */
+#define I2C_FUNC_I2C                            0x00000001
+#define I2C_FUNC_10BIT_ADDR                     0x00000002
+#define I2C_FUNC_PROTOCOL_MANGLING              0x00000004
+#define I2C_FUNC_SMBUS_HWPEC_CALC               0x00000008 /* SMBus 2.0

*/
+#define I2C_FUNC_SMBUS_READ_WORD_DATA_PEC 0x00000800 /* SMBus 2.0
*/
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA_PEC 0x00001000 /* SMBus 2.0
*/
+#define I2C_FUNC_SMBUS_PROC_CALL_PEC 0x00002000 /* SMBus 2.0
*/
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL_PEC 0x00004000 /* SMBus 2.0
*/
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL 0x00008000 /* SMBus 2.0
*/

hw/usb/dev-i2c-tiny.c new file +11 -0
+#define I2C_FUNC_SMBUS_QUICK                    0x00010000
+#define I2C_FUNC_SMBUS_READ_BYTE                0x00020000
+#define I2C_FUNC_SMBUS_WRITE_BYTE               0x00040000
+#define I2C_FUNC_SMBUS_READ_BYTE_DATA           0x00080000
+#define I2C_FUNC_SMBUS_WRITE_BYTE_DATA          0x00100000
+#define I2C_FUNC_SMBUS_READ_WORD_DATA           0x00200000
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA          0x00400000
+#define I2C_FUNC_SMBUS_PROC_CALL                0x00800000
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA          0x01000000
+#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA         0x02000000
+#define I2C_FUNC_SMBUS_READ_I2C_BLOCK           0x04000000 /*I2C-like

blk-xfr */ +#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x08000000
/*1-byte reg. addr.*/ +#define I2C_FUNC_SMBUS_READ_I2C_BLOCK_2 0x10000000 /*I2C-like blk-xfer*/ +#define
I2C_FUNC_SMBUS_WRITE_I2C_BLOCK_2 0x20000000 /* w/ 2-byte regadr*/
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA_PEC 0x40000000 /* SMBus 2.0
*/ +#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA_PEC 0x80000000 /* SMBus
2.0 */ +

hw/usb/dev-i2c-tiny.c new file +21 -0
+#define I2C_FUNC_SMBUS_BYTE (I2C_FUNC_SMBUS_READ_BYTE | \
+    I2C_FUNC_SMBUS_WRITE_BYTE)
+#define I2C_FUNC_SMBUS_BYTE_DATA (I2C_FUNC_SMBUS_READ_BYTE_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BYTE_DATA)
+#define I2C_FUNC_SMBUS_WORD_DATA (I2C_FUNC_SMBUS_READ_WORD_DATA | \
+    I2C_FUNC_SMBUS_WRITE_WORD_DATA)
+#define I2C_FUNC_SMBUS_BLOCK_DATA (I2C_FUNC_SMBUS_READ_BLOCK_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)
+#define I2C_FUNC_SMBUS_I2C_BLOCK (I2C_FUNC_SMBUS_READ_I2C_BLOCK | \
+    I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)
+
+#define I2C_FUNC_SMBUS_EMUL (I2C_FUNC_SMBUS_QUICK | \
+    I2C_FUNC_SMBUS_BYTE | \
+    I2C_FUNC_SMBUS_BYTE_DATA | \
+    I2C_FUNC_SMBUS_WORD_DATA | \
+    I2C_FUNC_SMBUS_PROC_CALL | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | \
+    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA_PEC | \
+    I2C_FUNC_SMBUS_I2C_BLOCK)
+
+#define DeviceOutVendor

((USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8) +#define DeviceInVendor ((USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8) +

hw/usb/dev-i2c-tiny.c new file +46 -0
+typedef struct {
+    USBDevice dev;
+    I2CBus *i2cbus;
+} UsbI2cTinyState;
+
+#define TYPE_USB_I2C_TINY "usb-i2c-dev"
+#define USB_I2C_TINY_DEV(obj) OBJECT_CHECK(UsbI2cTinyState, \
+    (obj), TYPE_USB_I2C_TINY)
+
+enum {
+    STR_MANUFACTURER = 1,
+    STR_PRODUCT_SERIAL,
+    STR_SERIALNUMBER,
+};
+
+static const USBDescStrings desc_strings = {
+    [STR_MANUFACTURER]    = "QEMU",
+    [STR_PRODUCT_SERIAL]  = "QEMU USB I2C Tiny",
+    [STR_SERIALNUMBER]    = "1",
+};
+
+static const USBDescIface desc_iface0 = {
+    .bInterfaceNumber              = 1,
+    .bNumEndpoints                 = 0,
+    .bInterfaceClass               = 0xff,
+    .bInterfaceSubClass            = 0xff,
+    .bInterfaceProtocol            = 0xff,
+    .eps = (USBDescEndpoint[]) {
+    }
+};
+
+static const USBDescDevice desc_device = {
+    .bcdUSB                        = 0x0110,
+    .bMaxPacketSize0               = 8,
+    .bNumConfigurations            = 1,
+    .confs = (USBDescConfig[]) {
+        {
+            .bNumInterfaces        = 1,
+            .bConfigurationValue   = 1,
+            .bmAttributes          = USB_CFG_ATT_ONE,
+            .bMaxPower             = 50,
+            .nif = 1,
+            .ifs = &desc_iface0,
+        },
+    },
+};
Quoted 8 lines

hw/usb/dev-i2c-tiny.c line 139

Quoted 6 lines
hw/usb/dev-i2c-tiny.c new file +6 -0
+
+static const USBDesc desc_usb_i2c = {
+    .id = {
+        .idVendor          = 0x0403,
+        .idProduct         = 0xc631,
+        .bcdDevice         = 0x0205,

Where did these magic numbers come from?

I guess i have forgotton to add the link of the hardware which i am aiming to emulate:
http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
Looking at the Linux driver it seems there are two supported manufacturer numbers, i just took one of them.

Quoted 40 lines
Quoted 40 lines
hw/usb/dev-i2c-tiny.c new file +33 -0
+        .iManufacturer     = STR_MANUFACTURER,
+        .iProduct          = STR_PRODUCT_SERIAL,
+        .iSerialNumber     = STR_SERIALNUMBER,
+    },
+    .full = &desc_device,
+    .str  = desc_strings,
+};
+
+static int usb_i2c_read_byte(I2CBus *bus, uint8_t addr)
+{
+    int retval;
+    i2c_start_transfer(bus, addr, 1);
+    retval = i2c_recv(bus);
+    i2c_end_transfer(bus);
+    return retval;
+}
+
+static int usb_i2c_write_byte(I2CBus *bus, uint8_t addr, uint8_t data)
+{
+    int retval;
+    i2c_start_transfer(bus, addr, 0);
+    retval = i2c_send(bus, data);
+    i2c_end_transfer(bus);
+    return retval;
+}
+
+static void usb_i2c_handle_reset(USBDevice *dev)
+{
+    DPRINTF("reset\n");
+}
+
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t

*data) +{

hw/usb/dev-i2c-tiny.c new file +6 -0
+    UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+    int ret;
+
+    DPRINTF("got control %x, value %x\n", request, value);
+    DPRINTF("iov_base:%lx pid:%x stream:%x param:%lx status:%x len:%x\n",
+            (uint64_t)(p->iov).iov->iov_base, p->pid, p->stream,
Quoted 8 lines
Quoted 4 lines

p->parameter, + p->status, p->actual_length);
+ ret = usb_desc_handle_control(dev, p, request, value, index, length,
data); + DPRINTF("usb_desc_handle_control return value: %i status:
%x\n", ret, + p->status);

I get that debug output is useful for debugging but if you want to maintain ability to look at the i2c transactions you might want to consider the tracing infrastructure.

Any pointers a quick search turned up the
http://wiki.qemu.org/Features/Tracing/Roadmap page but this seems pretty outdated?

Quoted 10 lines

hw/usb/dev-i2c-tiny.c line 191

Quoted 8 lines
hw/usb/dev-i2c-tiny.c new file +8 -0
+    if (ret >= 0) {
+        return;
+    }
+
+    switch (request) {
+    case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+        break;
+

Again where are these magic numbers coming from?

I choose this usb device as it has a mainline driver and is reasonable simple. These numbers are just what i have seen during reverse engineering: starting up the linux i2c-tiny-usb module and look for the results. So i don't know more than what i have written into the comments.

<snip>

Best regards
Tim

#3 lore raw

Tim Sander <[email protected]> writes:

[...]

Quoted 6 lines
Quoted 3 lines

I get that debug output is useful for debugging but if you want to maintain ability to look at the i2c transactions you might want to consider the tracing infrastructure.

Any pointers a quick search turned up the
http://wiki.qemu.org/Features/Tracing/Roadmap page but this seems pretty outdated?

Have a look at docs/tracing.txt.

[...]

#4 lore raw
In reply to Tim Sander
Quoted 8 lines · Tim Sander

Hi Alex

Thanks for your feedback, answers as usual inline.

Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée:

Quoted 3 lines
Quoted 2 lines · Tim Sander

Hi

<snip>

Quoted 6 lines
Quoted 2 lines

You are missing a s-o-b line and scripts/checkpatch.pl complains about a few things you should fix before your next submission.

I was unsure about the access length so i was just asking for feedback this time, thats why omitted the signed of by line.
It seems as if i grabbed an older version of checkpatch as these errors didn't come up with the slightly older script. These new occurences will be fixed.

Cool. FWIW I always run against the current $(QEMU_SRC)/scripts/checkpatch.pl as it is updated reasonably frequently.

--
Alex Bennée

#5 lore raw
In reply to Tim Sander
Quoted 3 lines · Tim Sander

Hi

Below is a patch implementing the i2c-tiny-usb device.

Is there a specification for this kind of device? Or does it mimic existing hardware?
Please add that into to the comment at the head of the file.

hw/usb/dev-i2c-tiny.c line 30

Quoted 6 lines
hw/usb/dev-i2c-tiny.c new file +6 -0
+#ifdef DEBUG_USBI2C
+#define DPRINTF(fmt, ...) \
+do { printf("usb-i2c-tiny: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif

Please consider turning them into trace points.

hw/usb/dev-i2c-tiny.c line 117

Quoted 9 lines
hw/usb/dev-i2c-tiny.c new file +9 -0
+static const USBDescIface desc_iface0 = {
+    .bInterfaceNumber              = 1,
+    .bNumEndpoints                 = 0,
+    .bInterfaceClass               = 0xff,
+    .bInterfaceSubClass            = 0xff,
+    .bInterfaceProtocol            = 0xff,
+    .eps = (USBDescEndpoint[]) {
+    }
+};

Hmm? No endpoints?

hw/usb/dev-i2c-tiny.c line 219

Quoted 13 lines
hw/usb/dev-i2c-tiny.c new file +12 -0
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t *data)
+{
 
+    case 0xc101:
+        {
+        /* thats what the real thing reports, FIXME: can we do better here? */
+        uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
+        DPRINTF("got functionality read %x, value %x\n", request, value);
+        memcpy(data, &func, sizeof(func));
+        p->actual_length = sizeof(func);
+        }
+        break;

Ah, it all goes over the control pipe.

Quoted 1 line

+static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)

Please drop this ...

Quoted 1 line

+ usb_legacy_register("usb-i2c-tiny", "i2c-bus-tiny", usb_i2c_init);

... and this.

It's for backward compatibility with old qemu versions (-usbdevice ...), and we don't need that for new devices.

cheers,
Gerd

author
#6 lore raw

Am Freitag, 27. November 2015, 07:48:21 schrieb Gerd Hoffmann:

Quoted 6 lines · Tim Sander
Quoted 3 lines

Hi

Below is a patch implementing the i2c-tiny-usb device.

Is there a specification for this kind of device?

http://www.harbaum.org/till/i2c_tiny_usb/index.shtml

Quoted 1 line

Or does it mimic existing hardware?

Yes, the reason i choose this device where:
*simple
*linux driver available
which makes a cmdline configurable i2c bus easy.

Quoted 1 line

Please add that into to the comment at the head of the file.

Will do.

Quoted 8 lines

hw/usb/dev-i2c-tiny.c line 30

Quoted 6 lines
hw/usb/dev-i2c-tiny.c new file +6 -0
+#ifdef DEBUG_USBI2C
+#define DPRINTF(fmt, ...) \
+do { printf("usb-i2c-tiny: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif

Please consider turning them into trace points.

Ok.

Quoted 11 lines

hw/usb/dev-i2c-tiny.c line 117

Quoted 9 lines
hw/usb/dev-i2c-tiny.c new file +9 -0
+static const USBDescIface desc_iface0 = {
+    .bInterfaceNumber              = 1,
+    .bNumEndpoints                 = 0,
+    .bInterfaceClass               = 0xff,
+    .bInterfaceSubClass            = 0xff,
+    .bInterfaceProtocol            = 0xff,
+    .eps = (USBDescEndpoint[]) {
+    }
+};

Hmm? No endpoints?

Yes this device has indeed no endpoints just control as you found out below.

Quoted 18 lines

hw/usb/dev-i2c-tiny.c line 219

Quoted 13 lines
(quoted diff) +2 -0
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t

*data) +{

hw/usb/dev-i2c-tiny.c new file +3 -0
+    case 0xc101:
+        {
+        /* thats what the real thing reports, FIXME: can we do better

here? */ + uint32_t func = htole32(I2C_FUNC_I2C |
I2C_FUNC_SMBUS_EMUL); + DPRINTF("got functionality read %x, value
%x\n", request, value); + memcpy(data, &func, sizeof(func));

hw/usb/dev-i2c-tiny.c new file +3 -0
+        p->actual_length = sizeof(func);
+        }
+        break;

Ah, it all goes over the control pipe.

Quoted 1 line

+static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)

Quoted 8 lines

Please drop this ...

Quoted 1 line

+ usb_legacy_register("usb-i2c-tiny", "i2c-bus-tiny", usb_i2c_init);

... and this.

It's for backward compatibility with old qemu versions (-usbdevice ...), and we don't need that for new devices.

Nice this makes the code even smaller :-).

Best regards
Tim

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#8 lore raw

Hi Paolo

Am Freitag, 27. November 2015, 10:32:25 schrieb Paolo Bonzini:

Quoted 8 lines · Tim Sander
Quoted 3 lines

Below is a patch implementing the i2c-tiny-usb device. I am currently not sure about the i2c semantics. I think incrementing the address on longer reads is wrong?

Yes, it is. This is a bus address, not an address inside the device.

Also you should only call i2c_start_transfer/i2c_end_transfer once.

Good to know.

I have one more thing, i2cdetect looks like this:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30: 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50: 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70: 70 71 72 73 74 75 76 77

But i only have a device at 0x50 so most numbers should be "--". I know that i just have to set the read length to zero, to archive that. Currently i don't know how to determine if there is something registered on the requested i2c address?

Best regards
Tim

#9 lore raw
Quoted 12 lines · Tim Sander

I have one more thing, i2cdetect looks like this:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30: 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50: 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70: 70 71 72 73 74 75 76 77

Quoted 4 lines

But i only have a device at 0x50 so most numbers should be "--". I know that i just have to set the read length to zero, to archive that. Currently i don't know how to determine if there is something registered on the requested i2c address?

If there is no slave at the requested address, i2c_start_transfer will return 1.

Paolo

author
#10 lore raw

[Qemu-devel] i2c data address question was Re: [PATCH RFC] i2c-tiny-usb

Hi Paolo and List

Am Freitag, 27. November 2015, 13:53:22 schrieb Paolo Bonzini:

Quoted 13 lines · Tim Sander
Quoted 12 lines

I have one more thing, i2cdetect looks like this:
0 1 2 3 4 5 6 7 8 9 a b c d e f

00: 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30: 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50: 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70: 70 71 72 73 74 75 76 77

Quoted 8 lines
Quoted 5 lines

But i only have a device at 0x50 so most numbers should be "--". I know that i just have to set the read length to zero, to archive that. Currently i don't know how to determine if there is something registered on
the requested i2c address?

If there is no slave at the requested address, i2c_start_transfer will return 1.

Ok, that works. Now probably the last problem i see is that i fail to set the data-address of the i2c-device?
I know the correct offset address for accesses on the bus e.g. i2cget -y 0 0x50 2
where 2 is an example offset for the access to this device.

So any hint how setting the data-address on the i2c bus in qemu works?

I have seen the function i2c_set_slave_address but i guess thats not the right function setting the data address.

Best regards
Tim

#11 lore raw

Re: [Qemu-devel] i2c data address question was Re: [PATCH RFC] i2c-tiny-usb

Quoted 9 lines · Tim Sander
Quoted 2 lines
Quoted 2 lines

If there is no slave at the requested address, i2c_start_transfer will return 1.

Ok, that works. Now probably the last problem i see is that i fail to set the data-address of the i2c-device?
I know the correct offset address for accesses on the bus e.g. i2cget -y 0 0x50 2
where 2 is an example offset for the access to this device.

So any hint how setting the data-address on the i2c bus in qemu works?

If you have a data address, you probably want to use functions like smbus_read_byte that do the right write-read sequence for you:

    if (i2c_start_transfer(bus, addr, 0)) {
        return -1;
    }
    i2c_send(bus, command);
    i2c_start_transfer(bus, addr, 1);
    data = i2c_recv(bus);
    i2c_nack(bus);
    i2c_end_transfer(bus);

Paolo

author
#12 lore raw

PATCH 1/1 i2c-tiny-usb: add new usb to i2c bridge

In reply to Tim Sander

i2c-tiny-usb is a small usb to i2c bridge:
http://www.harbaum.org/till/i2c_tiny_usb/index.shtml

It is pretty simple and has no usb endpoints just a control. Reasons for adding this device:
* Linux device driver available
* adding an additional i2c bus via command line e.g. -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50

---
 default-configs/i386-softmmu.mak |   1 +
 default-configs/usb.mak          |   1 +
 hw/usb/Makefile.objs             |   1 +
 hw/usb/dev-i2c-tiny.c            | 322 +++++++++++++++++++++++++++++++++++++++
 trace-events                     |  11 ++
 5 files changed, 336 insertions(+)
 create mode 100644 hw/usb/dev-i2c-tiny.c
default-configs/i386-softmmu.mak +1 -0
index c3020cf..a10ab74 100644
@@ -52,3 +52,4 @@ CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_GA12=y
 CONFIG_PTIMER=y
+CONFIG_TMP105=y
default-configs/usb.mak +1 -0
index f4b8568..01d2c9f 100644
@@ -8,3 +8,4 @@ CONFIG_USB_AUDIO=y
 CONFIG_USB_SERIAL=y
 CONFIG_USB_NETWORK=y
 CONFIG_USB_BLUETOOTH=y
+CONFIG_USB_I2C_TINY=y
hw/usb/Makefile.objs +1 -0
index 8f00fbd..3a4c337 100644
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_USB_AUDIO)        += dev-audio.o
 common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
 common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
+common-obj-$(CONFIG_USB_I2C_TINY)     += dev-i2c-tiny.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
hw/usb/dev-i2c-tiny.c new file +322 -0

322 lines not on this page · raw message

trace-events +11 -0
index 0b0ff02..e40aefc 100644
@@ -326,6 +326,17 @@ usb_port_attach(int bus, const char *port, const char *devspeed, const char *por
 usb_port_detach(int bus, const char *port) "bus %d, port %s"
 usb_port_release(int bus, const char *port) "bus %d, port %s"
 
+# hw/usb/dev-tiny-i2c.c
+usb_i2c_tiny_read(int request, int address,int offset,int data) "request:0x%x address:%i offset:%i data:%i"
+usb_i2c_tiny_write(int request, int address,int offset,int data) "request:0x%x address:%i offset:%i data:%i"
+usb_i2c_tiny_functionality_read(int length) "length:%i"
+usb_i2c_tiny_reset(void) ""
+usb_i2c_tiny_set_clock(void) ""
+usb_i2c_tiny_usb_i2c_handle_data(void) ""
+usb_i2c_tiny_i2c_start_transfer_failed(void) "i2c start transfer failed"
+usb_i2c_tiny_ignored_request(int index,int request,int value,int length) "index:%i request:0x%x value:%i length:%i"
+usb_i2c_tiny_unknown_request(int index,int request,int value,int length) "index:%i request:0x%x value:%i length:%i"
+
 # hw/usb/hcd-ohci.c
 usb_ohci_iso_td_read_failed(uint32_t addr) "ISO_TD read error at %x"
 usb_ohci_iso_td_head(uint32_t head, uint32_t tail, uint32_t flags, uint32_t bp, uint32_t next, uint32_t be, uint32_t framenum, uint32_t startframe, uint32_t framecount, int rel_frame_num) "ISO_TD ED head 0x%.8x tailp 0x%.8x\n0x%.8x 0x%.8x 0x%.8x 0x%.8x\nframe_number 0x%.8x starting_frame 0x%.8x\nframe_count  0x%.8x relative %d"
Signature · 1 line
1.9.1
#13 lore raw

Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge

Quoted 7 lines
hw/usb/dev-i2c-tiny.c new file +7 -0
+    case 0x4107:
+        {
+        int i;
+        /* this seems to be a byte type access */
+        if (i2c_start_transfer(s->i2cbus, /*address*/index, 0)) {
+            trace_usb_i2c_tiny_i2c_start_transfer_failed();
+            p->actual_length = 0; /* write failure */

hw/usb/dev-i2c-tiny.c line 185

Quoted 10 lines
hw/usb/dev-i2c-tiny.c new file +10 -0
+            break;
+        }
+        for (i = 0; i < length; i++) {
+            trace_usb_i2c_tiny_write(request, index, i, data[i]);
+            i2c_send(s->i2cbus, data[i]);
+        }
+        p->actual_length = length;
+        i2c_end_transfer(s->i2cbus);
+        }
+        break;

Braces look a bit displaced. How about moving the "int i"; to the start of the function and drop the braces then?

hw/usb/dev-i2c-tiny.c line 272

Quoted 3 lines
hw/usb/dev-i2c-tiny.c new file +3 -0
+static const VMStateDescription vmstate_usb_i2c = {
+    .name = "usb-i2c-tiny",
+    .unmigratable = 1,

I think you can drop the unmigratable line, or replace with /* nothing */ to make clear it is intentionally empty.

The device is so simple that there is no state to save. Being migrated with a half-done i2c transaction isn't possible too. So it should survive migration just fine.

Otherwise looks good.

cheers,
Gerd