> From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Friday, September 26, 2014 2:53 PM > To: Gonglei (Arei) > Cc: Jan Vesely; QEMU > Subject: Re: [Qemu-devel] [PATCH v2 1/3] usb-hid: Move descriptor decision to > usb-hid initfn > > Hi, > > > > -static void usb_hid_initfn(USBDevice *dev, int kind) > > > +static void usb_hid_initfn(USBDevice *dev, int kind, > > > + const USBDesc *usb1, const USBDesc > > > *usb2, > > > + Error **errp) > > > > I don't think it is a good idea that adding usb version as parameters. > > If we want to support usb3.0 tablet, we have to change usb_hid_initfn() too > > in the future and the usb-mouse, usb-kbd also need to changed despite > > usb-mouse and usb-kbd don't support it yet. > > Adding a usb3 parameter should the need arise in the future (unlikely > IMO as xhci can handle all three usb speeds) isn't much of a problem I > think. > > Error handling should be improved though: > > > > { > > > USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev); > > > + switch (us->usb_version) { > > > + case 1: > > > + dev->usb_desc = usb1; > > > + break; > > > + case 2: > > > + dev->usb_desc = usb2; > > > + break; > > > + default: > > dev->usb_desc = NULL; > } > if (dev->usb_desc) { > > > > + error_setg(errp, "Invalid usb version %d for usb hid device" > > > + "(must be 1 or 2)", us->usb_version); > > > + return; > > > + } > > This way it'll be possible to pass NULL for usb2 and have usb_hid_initfn > handle it correctly. > > > > static void usb_mouse_realize(USBDevice *dev, Error **errp) > > > { > > > - usb_hid_initfn(dev, HID_MOUSE); > > > + usb_hid_initfn(dev, HID_MOUSE, &desc_mouse, &desc_mouse, > errp); > > usb_hid_initfn(ev, HID_MOUSE, &desc_mouse, NULL, errp); >
Sounds good. :) Best regards, -Gonglei
