Re: [PATCH 2/5] husb: support for USB host device auto connect.

[Anthony Liguori <anthony@codemonkey.ws>]

Max Krasnyansky wrote:
> Anyway, it's implemented using a periodic timer that scans host devices
> and grabs those that match the filter. Timer is started when the first
> filter is added.
>   

Again, there has to be a way to get notified of usb device add/remove in 
Linux.

> +static USBHostDevice *hostdev_find(int bus_num, int addr)
> +{
> +    USBHostDevice *s = hostdev_list;
> +    while (s) {
> +        if (s->bus_num == bus_num && s->addr == addr)
> +            return s;
> +        s = s->next;
> +    }
> +    return NULL;
> +}
>   

There are generic list macros in sys-queue.h.  They aren't universally 
used though so it's up to you whether you think it's appropriate.

>  
> +struct USBAutoFilter {
> +    struct USBAutoFilter *next;
> +    int bus_num;
> +    int addr;
> +    int vendor_id;
> +    int product_id;
> +};
> +
> +static QEMUTimer *usb_auto_timer;
> +static struct USBAutoFilter *usb_auto_filter;
> +
> +static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
> +                     int class_id, int vendor_id, int product_id,
> +                     const char *product_name, int speed)
> +{
> +    struct USBAutoFilter *f;
> +    struct USBDevice *dev;
> +
> +    /* Ignore hubs */
> +    if (class_id == 9)
> +        return 0;
> +
> +    for (f = usb_auto_filter; f; f = f->next) {
> +        // printf("Auto match: bus_num %d addr %d vid %d pid %d\n",
> +	//    bus_num, addr, vendor_id, product_id);
>   

An #ifdef guard would be nicer.

> +	if (f->bus_num >= 0 && f->bus_num != bus_num)
> +            continue;
> +
> +	if (f->addr >= 0 && f->addr != addr)
> +            continue;
> +
> +	if (f->vendor_id >= 0 && f->vendor_id != vendor_id)
> +            continue;
> +
> +	if (f->product_id >= 0 && f->product_id != product_id)
> +            continue;
> +
> +        /* We got a match */
> +
> +        /* Allredy attached ? */
> +        if (hostdev_find(bus_num, addr))
> +            return 0;
> +
> +        printf("Auto open: bus_num %d addr %d\n", bus_num, addr);
>   

Please make this debug too.

> +	dev = usb_host_device_open_addr(bus_num, addr, product_name);
> +	if (dev)
> +	    usb_device_add_dev(dev);
> +    }
> +
> +    return 0;
> +}
> +
> +static void usb_host_auto_timer(void *unused)
> +{
> +    usb_host_scan(NULL, usb_host_auto_scan);
> +    qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
> +}
> +
> +/*
> + * Add autoconnect filter
> + * -1 means 'any' (device, vendor, etc)
> + */
> +static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_id)
> +{
> +    struct USBAutoFilter *f = qemu_mallocz(sizeof(*f));
> +    if (!f) {
> +        printf("Failed to allocate auto filter\n");
> +	return;
> +    }
> +
> +    f->bus_num = bus_num;
> +    f->addr    = addr;
> +    f->vendor_id  = vendor_id;
> +    f->product_id = product_id;
> +
> +    if (!usb_auto_filter) {
> +        /*
> +         * First entry. Init and start the monitor.
> +         * Right now we're using timer to check for new devices.
> +         * If this turns out to be too expensive we can move that into a 
> +         * separate thread.
> +         */
> +	usb_auto_timer = qemu_new_timer(rt_clock, usb_host_auto_timer, NULL);
> +	if (!usb_auto_timer) {
> +            printf("Failed to allocate timer\n");
>   

Please print errors to stderr.

Regards,

Anthony Liguori