* [Qemu-devel] [PATCH RFC] i2c-tiny-usb
@ 2015-11-26 16:35 Tim Sander
2015-11-26 18:07 ` Alex Bennée
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Tim Sander @ 2015-11-26 16:35 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini, Gerd Hoffmann
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
diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index f4b8568..01d2c9f 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -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
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 8f00fbd..3a4c337 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -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
diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
new file mode 100644
index 0000000..1dabb36
--- /dev/null
+++ b/hw/usb/dev-i2c-tiny.c
@@ -0,0 +1,383 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ * 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,
+ .ifs = &desc_iface0,
+ },
+ },
+};
+
+static const USBDesc desc_usb_i2c = {
+ .id = {
+ .idVendor = 0x0403,
+ .idProduct = 0xc631,
+ .bcdDevice = 0x0205,
+ .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)
+{
+ 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);
+ if (ret >= 0) {
+ return;
+ }
+
+ switch (request) {
+ case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+ break;
+
+ 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,
+};
+
+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)
--
1.9.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-26 16:35 [Qemu-devel] [PATCH RFC] i2c-tiny-usb Tim Sander
@ 2015-11-26 18:07 ` Alex Bennée
2015-11-27 8:41 ` Tim Sander
2015-11-27 6:48 ` Gerd Hoffmann
[not found] ` <56582329.5040304@redhat.com>
2 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2015-11-26 18:07 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, qemu-devel, Gerd Hoffmann
Tim Sander <tim@krieglstein.org> writes:
> 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.
You are missing a s-o-b line and scripts/checkpatch.pl complains about a
few things you should fix before your next submission.
>
> 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
>
> diff --git a/default-configs/usb.mak b/default-configs/usb.mak
> index f4b8568..01d2c9f 100644
> --- a/default-configs/usb.mak
> +++ b/default-configs/usb.mak
> @@ -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
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 8f00fbd..3a4c337 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -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
> diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
> new file mode 100644
> index 0000000..1dabb36
> --- /dev/null
> +++ b/hw/usb/dev-i2c-tiny.c
> @@ -0,0 +1,383 @@
> +/*
> + * I2C tiny usb device emulation
> + *
> + * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
> + *
> + * Loosly based on usb dev-serial.c:
> + * Copyright (c) 2006 CodeSourcery.
> + * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
> + * 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,
> + .ifs = &desc_iface0,
> + },
> + },
> +};
> +
> +static const USBDesc desc_usb_i2c = {
> + .id = {
> + .idVendor = 0x0403,
> + .idProduct = 0xc631,
> + .bcdDevice = 0x0205,
Where did these magic numbers come from?
> + .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)
> +{
> + 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.
> + if (ret >= 0) {
> + return;
> + }
> +
> + switch (request) {
> + case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
> + break;
> +
Again where are these magic numbers coming from?
> + 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,
> +};
> +
> +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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-26 16:35 [Qemu-devel] [PATCH RFC] i2c-tiny-usb Tim Sander
2015-11-26 18:07 ` Alex Bennée
@ 2015-11-27 6:48 ` Gerd Hoffmann
2015-11-27 10:59 ` Tim Sander
[not found] ` <56582329.5040304@redhat.com>
2 siblings, 1 reply; 21+ messages in thread
From: Gerd Hoffmann @ 2015-11-27 6:48 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, qemu-devel
On Do, 2015-11-26 at 17:35 +0100, Tim Sander wrote:
> 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.
> +#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.
> +static const USBDescIface desc_iface0 = {
> + .bInterfaceNumber = 1,
> + .bNumEndpoints = 0,
> + .bInterfaceClass = 0xff,
> + .bInterfaceSubClass = 0xff,
> + .bInterfaceProtocol = 0xff,
> + .eps = (USBDescEndpoint[]) {
> + }
> +};
Hmm? No endpoints?
> +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.
> +static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)
Please drop this ...
> + 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-26 18:07 ` Alex Bennée
@ 2015-11-27 8:41 ` Tim Sander
2015-11-27 9:35 ` Markus Armbruster
2015-11-27 11:57 ` Alex Bennée
0 siblings, 2 replies; 21+ messages in thread
From: Tim Sander @ 2015-11-27 8:41 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel, Gerd Hoffmann
Hi Alex
Thanks for your feedback, answers as usual inline.
Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée:
> Tim Sander <tim@krieglstein.org> writes:
> > 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.
>
> 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.
> > 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
> >
> > diff --git a/default-configs/usb.mak b/default-configs/usb.mak
> > index f4b8568..01d2c9f 100644
> > --- a/default-configs/usb.mak
> > +++ b/default-configs/usb.mak
> > @@ -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
> > diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> > index 8f00fbd..3a4c337 100644
> > --- a/hw/usb/Makefile.objs
> > +++ b/hw/usb/Makefile.objs
> > @@ -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
> >
> > diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
> > new file mode 100644
> > index 0000000..1dabb36
> > --- /dev/null
> > +++ b/hw/usb/dev-i2c-tiny.c
> > @@ -0,0 +1,383 @@
> > +/*
> > + * I2C tiny usb device emulation
> > + *
> > + * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
> > + *
> > + * Loosly based on usb dev-serial.c:
> > + * Copyright (c) 2006 CodeSourcery.
> > + * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
> > + * 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,
> > + .ifs = &desc_iface0,
> > + },
> > + },
> > +};
> > +
> > +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.
> > + .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) +{
> > + 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.
Any pointers a quick search turned up the
http://wiki.qemu.org/Features/Tracing/Roadmap page but this seems pretty
outdated?
> > + 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-27 8:41 ` Tim Sander
@ 2015-11-27 9:35 ` Markus Armbruster
2015-11-27 11:57 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-11-27 9:35 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, Alex Bennée, qemu-devel, Gerd Hoffmann
Tim Sander <tim@krieglstein.org> writes:
[...]
>> 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.
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-27 6:48 ` Gerd Hoffmann
@ 2015-11-27 10:59 ` Tim Sander
0 siblings, 0 replies; 21+ messages in thread
From: Tim Sander @ 2015-11-27 10:59 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Paolo Bonzini, qemu-devel
Am Freitag, 27. November 2015, 07:48:21 schrieb Gerd Hoffmann:
> On Do, 2015-11-26 at 17:35 +0100, Tim Sander wrote:
> > 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
> 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.
> Please add that into to the comment at the head of the file.
Will do.
> > +#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.
> > +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.
> > +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.
>
> > +static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)
>
> Please drop this ...
>
> > + 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-27 8:41 ` Tim Sander
2015-11-27 9:35 ` Markus Armbruster
@ 2015-11-27 11:57 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2015-11-27 11:57 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, qemu-devel, Gerd Hoffmann
Tim Sander <tim@krieglstein.org> writes:
> Hi Alex
>
> Thanks for your feedback, answers as usual inline.
>
> Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée:
>> Tim Sander <tim@krieglstein.org> writes:
>> > Hi
>> >
<snip>
>> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
[not found] ` <56582329.5040304@redhat.com>
@ 2015-11-27 12:39 ` Tim Sander
2015-11-27 12:53 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Tim Sander @ 2015-11-27 12:39 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Gerd Hoffmann
Hi Paolo
Am Freitag, 27. November 2015, 10:32:25 schrieb Paolo Bonzini:
> On 26/11/2015 17:35, Tim Sander wrote:
> > 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
2015-11-27 12:39 ` Tim Sander
@ 2015-11-27 12:53 ` Paolo Bonzini
2015-12-09 16:40 ` [Qemu-devel] i2c data address question was " Tim Sander
0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2015-11-27 12:53 UTC (permalink / raw)
To: Tim Sander; +Cc: qemu-devel, Gerd Hoffmann
On 27/11/2015 13:39, Tim Sander wrote:
>
> 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?
If there is no slave at the requested address, i2c_start_transfer will
return 1.
Paolo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] i2c data address question was Re: [PATCH RFC] i2c-tiny-usb
2015-11-27 12:53 ` Paolo Bonzini
@ 2015-12-09 16:40 ` Tim Sander
2015-12-09 17:04 ` Paolo Bonzini
2015-12-16 15:56 ` [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
0 siblings, 2 replies; 21+ messages in thread
From: Tim Sander @ 2015-12-09 16:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Gerd Hoffmann
Hi Paolo and List
Am Freitag, 27. November 2015, 13:53:22 schrieb Paolo Bonzini:
> On 27/11/2015 13:39, Tim Sander wrote:
> > 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?
>
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] i2c data address question was Re: [PATCH RFC] i2c-tiny-usb
2015-12-09 16:40 ` [Qemu-devel] i2c data address question was " Tim Sander
@ 2015-12-09 17:04 ` Paolo Bonzini
2015-12-16 15:56 ` [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
1 sibling, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-12-09 17:04 UTC (permalink / raw)
To: Tim Sander; +Cc: qemu-devel, Gerd Hoffmann
On 09/12/2015 17:40, Tim Sander wrote:
>> > 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2015-12-09 16:40 ` [Qemu-devel] i2c data address question was " Tim Sander
2015-12-09 17:04 ` Paolo Bonzini
@ 2015-12-16 15:56 ` Tim Sander
2015-12-17 16:15 ` Gerd Hoffmann
1 sibling, 1 reply; 21+ messages in thread
From: Tim Sander @ 2015-12-16 15:56 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Alex Bennée, Gerd Hoffmann, Markus Armbruster
Signed-off-by: Tim Sander <tim@krieglstein.org>
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
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index c3020cf..a10ab74 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -52,3 +52,4 @@ CONFIG_I82801B11=y
CONFIG_SMBIOS=y
CONFIG_GA12=y
CONFIG_PTIMER=y
+CONFIG_TMP105=y
diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index f4b8568..01d2c9f 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -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
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 8f00fbd..3a4c337 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -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
diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
new file mode 100644
index 0000000..a72244b
--- /dev/null
+++ b/hw/usb/dev-i2c-tiny.c
@@ -0,0 +1,322 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "trace.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/smbus.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* 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 = (const USBDescConfig[]) {
+ {
+ .bNumInterfaces = 1,
+ .bConfigurationValue = 1,
+ .bmAttributes = USB_CFG_ATT_ONE,
+ .bMaxPower = 50,
+ .nif = 1,
+ .ifs = &desc_iface0,
+ },
+ },
+};
+
+static const USBDesc desc_usb_i2c = {
+ .id = {
+ .idVendor = 0x0403,
+ .idProduct = 0xc631,
+ .bcdDevice = 0x0205,
+ .iManufacturer = STR_MANUFACTURER,
+ .iProduct = STR_PRODUCT_SERIAL,
+ .iSerialNumber = STR_SERIALNUMBER,
+ },
+ .full = &desc_device,
+ .str = desc_strings,
+};
+
+static void usb_i2c_handle_reset(USBDevice *dev)
+{
+ trace_usb_i2c_tiny_reset();
+}
+
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+ int request, int value, int index, int length, uint8_t *data)
+{
+ UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+ int ret;
+
+ ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
+ if (ret >= 0) {
+ return;
+ }
+
+ switch (request) {
+ case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+ break;
+ case 0x4102:
+ /* set clock, ignore */
+ trace_usb_i2c_tiny_set_clock();
+ break;
+
+ case 0x4105:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+
+ 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 */
+ 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;
+
+ 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);
+ memcpy(data, &func, sizeof(func));
+ p->actual_length = sizeof(func);
+ trace_usb_i2c_tiny_functionality_read(length);
+ }
+ break;
+
+ case 0xc103:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ p->actual_length = 1;
+ break;
+
+ case 0xc106:
+ {
+ int i;
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ trace_usb_i2c_tiny_unknown_request(data[0], data[1], data[2], data[3]);
+ if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0;
+ break;
+ }
+ i2c_send(s->i2cbus, data[0]);
+ i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ i2c_nack(s->i2cbus);
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ }
+ break;
+
+ case 0xc107:
+ {
+ int i;
+ if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0; /* read failure */
+ break;
+ }
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ }
+ break;
+
+ default:
+ p->status = USB_RET_STALL;
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+ }
+}
+
+static void usb_i2c_handle_data(USBDevice *dev, USBPacket *p)
+{
+ trace_usb_i2c_tiny_usb_i2c_handle_data();
+}
+
+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 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,
+};
+
+static void usb_i2c_register_types(void)
+{
+ type_register_static(&usb_i2c_dev_type_info);
+ type_register_static(&usbi2c_info);
+}
+
+type_init(usb_i2c_register_types)
diff --git a/trace-events b/trace-events
index 0b0ff02..e40aefc 100644
--- a/trace-events
+++ b/trace-events
@@ -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"
--
1.9.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2015-12-16 15:56 ` [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
@ 2015-12-17 16:15 ` Gerd Hoffmann
0 siblings, 0 replies; 21+ messages in thread
From: Gerd Hoffmann @ 2015-12-17 16:15 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, Alex Bennée, qemu-devel, Markus Armbruster
> + 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 */
> + 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?
> +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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
@ 2016-01-04 17:30 Tim Sander
2016-01-05 7:44 ` Gerd Hoffmann
0 siblings, 1 reply; 21+ messages in thread
From: Tim Sander @ 2016-01-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Alex Bennée, Markus Armbruster, Gerd Hoffmann
Version 3 with improvements suggested by Gerd Hoffmann
Signed-off-by: Tim Sander <tim@krieglstein.org>
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/usb.mak | 1 +
hw/usb/Makefile.objs | 1 +
hw/usb/dev-i2c-tiny.c | 313
++++++++++++++++++++++++++++++++++++++++++++++++
trace-events | 11 ++
4 files changed, 326 insertions(+)
create mode 100644 hw/usb/dev-i2c-tiny.c
diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index f4b8568..01d2c9f 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -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
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 8f00fbd..3a4c337 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -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
diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
new file mode 100644
index 0000000..492686a
--- /dev/null
+++ b/hw/usb/dev-i2c-tiny.c
@@ -0,0 +1,313 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "trace.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/smbus.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* 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 = (const USBDescConfig[]) {
+ {
+ .bNumInterfaces = 1,
+ .bConfigurationValue = 1,
+ .bmAttributes = USB_CFG_ATT_ONE,
+ .bMaxPower = 50,
+ .nif = 1,
+ .ifs = &desc_iface0,
+ },
+ },
+};
+
+static const USBDesc desc_usb_i2c = {
+ .id = {
+ .idVendor = 0x0403,
+ .idProduct = 0xc631,
+ .bcdDevice = 0x0205,
+ .iManufacturer = STR_MANUFACTURER,
+ .iProduct = STR_PRODUCT_SERIAL,
+ .iSerialNumber = STR_SERIALNUMBER,
+ },
+ .full = &desc_device,
+ .str = desc_strings,
+};
+
+static void usb_i2c_handle_reset(USBDevice *dev)
+{
+ trace_usb_i2c_tiny_reset();
+}
+
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+ int request, int value, int index, int length, uint8_t *data)
+{
+ UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+ int ret;
+ int i;
+
+ ret = usb_desc_handle_control(dev, p, request, value, index, length,
data);
+ if (ret >= 0) {
+ return;
+ }
+
+ switch (request) {
+ case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+ break;
+ case 0x4102:
+ /* set clock, ignore */
+ trace_usb_i2c_tiny_set_clock();
+ break;
+
+ case 0x4105:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+
+ case 0x4107:
+ /* 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 */
+ 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;
+
+ 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);
+ memcpy(data, &func, sizeof(func));
+ p->actual_length = sizeof(func);
+ trace_usb_i2c_tiny_functionality_read(length);
+ }
+ break;
+
+ case 0xc103:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ p->actual_length = 1;
+ break;
+
+ case 0xc106:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ trace_usb_i2c_tiny_unknown_request(data[0], data[1], data[2],
data[3]);
+ if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0;
+ break;
+ }
+ i2c_send(s->i2cbus, data[0]);
+ i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ i2c_nack(s->i2cbus);
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ break;
+
+ case 0xc107:
+ if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0; /* read failure */
+ break;
+ }
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ break;
+
+ default:
+ p->status = USB_RET_STALL;
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+ }
+}
+
+static void usb_i2c_handle_data(USBDevice *dev, USBPacket *p)
+{
+ trace_usb_i2c_tiny_usb_i2c_handle_data();
+}
+
+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 const VMStateDescription vmstate_usb_i2c = {
+ .name = "usb-i2c-tiny",
+};
+
+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,
+};
+
+static void usb_i2c_register_types(void)
+{
+ type_register_static(&usb_i2c_dev_type_info);
+ type_register_static(&usbi2c_info);
+}
+
+type_init(usb_i2c_register_types)
diff --git a/trace-events b/trace-events
index 6f03638..89f694b 100644
--- a/trace-events
+++ b/trace-events
@@ -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"
--
1.9.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-04 17:30 Tim Sander
@ 2016-01-05 7:44 ` Gerd Hoffmann
2016-01-05 12:23 ` Tim Sander
0 siblings, 1 reply; 21+ messages in thread
From: Gerd Hoffmann @ 2016-01-05 7:44 UTC (permalink / raw)
To: Tim Sander; +Cc: Paolo Bonzini, Alex Bennée, qemu-devel, Markus Armbruster
> + case 0x4107:
> + /* 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 */
> + 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;
I think most of the tracepoints should be moved into i2c code (or just
dropped in case we already have tracepoints there).
One (high-level) tracepoint per transfer request makes sense in the usb
code, i.e. trace_usb_i2c_transfer_{read,write}, so one can see in the
trace log which usb request triggered which i2c transaction.
> + case 0xc101:
> + {
> + /* thats what the real thing reports, FIXME: can we do better here?
> */
Hmm, didn't we agree on adding a note about what the "real thing" we
mimic here is, to the comment at the start of the file?
> + uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
Can we move 'func' to the start of the function too, like we did with
'i'?
> + case 0xc106:
> + trace_usb_i2c_tiny_unknown_request(index, request, value, length);
> + trace_usb_i2c_tiny_unknown_request(data[0], data[1], data[2],
> data[3]);
> + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
> + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> + p->actual_length = 0;
> + break;
> + }
Doesn't look like this request is unknown ...
> + for (i = 0; i < length; i++) {
> + data[i] = i2c_recv(s->i2cbus);
Can this fail?
cheers,
Gerd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-05 7:44 ` Gerd Hoffmann
@ 2016-01-05 12:23 ` Tim Sander
2016-01-07 9:48 ` Peter Crosthwaite
0 siblings, 1 reply; 21+ messages in thread
From: Tim Sander @ 2016-01-05 12:23 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Paolo Bonzini, Alex Bennée, qemu-devel, Markus Armbruster
Hi Gerd
Thanks for your review.
Am Dienstag, 5. Januar 2016, 08:44:30 schrieb Gerd Hoffmann:
> > + case 0x4107:
> > + /* 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 */
> > + 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;
>
> I think most of the tracepoints should be moved into i2c code (or just
> dropped in case we already have tracepoints there).
I don't think that there are any tracepoints in there.
> One (high-level) tracepoint per transfer request makes sense in the usb
> code, i.e. trace_usb_i2c_transfer_{read,write}, so one can see in the
> trace log which usb request triggered which i2c transaction.
>
> > + case 0xc101:
> > + {
> > + /* thats what the real thing reports, FIXME: can we do better
> > here? */
>
> Hmm, didn't we agree on adding a note about what the "real thing" we
> mimic here is, to the comment at the start of the file?
Ok, that was a missunderstanding. I thought you wanted a description on top of
the patch i was sending but having a description in the file makes sense and i
will add it.
> > + uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
>
> Can we move 'func' to the start of the function too, like we did with
> 'i'?
will do.
> > + case 0xc106:
> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
> > length);
> > + trace_usb_i2c_tiny_unknown_request(data[0], data[1], data[2],
> > data[3]);
> > + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> > + p->actual_length = 0;
> > + break;
> > + }
>
> Doesn't look like this request is unknown ...
>
> > + for (i = 0; i < length; i++) {
> > + data[i] = i2c_recv(s->i2cbus);
>
> Can this fail?
I think failure is just returning 255 as a value? AFAIK thats what real i2c
hardware returns.
Best regards
Tim
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
@ 2016-01-06 14:58 Tim Sander
2016-01-07 10:14 ` Peter Crosthwaite
0 siblings, 1 reply; 21+ messages in thread
From: Tim Sander @ 2016-01-06 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Alex Bennée, Gerd Hoffmann, Markus Armbruster
Version 4 with improvements suggested by Gerd Hoffmann:
Signed-off-by: Tim Sander <tim@krieglstein.org>
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/usb.mak | 1 +
hw/usb/Makefile.objs | 1 +
hw/usb/dev-i2c-tiny.c | 320 ++++++++++++++++++++++++++++++++++++++++++++++++
trace-events | 11 ++
4 files changed, 333 insertions(+)
create mode 100644 hw/usb/dev-i2c-tiny.c
diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index f4b8568..01d2c9f 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -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
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 8f00fbd..3a4c337 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -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
diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
new file mode 100644
index 0000000..c28d7a5
--- /dev/null
+++ b/hw/usb/dev-i2c-tiny.c
@@ -0,0 +1,320 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * i2c-tiny-usb is a small usb to i2c bridge:
+ *
+ * http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
+ *
+ * The simulated device is pretty simple and has no usb endpoints.
+ * There is a Linux device driver available named i2c-tiny-usb.
+ *
+ * Below is an example how to use this device from command line:
+ * -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
+ *
+ * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "trace.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/smbus.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* 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 = (const USBDescConfig[]) {
+ {
+ .bNumInterfaces = 1,
+ .bConfigurationValue = 1,
+ .bmAttributes = USB_CFG_ATT_ONE,
+ .bMaxPower = 50,
+ .nif = 1,
+ .ifs = &desc_iface0,
+ },
+ },
+};
+
+static const USBDesc desc_usb_i2c = {
+ .id = {
+ .idVendor = 0x0403,
+ .idProduct = 0xc631,
+ .bcdDevice = 0x0205,
+ .iManufacturer = STR_MANUFACTURER,
+ .iProduct = STR_PRODUCT_SERIAL,
+ .iSerialNumber = STR_SERIALNUMBER,
+ },
+ .full = &desc_device,
+ .str = desc_strings,
+};
+
+static void usb_i2c_handle_reset(USBDevice *dev)
+{
+ trace_usb_i2c_tiny_reset();
+}
+
+static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
+ int request, int value, int index, int length, uint8_t *data)
+{
+ UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
+ int ret;
+ int i;
+ uint32_t func;
+
+ ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
+ if (ret >= 0) {
+ return;
+ }
+
+ switch (request) {
+ case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+ break;
+ case 0x4102:
+ /* set clock, ignore */
+ trace_usb_i2c_tiny_set_clock();
+ break;
+
+ case 0x4105:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+
+ case 0x4107:
+ /* 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 */
+ 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;
+
+ case 0xc101:
+ /* thats what the real thing reports, FIXME: can we do better here? */
+ func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
+ memcpy(data, &func, sizeof(func));
+ p->actual_length = sizeof(func);
+ trace_usb_i2c_tiny_functionality_read(length);
+ break;
+
+ case 0xc103:
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ p->actual_length = 1;
+ break;
+
+ case 0xc106:
+ if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0;
+ break;
+ }
+ i2c_send(s->i2cbus, data[0]);
+ i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ i2c_nack(s->i2cbus);
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ break;
+
+ case 0xc107:
+ if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
+ trace_usb_i2c_tiny_i2c_start_transfer_failed();
+ p->actual_length = 0; /* read failure */
+ break;
+ }
+ for (i = 0; i < length; i++) {
+ data[i] = i2c_recv(s->i2cbus);
+ trace_usb_i2c_tiny_read(request, index, i, data[i]);
+ }
+ p->actual_length = length;
+ i2c_end_transfer(s->i2cbus);
+ break;
+
+ default:
+ p->status = USB_RET_STALL;
+ trace_usb_i2c_tiny_unknown_request(index, request, value, length);
+ break;
+ }
+}
+
+static void usb_i2c_handle_data(USBDevice *dev, USBPacket *p)
+{
+ trace_usb_i2c_tiny_usb_i2c_handle_data();
+}
+
+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 const VMStateDescription vmstate_usb_i2c = {
+ .name = "usb-i2c-tiny",
+};
+
+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,
+};
+
+static void usb_i2c_register_types(void)
+{
+ type_register_static(&usb_i2c_dev_type_info);
+ type_register_static(&usbi2c_info);
+}
+
+type_init(usb_i2c_register_types)
diff --git a/trace-events b/trace-events
index 6f03638..89f694b 100644
--- a/trace-events
+++ b/trace-events
@@ -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"
--
1.9.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-05 12:23 ` Tim Sander
@ 2016-01-07 9:48 ` Peter Crosthwaite
0 siblings, 0 replies; 21+ messages in thread
From: Peter Crosthwaite @ 2016-01-07 9:48 UTC (permalink / raw)
To: Tim Sander
Cc: Paolo Bonzini, Alex Bennée, Gerd Hoffmann, Markus Armbruster,
qemu-devel@nongnu.org Developers
On Tue, Jan 5, 2016 at 4:23 AM, Tim Sander <tim@krieglstein.org> wrote:
> Hi Gerd
>
> Thanks for your review.
>
> Am Dienstag, 5. Januar 2016, 08:44:30 schrieb Gerd Hoffmann:
>> > + case 0x4107:
>> > + /* 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 */
>> > + 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;
>>
>> I think most of the tracepoints should be moved into i2c code (or just
>> dropped in case we already have tracepoints there).
> I don't think that there are any tracepoints in there.
>
>> One (high-level) tracepoint per transfer request makes sense in the usb
>> code, i.e. trace_usb_i2c_transfer_{read,write}, so one can see in the
>> trace log which usb request triggered which i2c transaction.
>>
>> > + case 0xc101:
>> > + {
>> > + /* thats what the real thing reports, FIXME: can we do better
>> > here? */
>>
>> Hmm, didn't we agree on adding a note about what the "real thing" we
>> mimic here is, to the comment at the start of the file?
> Ok, that was a missunderstanding. I thought you wanted a description on top of
> the patch i was sending but having a description in the file makes sense and i
> will add it.
>
>> > + uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
>>
>> Can we move 'func' to the start of the function too, like we did with
>> 'i'?
> will do.
>
>> > + case 0xc106:
>> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
>> > length);
>> > + trace_usb_i2c_tiny_unknown_request(data[0], data[1], data[2],
>> > data[3]);
>> > + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
>> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
>> > + p->actual_length = 0;
>> > + break;
>> > + }
>>
>> Doesn't look like this request is unknown ...
>>
>> > + for (i = 0; i < length; i++) {
>> > + data[i] = i2c_recv(s->i2cbus);
>>
>> Can this fail?
Yes it can. No device would return -1. Positive values indicate
non-errors I think.
Regards,
Peter
> I think failure is just returning 255 as a value? AFAIK thats what real i2c
> hardware returns.
>
> Best regards
> Tim
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-06 14:58 Tim Sander
@ 2016-01-07 10:14 ` Peter Crosthwaite
2016-01-13 16:07 ` Tim Sander
0 siblings, 1 reply; 21+ messages in thread
From: Peter Crosthwaite @ 2016-01-07 10:14 UTC (permalink / raw)
To: Tim Sander
Cc: Paolo Bonzini, Alex Bennée, qemu-devel@nongnu.org Developers,
Markus Armbruster, Gerd Hoffmann
Patch subject prefix should contain the version number. Use the
--subject-prefix or -v options to git format-patch.
On Wed, Jan 6, 2016 at 6:58 AM, Tim Sander <tim@krieglstein.org> wrote:
> Version 4 with improvements suggested by Gerd Hoffmann:
>
Changelog information should go below the line ...
> Signed-off-by: Tim Sander <tim@krieglstein.org>
>
Signed-off-by usually at end of the commit message.
> 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
> ---
... here.
> default-configs/usb.mak | 1 +
> hw/usb/Makefile.objs | 1 +
> hw/usb/dev-i2c-tiny.c | 320 ++++++++++++++++++++++++++++++++++++++++++++++++
> trace-events | 11 ++
> 4 files changed, 333 insertions(+)
> create mode 100644 hw/usb/dev-i2c-tiny.c
>
> diff --git a/default-configs/usb.mak b/default-configs/usb.mak
> index f4b8568..01d2c9f 100644
> --- a/default-configs/usb.mak
> +++ b/default-configs/usb.mak
> @@ -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
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 8f00fbd..3a4c337 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -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
> diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
> new file mode 100644
> index 0000000..c28d7a5
> --- /dev/null
> +++ b/hw/usb/dev-i2c-tiny.c
> @@ -0,0 +1,320 @@
> +/*
> + * I2C tiny usb device emulation
> + *
> + * i2c-tiny-usb is a small usb to i2c bridge:
> + *
> + * http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
> + *
> + * The simulated device is pretty simple and has no usb endpoints.
> + * There is a Linux device driver available named i2c-tiny-usb.
> + *
> + * Below is an example how to use this device from command line:
> + * -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
> + *
> + * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
> + *
> + * Loosly based on usb dev-serial.c:
> + * Copyright (c) 2006 CodeSourcery.
> + * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
> + * Written by Paul Brook, reused for FTDI by Samuel Thibault
> + *
> + * This code is licensed under the LGPL.
> + *
> + */
> +
> +#include "trace.h"
> +#include "qemu-common.h"
> +#include "qemu/error-report.h"
> +#include "hw/usb.h"
> +#include "hw/usb/desc.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/i2c/smbus.h"
> +#include "sysemu/char.h"
> +#include "endian.h"
> +
> +/* 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;
Acronyms in CamelCase should be all-caps, making this USBI2CTinyState.
> +
> +#define TYPE_USB_I2C_TINY "usb-i2c-dev"
do we need the -dev suffix?
> +#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 = (const USBDescConfig[]) {
> + {
> + .bNumInterfaces = 1,
> + .bConfigurationValue = 1,
> + .bmAttributes = USB_CFG_ATT_ONE,
> + .bMaxPower = 50,
> + .nif = 1,
> + .ifs = &desc_iface0,
> + },
> + },
> +};
> +
> +static const USBDesc desc_usb_i2c = {
> + .id = {
> + .idVendor = 0x0403,
> + .idProduct = 0xc631,
> + .bcdDevice = 0x0205,
> + .iManufacturer = STR_MANUFACTURER,
> + .iProduct = STR_PRODUCT_SERIAL,
> + .iSerialNumber = STR_SERIALNUMBER,
> + },
> + .full = &desc_device,
> + .str = desc_strings,
> +};
> +
> +static void usb_i2c_handle_reset(USBDevice *dev)
> +{
> + trace_usb_i2c_tiny_reset();
> +}
> +
> +static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
> + int request, int value, int index, int length, uint8_t *data)
> +{
> + UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
> + int ret;
> + int i;
> + uint32_t func;
> +
> + ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
> + if (ret >= 0) {
> + return;
> + }
> +
> + switch (request) {
> + case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
> + break;
> + case 0x4102:
> + /* set clock, ignore */
> + trace_usb_i2c_tiny_set_clock();
> + break;
> +
> + case 0x4105:
> + trace_usb_i2c_tiny_unknown_request(index, request, value, length);
> + break;
> +
> + case 0x4107:
> + /* 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 */
> + 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;
> +
> + case 0xc101:
> + /* thats what the real thing reports, FIXME: can we do better here? */
> + func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
> + memcpy(data, &func, sizeof(func));
> + p->actual_length = sizeof(func);
> + trace_usb_i2c_tiny_functionality_read(length);
> + break;
> +
> + case 0xc103:
> + trace_usb_i2c_tiny_unknown_request(index, request, value, length);
> + p->actual_length = 1;
> + break;
> +
> + case 0xc106:
> + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
> + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> + p->actual_length = 0;
> + break;
> + }
> + i2c_send(s->i2cbus, data[0]);
> + i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
> + for (i = 0; i < length; i++) {
> + data[i] = i2c_recv(s->i2cbus);
> + trace_usb_i2c_tiny_read(request, index, i, data[i]);
> + }
> + i2c_nack(s->i2cbus);
This looks odd in that c106 is the only instruction format terminating
with nack.
> + p->actual_length = length;
> + i2c_end_transfer(s->i2cbus);
> + break;
> +
> + case 0xc107:
Your three transacting cases all have very similar functionality. 4107
and c107 only differ in data directionality and only one bit differs
in opcode encoding. Is request an expanding instruction format? The
code should at least be de-duplicated.
You may find this patch helps merge at least 4107 and c107:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04237.html
Regards,
Peter
> + if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
> + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> + p->actual_length = 0; /* read failure */
> + break;
> + }
> + for (i = 0; i < length; i++) {
> + data[i] = i2c_recv(s->i2cbus);
> + trace_usb_i2c_tiny_read(request, index, i, data[i]);
> + }
> + p->actual_length = length;
> + i2c_end_transfer(s->i2cbus);
> + break;
> +
> + default:
> + p->status = USB_RET_STALL;
> + trace_usb_i2c_tiny_unknown_request(index, request, value, length);
> + break;
> + }
> +}
> +
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-07 10:14 ` Peter Crosthwaite
@ 2016-01-13 16:07 ` Tim Sander
2016-01-13 20:59 ` Peter Crosthwaite
0 siblings, 1 reply; 21+ messages in thread
From: Tim Sander @ 2016-01-13 16:07 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Alex Bennée,
Markus Armbruster, Gerd Hoffmann
Hi
Am Donnerstag, 7. Januar 2016, 02:14:23 schrieb Peter Crosthwaite:
> Patch subject prefix should contain the version number. Use the
> --subject-prefix or -v options to git format-patch.
Ok, i will try to remember this next time.
>
> On Wed, Jan 6, 2016 at 6:58 AM, Tim Sander <tim@krieglstein.org> wrote:
> > Version 4 with improvements suggested by Gerd Hoffmann:
> Changelog information should go below the line ...
>
> > Signed-off-by: Tim Sander <tim@krieglstein.org>
>
> Signed-off-by usually at end of the commit message.
>
> > 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
> >
> > ---
>
> ... here.
Ok.
> > default-configs/usb.mak | 1 +
> > hw/usb/Makefile.objs | 1 +
> > hw/usb/dev-i2c-tiny.c | 320
> > ++++++++++++++++++++++++++++++++++++++++++++++++ trace-events
> > | 11 ++
> > 4 files changed, 333 insertions(+)
> > create mode 100644 hw/usb/dev-i2c-tiny.c
> >
> > diff --git a/default-configs/usb.mak b/default-configs/usb.mak
> > index f4b8568..01d2c9f 100644
> > --- a/default-configs/usb.mak
> > +++ b/default-configs/usb.mak
> > @@ -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
> > diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> > index 8f00fbd..3a4c337 100644
> > --- a/hw/usb/Makefile.objs
> > +++ b/hw/usb/Makefile.objs
> > @@ -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
> >
> > diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
> > new file mode 100644
> > index 0000000..c28d7a5
> > --- /dev/null
> > +++ b/hw/usb/dev-i2c-tiny.c
> > @@ -0,0 +1,320 @@
> > +/*
> > + * I2C tiny usb device emulation
> > + *
> > + * i2c-tiny-usb is a small usb to i2c bridge:
> > + *
> > + * http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
> > + *
> > + * The simulated device is pretty simple and has no usb endpoints.
> > + * There is a Linux device driver available named i2c-tiny-usb.
> > + *
> > + * Below is an example how to use this device from command line:
> > + * -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
> > + *
> > + * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
> > + *
> > + * Loosly based on usb dev-serial.c:
> > + * Copyright (c) 2006 CodeSourcery.
> > + * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
> > + * Written by Paul Brook, reused for FTDI by Samuel Thibault
> > + *
> > + * This code is licensed under the LGPL.
> > + *
> > + */
> > +
> > +#include "trace.h"
> > +#include "qemu-common.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/usb.h"
> > +#include "hw/usb/desc.h"
> > +#include "hw/i2c/i2c.h"
> > +#include "hw/i2c/smbus.h"
> > +#include "sysemu/char.h"
> > +#include "endian.h"
> > +
> > +/* 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;
>
> Acronyms in CamelCase should be all-caps, making this USBI2CTinyState.
Will do.
> > +
> > +#define TYPE_USB_I2C_TINY "usb-i2c-dev"
>
> do we need the -dev suffix?
This was just resembling the file name in reverse but i will remove it.
> > +#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 = (const USBDescConfig[]) {
> > + {
> > + .bNumInterfaces = 1,
> > + .bConfigurationValue = 1,
> > + .bmAttributes = USB_CFG_ATT_ONE,
> > + .bMaxPower = 50,
> > + .nif = 1,
> > + .ifs = &desc_iface0,
> > + },
> > + },
> > +};
> > +
> > +static const USBDesc desc_usb_i2c = {
> > + .id = {
> > + .idVendor = 0x0403,
> > + .idProduct = 0xc631,
> > + .bcdDevice = 0x0205,
> > + .iManufacturer = STR_MANUFACTURER,
> > + .iProduct = STR_PRODUCT_SERIAL,
> > + .iSerialNumber = STR_SERIALNUMBER,
> > + },
> > + .full = &desc_device,
> > + .str = desc_strings,
> > +};
> > +
> > +static void usb_i2c_handle_reset(USBDevice *dev)
> > +{
> > + trace_usb_i2c_tiny_reset();
> > +}
> > +
> > +static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
> > + int request, int value, int index, int length, uint8_t
> > *data) +{
> > + UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
> > + int ret;
> > + int i;
> > + uint32_t func;
> > +
> > + ret = usb_desc_handle_control(dev, p, request, value, index, length,
> > data); + if (ret >= 0) {
> > + return;
> > + }
> > +
> > + switch (request) {
> > + case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
> > + break;
> > + case 0x4102:
> > + /* set clock, ignore */
> > + trace_usb_i2c_tiny_set_clock();
> > + break;
> > +
> > + case 0x4105:
> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
> > length);
> > + break;
> > +
> > + case 0x4107:
> > + /* 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 */
> > + 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;
> > +
> > + case 0xc101:
> > + /* thats what the real thing reports, FIXME: can we do better
> > here? */ + func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
> > + memcpy(data, &func, sizeof(func));
> > + p->actual_length = sizeof(func);
> > + trace_usb_i2c_tiny_functionality_read(length);
> > + break;
> > +
> > + case 0xc103:
> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
> > length);
> > + p->actual_length = 1;
> > + break;
> > +
> > + case 0xc106:
> > + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> > + p->actual_length = 0;
> > + break;
> > + }
> > + i2c_send(s->i2cbus, data[0]);
> > + i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
> > + for (i = 0; i < length; i++) {
> > + data[i] = i2c_recv(s->i2cbus);
> > + trace_usb_i2c_tiny_read(request, index, i, data[i]);
> > + }
> > + i2c_nack(s->i2cbus);
>
> This looks odd in that c106 is the only instruction format terminating
> with nack.
This kind of resembles smbus_receive_byte for byte adressing.
As i want to return multiple bytes i could not use smbus_receive_byte directly
AFAIK.
>
> > + p->actual_length = length;
> > + i2c_end_transfer(s->i2cbus);
> > + break;
> > +
>
> > + case 0xc107:
> Your three transacting cases all have very similar functionality. 4107
> and c107 only differ in data directionality and only one bit differs
> in opcode encoding. Is request an expanding instruction format? The
> code should at least be de-duplicated.
>
> You may find this patch helps merge at least 4107 and c107:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04237.html
Is it ok to use the conditional operator for this? Haven't found anything
about it in the coding style.
Best Regards
Tim
> > + if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
> > + p->actual_length = 0; /* read failure */
> > + break;
> > + }
> > + for (i = 0; i < length; i++) {
> > + data[i] = i2c_recv(s->i2cbus);
> > + trace_usb_i2c_tiny_read(request, index, i, data[i]);
> > + }
> > + p->actual_length = length;
> > + i2c_end_transfer(s->i2cbus);
> > + break;
> > +
> > + default:
> > + p->status = USB_RET_STALL;
> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
> > length);
> > + break;
> > + }
> > +}
> > +
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-13 16:07 ` Tim Sander
@ 2016-01-13 20:59 ` Peter Crosthwaite
0 siblings, 0 replies; 21+ messages in thread
From: Peter Crosthwaite @ 2016-01-13 20:59 UTC (permalink / raw)
To: Tim Sander, Frederic Konrad
Cc: Paolo Bonzini, Gerd Hoffmann, Alex Bennée,
qemu-devel@nongnu.org Developers, Markus Armbruster
On Wed, Jan 13, 2016 at 8:07 AM, Tim Sander <tim@krieglstein.org> wrote:
> Hi
> Am Donnerstag, 7. Januar 2016, 02:14:23 schrieb Peter Crosthwaite:
>> Patch subject prefix should contain the version number. Use the
>> --subject-prefix or -v options to git format-patch.
> Ok, i will try to remember this next time.
>>
>> On Wed, Jan 6, 2016 at 6:58 AM, Tim Sander <tim@krieglstein.org> wrote:
>> > Version 4 with improvements suggested by Gerd Hoffmann:
>> Changelog information should go below the line ...
>>
>> > Signed-off-by: Tim Sander <tim@krieglstein.org>
>>
>> Signed-off-by usually at end of the commit message.
>>
>> > 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
>> >
>> > ---
>>
>> ... here.
> Ok.
>
>> > default-configs/usb.mak | 1 +
>> > hw/usb/Makefile.objs | 1 +
>> > hw/usb/dev-i2c-tiny.c | 320
>> > ++++++++++++++++++++++++++++++++++++++++++++++++ trace-events
>> > | 11 ++
>> > 4 files changed, 333 insertions(+)
>> > create mode 100644 hw/usb/dev-i2c-tiny.c
>> >
>> > diff --git a/default-configs/usb.mak b/default-configs/usb.mak
>> > index f4b8568..01d2c9f 100644
>> > --- a/default-configs/usb.mak
>> > +++ b/default-configs/usb.mak
>> > @@ -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
>> > diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
>> > index 8f00fbd..3a4c337 100644
>> > --- a/hw/usb/Makefile.objs
>> > +++ b/hw/usb/Makefile.objs
>> > @@ -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
>> >
>> > diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
>> > new file mode 100644
>> > index 0000000..c28d7a5
>> > --- /dev/null
>> > +++ b/hw/usb/dev-i2c-tiny.c
>> > @@ -0,0 +1,320 @@
>> > +/*
>> > + * I2C tiny usb device emulation
>> > + *
>> > + * i2c-tiny-usb is a small usb to i2c bridge:
>> > + *
>> > + * http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
>> > + *
>> > + * The simulated device is pretty simple and has no usb endpoints.
>> > + * There is a Linux device driver available named i2c-tiny-usb.
>> > + *
>> > + * Below is an example how to use this device from command line:
>> > + * -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
>> > + *
>> > + * Copyright (c) 2015 Tim Sander <tim@krieglstein.org>
>> > + *
>> > + * Loosly based on usb dev-serial.c:
>> > + * Copyright (c) 2006 CodeSourcery.
>> > + * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org>
>> > + * Written by Paul Brook, reused for FTDI by Samuel Thibault
>> > + *
>> > + * This code is licensed under the LGPL.
>> > + *
>> > + */
>> > +
>> > +#include "trace.h"
>> > +#include "qemu-common.h"
>> > +#include "qemu/error-report.h"
>> > +#include "hw/usb.h"
>> > +#include "hw/usb/desc.h"
>> > +#include "hw/i2c/i2c.h"
>> > +#include "hw/i2c/smbus.h"
>> > +#include "sysemu/char.h"
>> > +#include "endian.h"
>> > +
>> > +/* 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;
>>
>> Acronyms in CamelCase should be all-caps, making this USBI2CTinyState.
> Will do.
>> > +
>> > +#define TYPE_USB_I2C_TINY "usb-i2c-dev"
>>
>> do we need the -dev suffix?
> This was just resembling the file name in reverse but i will remove it.
>
>> > +#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 = (const USBDescConfig[]) {
>> > + {
>> > + .bNumInterfaces = 1,
>> > + .bConfigurationValue = 1,
>> > + .bmAttributes = USB_CFG_ATT_ONE,
>> > + .bMaxPower = 50,
>> > + .nif = 1,
>> > + .ifs = &desc_iface0,
>> > + },
>> > + },
>> > +};
>> > +
>> > +static const USBDesc desc_usb_i2c = {
>> > + .id = {
>> > + .idVendor = 0x0403,
>> > + .idProduct = 0xc631,
>> > + .bcdDevice = 0x0205,
>> > + .iManufacturer = STR_MANUFACTURER,
>> > + .iProduct = STR_PRODUCT_SERIAL,
>> > + .iSerialNumber = STR_SERIALNUMBER,
>> > + },
>> > + .full = &desc_device,
>> > + .str = desc_strings,
>> > +};
>> > +
>> > +static void usb_i2c_handle_reset(USBDevice *dev)
>> > +{
>> > + trace_usb_i2c_tiny_reset();
>> > +}
>> > +
>> > +static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
>> > + int request, int value, int index, int length, uint8_t
>> > *data) +{
>> > + UsbI2cTinyState *s = (UsbI2cTinyState *)dev;
>> > + int ret;
>> > + int i;
>> > + uint32_t func;
>> > +
>> > + ret = usb_desc_handle_control(dev, p, request, value, index, length,
>> > data); + if (ret >= 0) {
>> > + return;
>> > + }
>> > +
>> > + switch (request) {
>> > + case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
>> > + break;
>> > + case 0x4102:
>> > + /* set clock, ignore */
>> > + trace_usb_i2c_tiny_set_clock();
>> > + break;
>> > +
>> > + case 0x4105:
>> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
>> > length);
>> > + break;
>> > +
>> > + case 0x4107:
>> > + /* 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 */
>> > + 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;
>> > +
>> > + case 0xc101:
>> > + /* thats what the real thing reports, FIXME: can we do better
>> > here? */ + func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
>> > + memcpy(data, &func, sizeof(func));
>> > + p->actual_length = sizeof(func);
>> > + trace_usb_i2c_tiny_functionality_read(length);
>> > + break;
>> > +
>> > + case 0xc103:
>> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
>> > length);
>> > + p->actual_length = 1;
>> > + break;
>> > +
>> > + case 0xc106:
>> > + if (i2c_start_transfer(s->i2cbus, /*address*/ index, 1)) {
>> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
>> > + p->actual_length = 0;
>> > + break;
>> > + }
>> > + i2c_send(s->i2cbus, data[0]);
>> > + i2c_start_transfer(s->i2cbus, /*address*/ index, 1);
>> > + for (i = 0; i < length; i++) {
>> > + data[i] = i2c_recv(s->i2cbus);
>> > + trace_usb_i2c_tiny_read(request, index, i, data[i]);
>> > + }
>> > + i2c_nack(s->i2cbus);
>>
>> This looks odd in that c106 is the only instruction format terminating
>> with nack.
> This kind of resembles smbus_receive_byte for byte adressing.
> As i want to return multiple bytes i could not use smbus_receive_byte directly
> AFAIK.
>
>>
>> > + p->actual_length = length;
>> > + i2c_end_transfer(s->i2cbus);
>> > + break;
>> > +
>>
>> > + case 0xc107:
>> Your three transacting cases all have very similar functionality. 4107
>> and c107 only differ in data directionality and only one bit differs
>> in opcode encoding. Is request an expanding instruction format? The
>> code should at least be de-duplicated.
>>
>> You may find this patch helps merge at least 4107 and c107:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04237.html
> Is it ok to use the conditional operator for this? Haven't found anything
> about it in the coding style.
>
Conditional operator is sometimes discouraged but still OK (I am a fan
of conditional operator FWIW and think it should be used over one
liner if-else setters always). The problem with conditional operator
in this case was the prototype for the functions is different, with
the data path being either and argument or return value. That patch of
mine makes it consistent for both read and write with a pointer arg
and the return value is always just an error code.
If you want to use conditional operator for the de-duplication that
should be ok until we fix I2C. But this is second controller on list
now with this same problem, the other being the I2C AUX controller for
the Xilinx DP work from Fred Konrad, so we should fix that core API.
Regards,
Peter
> Best Regards
> Tim
>> > + if (i2c_start_transfer(s->i2cbus, /*address*/index, 1)) {
>> > + trace_usb_i2c_tiny_i2c_start_transfer_failed();
>> > + p->actual_length = 0; /* read failure */
>> > + break;
>> > + }
>> > + for (i = 0; i < length; i++) {
>> > + data[i] = i2c_recv(s->i2cbus);
>> > + trace_usb_i2c_tiny_read(request, index, i, data[i]);
>> > + }
>> > + p->actual_length = length;
>> > + i2c_end_transfer(s->i2cbus);
>> > + break;
>> > +
>> > + default:
>> > + p->status = USB_RET_STALL;
>> > + trace_usb_i2c_tiny_unknown_request(index, request, value,
>> > length);
>> > + break;
>> > + }
>> > +}
>> > +
>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-01-13 20:59 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-26 16:35 [Qemu-devel] [PATCH RFC] i2c-tiny-usb Tim Sander
2015-11-26 18:07 ` Alex Bennée
2015-11-27 8:41 ` Tim Sander
2015-11-27 9:35 ` Markus Armbruster
2015-11-27 11:57 ` Alex Bennée
2015-11-27 6:48 ` Gerd Hoffmann
2015-11-27 10:59 ` Tim Sander
[not found] ` <56582329.5040304@redhat.com>
2015-11-27 12:39 ` Tim Sander
2015-11-27 12:53 ` Paolo Bonzini
2015-12-09 16:40 ` [Qemu-devel] i2c data address question was " Tim Sander
2015-12-09 17:04 ` Paolo Bonzini
2015-12-16 15:56 ` [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
2015-12-17 16:15 ` Gerd Hoffmann
-- strict thread matches above, loose matches on Subject: below --
2016-01-04 17:30 Tim Sander
2016-01-05 7:44 ` Gerd Hoffmann
2016-01-05 12:23 ` Tim Sander
2016-01-07 9:48 ` Peter Crosthwaite
2016-01-06 14:58 Tim Sander
2016-01-07 10:14 ` Peter Crosthwaite
2016-01-13 16:07 ` Tim Sander
2016-01-13 20:59 ` Peter Crosthwaite
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).