All of lore.kernel.org
 help / color / mirror / Atom feed
From: Erik Rull <erik.rull@rdsoftware.de>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] usb-host: rip out legacy procfs support
Date: Mon, 19 Dec 2011 18:33:43 +0100	[thread overview]
Message-ID: <4EEF7577.3060107@rdsoftware.de> (raw)
In-Reply-To: <1324034837-13962-1-git-send-email-kraxel@redhat.com>

Gerd Hoffmann wrote:
> This patch removes support for parsing /proc/bus/usb/devices for device
> discovery.  The code lacks a few features compared to the sysfs code and
> is also bitrotting as everybody has sysfs these days.
>
> This implies having sysfs mounted is mandatory now to use the usb-host
> driver.  udev isn't required though.  qemu will prefer the udev-managed
> device nodes below /dev/bus/usb, but in case this directory isn't preset
> qemu will use the device nodes below /proc/bus/usb (default usbfs mount
> point).
>
> Bottom line: make sure you have both sysfs and usbfs mounted properly,
> and everything should continue to work as it did before.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   usb-linux.c |  327 ++++++++---------------------------------------------------
>   1 files changed, 42 insertions(+), 285 deletions(-)
>
> diff --git a/usb-linux.c b/usb-linux.c
> index ed14bb1..db41104 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -66,23 +66,9 @@ typedef int USBScanFunc(void *opaque, int bus_num, int addr, const char *port,
>   #define DPRINTF(...)
>   #endif
>
> -#define USBDBG_DEVOPENED "husb: opened %s/devices\n"
> -
> -#define USBPROCBUS_PATH "/proc/bus/usb"
>   #define PRODUCT_NAME_SZ 32
>   #define MAX_ENDPOINTS 15
>   #define MAX_PORTLEN 16
> -#define USBDEVBUS_PATH "/dev/bus/usb"
> -#define USBSYSBUS_PATH "/sys/bus/usb"
> -
> -static char *usb_host_device_path;
> -
> -#define USB_FS_NONE 0
> -#define USB_FS_PROC 1
> -#define USB_FS_DEV 2
> -#define USB_FS_SYS 3
> -
> -static int usb_fs_type;
>
>   /* endpoint association data */
>   #define ISO_FRAME_DESC_PER_URB 32
> @@ -430,6 +416,31 @@ static void usb_host_async_cancel(USBDevice *dev, USBPacket *p)
>       }
>   }
>
> +static int usb_host_open_device(int bus, int addr)
> +{
> +    const char *usbfs = NULL;
> +    char filename[32];
> +    struct stat st;
> +    int fd, rc;
> +
> +    rc = stat("/dev/bus/usb",&st);
> +    if (rc == 0&&  S_ISDIR(st.st_mode)) {
> +        /* udev-created device nodes available */
> +        usbfs = "/dev/bus/usb";
> +    } else {
> +        /* fallback: usbfs mounted below /proc */
> +        usbfs = "/proc/bus/usb";
> +    }
> +
> +    snprintf(filename, sizeof(filename), "%s/%03d/%03d",
> +             usbfs, bus, addr);
> +    fd = open(filename, O_RDWR | O_NONBLOCK);
> +    if (fd<  0) {
> +        fprintf(stderr, "husb: open %s: %s\n", filename, strerror(errno));
> +    }
> +    return fd;
> +}
> +
>   static int usb_host_claim_port(USBHostDevice *s)
>   {
>   #ifdef USBDEVFS_CLAIM_PORT
> @@ -459,12 +470,7 @@ static int usb_host_claim_port(USBHostDevice *s)
>           return -1;
>       }
>
> -    if (!usb_host_device_path) {
> -        return -1;
> -    }
> -    snprintf(line, sizeof(line), "%s/%03d/%03d",
> -             usb_host_device_path, s->match.bus_num, hub_addr);
> -    s->hub_fd = open(line, O_RDWR | O_NONBLOCK);
> +    s->hub_fd = usb_host_open_device(s->match.bus_num, hub_addr);
>       if (s->hub_fd<  0) {
>           return -1;
>       }
> @@ -509,10 +515,6 @@ static int usb_linux_get_num_interfaces(USBHostDevice *s)
>       char device_name[64], line[1024];
>       int num_interfaces = 0;
>
> -    if (usb_fs_type != USB_FS_SYS) {
> -        return -1;
> -    }
> -
>       sprintf(device_name, "%d-%s", s->bus_num, s->port);
>       if (!usb_host_read_file(line, sizeof(line), "bNumInterfaces",
>                               device_name)) {
> @@ -1079,41 +1081,21 @@ static int usb_host_handle_control(USBDevice *dev, USBPacket *p,
>   static uint8_t usb_linux_get_alt_setting(USBHostDevice *s,
>       uint8_t configuration, uint8_t interface)
>   {
> -    uint8_t alt_setting;
> -    struct usb_ctrltransfer ct;
> -    int ret;
> -
> -    if (usb_fs_type == USB_FS_SYS) {
> -        char device_name[64], line[1024];
> -        int alt_setting;
> +    char device_name[64], line[1024];
> +    int alt_setting;
>
> -        sprintf(device_name, "%d-%s:%d.%d", s->bus_num, s->port,
> -                (int)configuration, (int)interface);
> +    sprintf(device_name, "%d-%s:%d.%d", s->bus_num, s->port,
> +            (int)configuration, (int)interface);
>
> -        if (!usb_host_read_file(line, sizeof(line), "bAlternateSetting",
> -                                device_name)) {
> -            goto usbdevfs;
> -        }
> -        if (sscanf(line, "%d",&alt_setting) != 1) {
> -            goto usbdevfs;
> -        }
> -        return alt_setting;
> -    }
> -
> -usbdevfs:
> -    ct.bRequestType = USB_DIR_IN | USB_RECIP_INTERFACE;
> -    ct.bRequest = USB_REQ_GET_INTERFACE;
> -    ct.wValue = 0;
> -    ct.wIndex = interface;
> -    ct.wLength = 1;
> -    ct.data =&alt_setting;
> -    ct.timeout = 50;
> -    ret = ioctl(s->fd, USBDEVFS_CONTROL,&ct);
> -    if (ret<  0) {
> +    if (!usb_host_read_file(line, sizeof(line), "bAlternateSetting",
> +                            device_name)) {
> +        /* Assume alt 0 on error */
> +        return 0;
> +    }
> +    if (sscanf(line, "%d",&alt_setting) != 1) {
>           /* Assume alt 0 on error */
>           return 0;
>       }
> -
>       return alt_setting;
>   }
>
> @@ -1262,7 +1244,6 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
>                            const char *prod_name, int speed)
>   {
>       int fd = -1, ret;
> -    char buf[1024];
>
>       trace_usb_host_open_started(bus_num, addr);
>
> @@ -1270,15 +1251,8 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
>           goto fail;
>       }
>
> -    if (!usb_host_device_path) {
> -        perror("husb: USB Host Device Path not set");
> -        goto fail;
> -    }
> -    snprintf(buf, sizeof(buf), "%s/%03d/%03d", usb_host_device_path,
> -             bus_num, addr);
> -    fd = open(buf, O_RDWR | O_NONBLOCK);
> +    fd = usb_host_open_device(bus_num, addr);
>       if (fd<  0) {
> -        perror(buf);
>           goto fail;
>       }
>       DPRINTF("husb: opened %s\n", buf);
> @@ -1526,149 +1500,6 @@ int usb_host_device_close(const char *devname)
>       return -1;
>   }
>
> -static int get_tag_value(char *buf, int buf_size,
> -                         const char *str, const char *tag,
> -                         const char *stopchars)
> -{
> -    const char *p;
> -    char *q;
> -    p = strstr(str, tag);
> -    if (!p) {
> -        return -1;
> -    }
> -    p += strlen(tag);
> -    while (qemu_isspace(*p)) {
> -        p++;
> -    }
> -    q = buf;
> -    while (*p != '\0'&&  !strchr(stopchars, *p)) {
> -        if ((q - buf)<  (buf_size - 1)) {
> -            *q++ = *p;
> -        }
> -        p++;
> -    }
> -    *q = '\0';
> -    return q - buf;
> -}
> -
> -/*
> - * Use /proc/bus/usb/devices or /dev/bus/usb/devices file to determine
> - * host's USB devices. This is legacy support since many distributions
> - * are moving to /sys/bus/usb
> - */
> -static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
> -{
> -    FILE *f = NULL;
> -    char line[1024];
> -    char buf[1024];
> -    int bus_num, addr, speed, device_count;
> -    int class_id, product_id, vendor_id, port;
> -    char product_name[512];
> -    int ret = 0;
> -
> -    if (!usb_host_device_path) {
> -        perror("husb: USB Host Device Path not set");
> -        goto the_end;
> -    }
> -    snprintf(line, sizeof(line), "%s/devices", usb_host_device_path);
> -    f = fopen(line, "r");
> -    if (!f) {
> -        perror("husb: cannot open devices file");
> -        goto the_end;
> -    }
> -
> -    device_count = 0;
> -    bus_num = addr = class_id = product_id = vendor_id = port = 0;
> -    speed = -1; /* Can't get the speed from /[proc|dev]/bus/usb/devices */
> -    for(;;) {
> -        if (fgets(line, sizeof(line), f) == NULL) {
> -            break;
> -        }
> -        if (strlen(line)>  0) {
> -            line[strlen(line) - 1] = '\0';
> -        }
> -        if (line[0] == 'T'&&  line[1] == ':') {
> -            if (device_count&&  (vendor_id || product_id)) {
> -                /* New device.  Add the previously discovered device.  */
> -                if (port>  0) {
> -                    snprintf(buf, sizeof(buf), "%d", port);
> -                } else {
> -                    snprintf(buf, sizeof(buf), "?");
> -                }
> -                ret = func(opaque, bus_num, addr, buf, class_id, vendor_id,
> -                           product_id, product_name, speed);
> -                if (ret) {
> -                    goto the_end;
> -                }
> -            }
> -            if (get_tag_value(buf, sizeof(buf), line, "Bus=", " ")<  0) {
> -                goto fail;
> -            }
> -            bus_num = atoi(buf);
> -            if (get_tag_value(buf, sizeof(buf), line, "Port=", " ")<  0) {
> -                goto fail;
> -            }
> -            port = atoi(buf);
> -            if (get_tag_value(buf, sizeof(buf), line, "Dev#=", " ")<  0) {
> -                goto fail;
> -            }
> -            addr = atoi(buf);
> -            if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ")<  0) {
> -                goto fail;
> -            }
> -            if (!strcmp(buf, "5000")) {
> -                speed = USB_SPEED_SUPER;
> -            } else if (!strcmp(buf, "480")) {
> -                speed = USB_SPEED_HIGH;
> -            } else if (!strcmp(buf, "1.5")) {
> -                speed = USB_SPEED_LOW;
> -            } else {
> -                speed = USB_SPEED_FULL;
> -            }
> -            product_name[0] = '\0';
> -            class_id = 0xff;
> -            device_count++;
> -            product_id = 0;
> -            vendor_id = 0;
> -        } else if (line[0] == 'P'&&  line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Vendor=", " ")<  0) {
> -                goto fail;
> -            }
> -            vendor_id = strtoul(buf, NULL, 16);
> -            if (get_tag_value(buf, sizeof(buf), line, "ProdID=", " ")<  0) {
> -                goto fail;
> -            }
> -            product_id = strtoul(buf, NULL, 16);
> -        } else if (line[0] == 'S'&&  line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Product=", "")<  0) {
> -                goto fail;
> -            }
> -            pstrcpy(product_name, sizeof(product_name), buf);
> -        } else if (line[0] == 'D'&&  line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Cls=", " (")<  0) {
> -                goto fail;
> -            }
> -            class_id = strtoul(buf, NULL, 16);
> -        }
> -    fail: ;
> -    }
> -    if (device_count&&  (vendor_id || product_id)) {
> -        /* Add the last device.  */
> -        if (port>  0) {
> -            snprintf(buf, sizeof(buf), "%d", port);
> -        } else {
> -            snprintf(buf, sizeof(buf), "?");
> -        }
> -        ret = func(opaque, bus_num, addr, buf, class_id, vendor_id,
> -                   product_id, product_name, speed);
> -    }
> - the_end:
> -    if (f) {
> -        fclose(f);
> -    }
> -    return ret;
> -}
> -
>   /*
>    * Read sys file-system device file
>    *
> @@ -1686,7 +1517,7 @@ static int usb_host_read_file(char *line, size_t line_size,
>       int ret = 0;
>       char filename[PATH_MAX];
>
> -    snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/%s", device_name,
> +    snprintf(filename, PATH_MAX, "/sys/bus/usb/devices/%s/%s", device_name,
>                device_file);
>       f = fopen(filename, "r");
>       if (f) {
> @@ -1704,7 +1535,7 @@ static int usb_host_read_file(char *line, size_t line_size,
>    * This code is based on Robert Schiele's original patches posted to
>    * the Novell bug-tracker https://bugzilla.novell.com/show_bug.cgi?id=241950
>    */
> -static int usb_host_scan_sys(void *opaque, USBScanFunc *func)
> +static int usb_host_scan(void *opaque, USBScanFunc *func)
>   {
>       DIR *dir = NULL;
>       char line[1024];
> @@ -1714,9 +1545,10 @@ static int usb_host_scan_sys(void *opaque, USBScanFunc *func)
>       char product_name[512];
>       struct dirent *de;
>
> -    dir = opendir(USBSYSBUS_PATH "/devices");
> +    dir = opendir("/sys/bus/usb/devices");
>       if (!dir) {
> -        perror("husb: cannot open devices directory");
> +        perror("husb: opendir /sys/bus/usb/devices");
> +        fprintf(stderr, "husb: please make sure sysfs is mounted at /sys\n");
>           goto the_end;
>       }
>
> @@ -1791,81 +1623,6 @@ static int usb_host_scan_sys(void *opaque, USBScanFunc *func)
>       return ret;
>   }
>
> -/*
> - * Determine how to access the host's USB devices and call the
> - * specific support function.
> - */
> -static int usb_host_scan(void *opaque, USBScanFunc *func)
> -{
> -    Monitor *mon = cur_mon;
> -    FILE *f = NULL;
> -    DIR *dir = NULL;
> -    int ret = 0;
> -    const char *fs_type[] = {"unknown", "proc", "dev", "sys"};
> -    char devpath[PATH_MAX];
> -
> -    /* only check the host once */
> -    if (!usb_fs_type) {
> -        dir = opendir(USBSYSBUS_PATH "/devices");
> -        if (dir) {
> -            /* devices found in /dev/bus/usb/ (yes - not a mistake!) */
> -            strcpy(devpath, USBDEVBUS_PATH);
> -            usb_fs_type = USB_FS_SYS;
> -            closedir(dir);
> -            DPRINTF(USBDBG_DEVOPENED, USBSYSBUS_PATH);
> -            goto found_devices;
> -        }
> -        f = fopen(USBPROCBUS_PATH "/devices", "r");
> -        if (f) {
> -            /* devices found in /proc/bus/usb/ */
> -            strcpy(devpath, USBPROCBUS_PATH);
> -            usb_fs_type = USB_FS_PROC;
> -            fclose(f);
> -            DPRINTF(USBDBG_DEVOPENED, USBPROCBUS_PATH);
> -            goto found_devices;
> -        }
> -        /* try additional methods if an access method hasn't been found yet */
> -        f = fopen(USBDEVBUS_PATH "/devices", "r");
> -        if (f) {
> -            /* devices found in /dev/bus/usb/ */
> -            strcpy(devpath, USBDEVBUS_PATH);
> -            usb_fs_type = USB_FS_DEV;
> -            fclose(f);
> -            DPRINTF(USBDBG_DEVOPENED, USBDEVBUS_PATH);
> -            goto found_devices;
> -        }
> -    found_devices:
> -        if (!usb_fs_type) {
> -            if (mon) {
> -                monitor_printf(mon, "husb: unable to access USB devices\n");
> -            }
> -            return -ENOENT;
> -        }
> -
> -        /* the module setting (used later for opening devices) */
> -        usb_host_device_path = g_malloc0(strlen(devpath)+1);
> -        strcpy(usb_host_device_path, devpath);
> -        if (mon) {
> -            monitor_printf(mon, "husb: using %s file-system with %s\n",
> -                           fs_type[usb_fs_type], usb_host_device_path);
> -        }
> -    }
> -
> -    switch (usb_fs_type) {
> -    case USB_FS_PROC:
> -    case USB_FS_DEV:
> -        ret = usb_host_scan_dev(opaque, func);
> -        break;
> -    case USB_FS_SYS:
> -        ret = usb_host_scan_sys(opaque, func);
> -        break;
> -    default:
> -        ret = -EINVAL;
> -        break;
> -    }
> -    return ret;
> -}
> -
>   static QEMUTimer *usb_auto_timer;
>
>   static int usb_host_auto_scan(void *opaque, int bus_num,

Works fine from my side!

Best regards,

Erik

      reply	other threads:[~2011-12-19 17:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-16 11:27 [Qemu-devel] [PATCH] usb-host: rip out legacy procfs support Gerd Hoffmann
2011-12-19 17:33 ` Erik Rull [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EEF7577.3060107@rdsoftware.de \
    --to=erik.rull@rdsoftware.de \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.