PERFORCE change 155820 for review

[M. Warner Losh]

In message: <200901081532.n08FWPDx031982 at repoman.freebsd.org>
            Hans Petter Selasky <hselasky at FreeBSD.org> writes:
: 	USB memory usage reduction patch.

This likely needs to be more descriptive.

: ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#45 (text+ko) ====
: 
: @@ -1290,19 +1290,23 @@
:  	 * Find an unused device index. In USB Host mode this is the
:  	 * same as the device address.
:  	 *
: -	 * NOTE: Index 1 is reserved for the Root HUB.
: +	 * Device index zero is not used and device index 1 should
: +	 * always be the root hub.
:  	 */
: -	for (device_index = USB_ROOT_HUB_ADDR; device_index !=
: -	    USB_MAX_DEVICES; device_index++) {
: +	for (device_index = USB_ROOT_HUB_ADDR;; device_index++) {

This looks wrong.  ';;' seems wrong to me.  While it is acceptable 'C'
code, the fact that you have an if statement at the end means that you
should write this like:

	for (device_index = USB_ROOT_HUB_ADDR;
	    bus->devices[device_index] != NULL; device_index++) {

: +#if (USB_ROOT_HUB_ADDR > USB_MIN_DEVICES)
: +#error "Incorrect device limit."
: +#endif

This likely is the wrong place for this #ifdef.

: +		if (device_index == bus->devices_max) {
: +			device_printf(bus->bdev,
: +			    "No free USB device "
: +			    "index for new device!\n");
: +			return (NULL);
: +		}
:  		if (bus->devices[device_index] == NULL)
:  			break;

See above: likely want to merge this statement into the for loop.

: ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_hub.c#28 (text+ko) ====
: 
: @@ -1520,8 +1520,12 @@
:  	 * The root HUB device is never suspended
:  	 * and we simply skip it.
:  	 */
: -	for (x = USB_ROOT_HUB_ADDR + 1;
: -	    x != USB_MAX_DEVICES; x++) {
: +	for (x = USB_ROOT_HUB_ADDR + 1;; x++) {
: +#if ((USB_ROOT_HUB_ADDR + 1) > USB_MIN_DEVICES)
: +#error "Incorrect device limit."
: +#endif
: +		if (x == bus->devices_max)
: +			break;

Same comments as above.  This #if is in the wrong place for a compile
time assert.  The for loop is weirdly constructed.

:  
:  		udev = bus->devices[x];
:  		if (udev == NULL)
: @@ -1564,8 +1568,12 @@
:  
:  	/* Re-loop all the devices to get the actual state */
:  
: -	for (x = USB_ROOT_HUB_ADDR + 1;
: -	    x != USB_MAX_DEVICES; x++) {
: +	for (x = USB_ROOT_HUB_ADDR + 1;; x++) {
: +#if ((USB_ROOT_HUB_ADDR + 1) > USB_MIN_DEVICES)
: +#error "Incorrect device limit."
: +#endif
: +		if (x == bus->devices_max)
: +			break;

Same comments as above.  This #if is in the wrong place for a compile
time assert.  The for loop is weirdly constructed.

: ==== //depot/projects/usb/src/sys/dev/usb2/include/usb2_defs.h#7 (text+ko) ====
: 
: @@ -35,6 +35,8 @@
:  #define	USB_EP_MAX (2*16)		/* hardcoded */
:  #define	USB_FIFO_MAX (4 * USB_EP_MAX)
:  
: +#define	USB_MIN_DEVICES 2		/* unused + root HUB */
: +
:  #define	USB_MAX_DEVICES USB_DEV_MAX	/* including virtual root HUB and
:  					 * address zero */
:  #define	USB_MAX_ENDPOINTS USB_EP_MAX	/* 2 directions on 16 endpoints */
: @@ -64,5 +66,7 @@
:  #if (USB_EP_MAX < (2*16))
:  #error "Misconfigured limits #3"
:  #endif
: -
: +#if (USB_MAX_DEVICES < USB_MIN_DEVICES)
: +#error "Misconfigured limits #4"
: +#endif
:  #endif					/* _USB2_DEFS_H_ */

These #error messages are lame.  Please make them less lame and more
descriptive.

Warner