
Dear Vivek Gautam,
[...]
This comes as an affect of introduction of "struct usb_ep_desc" which eventually contains "struct usb_endpoint_descriptor" and "struct usb_ss_ep_comp_descriptor".
I see, can we split this? I really enjoy small incremental patches, it's much easier to bisect issues then.
put_unaligned(le16_to_cpu(ep_wMaxPacketSize), &dev->config.\ if_desc[ifno].\ ep_desc[epno].\
wMaxPacketSize);
ep_desc.wMaxPacketSize); USB_PRINTF("if %d, ep %d\n", ifno, epno); break;
case USB_DT_SS_ENDPOINT_COMP:
if_desc = &dev->config.if_desc[ifno];
memcpy(&(if_desc->ep_desc[epno].ss_ep_comp),
Do you need these braces in &(...) ?
True, we can remove these braces.
&buffer[index], buffer[index]);
break; default: if (head->bLength == 0) return 1;
@@ -659,6 +666,18 @@ static int usb_get_string(struct usb_device *dev, unsigned short langid, return result;
}
+/* Allocate usb device */ +int usb_alloc_dev(struct usb_device *dev) +{
int res;
USB_PRINTF("alloc device\n");
this is a debug print.
Isn't USB_PRINTF itself a conditional debug print ?
Yes, but I'd prefer to kill USB_PRINTF altogether in favor of debug().
res = usb_control_msg(dev, usb_sndctrlpipe(dev->parent, 0),
USB_REQ_ALLOC_DEV, 0, 0, 0,
NULL, 0, USB_CNTL_TIMEOUT);
How does this "allocate" a device? Please, do a proper documentation. Actually, take a look at include/linker_lists.h
Or see here: http://git.denx.de/?p=u- boot.git;a=blob;f=include/linker_lists.h;h=0b405d78ea34df1c528fbc4e24ed2a ad756ac4a2;hb=HEAD
And here (U-Boot Code Documentation): http://www.denx.de/wiki/U-Boot/CodingStyle
It'd be really nice if you could follow such pattern of documentation!
Yes, thanks for pointing out. Will document it properly to make things more understandable.
_AWESOME_ !
return res;
+}
[...]
diff --git a/include/common.h b/include/common.h index b23e90b..ef5f687 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *);
#define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y)
+#define min3(a, b, c) min(min(a, b), c)
Isn't this defined somewhere already?
Can you please guide me here, i can see a similar inline function in ehci-hcd only :(
ICK, so we have ad-hoc implementation of this :-( I'd say, put it into common.h and remove the ehci's ad-hoc implementation.
[...]
Rest is just great.
Thanks for reviewing this Marek. I shall update the patchset soon.
Welcome, it's pleasure to work with you ;-)