LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/5] staging: ft1000: Remove unused variables.
@ 2011-01-26 11:49 Marek Belisko
  2011-01-26 11:49 ` [PATCH 2/5] staging: ft1000: Remove unnecessary assignment Marek Belisko
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Marek Belisko @ 2011-01-26 11:49 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Marek Belisko

Remove variables which was defined and assigned
but never used in function.

Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index 05f3e22..9e9709a 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -492,9 +492,7 @@ static u32 write_blk (struct ft1000_device *ft1000dev, u16 **pUsFile, u8 **pUcFi
 {
    u32 Status = STATUS_SUCCESS;
    u16 dpram;
-   long temp_word_length;
    int loopcnt, i, j;
-   u16 *pTempFile;
    u16 tempword;
    u16 tempbuffer[64];
    u16 resultbuffer[64];
@@ -513,8 +511,6 @@ static u32 write_blk (struct ft1000_device *ft1000dev, u16 **pUsFile, u8 **pUcFi
    word_length--;
    tempword = (u16)word_length;
    word_length = (word_length / 16) + 1;
-   pTempFile = *pUsFile;
-   temp_word_length = word_length;
    for (; word_length > 0; word_length--) /* In words */
    {
 	   loopcnt = 0;
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/5] staging: ft1000: Remove unnecessary assignment.
  2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
@ 2011-01-26 11:49 ` Marek Belisko
  2011-01-26 11:49 ` [PATCH 3/5] staging: ft1000: Create common function for buffers check Marek Belisko
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Marek Belisko @ 2011-01-26 11:49 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Marek Belisko

dsp_img_info->version and requested_version have same type
so additional temporary variable creation could be omitted
because variables could be compares directly.

Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index 9e9709a..b99553b 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -1026,12 +1026,7 @@ u16 scram_dnldr(struct ft1000_device *ft1000dev, void *pFileStart, u32  FileLeng
 
                for (image = 0; image < file_hdr->nDspImages; image++)
                {
-
-                       temp = (u16)(dsp_img_info->version);
-                       templong = temp;
-                       temp = (u16)(dsp_img_info->version >> 16);
-                       templong |= (temp << 16);
-                   if (templong == (u32)requested_version)
+                   if (dsp_img_info->version == requested_version)
                        {
                            correct_version = TRUE;
                            DEBUG("FT1000:download: correct_version is TRUE\n");
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/5] staging: ft1000: Create common function for buffers check.
  2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
  2011-01-26 11:49 ` [PATCH 2/5] staging: ft1000: Remove unnecessary assignment Marek Belisko
