[OPW,kernel] staging: ft1000: function request_code_segment extracted
diff mbox

Message ID 20131025185205.GA16418@kelleynnn-HP-Compaq-8510w
State Changes Requested
Headers show

Commit Message

Kelley Nielsen Oct. 25, 2013, 6:52 p.m. UTC
function scram_dnldr in ft1000_download.c is very long and contains many
coding style errors and best practice violations. It consists of nested
switch statements inside a while loop. One of the inner switch cases has
been extracted as a helper function. Also, some style errors (such as
C99 comments) have been fixed, an assignment to an unread variable has
been removed, and break statements inside ifs have been converted to
returns.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    | 66 ++++++++++------------
 1 file changed, 31 insertions(+), 35 deletions(-)

Comments

Greg Kroah-Hartman Oct. 27, 2013, 1:40 p.m. UTC | #1
On Fri, Oct 25, 2013 at 11:52:05AM -0700, Kelley Nielsen wrote:
> function scram_dnldr in ft1000_download.c is very long and contains many
> coding style errors and best practice violations. It consists of nested
> switch statements inside a while loop. One of the inner switch cases has
> been extracted as a helper function. Also, some style errors (such as
> C99 comments) have been fixed, an assignment to an unread variable has
> been removed, and break statements inside ifs have been converted to
> returns.
> 
> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> ---
>  .../staging/ft1000/ft1000-usb/ft1000_download.c    | 66 ++++++++++------------
>  1 file changed, 31 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> index 8fa0620..4b3bad2 100644
> --- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
> @@ -574,6 +574,33 @@ static u16 scram_start_dwnld(struct ft1000_usb *ft1000dev, u16 *hshake,
>  	return status;
>  }
>  
> +static u16 request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
> +		 u8 **c_file, const u8 *boot_end)
> +{
> +	long word_length;
> +	u16 status = STATUS_SUCCESS;

There's no need to set this here, is there?

> +
> +	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
> +	word_length = get_request_value(ft1000dev);
> +	/*DEBUG("FT1000:word_length = 0x%x\n", (int)word_length); */
> +	/*NdisMSleep (100); */
> +	if (word_length > MAX_LENGTH) {
> +		DEBUG("FT1000:download:Download error: Max length exceeded\n");
> +		return STATUS_FAILURE;
> +	}
> +	if ((word_length * 2 + (long)c_file) > (long)boot_end) {
> +		/* Error, beyond boot code range.*/
> +		DEBUG("FT1000:download:Download error: Requested len=%d exceeds BOOT code boundary.\n", (int)word_length);
> +		return STATUS_FAILURE;
> +	}
> +	if (word_length & 0x1)
> +		word_length++;
> +	word_length = word_length / 2;
> +	status = write_blk(ft1000dev, s_file, c_file, word_length);

Because you always set it here when you return.

Care to fix this up?

thanks,

greg k-h

Patch
diff mbox

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index 8fa0620..4b3bad2 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -574,6 +574,33 @@  static u16 scram_start_dwnld(struct ft1000_usb *ft1000dev, u16 *hshake,
 	return status;
 }
 
+static u16 request_code_segment(struct ft1000_usb *ft1000dev, u16 **s_file,
+		 u8 **c_file, const u8 *boot_end)
+{
+	long word_length;
+	u16 status = STATUS_SUCCESS;
+
+	/*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/
+	word_length = get_request_value(ft1000dev);
+	/*DEBUG("FT1000:word_length = 0x%x\n", (int)word_length); */
+	/*NdisMSleep (100); */
+	if (word_length > MAX_LENGTH) {
+		DEBUG("FT1000:download:Download error: Max length exceeded\n");
+		return STATUS_FAILURE;
+	}
+	if ((word_length * 2 + (long)c_file) > (long)boot_end) {
+		/* Error, beyond boot code range.*/
+		DEBUG("FT1000:download:Download error: Requested len=%d exceeds BOOT code boundary.\n", (int)word_length);
+		return STATUS_FAILURE;
+	}
+	if (word_length & 0x1)
+		word_length++;
+	word_length = word_length / 2;
+	status = write_blk(ft1000dev, s_file, c_file, word_length);
+	/*DEBUG("write_blk returned %d\n", status); */
+
+	return status;
+}
 
 /* Scramble downloader for Harley based ASIC via USB interface */
 u16 scram_dnldr(struct ft1000_usb *ft1000dev, void *pFileStart,
@@ -673,41 +700,10 @@  u16 scram_dnldr(struct ft1000_usb *ft1000dev, void *pFileStart,
 					ft1000dev->fcodeldr = 1;
 					break;
 				case REQUEST_CODE_SEGMENT:
-					//DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");
-					word_length =
-					    get_request_value(ft1000dev);
-					//DEBUG("FT1000:word_length = 0x%x\n", (int)word_length);
-					//NdisMSleep (100);
-					if (word_length > MAX_LENGTH) {
-						DEBUG
-						    ("FT1000:download:Download error: Max length exceeded\n");
-						status = STATUS_FAILURE;
-						break;
-					}
-					if ((word_length * 2 + c_file) >
-					    boot_end) {
-						/*
-						 * Error, beyond boot code range.
-						 */
-						DEBUG
-						    ("FT1000:download:Download error: Requested len=%d exceeds BOOT code boundary.\n",
-						     (int)word_length);
-						status = STATUS_FAILURE;
-						break;
-					}
-					/*
-					 * Position ASIC DPRAM auto-increment pointer.
-					 */
-					dpram = (u16) DWNLD_MAG1_PS_HDR_LOC;
-					if (word_length & 0x1)
-						word_length++;
-					word_length = word_length / 2;
-
-					status =
-					    write_blk(ft1000dev, &s_file,
-						      &c_file, word_length);
-					//DEBUG("write_blk returned %d\n", status);
-					break;
+					status = request_code_segment(ft1000dev,
+							&s_file, &c_file,
+							(const u8 *)boot_end);
+				break;
 				default:
 					DEBUG
 					    ("FT1000:download:Download error: Bad request type=%d in BOOT download state.\n",