Am Freitag, 27. November 2015, 07:48:21 schrieb Gerd Hoffmann:
> 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?
http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
> Or does it mimic existing hardware?
Yes, the reason i choose this device where:
*simple
*linux driver available
which makes a cmdline configurable i2c bus easy.
> Please add that into to the comment at the head of the file.
Will do.
> > +#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.
Ok.
> > +static const USBDescIface desc_iface0 = {
> > + .bInterfaceNumber = 1,
> > + .bNumEndpoints = 0,
> > + .bInterfaceClass = 0xff,
> > + .bInterfaceSubClass = 0xff,
> > + .bInterfaceProtocol = 0xff,
> > + .eps = (USBDescEndpoint[]) {
> > + }
> > +};
>
> Hmm? No endpoints?
Yes this device has indeed no endpoints just control as you found out below.
> > +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.
Nice this makes the code even smaller :-).
Best regards
Tim