@ 2011-01-26 11:49 ` Marek Belisko
  2011-01-26 11:49 ` [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function Marek Belisko
  2011-01-26 11:49 ` [PATCH 5/5] staging: ft1000: Fix indentation in scram_dnldr() function Marek Belisko
  3 siblings, 0 replies; 16+ messages in thread
From: Marek Belisko @ 2011-01-26 11:49 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Marek Belisko

Same check was done on three places which make code unreadable.
Put repeat routine to separate function.

Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |   59 ++++++++++----------
 1 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index b99553b..2537a11 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -470,6 +470,17 @@ static u16 hdr_checksum(struct pseudo_hdr *pHdr)
 	return chksum;
 }
 
+static int check_buffers(u16 *buff_w, u16 *buff_r, int len, int offset)
+{
+	int i;
+
+	for (i = 0; i < len; i++) {
+		if (buff_w[i] != buff_r[i + offset])
+			return -1;
+	}
+
+	return 0;
+}
 
 //---------------------------------------------------------------------------
 // Function:    write_blk
@@ -560,43 +571,31 @@ static u32 write_blk (struct ft1000_device *ft1000dev, u16 **pUsFile, u8 **pUcFi
 	    	       Status = ft1000_read_dpram32 (ft1000dev, dpram, (u8 *)&resultbuffer[0], 64);
 		       if ( (tempbuffer[31] & 0xfe00) == 0xfe00)
 		       {
-		           for (i=0; i<28; i++)
-		           {
-			       if (resultbuffer[i] != tempbuffer[i])
-			       {
-			           //NdisMSleep (100);
-                                   DEBUG("FT1000:download:DPRAM write failed 1 during bootloading\n");
-				   msleep(10);
-     	  			   Status = STATUS_FAILURE;
-				   break;
+				if (check_buffers(tempbuffer, resultbuffer, 28, 0)) {
+					DEBUG("FT1000:download:DPRAM write failed 1 during bootloading\n");
+					msleep(10);
+					Status = STATUS_FAILURE;
+					break;
 				}
-			   }
    			   Status = ft1000_read_dpram32 (ft1000dev, dpram+12, (u8 *)&resultbuffer[0], 64);
-		           for (i=0; i<16; i++)
-		           {
-    			       if (resultbuffer[i] != tempbuffer[i+24])
-    			       {
-                                   //NdisMSleep (100);
-                                   DEBUG("FT1000:download:DPRAM write failed 2 during bootloading\n");
-				   msleep(10);
-				   Status = STATUS_FAILURE;
-				   break;
+
+				if (check_buffers(tempbuffer, resultbuffer, 16, 24)) {
+					DEBUG("FT1000:download:DPRAM write failed 2 during bootloading\n");
+					msleep(10);
+					Status = STATUS_FAILURE;
+					break;
 				}
-			   }
+			   
 			}
 			else
 			{
-			    for (i=0; i<32; i++)
-			    {
-    			        if (resultbuffer[i] != tempbuffer[i])
-    			        {
-                                    //NdisMSleep (100);
-                                    DEBUG("FT1000:download:DPRAM write failed 3 during bootloading\n");
-				    msleep(10);
-				    Status = STATUS_FAILURE;
-				    break;
+				if (check_buffers(tempbuffer, resultbuffer, 32, 0)) {
+					DEBUG("FT1000:download:DPRAM write failed 3 during bootloading\n");
+					msleep(10);
+					Status = STATUS_FAILURE;
+					break;
 				}
-			    }
+			    
 			}
 
 			if (Status == STATUS_SUCCESS)
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
  2011-01-26 11:49 ` [PATCH 2/5] staging: ft1000: Remove unnecessary assignment Marek Belisko
  2011-01-26 11:49 ` [PATCH 3/5] staging: ft1000: Create common function for buffers check Marek Belisko
@ 2011-01-26 11:49 ` Marek Belisko
  2011-01-26 13:07   ` Dan Carpenter
  2011-01-26 11:49 ` [PATCH 5/5] staging: ft1000: Fix indentation in scram_dnldr() function Marek Belisko
  3 siblings, 1 reply; 16+ messages in thread
From: Marek Belisko @ 2011-01-26 11:49 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Marek Belisko

Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |   56 +++++++++----------
 1 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index 2537a11..1e12c41 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -639,44 +639,42 @@ static void usb_dnld_complete (struct urb *urb)
 // Notes:
 //
 //---------------------------------------------------------------------------
-static u32 write_blk_fifo (struct ft1000_device *ft1000dev, u16 **pUsFile, u8 **pUcFile, long word_length)
+static u32 write_blk_fifo(struct ft1000_device *ft1000dev, u16 **pUsFile,
+			  u8 **pUcFile, long word_length)
 {
-   u32 Status = STATUS_SUCCESS;
-   int byte_length;
-   long aligncnt;
-
-   byte_length = word_length * 4;
+	u32 Status = STATUS_SUCCESS;
+	int byte_length;
+	long aligncnt;
 
-   if (byte_length % 4)
-      aligncnt = 4 - (byte_length % 4);
-   else
-      aligncnt = 0;
-   byte_length += aligncnt;
+	byte_length = word_length * 4;
 
-   if (byte_length && ((byte_length % 64) == 0)) {
-      byte_length += 4;
-   }
+	if (byte_length % 4)
+		aligncnt = 4 - (byte_length % 4);
+	else
+		aligncnt = 0;
+	byte_length += aligncnt;
 
-   if (byte_length < 64)
-       byte_length = 68;
+	if (byte_length && ((byte_length % 64) == 0))
+		byte_length += 4;
 
+	if (byte_length < 64)
+		byte_length = 68;
 
-    usb_init_urb(ft1000dev->tx_urb);
-    memcpy (ft1000dev->tx_buf, *pUcFile, byte_length);
-    usb_fill_bulk_urb(ft1000dev->tx_urb,
-                      ft1000dev->dev,
-                      usb_sndbulkpipe(ft1000dev->dev, ft1000dev->bulk_out_endpointAddr),
-                      ft1000dev->tx_buf,
-                      byte_length,
-                      usb_dnld_complete,
-                      (void*)ft1000dev);
+	usb_init_urb(ft1000dev->tx_urb);
+	memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
+	usb_fill_bulk_urb(ft1000dev->tx_urb,
+			  ft1000dev->dev,
+			  usb_sndbulkpipe(ft1000dev->dev,
+					  ft1000dev->bulk_out_endpointAddr),
+			  ft1000dev->tx_buf, byte_length, usb_dnld_complete,
+			  (void *)ft1000dev);
 
-    usb_submit_urb(ft1000dev->tx_urb, GFP_ATOMIC);
+	usb_submit_urb(ft1000dev->tx_urb, GFP_ATOMIC);
 
-   *pUsFile = *pUsFile + (word_length << 1);
-   *pUcFile = *pUcFile + (word_length << 2);
+	*pUsFile = *pUsFile + (word_length << 1);
+	*pUcFile = *pUcFile + (word_length << 2);
 
-   return Status;
+	return Status;
 }
 
 //---------------------------------------------------------------------------
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 5/5] staging: ft1000: Fix indentation in scram_dnldr() function.
  2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
                   ` (2 preceding siblings ...)
  2011-01-26 11:49 ` [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function Marek Belisko
@ 2011-01-26 11:49 ` Marek Belisko
  3 siblings, 0 replies; 16+ messages in thread
From: Marek Belisko @ 2011-01-26 11:49 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, Marek Belisko

Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 .../staging/ft1000/ft1000-usb/ft1000_download.c    |  934 +++++++++++---------
 1 files changed, 509 insertions(+), 425 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index 1e12c41..942d923 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -689,450 +689,533 @@ static u32 write_blk_fifo(struct ft1000_device *ft1000dev, u16 **pUsFile,
 //  Returns:    status                  - return code
 //---------------------------------------------------------------------------
 
-u16 scram_dnldr(struct ft1000_device *ft1000dev, void *pFileStart, u32  FileLength)
+u16 scram_dnldr(struct ft1000_device *ft1000dev, void *pFileStart,
+		u32 FileLength)
 {
-   u16                     status = STATUS_SUCCESS;
-   u32                    state;
-   u16                  handshake;
+	u16 status = STATUS_SUCCESS;
+	u32 state;
+	u16 handshake;
 	struct pseudo_hdr *pseudo_header;
-   u16                  pseudo_header_len;
-   long                    word_length;
-   u16                  request;
-   u16                  temp;
-   u16                  tempword;
+	u16 pseudo_header_len;
+	long word_length;
+	u16 request;
+	u16 temp;
+	u16 tempword;
 
 	struct dsp_file_hdr *file_hdr;
 	struct dsp_image_info *dsp_img_info = NULL;
-   long                    requested_version;
-   bool                 correct_version;
+	long requested_version;
+	bool correct_version;
 	struct drv_msg *mailbox_data;
-   u16                  *data = NULL;
-   u16                  *s_file = NULL;
-   u8                   *c_file = NULL;
-   u8                   *boot_end = NULL, *code_end= NULL;
-   int                     image;
-   long                    loader_code_address, loader_code_size = 0;
-   long                    run_address = 0, run_size = 0;
-
-   u32                   templong;
-   u32                   image_chksum = 0;
-
-   u16                  dpram = 0;
-   u8 *pbuffer;
+	u16 *data = NULL;
+	u16 *s_file = NULL;
+	u8 *c_file = NULL;
+	u8 *boot_end = NULL, *code_end = NULL;
+	int image;
+	long loader_code_address, loader_code_size = 0;
+	long run_address = 0, run_size = 0;
+
+	u32 templong;
+	u32 image_chksum = 0;
+
+	u16 dpram = 0;
+	u8 *pbuffer;
 	struct prov_record *pprov_record;
 	struct ft1000_info *pft1000info = netdev_priv(ft1000dev->net);
 
-   DEBUG("Entered   scram_dnldr...\n");
+	DEBUG("Entered   scram_dnldr...\n");
 
-   pft1000info->fcodeldr = 0;
-   pft1000info->usbboot = 0;
-   pft1000info->dspalive = 0xffff;
+	pft1000info->fcodeldr = 0;
+	pft1000info->usbboot = 0;
+	pft1000info->dspalive = 0xffff;
 
+	//
+	// Get version id of file, at first 4 bytes of file, for newer files.
+	//
 
-   //
-   // Get version id of file, at first 4 bytes of file, for newer files.
-   //
+	state = STATE_START_DWNLD;
 
-   state = STATE_START_DWNLD;
+	file_hdr = (struct dsp_file_hdr *)pFileStart;
 
-   file_hdr = (struct dsp_file_hdr *)pFileStart;
+	ft1000_write_register(ft1000dev, 0x800, FT1000_REG_MAG_WATERMARK);
 
-   ft1000_write_register (ft1000dev, 0x800, FT1000_REG_MAG_WATERMARK);
+	s_file = (u16 *) (pFileStart + file_hdr->loader_offset);
+	c_file = (u8 *) (pFileStart + file_hdr->loader_offset);
 
-      s_file = (u16 *)(pFileStart + file_hdr->loader_offset);
-      c_file = (u8 *)(pFileStart + file_hdr->loader_offset);
+	boot_end = (u8 *) (pFileStart + file_hdr->loader_code_end);
 
-      boot_end = (u8 *)(pFileStart + file_hdr->loader_code_end);
+	loader_code_address = file_hdr->loader_code_address;
+	loader_code_size = file_hdr->loader_code_size;
+	correct_version = FALSE;
+
+	while ((status == STATUS_SUCCESS) && (state != STATE_DONE_FILE)) {
+		switch (state) {
+		case STATE_START_DWNLD:
+			DEBUG("FT1000:STATE_START_DWNLD\n");
+			if (pft1000info->usbboot)
+				handshake =
+				    get_handshake_usb(ft1000dev,
+						      HANDSHAKE_DSP_BL_READY);
+			else
+				handshake =
+				    get_handshake(ft1000dev,
+						  HANDSHAKE_DSP_BL_READY);
+
+			if (handshake == HANDSHAKE_DSP_BL_READY) {
+				DEBUG
+				    ("scram_dnldr: handshake is HANDSHAKE_DSP_BL_READY, call put_handshake(HANDSHAKE_DRIVER_READY)\n");
+				put_handshake(ft1000dev,
+					      HANDSHAKE_DRIVER_READY);
+			} else {
+				DEBUG
+				    ("FT1000:download:Download error: Handshake failed\n");
+				status = STATUS_FAILURE;
+			}
 
-      loader_code_address = file_hdr->loader_code_address;
-      loader_code_size = file_hdr->loader_code_size;
-      correct_version = FALSE;
+			state = STATE_BOOT_DWNLD;
 
-   while ((status == STATUS_SUCCESS) && (state != STATE_DONE_FILE))
-   {
-      switch (state)
-      {
-      case  STATE_START_DWNLD:
-         DEBUG("FT1000:STATE_START_DWNLD\n");
-         if (pft1000info->usbboot)
-             handshake = get_handshake_usb(ft1000dev, HANDSHAKE_DSP_BL_READY);
-         else
-             handshake = get_handshake(ft1000dev, HANDSHAKE_DSP_BL_READY);
-
-         if (handshake == HANDSHAKE_DSP_BL_READY)
-         {
-            DEBUG("scram_dnldr: handshake is HANDSHAKE_DSP_BL_READY, call put_handshake(HANDSHAKE_DRIVER_READY)\n");
-            put_handshake(ft1000dev, HANDSHAKE_DRIVER_READY);
-         }
-         else
-         {
-            DEBUG("FT1000:download:Download error: Handshake failed\n");
-            status = STATUS_FAILURE;
-         }
-
-         state = STATE_BOOT_DWNLD;
-
-         break;
-
-      case STATE_BOOT_DWNLD:
-         DEBUG("FT1000:STATE_BOOT_DWNLD\n");
-         pft1000info->bootmode = 1;
-         handshake = get_handshake(ft1000dev, HANDSHAKE_REQUEST);
-         if (handshake == HANDSHAKE_REQUEST)
-         {
-            /*
-             * Get type associated with the request.
-             */
-            request = get_request_type(ft1000dev);
-            switch (request)
-            {
-            case  REQUEST_RUN_ADDRESS:
-               DEBUG("FT1000:REQUEST_RUN_ADDRESS\n");
-               put_request_value(ft1000dev, loader_code_address);
-               break;
-            case  REQUEST_CODE_LENGTH:
-               DEBUG("FT1000:REQUEST_CODE_LENGTH\n");
-               put_request_value(ft1000dev, loader_code_size);
-               break;
-            case  REQUEST_DONE_BL:
-               DEBUG("FT1000:REQUEST_DONE_BL\n");
-               /* Reposition ptrs to beginning of code section */
-               s_file = (u16 *)(boot_end);
-               c_file = (u8 *)(boot_end);
-               //DEBUG("FT1000:download:s_file = 0x%8x\n", (int)s_file);
-               //DEBUG("FT1000:download:c_file = 0x%8x\n", (int)c_file);
-               state = STATE_CODE_DWNLD;
-               pft1000info->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 boundry.\n",
-                                                            (int)word_length);
-                  status = STATUS_FAILURE;
-                  break;
-               }
-               /*
-                * Position ASIC DPRAM auto-increment pointer.
-                */
-				    dpram = (u16)DWNLD_MAG1_PS_HDR_LOC;
+			break;
+
+		case STATE_BOOT_DWNLD:
+			DEBUG("FT1000:STATE_BOOT_DWNLD\n");
+			pft1000info->bootmode = 1;
+			handshake = get_handshake(ft1000dev, HANDSHAKE_REQUEST);
+			if (handshake == HANDSHAKE_REQUEST) {
+				/*
+				 * Get type associated with the request.
+				 */
+				request = get_request_type(ft1000dev);
+				switch (request) {
+				case REQUEST_RUN_ADDRESS:
+					DEBUG("FT1000:REQUEST_RUN_ADDRESS\n");
+					put_request_value(ft1000dev,
+							  loader_code_address);
+					break;
+				case REQUEST_CODE_LENGTH:
+					DEBUG("FT1000:REQUEST_CODE_LENGTH\n");
+					put_request_value(ft1000dev,
+							  loader_code_size);
+					break;
+				case REQUEST_DONE_BL:
+					DEBUG("FT1000:REQUEST_DONE_BL\n");
+					/* Reposition ptrs to beginning of code section */
+					s_file = (u16 *) (boot_end);
+					c_file = (u8 *) (boot_end);
+					//DEBUG("FT1000:download:s_file = 0x%8x\n", (int)s_file);
+					//DEBUG("FT1000:download:c_file = 0x%8x\n", (int)c_file);
+					state = STATE_CODE_DWNLD;
+					pft1000info->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 boundry.\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;
-            default:
-               DEBUG("FT1000:download:Download error: Bad request type=%d in BOOT download state.\n",request);
-               status = STATUS_FAILURE;
-               break;
-            }
-            if (pft1000info->usbboot)
-                put_handshake_usb(ft1000dev, HANDSHAKE_RESPONSE);
-            else
-                put_handshake(ft1000dev, HANDSHAKE_RESPONSE);
-         }
-         else
-         {
-            DEBUG("FT1000:download:Download error: Handshake failed\n");
-            status = STATUS_FAILURE;
-         }
-
-         break;
-
-      case STATE_CODE_DWNLD:
-         //DEBUG("FT1000:STATE_CODE_DWNLD\n");
-         pft1000info->bootmode = 0;
-         if (pft1000info->usbboot)
-            handshake = get_handshake_usb(ft1000dev, HANDSHAKE_REQUEST);
-         else
-            handshake = get_handshake(ft1000dev, HANDSHAKE_REQUEST);
-         if (handshake == HANDSHAKE_REQUEST)
-         {
-            /*
-             * Get type associated with the request.
-             */
-            if (pft1000info->usbboot)
-                request = get_request_type_usb(ft1000dev);
-            else
-                request = get_request_type(ft1000dev);
-            switch (request)
-            {
-            case REQUEST_FILE_CHECKSUM:
-                DEBUG("FT1000:download:image_chksum = 0x%8x\n", image_chksum);
-                put_request_value(ft1000dev, image_chksum);
-                break;
-            case  REQUEST_RUN_ADDRESS:
-               DEBUG("FT1000:download:  REQUEST_RUN_ADDRESS\n");
-               if (correct_version)
-               {
-                  DEBUG("FT1000:download:run_address = 0x%8x\n", (int)run_address);
-                  put_request_value(ft1000dev, run_address);
-               }
-               else
-               {
-                  DEBUG("FT1000:download:Download error: Got Run address request before image offset request.\n");
-                  status = STATUS_FAILURE;
-                  break;
-               }
-               break;
-            case  REQUEST_CODE_LENGTH:
-               DEBUG("FT1000:download:REQUEST_CODE_LENGTH\n");
-               if (correct_version)
-               {
-                  DEBUG("FT1000:download:run_size = 0x%8x\n", (int)run_size);
-                  put_request_value(ft1000dev, run_size);
-               }
-               else
-               {
-                  DEBUG("FT1000:download:Download error: Got Size request before image offset request.\n");
-                  status = STATUS_FAILURE;
-                  break;
-               }
-               break;
-            case  REQUEST_DONE_CL:
-               pft1000info->usbboot = 3;
-               /* Reposition ptrs to beginning of provisioning section */
-                  s_file = (u16 *)(pFileStart + file_hdr->commands_offset);
-                  c_file = (u8 *)(pFileStart + file_hdr->commands_offset);
-               state = STATE_DONE_DWNLD;
-               break;
-            case  REQUEST_CODE_SEGMENT:
-               //DEBUG("FT1000:download: REQUEST_CODE_SEGMENT - CODELOADER\n");
-               if (!correct_version)
-               {
-                  DEBUG("FT1000:download:Download error: Got Code Segment request before image offset request.\n");
-                  status = STATUS_FAILURE;
-                  break;
-               }
-
-               word_length = get_request_value(ft1000dev);
-               //DEBUG("FT1000:download:word_length = %d\n", (int)word_length);
-               if (word_length > MAX_LENGTH)
-               {
-                  DEBUG("FT1000:download:Download error: Max length exceeded\n");
-                  status = STATUS_FAILURE;
-                  break;
-               }
-               if ( (word_length*2 + c_file) > code_end)
-               {
-                  /*
-                   * Error, beyond boot code range.
-                   */
-                  DEBUG("FT1000:download:Download error: Requested len=%d exceeds DSP code boundry.\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;
-
-   	       write_blk_fifo (ft1000dev, &s_file, &c_file, word_length);
-               if (pft1000info->usbboot == 0)
-                   pft1000info->usbboot++;
-               if (pft1000info->usbboot == 1) {
-                   tempword = 0;
-                   ft1000_write_dpram16 (ft1000dev, DWNLD_MAG1_PS_HDR_LOC, tempword, 0);
-               }
-
-               break;
-
-            case  REQUEST_MAILBOX_DATA:
-               DEBUG("FT1000:download: REQUEST_MAILBOX_DATA\n");
-               // Convert length from byte count to word count. Make sure we round up.
-               word_length = (long)(pft1000info->DSPInfoBlklen + 1)/2;
-               put_request_value(ft1000dev, word_length);
-		mailbox_data = (struct drv_msg *)&(pft1000info->DSPInfoBlk[0]);
-               /*
-                * Position ASIC DPRAM auto-increment pointer.
-                */
-
-
-                   data = (u16 *)&mailbox_data->data[0];
-                   dpram = (u16)DWNLD_MAG1_PS_HDR_LOC;
-                   if (word_length & 0x1)
-                       word_length++;
-
-                   word_length = (word_length / 2);
-
-
-               for (; word_length > 0; word_length--) /* In words */
-               {
-
-                      templong = *data++;
-					  templong |= (*data++ << 16);
-                      status = fix_ft1000_write_dpram32 (ft1000dev, dpram++, (u8 *)&templong);
-
-               }
-               break;
-
-            case  REQUEST_VERSION_INFO:
-               DEBUG("FT1000:download:REQUEST_VERSION_INFO\n");
-               word_length = file_hdr->version_data_size;
-               put_request_value(ft1000dev, word_length);
-               /*
-                * Position ASIC DPRAM auto-increment pointer.
-                */
-
-               s_file = (u16 *)(pFileStart + file_hdr->version_data_offset);
-
-
-                   dpram = (u16)DWNLD_MAG1_PS_HDR_LOC;
-                   if (word_length & 0x1)
-                       word_length++;
-
-                   word_length = (word_length / 2);
-
-
-               for (; word_length > 0; word_length--) /* In words */
-               {
-
-                      templong = ntohs(*s_file++);
-					  temp = ntohs(*s_file++);
-					  templong |= (temp << 16);
-                      status = fix_ft1000_write_dpram32 (ft1000dev, dpram++, (u8 *)&templong);
-
-               }
-               break;
-
-            case  REQUEST_CODE_BY_VERSION:
-               DEBUG("FT1000:download:REQUEST_CODE_BY_VERSION\n");
-               correct_version = FALSE;
-               requested_version = get_request_value(ft1000dev);
-
-                   dsp_img_info = (struct dsp_image_info *)(pFileStart + sizeof(struct dsp_file_hdr ));
-
-               for (image = 0; image < file_hdr->nDspImages; image++)
-               {
-                   if (dsp_img_info->version == requested_version)
-                       {
-                           correct_version = TRUE;
-                           DEBUG("FT1000:download: correct_version is TRUE\n");
-                           s_file = (u16 *)(pFileStart + dsp_img_info->begin_offset);
-                           c_file = (u8 *)(pFileStart + dsp_img_info->begin_offset);
-                           code_end = (u8 *)(pFileStart + dsp_img_info->end_offset);
-                           run_address = dsp_img_info->run_address;
-                           run_size = dsp_img_info->image_size;
-                           image_chksum = (u32)dsp_img_info->checksum;
-                           break;
-                        }
-                        dsp_img_info++;
-
-
-               } //end of for
-
-               if (!correct_version)
-               {
-                  /*
-                   * Error, beyond boot code range.
-                   */
-                  DEBUG("FT1000:download:Download error: Bad Version Request = 0x%x.\n",(int)requested_version);
-                  status = STATUS_FAILURE;
-                  break;
-               }
-               break;
-
-            default:
-               DEBUG("FT1000:download:Download error: Bad request type=%d in CODE download state.\n",request);
-               status = STATUS_FAILURE;
-               break;
-            }
-            if (pft1000info->usbboot)
-                put_handshake_usb(ft1000dev, HANDSHAKE_RESPONSE);
-            else
-                put_handshake(ft1000dev, HANDSHAKE_RESPONSE);
-         }
-         else
-         {
-            DEBUG("FT1000:download:Download error: Handshake failed\n");
-            status = STATUS_FAILURE;
-         }
-
-         break;
-
-      case STATE_DONE_DWNLD:
-         DEBUG("FT1000:download:Code loader is done...\n");
-         state = STATE_SECTION_PROV;
-         break;
-
-      case  STATE_SECTION_PROV:
-         DEBUG("FT1000:download:STATE_SECTION_PROV\n");
-		pseudo_header = (struct pseudo_hdr *)c_file;
-
-         if (pseudo_header->checksum == hdr_checksum(pseudo_header))
-         {
-            if (pseudo_header->portdest != 0x80 /* Dsp OAM */)
-            {
-               state = STATE_DONE_PROV;
-               break;
-            }
-            pseudo_header_len = ntohs(pseudo_header->length);    /* Byte length for PROV records */
-
-            // Get buffer for provisioning data
-		pbuffer = kmalloc((pseudo_header_len + sizeof(struct pseudo_hdr)), GFP_ATOMIC);
-            if (pbuffer) {
-		memcpy(pbuffer, (void *)c_file, (u32)(pseudo_header_len + sizeof(struct pseudo_hdr)));
-                // link provisioning data
-		pprov_record = kmalloc(sizeof(struct prov_record), GFP_ATOMIC);
-                if (pprov_record) {
-                    pprov_record->pprov_data = pbuffer;
-                    list_add_tail (&pprov_record->list, &pft1000info->prov_list);
-                    // Move to next entry if available
-			c_file = (u8 *)((unsigned long)c_file + (u32)((pseudo_header_len + 1) & 0xFFFFFFFE) + sizeof(struct pseudo_hdr));
-                    if ( (unsigned long)(c_file) - (unsigned long)(pFileStart) >= (unsigned long)FileLength) {
-                       state = STATE_DONE_FILE;
-                    }
-                }
-                else {
-                    kfree(pbuffer);
-                    status = STATUS_FAILURE;
-                }
-            }
-            else {
-                status = STATUS_FAILURE;
-            }
-         }
-         else
-         {
-            /* Checksum did not compute */
-            status = STATUS_FAILURE;
-         }
-         DEBUG("ft1000:download: after STATE_SECTION_PROV, state = %d, status= %d\n", state, status);
-         break;
-
-      case  STATE_DONE_PROV:
-         DEBUG("FT1000:download:STATE_DONE_PROV\n");
-         state = STATE_DONE_FILE;
-         break;
-
-
-      default:
-         status = STATUS_FAILURE;
-         break;
-      } /* End Switch */
-
-      if (status != STATUS_SUCCESS) {
-          break;
-      }
+					status =
+					    write_blk(ft1000dev, &s_file,
+						      &c_file, word_length);
+					//DEBUG("write_blk returned %d\n", status);
+					break;
+				default:
+					DEBUG
+					    ("FT1000:download:Download error: Bad request type=%d in BOOT download state.\n",
+					     request);
+					status = STATUS_FAILURE;
+					break;
+				}
+				if (pft1000info->usbboot)
+					put_handshake_usb(ft1000dev,
+							  HANDSHAKE_RESPONSE);
+				else
+					put_handshake(ft1000dev,
+						      HANDSHAKE_RESPONSE);
+			} else {
+				DEBUG
+				    ("FT1000:download:Download error: Handshake failed\n");
+				status = STATUS_FAILURE;
+			}
+
+			break;
+
+		case STATE_CODE_DWNLD:
+			//DEBUG("FT1000:STATE_CODE_DWNLD\n");
+			pft1000info->bootmode = 0;
+			if (pft1000info->usbboot)
+				handshake =
+				    get_handshake_usb(ft1000dev,
+						      HANDSHAKE_REQUEST);
+			else
+				handshake =
+				    get_handshake(ft1000dev, HANDSHAKE_REQUEST);
+			if (handshake == HANDSHAKE_REQUEST) {
+				/*
+				 * Get type associated with the request.
+				 */
+				if (pft1000info->usbboot)
+					request =
+					    get_request_type_usb(ft1000dev);
+				else
+					request = get_request_type(ft1000dev);
+				switch (request) {
+				case REQUEST_FILE_CHECKSUM:
+					DEBUG
+					    ("FT1000:download:image_chksum = 0x%8x\n",
+					     image_chksum);
+					put_request_value(ft1000dev,
+							  image_chksum);
+					break;
+				case REQUEST_RUN_ADDRESS:
+					DEBUG
+					    ("FT1000:download:  REQUEST_RUN_ADDRESS\n");
+					if (correct_version) {
+						DEBUG
+						    ("FT1000:download:run_address = 0x%8x\n",
+						     (int)run_address);
+						put_request_value(ft1000dev,
+								  run_address);
+					} else {
+						DEBUG
+						    ("FT1000:download:Download error: Got Run address request before image offset request.\n");
+						status = STATUS_FAILURE;
+						break;
+					}
+					break;
+				case REQUEST_CODE_LENGTH:
+					DEBUG
+					    ("FT1000:download:REQUEST_CODE_LENGTH\n");
+					if (correct_version) {
+						DEBUG
+						    ("FT1000:download:run_size = 0x%8x\n",
+						     (int)run_size);
+						put_request_value(ft1000dev,
+								  run_size);
+					} else {
+						DEBUG
+						    ("FT1000:download:Download error: Got Size request before image offset request.\n");
+						status = STATUS_FAILURE;
+						break;
+					}
+					break;
+				case REQUEST_DONE_CL:
+					pft1000info->usbboot = 3;
+					/* Reposition ptrs to beginning of provisioning section */
+					s_file =
+					    (u16 *) (pFileStart +
+						     file_hdr->commands_offset);
+					c_file =
+					    (u8 *) (pFileStart +
+						    file_hdr->commands_offset);
+					state = STATE_DONE_DWNLD;
+					break;
+				case REQUEST_CODE_SEGMENT:
+					//DEBUG("FT1000:download: REQUEST_CODE_SEGMENT - CODELOADER\n");
+					if (!correct_version) {
+						DEBUG
+						    ("FT1000:download:Download error: Got Code Segment request before image offset request.\n");
+						status = STATUS_FAILURE;
+						break;
+					}
+
+					word_length =
+					    get_request_value(ft1000dev);
+					//DEBUG("FT1000:download:word_length = %d\n", (int)word_length);
+					if (word_length > MAX_LENGTH) {
+						DEBUG
+						    ("FT1000:download:Download error: Max length exceeded\n");
+						status = STATUS_FAILURE;
+						break;
+					}
+					if ((word_length * 2 + c_file) >
+					    code_end) {
+						/*
+						 * Error, beyond boot code range.
+						 */
+						DEBUG
+						    ("FT1000:download:Download error: Requested len=%d exceeds DSP code boundry.\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;
+
+					write_blk_fifo(ft1000dev, &s_file,
+						       &c_file, word_length);
+					if (pft1000info->usbboot == 0)
+						pft1000info->usbboot++;
+					if (pft1000info->usbboot == 1) {
+						tempword = 0;
+						ft1000_write_dpram16(ft1000dev,
+								     DWNLD_MAG1_PS_HDR_LOC,
+								     tempword,
+								     0);
+					}
+
+					break;
+
+				case REQUEST_MAILBOX_DATA:
+					DEBUG
+					    ("FT1000:download: REQUEST_MAILBOX_DATA\n");
+					// Convert length from byte count to word count. Make sure we round up.
+					word_length =
+					    (long)(pft1000info->DSPInfoBlklen +
+						   1) / 2;
+					put_request_value(ft1000dev,
+							  word_length);
+					mailbox_data =
+					    (struct drv_msg *)&(pft1000info->
+								DSPInfoBlk[0]);
+					/*
+					 * Position ASIC DPRAM auto-increment pointer.
+					 */
+
+					data = (u16 *) & mailbox_data->data[0];
+					dpram = (u16) DWNLD_MAG1_PS_HDR_LOC;
+					if (word_length & 0x1)
+						word_length++;
+
+					word_length = (word_length / 2);
+
+					for (; word_length > 0; word_length--) {	/* In words */
+
+						templong = *data++;
+						templong |= (*data++ << 16);
+						status =
+						    fix_ft1000_write_dpram32
+						    (ft1000dev, dpram++,
+						     (u8 *) & templong);
+
+					}
+					break;
+
+				case REQUEST_VERSION_INFO:
+					DEBUG
+					    ("FT1000:download:REQUEST_VERSION_INFO\n");
+					word_length =
+					    file_hdr->version_data_size;
+					put_request_value(ft1000dev,
+							  word_length);
+					/*
+					 * Position ASIC DPRAM auto-increment pointer.
+					 */
+
+					s_file =
+					    (u16 *) (pFileStart +
+						     file_hdr->
+						     version_data_offset);
+
+					dpram = (u16) DWNLD_MAG1_PS_HDR_LOC;
+					if (word_length & 0x1)
+						word_length++;
+
+					word_length = (word_length / 2);
+
+					for (; word_length > 0; word_length--) {	/* In words */
+
+						templong = ntohs(*s_file++);
+						temp = ntohs(*s_file++);
+						templong |= (temp << 16);
+						status =
+						    fix_ft1000_write_dpram32
+						    (ft1000dev, dpram++,
+						     (u8 *) & templong);
+
+					}
+					break;
+
+				case REQUEST_CODE_BY_VERSION:
+					DEBUG
+					    ("FT1000:download:REQUEST_CODE_BY_VERSION\n");
+					correct_version = FALSE;
+					requested_version =
+					    get_request_value(ft1000dev);
+
+					dsp_img_info =
+					    (struct dsp_image_info *)(pFileStart
+								      +
+								      sizeof
+								      (struct
+								       dsp_file_hdr));
+
+					for (image = 0;
+					     image < file_hdr->nDspImages;
+					     image++) {
+						if (dsp_img_info->version ==
+						    requested_version) {
+							correct_version = TRUE;
+							DEBUG
+							    ("FT1000:download: correct_version is TRUE\n");
+							s_file =
+							    (u16 *) (pFileStart
+								     +
+								     dsp_img_info->
+								     begin_offset);
+							c_file =
+							    (u8 *) (pFileStart +
+								    dsp_img_info->
+								    begin_offset);
+							code_end =
+							    (u8 *) (pFileStart +
+								    dsp_img_info->
+								    end_offset);
+							run_address =
+							    dsp_img_info->
+							    run_address;
+							run_size =
+							    dsp_img_info->
+							    image_size;
+							image_chksum =
+							    (u32) dsp_img_info->
+							    checksum;
+							break;
+						}
+						dsp_img_info++;
+
+					}	//end of for
+
+					if (!correct_version) {
+						/*
+						 * Error, beyond boot code range.
+						 */
+						DEBUG
+						    ("FT1000:download:Download error: Bad Version Request = 0x%x.\n",
+						     (int)requested_version);
+						status = STATUS_FAILURE;
+						break;
+					}
+					break;
+
+				default:
+					DEBUG
+					    ("FT1000:download:Download error: Bad request type=%d in CODE download state.\n",
+					     request);
+					status = STATUS_FAILURE;
+					break;
+				}
+				if (pft1000info->usbboot)
+					put_handshake_usb(ft1000dev,
+							  HANDSHAKE_RESPONSE);
+				else
+					put_handshake(ft1000dev,
+						      HANDSHAKE_RESPONSE);
+			} else {
+				DEBUG
+				    ("FT1000:download:Download error: Handshake failed\n");
+				status = STATUS_FAILURE;
+			}
+
+			break;
+
+		case STATE_DONE_DWNLD:
+			DEBUG("FT1000:download:Code loader is done...\n");
+			state = STATE_SECTION_PROV;
+			break;
+
+		case STATE_SECTION_PROV:
+			DEBUG("FT1000:download:STATE_SECTION_PROV\n");
+			pseudo_header = (struct pseudo_hdr *)c_file;
+
+			if (pseudo_header->checksum ==
+			    hdr_checksum(pseudo_header)) {
+				if (pseudo_header->portdest !=
+				    0x80 /* Dsp OAM */ ) {
+					state = STATE_DONE_PROV;
+					break;
+				}
+				pseudo_header_len = ntohs(pseudo_header->length);	/* Byte length for PROV records */
+
+				// Get buffer for provisioning data
+				pbuffer =
+				    kmalloc((pseudo_header_len +
+					     sizeof(struct pseudo_hdr)),
+					    GFP_ATOMIC);
+				if (pbuffer) {
+					memcpy(pbuffer, (void *)c_file,
+					       (u32) (pseudo_header_len +
+						      sizeof(struct
+							     pseudo_hdr)));
+					// link provisioning data
+					pprov_record =
+					    kmalloc(sizeof(struct prov_record),
+						    GFP_ATOMIC);
+					if (pprov_record) {
+						pprov_record->pprov_data =
+						    pbuffer;
+						list_add_tail(&pprov_record->
+							      list,
+							      &pft1000info->
+							      prov_list);
+						// Move to next entry if available
+						c_file =
+						    (u8 *) ((unsigned long)
+							    c_file +
+							    (u32) ((pseudo_header_len + 1) & 0xFFFFFFFE) + sizeof(struct pseudo_hdr));
+						if ((unsigned long)(c_file) -
+						    (unsigned long)(pFileStart)
+						    >=
+						    (unsigned long)FileLength) {
+							state = STATE_DONE_FILE;
+						}
+					} else {
+						kfree(pbuffer);
+						status = STATUS_FAILURE;
+					}
+				} else {
+					status = STATUS_FAILURE;
+				}
+			} else {
+				/* Checksum did not compute */
+				status = STATUS_FAILURE;
+			}
+			DEBUG
+			    ("ft1000:download: after STATE_SECTION_PROV, state = %d, status= %d\n",
+			     state, status);
+			break;
+
+		case STATE_DONE_PROV:
+			DEBUG("FT1000:download:STATE_DONE_PROV\n");
+			state = STATE_DONE_FILE;
+			break;
+
+		default:
+			status = STATUS_FAILURE;
+			break;
+		}		/* End Switch */
+
+		if (status != STATUS_SUCCESS) {
+			break;
+		}
 
 /****
       // Check if Card is present
@@ -1147,11 +1230,12 @@ u16 scram_dnldr(struct ft1000_device *ft1000dev, void *pFileStart, u32  FileLeng
       }
 ****/
 
-   } /* End while */
+	}			/* End while */
 
-   DEBUG("Download exiting with status = 0x%8x\n", status);
-   ft1000_write_register(ft1000dev, FT1000_DB_DNLD_TX, FT1000_REG_DOORBELL);
+	DEBUG("Download exiting with status = 0x%8x\n", status);
+	ft1000_write_register(ft1000dev, FT1000_DB_DNLD_TX,
+			      FT1000_REG_DOORBELL);
 
-   return status;
+	return status;
 }
 
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 11:49 ` [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function Marek Belisko
@ 2011-01-26 13:07   ` Dan Carpenter
  2011-01-26 13:34     ` Belisko Marek
                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Dan Carpenter @ 2011-01-26 13:07 UTC (permalink / raw)
  To: Marek Belisko; +Cc: gregkh, devel, linux-kernel

You didn't introduce it but do you know what the code is trying to do
here?

        byte_length = word_length * 4;

        if (byte_length % 4)
                aligncnt = 4 - (byte_length % 4);
        else
                aligncnt = 0;
        byte_length += aligncnt;

        if (byte_length && ((byte_length % 64) == 0))
                byte_length += 4;

        if (byte_length < 64)
                byte_length = 68;


Apparently the stuff has to be aligned to 4 bytes, but it can't be
aligned at 64 bytes and it can't be less than 68 bytes long.  The 
part that especially confuses me is why it can't be aligned at 64 bytes.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 13:07   ` Dan Carpenter
@ 2011-01-26 13:34     ` Belisko Marek
  2011-01-26 13:43     ` Dan Carpenter
  2011-01-26 14:30     ` Dan Carpenter
  2 siblings, 0 replies; 16+ messages in thread
From: Belisko Marek @ 2011-01-26 13:34 UTC (permalink / raw)
  To: Dan Carpenter, Marek Belisko, gregkh, devel, linux-kernel

On Wed, Jan 26, 2011 at 2:07 PM, Dan Carpenter <error27@gmail.com> wrote:
> You didn't introduce it but do you know what the code is trying to do
> here?
>
>        byte_length = word_length * 4;
>
>        if (byte_length % 4)
>                aligncnt = 4 - (byte_length % 4);
>        else
>                aligncnt = 0;
>        byte_length += aligncnt;
>
>        if (byte_length && ((byte_length % 64) == 0))
>                byte_length += 4;
>
>        if (byte_length < 64)
>                byte_length = 68;
>
>
> Apparently the stuff has to be aligned to 4 bytes, but it can't be
> aligned at 64 bytes and it can't be less than 68 bytes long.  The
> part that especially confuses me is why it can't be aligned at 64 bytes.
Yes this code seems little bit confusing but basically this routine send data
to device when firmware code is downloaded. Length which should be send is
asked by device itself (word_length) so maybe it should be somehow
aligned in some cases
(don't know details).  Just to get some idea about length I add some
printk(before 4*word_length
and after byte_length is counted before sending to usb):

Jan 26 14:22:24 linux kernel: [29632.843980] write_blk_fifo: word_length:4
Jan 26 14:22:24 linux kernel: [29632.843984] write_blk_fifo: byte_length:68
Jan 26 14:22:24 linux kernel: [29632.859982] write_blk_fifo: word_length:506
Jan 26 14:22:24 linux kernel: [29632.859986] write_blk_fifo: byte_length:2024
Jan 26 14:22:24 linux kernel: [29632.874984] write_blk_fifo: word_length:505
Jan 26 14:22:24 linux kernel: [29632.874988] write_blk_fifo: byte_length:2020
Jan 26 14:22:24 linux kernel: [29632.890046] write_blk_fifo: word_length:4
Jan 26 14:22:24 linux kernel: [29632.890051] write_blk_fifo: byte_length:68


>
> regards,
> dan carpenter
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 13:07   ` Dan Carpenter
  2011-01-26 13:34     ` Belisko Marek
@ 2011-01-26 13:43     ` Dan Carpenter
  2011-01-26 13:56       ` Belisko Marek
  2011-01-26 14:30     ` Dan Carpenter
  2 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2011-01-26 13:43 UTC (permalink / raw)
  To: Marek Belisko, gregkh, devel, linux-kernel

On Wed, Jan 26, 2011 at 04:07:18PM +0300, Dan Carpenter wrote:
> You didn't introduce it but do you know what the code is trying to do
> here?
> 
>         byte_length = word_length * 4;
> 
>         if (byte_length % 4)

Actually this check is pointless.  word_length * 4 is always
aligned at 4 bytes.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 13:43     ` Dan Carpenter
@ 2011-01-26 13:56       ` Belisko Marek
  0 siblings, 0 replies; 16+ messages in thread
From: Belisko Marek @ 2011-01-26 13:56 UTC (permalink / raw)
  To: Dan Carpenter, Marek Belisko, gregkh, devel, linux-kernel

On Wed, Jan 26, 2011 at 2:43 PM, Dan Carpenter <error27@gmail.com> wrote:
> On Wed, Jan 26, 2011 at 04:07:18PM +0300, Dan Carpenter wrote:
>> You didn't introduce it but do you know what the code is trying to do
>> here?
>>
>>         byte_length = word_length * 4;
>>
>>         if (byte_length % 4)
>
> Actually this check is pointless.  word_length * 4 is always
> aligned at 4 bytes.
OK will post in separate patch. Original was just about coding style.
Thanks for reviewing.
>
> regards,
> dan carpenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 13:07   ` Dan Carpenter
  2011-01-26 13:34     ` Belisko Marek
  2011-01-26 13:43     ` Dan Carpenter
@ 2011-01-26 14:30     ` Dan Carpenter
  2011-02-08 13:40       ` Belisko Marek
  2 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2011-01-26 14:30 UTC (permalink / raw)
  To: Marek Belisko, gregkh, devel, linux-kernel

Also when it does:
	memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);

That should probably be:
	memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);

Otherwise we're probably copying garbage from beyond the end of *pUcFile
to the ->tx_buf.  ft1000dev->tx_buf is hopefully initialized to zero at
this point?

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-01-26 14:30     ` Dan Carpenter
@ 2011-02-08 13:40       ` Belisko Marek
  2011-02-08 16:35         ` Dan Carpenter
  0 siblings, 1 reply; 16+ messages in thread
From: Belisko Marek @ 2011-02-08 13:40 UTC (permalink / raw)
  To: Dan Carpenter, Marek Belisko, gregkh, devel, linux-kernel

On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@gmail.com> wrote:
> Also when it does:
>        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
>
> That should probably be:
>        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
No this shouldn't because before you have additional check:
if (byte_length && ((byte_length % 64) == 0))
        byte_length += 4;

if (byte_length < 64)
        byte_length = 68;
So in my opinion byte_length should stay.
>
> Otherwise we're probably copying garbage from beyond the end of *pUcFile
> to the ->tx_buf.  ft1000dev->tx_buf is hopefully initialized to zero at
> this point?
Yes usb_init_urb set it to zeroes.
>
> regards,
> dan carpenter
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-02-08 13:40       ` Belisko Marek
@ 2011-02-08 16:35         ` Dan Carpenter
  2011-02-09  8:27           ` Belisko Marek
  2011-02-09 10:07           ` Belisko Marek
  0 siblings, 2 replies; 16+ messages in thread
From: Dan Carpenter @ 2011-02-08 16:35 UTC (permalink / raw)
  To: Belisko Marek; +Cc: Marek Belisko, gregkh, devel, linux-kernel

On Tue, Feb 08, 2011 at 02:40:49PM +0100, Belisko Marek wrote:
> On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@gmail.com> wrote:
> > Also when it does:
> >        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
> >
> > That should probably be:
> >        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
> No this shouldn't because before you have additional check:
> if (byte_length && ((byte_length % 64) == 0))
>         byte_length += 4;
> 
> if (byte_length < 64)
>         byte_length = 68;
> So in my opinion byte_length should stay.

Yes.  We make byte_length longer than the caller intended.  The caller
knows the size of the source buffer.  We have to pad the length of the
other buffer, but we should fill up the last part with zeroes instead
of reading past the end of the source buffer.

(I am not very familiar with the code and I haven't looked outside this
function, so I may be wrong).

Also I really bet that the thing where byte_length can't be a multiple
of 64 is bogus.  I've never heard of anything with a requirement like
that.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-02-08 16:35         ` Dan Carpenter
@ 2011-02-09  8:27           ` Belisko Marek
  2011-02-09 10:07           ` Belisko Marek
  1 sibling, 0 replies; 16+ messages in thread
From: Belisko Marek @ 2011-02-09  8:27 UTC (permalink / raw)
  To: Dan Carpenter, Belisko Marek, Marek Belisko, gregkh, devel, linux-kernel

On Tue, Feb 8, 2011 at 5:35 PM, Dan Carpenter <error27@gmail.com> wrote:
> On Tue, Feb 08, 2011 at 02:40:49PM +0100, Belisko Marek wrote:
>> On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@gmail.com> wrote:
>> > Also when it does:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
>> >
>> > That should probably be:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
>> No this shouldn't because before you have additional check:
>> if (byte_length && ((byte_length % 64) == 0))
>>         byte_length += 4;
>>
>> if (byte_length < 64)
>>         byte_length = 68;
>> So in my opinion byte_length should stay.
>
> Yes.  We make byte_length longer than the caller intended.  The caller
> knows the size of the source buffer.  We have to pad the length of the
> other buffer, but we should fill up the last part with zeroes instead
> of reading past the end of the source buffer.
>
> (I am not very familiar with the code and I haven't looked outside this
> function, so I may be wrong).
>
> Also I really bet that the thing where byte_length can't be a multiple
> of 64 is bogus.  I've never heard of anything with a requirement like
> that.
You're right. Today will make test when remove all opaque code.
Anyway at the end file position is moved in that way:
*pUsFile = *pUsFile + (word_length << 1);
*pUcFile = *pUcFile + (word_length << 2);

So short pointer multiplied by 2 and char pointer by 4 with
word_length. So my assume
all check and byte_length increasing is not correct (will see what test shows).
>
> regards,
> dan carpenter
>
>
>

regards,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-02-08 16:35         ` Dan Carpenter
  2011-02-09  8:27           ` Belisko Marek
@ 2011-02-09 10:07           ` Belisko Marek
  2011-02-09 11:08             ` Dan Carpenter
  1 sibling, 1 reply; 16+ messages in thread
From: Belisko Marek @ 2011-02-09 10:07 UTC (permalink / raw)
  To: Dan Carpenter, Belisko Marek, Marek Belisko, gregkh, devel, linux-kernel

On Tue, Feb 8, 2011 at 5:35 PM, Dan Carpenter <error27@gmail.com> wrote:
> On Tue, Feb 08, 2011 at 02:40:49PM +0100, Belisko Marek wrote:
>> On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@gmail.com> wrote:
>> > Also when it does:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
>> >
>> > That should probably be:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
>> No this shouldn't because before you have additional check:
>> if (byte_length && ((byte_length % 64) == 0))
>>         byte_length += 4;
>>
>> if (byte_length < 64)
>>         byte_length = 68;
>> So in my opinion byte_length should stay.
>
> Yes.  We make byte_length longer than the caller intended.  The caller
> knows the size of the source buffer.  We have to pad the length of the
> other buffer, but we should fill up the last part with zeroes instead
> of reading past the end of the source buffer.
>
> (I am not very familiar with the code and I haven't looked outside this
> function, so I may be wrong).
>
> Also I really bet that the thing where byte_length can't be a multiple
> of 64 is bogus.  I've never heard of anything with a requirement like
> that.
Well I test it and it seems very strange and can't figure out why.
Will remove all byte_length manipulations and device doesn't boot properly
(finish with error). Add some prinkt to code figure out following:

1. byte_length = word_length *4 is < 64 we need to send via usb 68
bytes (despite
4 bytes are behind 64 (without this it can't boot).

2. also when e.g. word_length is 400 (400*4 = 1600) condition
1600%64 == 0 is valid and we send 1604 bytes to usb (also not sure why
but without this change it also doesn't work).

For little explanation when we get to state code load we ask device
how many bytes should send
so we will get reply and send block via usb. So maybe it's related
that we send in our assumption correct data
but usb request something else to properly working ;)
>
> regards,
> dan carpenter
>
>
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-02-09 10:07           ` Belisko Marek
@ 2011-02-09 11:08             ` Dan Carpenter
  2011-02-09 11:16               ` Belisko Marek
  0 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2011-02-09 11:08 UTC (permalink / raw)
  To: Belisko Marek; +Cc: Marek Belisko, gregkh, devel, linux-kernel

On Wed, Feb 09, 2011 at 11:07:19AM +0100, Belisko Marek wrote:
> Well I test it and it seems very strange and can't figure out why.
> Will remove all byte_length manipulations and device doesn't boot properly
> (finish with error). Add some prinkt to code figure out following:
> 
> 1. byte_length = word_length *4 is < 64 we need to send via usb 68
> bytes (despite
> 4 bytes are behind 64 (without this it can't boot).
> 
> 2. also when e.g. word_length is 400 (400*4 = 1600) condition
> 1600%64 == 0 is valid and we send 1604 bytes to usb (also not sure why
> but without this change it also doesn't work).
> 

Huh.  Strange.  Thanks for testing.  Sorry for the noise.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
  2011-02-09 11:08             ` Dan Carpenter
@ 2011-02-09 11:16               ` Belisko Marek
  0 siblings, 0 replies; 16+ messages in thread
From: Belisko Marek @ 2011-02-09 11:16 UTC (permalink / raw)
  To: Dan Carpenter, Belisko Marek, Marek Belisko, gregkh, devel, linux-kernel

On Wed, Feb 9, 2011 at 12:08 PM, Dan Carpenter <error27@gmail.com> wrote:
> On Wed, Feb 09, 2011 at 11:07:19AM +0100, Belisko Marek wrote:
>> Well I test it and it seems very strange and can't figure out why.
>> Will remove all byte_length manipulations and device doesn't boot properly
>> (finish with error). Add some prinkt to code figure out following:
>>
>> 1. byte_length = word_length *4 is < 64 we need to send via usb 68
>> bytes (despite
>> 4 bytes are behind 64 (without this it can't boot).
>>
>> 2. also when e.g. word_length is 400 (400*4 = 1600) condition
>> 1600%64 == 0 is valid and we send 1604 bytes to usb (also not sure why
>> but without this change it also doesn't work).
>>
>
> Huh.  Strange.  Thanks for testing.  Sorry for the noise.
No problem ;). Thanks for reviewing and making noise ;).
@greg: could you please apply posted patch(thanks):
http://driverdev.linuxdriverproject.org/pipermail/devel/2011-February/011942.html
>
> regards,
> dan carpenter
>
>
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2011-02-09 11:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
2011-01-26 11:49 ` [PATCH 2/5] staging: ft1000: Remove unnecessary assignment Marek Belisko
2011-01-26 11:49 ` [PATCH 3/5] staging: ft1000: Create common function for buffers check Marek Belisko
2011-01-26 11:49 ` [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function Marek Belisko
2011-01-26 13:07   ` Dan Carpenter
2011-01-26 13:34     ` Belisko Marek
2011-01-26 13:43     ` Dan Carpenter
2011-01-26 13:56       ` Belisko Marek
2011-01-26 14:30     ` Dan Carpenter
2011-02-08 13:40       ` Belisko Marek
2011-02-08 16:35         ` Dan Carpenter
2011-02-09  8:27           ` Belisko Marek
2011-02-09 10:07           ` Belisko Marek
2011-02-09 11:08             ` Dan Carpenter
2011-02-09 11:16               ` Belisko Marek
2011-01-26 11:49 ` [PATCH 5/5] staging: ft1000: Fix indentation in scram_dnldr() function Marek Belisko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).