On Do, 2015-11-26 at 17:35 +0100, Tim Sander wrote:
> Hi
>
> Below is a patch implementing the i2c-tiny-usb device.
Is there a specification for this kind of device?
Or does it mimic existing hardware?
Please add that into to the comment at the head of the file.
> +#ifdef DEBUG_USBI2C
> +#define DPRINTF(fmt, ...) \
> +do { printf("usb-i2c-tiny: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
Please consider turning them into trace points.
> +static const USBDescIface desc_iface0 = {
> + .bInterfaceNumber = 1,
> + .bNumEndpoints = 0,
> + .bInterfaceClass = 0xff,
> + .bInterfaceSubClass = 0xff,
> + .bInterfaceProtocol = 0xff,
> + .eps = (USBDescEndpoint[]) {
> + }
> +};
Hmm? No endpoints?
> +static void usb_i2c_handle_control(USBDevice *dev, USBPacket *p,
> + int request, int value, int index, int length, uint8_t *data)
> +{
> + case 0xc101:
> + {
> + /* thats what the real thing reports, FIXME: can we do better here?
> */
> + uint32_t func = htole32(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL);
> + DPRINTF("got functionality read %x, value %x\n", request, value);
> + memcpy(data, &func, sizeof(func));
> + p->actual_length = sizeof(func);
> + }
> + break;
Ah, it all goes over the control pipe.
> +static USBDevice *usb_i2c_init(USBBus *bus, const char *filename)
Please drop this ...
> + usb_legacy_register("usb-i2c-tiny", "i2c-bus-tiny", usb_i2c_init);
... and this.
It's for backward compatibility with old qemu versions (-usbdevice ...),
and we don't need that for new devices.
cheers,
Gerd
