From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KjF0p-0006qQ-PA for qemu-devel@nongnu.org; Fri, 26 Sep 2008 11:13:43 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KjF0o-0006p0-EF for qemu-devel@nongnu.org; Fri, 26 Sep 2008 11:13:42 -0400 Received: from [199.232.76.173] (port=37496 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KjF0n-0006oh-NR for qemu-devel@nongnu.org; Fri, 26 Sep 2008 11:13:41 -0400 Received: from mail.windriver.com ([147.11.1.11]:37167 helo=mail.wrs.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KjF0m-0008Lm-QI for qemu-devel@nongnu.org; Fri, 26 Sep 2008 11:13:41 -0400 Message-ID: <48DCFC18.5010506@windriver.com> Date: Fri, 26 Sep 2008 10:13:28 -0500 From: Jason Wessel MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] Add USB sys file-system support (v6) References: <1220571341.2638.6.camel@hephaestion> <1220580385.2638.15.camel@hephaestion> <48C1346F.3000405@windriver.com> <1220640699.5470.15.camel@hephaestion> <48C1862C.3050307@windriver.com> <1220649226.9611.13.camel@hephaestion> <48C53B04.9030006@windriver.com> <1221679892.17792.6.camel@hephaestion> <48D16904.30104@codemonkey.ws> <1221691647.17792.55.camel@hephaestion> <48D81E26.9080802@codemonkey.ws> <1222133639.18297.13.camel@hephaestion> <48DBCE35.4030607@codemonkey.ws> <48DCEA3E.7010006@windriver.com> <48DCF168.5080502@codemonkey.ws> In-Reply-To: <48DCF168.5080502@codemonkey.ws> Content-Type: multipart/mixed; boundary="------------060403040504070209000003" Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: TJ , qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------060403040504070209000003 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Anthony Liguori wrote: > Jason Wessel wrote: >> Anthony if that is all that you consider to be blocking a commit, >> attached is a fixed version with white space and comments fixups. >> > > Hi Jason, > > Thanks for trying to help, but it's *still* whitespace damaged. This > may seem like a minor nit but not taking the time to try and make code > match up with the rest of the source tree doesn't suggest that much time > has been taking to make the code fit in architecturally or to think > about any sort of wider interactions. I also like to give an author the > chance to fix something because it helps encourage people to do the > right thing the first time around. > While I agree in part, it is also frustrating when you want to put changes on top someone else's work that is changing. It is in the spirit of trying to stabilize this patch that I offered a bit of assistance. The rest of your concerns to the white space fixes are addressed in the next version here. I also fixed the instances of: if (f) ... to be: if (f) ... As well a closing else {} problem: if () { ... } else ... to be: if () { ... } else { ... } Perhaps further technical review can occur with no more white space issues? My comment on technical content would be that the several places that atoi() and strtoul() the values are not checked for any kind of correctness and subsequently pass on. Perhaps no validity check is needed because it is dealt with in the later call to the passed in function... It just was not immediately obvious. Jason. --------------060403040504070209000003 Content-Type: text/x-diff; name="sys-usb-v2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="sys-usb-v2.patch" From: TJ Subject: [PATCH] Add USB sys file-system support This patch adds support for host USB devices discovered via: /sys/bus/usb/devices/* and opened from /dev/bus/usb/*/* /dev/bus/usb/devices and opened from /dev/bus/usb/*/* in addition to the existing discovery via: /proc/bus/usb/devices and opened from /proc/bus/usb/*/* Signed-off-by: TJ --- usb-linux.c | 229 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 219 insertions(+), 10 deletions(-) --- a/usb-linux.c +++ b/usb-linux.c @@ -7,6 +7,10 @@ * Support for host device auto connect & disconnect * Major rewrite to support fully async operation * + * Copyright 2008 TJ + * Added flexible support for /dev/bus/usb /sys/bus/usb/devices in addition + * to the legacy /proc/bus/usb USB device discovery and handling + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal * in the Software without restriction, including without limitation the rights @@ -72,9 +76,20 @@ static int usb_host_find_device(int *pbu #define dprintf(...) #endif -#define USBDEVFS_PATH "/proc/bus/usb" +#define USBPROCBUS_PATH "/proc/bus/usb" #define PRODUCT_NAME_SZ 32 #define MAX_ENDPOINTS 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 = 0; /* endpoint association data */ struct endp_data { @@ -890,13 +905,18 @@ static USBDevice *usb_host_device_open_a printf("husb: open device %d.%d\n", bus_num, addr); - snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d", + 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); if (fd < 0) { perror(buf); goto fail; } + dprintf("husb: opened %s\n", buf); /* read the device description */ dev->descr_len = read(fd, dev->descr, sizeof(dev->descr)); @@ -1038,23 +1058,29 @@ static int get_tag_value(char *buf, int return q - buf; } -static int usb_host_scan(void *opaque, USBScanFunc *func) +/* + * 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; + FILE *f = 0; char line[1024]; char buf[1024]; int bus_num, addr, speed, device_count, class_id, product_id, vendor_id; - int ret; char product_name[512]; + int ret = 0; - f = fopen(USBDEVFS_PATH "/devices", "r"); + snprintf(line, sizeof(line), "%s/devices", usb_host_device_path); + f = fopen(line, "r"); if (!f) { - term_printf("husb: could not open %s\n", USBDEVFS_PATH "/devices"); - return 0; + perror("husb: cannot open devices file"); + goto the_end; } + device_count = 0; bus_num = addr = speed = class_id = product_id = vendor_id = 0; - ret = 0; for(;;) { if (fgets(line, sizeof(line), f) == NULL) break; @@ -1111,7 +1137,190 @@ static int usb_host_scan(void *opaque, U product_id, product_name, speed); } the_end: - fclose(f); + if (f) + fclose(f); + return ret; +} + +/* + * Use /sys/bus/usb/devices/ directory to determine host's USB + * devices. + * + * This code is taken from 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) +{ + FILE *f; + DIR *dir = 0; + char line[1024]; + int bus_num, addr, speed, class_id, product_id, vendor_id; + int ret = 0; + char product_name[512]; + struct dirent* de; + + dir = opendir(USBSYSBUS_PATH "/devices"); + if (!dir) { + perror("husb: cannot open devices directory"); + goto the_end; + } + + while ((de = readdir(dir))) { + if (de->d_name[0] != '.' && ! strchr(de->d_name, ':')) { + char filename[PATH_MAX]; + char* tmpstr = de->d_name; + if (!strncmp(de->d_name, "usb", 3)) + tmpstr += 3; + + bus_num = atoi(tmpstr); + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/devnum", de->d_name); + f = fopen(filename, "r"); + if (!f) { + term_printf("Could not open %s\n", filename); + goto the_end; + } + fgets(line, sizeof(line), f); + fclose(f); + addr = atoi(line); + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/bDeviceClass", de->d_name); + f = fopen(filename, "r"); + if (!f) { + term_printf("Could not open %s\n", filename); + goto the_end; + } + fgets(line, sizeof(line), f); + fclose(f); + class_id = strtoul(line, NULL, 16); + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/idVendor", de->d_name); + f = fopen(filename, "r"); + if (!f) { + term_printf("Could not open %s\n", filename); + goto the_end; + } + fgets(line, sizeof(line), f); + fclose(f); + vendor_id = strtoul(line, NULL, 16); + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/idProduct", de->d_name); + f = fopen(filename, "r"); + if (!f) { + term_printf("Could not open %s\n", filename); + goto the_end; + } + fgets(line, sizeof(line), f); + fclose(f); + product_id = strtoul(line, NULL, 16); + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/product", de->d_name); + f = fopen(filename, "r"); + if (f) { + fgets(line, sizeof(line), f); + fclose(f); + if (strlen(line) > 0) + line[strlen(line) - 1] = '\0'; + + pstrcpy(product_name, sizeof(product_name), line); + } else { + *product_name = 0; + } + + snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/speed", de->d_name); + f = fopen(filename, "r"); + if (!f) { + term_printf("Could not open %s\n", filename); + goto the_end; + } + fgets(line, sizeof(line), f); + fclose(f); + if (!strcmp(line, "480\n")) + speed = USB_SPEED_HIGH; + else if (!strcmp(line, "1.5\n")) + speed = USB_SPEED_LOW; + else + speed = USB_SPEED_FULL; + + ret = func(opaque, bus_num, addr, class_id, vendor_id, + product_id, product_name, speed); + if (ret) + goto the_end; + } + } + the_end: + if (dir) closedir(dir); + 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) +{ + FILE *f = 0; + DIR *dir = 0; + int ret = 0; + const char *devices = "/devices"; + const char *trying = "husb: trying to open %s%s\n"; + const char *failed = "husb: could not open %s%s\n"; + char devpath[PATH_MAX]; + + /* only check the host once */ + if (!usb_fs_type) { + /* test for dev file-system access in /proc/ */ + dprintf(trying, USBPROCBUS_PATH, devices); + f = fopen(USBPROCBUS_PATH "/devices", "r"); + if (!f) { + dprintf(failed, USBPROCBUS_PATH, devices); + /* maybe it has been moved to the /dev/ base */ + dprintf(trying, USBDEVBUS_PATH, devices); + f = fopen(USBDEVBUS_PATH "/devices", "r"); + if (!f) { + dprintf(failed, USBDEVBUS_PATH, devices); + /* test for newer sys file-system access */ + dprintf(trying, USBSYSBUS_PATH, devices); + dir = opendir(USBSYSBUS_PATH "/devices"); + if (!dir) { + dprintf(failed, USBSYSBUS_PATH, devices); + goto the_end; + } else { + /* devices found in /dev/bus/usb/ (yes - not a mistake!) */ + strcpy(devpath, USBDEVBUS_PATH); + usb_fs_type = USB_FS_SYS; + } + if (dir) closedir(dir); + } else { + /* devices found in /dev/bus/usb/ */ + strcpy(devpath, USBDEVBUS_PATH); + usb_fs_type = USB_FS_DEV; + } + } else { + /* devices found in /proc/bus/usb/ */ + strcpy(devpath, USBPROCBUS_PATH); + usb_fs_type = USB_FS_PROC; + } + if (f) + fclose(f); + + /* the module setting (used later for opening devices) */ + usb_host_device_path = qemu_mallocz(strlen(devpath)+1); + if (usb_host_device_path) { + strcpy(usb_host_device_path, devpath); + term_printf("husb: using %s\n", usb_host_device_path); + } else { + /* out of memory? */ + perror("husb: unable to allocate memory for device path"); + goto the_end; + } + } + + 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; + } + the_end: return ret; } --------------060403040504070209000003--