From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [PATCH 2/5] husb: support for USB host device auto connect.
Date: Thu, 14 Aug 2008 11:41:23 -0500
Message-ID: <48A46033.3070200@codemonkey.ws>
References: <cover.1218685607.git.maxk@kernel.org> <bc54baf4e955c96be17282e99323f11dcf070a4e.1218685608.git.maxk@kernel.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
To: Max Krasnyansky <maxk@kernel.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from wr-out-0506.google.com ([64.233.184.234]:26432 "EHLO
	wr-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756095AbYHNQmE (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 14 Aug 2008 12:42:04 -0400
Received: by wr-out-0506.google.com with SMTP id 69so611648wri.5
        for <kvm@vger.kernel.org>; Thu, 14 Aug 2008 09:42:03 -0700 (PDT)
In-Reply-To: <bc54baf4e955c96be17282e99323f11dcf070a4e.1218685608.git.maxk@kernel.org>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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