diff mbox

[U-Boot,4/4] USB: Staticize usb_storage.c

Message ID 1329195499-20414-5-git-send-email-marek.vasut@gmail.com
State Changes Requested
Delegated to: Marek Vasut
Headers show

Commit Message

Marek Vasut Feb. 14, 2012, 4:58 a.m. UTC 
Also, make usb_stor_buf local.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Remy Bohmer <linux@bohmer.net>
---
 common/usb_storage.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

Comments

Mike Frysinger Feb. 14, 2012, 5:37 a.m. UTC | #1 
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
diff mbox

Patch

diff --git a/common/usb_storage.c b/common/usb_storage.c
index de84c8d..1446fb0 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -79,7 +79,6 @@  static const unsigned char us_direction[256/8] = {
 };
 #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
 
-static unsigned char usb_stor_buf[512];
 static ccb usb_ccb;
 
 /*
@@ -164,15 +163,14 @@  static struct us_data usb_stor[USB_MAX_STOR_DEV];
 #define USB_STOR_TRANSPORT_FAILED -1
 #define USB_STOR_TRANSPORT_ERROR  -2
 
-int usb_stor_get_info(struct usb_device *dev, struct us_data *us,
+static int usb_stor_get_info(struct usb_device *dev, struct us_data *us,
 		      block_dev_desc_t *dev_desc);
-int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
+static int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
 		      struct us_data *ss);
-unsigned long usb_stor_read(int device, unsigned long blknr,
+static unsigned long usb_stor_read(int device, unsigned long blknr,
 			    unsigned long blkcnt, void *buffer);
-unsigned long usb_stor_write(int device, unsigned long blknr,
+static unsigned long usb_stor_write(int device, unsigned long blknr,
 			     unsigned long blkcnt, const void *buffer);
-struct usb_device * usb_get_dev_index(int index);
 void uhci_show_temp_int_td(void);
 
 #ifdef CONFIG_PARTITIONS
@@ -182,7 +180,7 @@  block_dev_desc_t *usb_stor_get_dev(int index)
 }
 #endif
 
-void usb_show_progress(void)
+static void usb_show_progress(void)
 {
 	debug(".");
 }
@@ -233,9 +231,6 @@  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... ");
 
@@ -493,7 +488,7 @@  static int usb_stor_CB_reset(struct us_data *us)
  * Set up the command for a BBB device. Note that the actual SCSI
  * command is copied into cbw.CBWCDB.
  */
-int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
+static int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 {
 	int result;
 	int actlen;
@@ -541,7 +536,7 @@  int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 /* FIXME: we also need a CBI_command which sets up the completion
  * interrupt, and waits for it
  */
-int usb_stor_CB_comdat(ccb *srb, struct us_data *us)
+static int usb_stor_CB_comdat(ccb *srb, struct us_data *us)
 {
 	int result = 0;
 	int dir_in, retry;
@@ -610,7 +605,7 @@  int usb_stor_CB_comdat(ccb *srb, struct us_data *us)
 }
 
 
-int usb_stor_CBI_get_status(ccb *srb, struct us_data *us)
+static int usb_stor_CBI_get_status(ccb *srb, struct us_data *us)
 {
 	int timeout;
 
@@ -658,7 +653,7 @@  int usb_stor_CBI_get_status(ccb *srb, struct us_data *us)
 #define USB_TRANSPORT_NOT_READY_RETRY 10
 
 /* clear a stall on an endpoint - special for BBB devices */
-int usb_stor_BBB_clear_endpt_stall(struct us_data *us, __u8 endpt)
+static int usb_stor_BBB_clear_endpt_stall(struct us_data *us, __u8 endpt)
 {
 	int result;
 
@@ -669,7 +664,7 @@  int usb_stor_BBB_clear_endpt_stall(struct us_data *us, __u8 endpt)
 	return result;
 }
 
-int usb_stor_BBB_transport(ccb *srb, struct us_data *us)
+static int usb_stor_BBB_transport(ccb *srb, struct us_data *us)
 {
 	int result, retry;
 	int dir_in;
@@ -790,7 +785,7 @@  again:
 	return result;
 }
 
-int usb_stor_CB_transport(ccb *srb, struct us_data *us)
+static int usb_stor_CB_transport(ccb *srb, struct us_data *us)
 {
 	int result, status;
 	ccb *psrb;
@@ -1042,7 +1037,7 @@  static void usb_bin_fixup(struct usb_device_descriptor descriptor,
 }
 #endif /* CONFIG_USB_BIN_FIXUP */
 
-unsigned long usb_stor_read(int device, unsigned long blknr,
+static unsigned long usb_stor_read(int device, unsigned long blknr,
 			    unsigned long blkcnt, void *buffer)
 {
 	unsigned long start, blks, buf_addr;
@@ -1118,7 +1113,7 @@  retry_it:
 	return blkcnt;
 }
 
-unsigned long usb_stor_write(int device, unsigned long blknr,
+static unsigned long usb_stor_write(int device, unsigned long blknr,
 				unsigned long blkcnt, const void *buffer)
 {
 	unsigned long start, blks, buf_addr;
@@ -1199,7 +1194,7 @@  retry_it:
 }
 
 /* Probe to see if a new device is actually a Storage device */
-int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
+static int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
 		      struct us_data *ss)
 {
 	struct usb_interface *iface;
@@ -1339,13 +1334,17 @@  int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
 	return 1;
 }
 
-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));
 
 	pccb->pdata = usb_stor_buf;