* [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; 10+ 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] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge
2016-01-04 17:30 [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
@ 2016-01-05 7:44 ` Gerd Hoffmann
2016-01-05 12:23 ` Tim Sander
0 siblings, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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-16 15:56 ` Tim Sander
2015-12-17 16:15 ` Gerd Hoffmann
0 siblings, 1 reply; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2016-01-13 20:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-04 17:30 [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
2016-01-05 7:44 ` Gerd Hoffmann
2016-01-05 12:23 ` Tim Sander
2016-01-07 9:48 ` Peter Crosthwaite
-- strict thread matches above, loose matches on Subject: below --
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
2015-11-26 16:35 [Qemu-devel] [PATCH RFC] i2c-tiny-usb 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-16 15:56 ` [Qemu-devel] [PATCH] i2c-tiny-usb: add new usb to i2c bridge Tim Sander
2015-12-17 16:15 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.