On Tue, Apr 2, 2013 at 11:24 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev, int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],&buffer[index], buffer[index]);dev->config.if_desc[ifno].no_of_ep = 0;dev->config.if_desc[ifno].num_altsetting = 1;
memcpy(if_desc, &buffer[index], buffer[index]);if_desc->no_of_ep = 0;if_desc->num_altsetting = 1; curr_if_num =
dev-config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++;
if_desc->num_altsetting++;This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
Do we really need this brace here? We just have one statement under this if.
usb_max_devs++;}
&usb_dev_desc[usb_max_devs]) == 1)usb_max_devs++; }What is this change here doing?
There were these unaligned braces for if part, which we removed.
Please ignore above comment. It was meant for change prior to this, above. Will remove this extra line here.
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {[..]
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Thanks & Regards Vivek