On Monday 13 February 2012 23:58:19 Marek Vasut wrote: > Also, make usb_stor_buf local.
probably should get split out into its own patch
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
>
> int usb_stor_scan(int mode)
> ...
> unsigned char i;
> struct usb_device *dev;
>
> - /* GJ */
> - memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
> -
> if (mode == 1)
> printf(" scanning bus for storage devices... ");
> ...
> -int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
> +static int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
> block_dev_desc_t *dev_desc)
> {
> unsigned char perq, modi;
> unsigned long cap[2];
> unsigned long *capacity, *blksz;
> ccb *pccb = &usb_ccb;
> + unsigned char usb_stor_buf[512];
> +
> + /* GJ */
> + memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
before, the code would do:
usb_stor_scan() {
memset(usb_stor_buf)
for (...) {
usb_stor_get_info() {
... use usb_stor_buf ...
}
}
}
now the new code calls memset multiple times. in a cursory reading of the
code, i'd say just drop the memset altogether. the get_info func will return
if the usb_inquiry call failed and the buffer contents don't matter. if
usb_query worked, then the buffer should contain valid data and the memset was
pointless.
looking a bit more, i think 512 is too big. it makes sense when it's a global
dumping ground that random parts of the code would use. but this buffer is
only used with usb_inquiry (which in reality is the SCSI INQUIRY command), and
that should require just 36 bytes.
so my suggestion would be:
- drop usb_stor_buf changes from this patch and make a dedicated one
- drop the useless GJ comment
- drop the useless memset
- shrink the buffer to 36 bytes
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
