Hi Alex Thanks for your feedback, answers as usual inline.
Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée: > Tim Sander <t...@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 <t...@krieglstein.org> > > + * > > + * Loosly based on usb dev-serial.c: > > + * Copyright (c) 2006 CodeSourcery. > > + * Copyright (c) 2008 Samuel Thibault <samuel.thiba...@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