Re: [PATCH] Add USB sys file-system support (v6)

[Anthony Liguori <anthony@codemonkey.ws>]

TJ wrote:
> 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 <linux@tjworld.net>
> ---
> --- a/usb-linux.c	2008-09-17 22:39:38.000000000 +0100
> +++ b/usb-linux.c	2008-09-23 02:28:48.000000000 +0100
> @@ -7,6 +7,10 @@
>   *      Support for host device auto connect & disconnect
>   *      Major rewrite to support fully async operation
>   *
> + * Copyright 2008 TJ <linux@tjworld.net>
> + *      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 @@
>  #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 @@
>  
>      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);
>   

You have tabs here.

>      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 @@
>      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;
>      }
>   

And here and almost everywhere.

> +
>      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;
> @@ -1106,12 +1132,191 @@
>      fail: ;
>      }
>      if (device_count && (vendor_id || product_id)) {
> -        /* Add the last device.  */
> -        ret = func(opaque, bus_num, addr, class_id, vendor_id,
> -                   product_id, product_name, speed);
> +    	/* Add the last device.  */
> +    	ret = func(opaque, bus_num, addr, class_id, vendor_id,
> +    				product_id, product_name, speed);
> +    }
> + the_end:
> +    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;
>   

This is indented wrong.

> +
> +    		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!)
>   

else belongs on the same line as curly brackets.  Please avoid C99 
comments too.

Regards,

Anthony Liguori