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 <t...@krieglstein.org> wrote: > > Version 4 with improvements suggested by Gerd Hoffmann: > Changelog information should go below the line ... > > > Signed-off-by: Tim Sander <t...@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 <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 "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; > > + } > > +} > > +