* [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 related [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 related [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 related [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 related [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 related [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.