[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb
|
From: |
Tim Sander |
|
Subject: |
Re: [Qemu-devel] [PATCH RFC] i2c-tiny-usb |
|
Date: |
Fri, 27 Nov 2015 09:41:35 +0100 |
|
User-agent: |
KMail/4.14.3 (Linux/4.1.0quirk-00001-gf1e27b0-dirty; KDE/4.14.13; x86_64; ; ) |
Hi Alex
Thanks for your feedback, answers as usual inline.
Am Donnerstag, 26. November 2015, 18:07:35 schrieb Alex Bennée:
> Tim Sander <address@hidden> 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 <address@hidden>
> > + *
> > + * Loosly based on usb dev-serial.c:
> > + * Copyright (c) 2006 CodeSourcery.
> > + * Copyright (c) 2008 Samuel Thibault <address@hidden>
> > + * 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