2011-03-23 07:08:26

by Marek Belisko

[permalink] [raw]
Subject: [PATCH] staging: ft1000-pcmcia: Fix ft1000_dnld() to work also on 64bit architectures.

From: pixo <pixo@pixo-LIFEBOOK-E8310.(none)>

Firmware file needs to be read by 4bytes also on 64 bit architectures.
Change long type to u32 and also extend checking. Tested on 32 and also
64 bit architectures.

Signed-off-by: Stano Lanci <[email protected]>
Signed-off-by: Marek Belisko <[email protected]>

Tested-by: Stano Lanci <[email protected]>
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c | 78 ++++++++++----------
1 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
index b0729fc..fb375ea 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
@@ -95,47 +95,47 @@ void put_request_value(struct net_device *dev, long lvalue);
USHORT hdr_checksum(PPSEUDO_HDR pHdr);

typedef struct _DSP_FILE_HDR {
- long build_date;
- long dsp_coff_date;
- long loader_code_address;
- long loader_code_size;
- long loader_code_end;
- long dsp_code_address;
- long dsp_code_size;
- long dsp_code_end;
- long reserved[8];
+ u32 build_date;
+ u32 dsp_coff_date;
+ u32 loader_code_address;
+ u32 loader_code_size;
+ u32 loader_code_end;
+ u32 dsp_code_address;
+ u32 dsp_code_size;
+ u32 dsp_code_end;
+ u32 reserved[8];
} __attribute__ ((packed)) DSP_FILE_HDR, *PDSP_FILE_HDR;

typedef struct _DSP_FILE_HDR_5 {
- long version_id; // Version ID of this image format.
- long package_id; // Package ID of code release.
- long build_date; // Date/time stamp when file was built.
- long commands_offset; // Offset to attached commands in Pseudo Hdr format.
- long loader_offset; // Offset to bootloader code.
- long loader_code_address; // Start address of bootloader.
- long loader_code_end; // Where bootloader code ends.
- long loader_code_size;
- long version_data_offset; // Offset were scrambled version data begins.
- long version_data_size; // Size, in words, of scrambled version data.
- long nDspImages; // Number of DSP images in file.
+ u32 version_id; // Version ID of this image format.
+ u32 package_id; // Package ID of code release.
+ u32 build_date; // Date/time stamp when file was built.
+ u32 commands_offset; // Offset to attached commands in Pseudo Hdr format.
+ u32 loader_offset; // Offset to bootloader code.
+ u32 loader_code_address; // Start address of bootloader.
+ u32 loader_code_end; // Where bootloader code ends.
+ u32 loader_code_size;
+ u32 version_data_offset; // Offset were scrambled version data begins.
+ u32 version_data_size; // Size, in words, of scrambled version data.
+ u32 nDspImages; // Number of DSP images in file.
} __attribute__ ((packed)) DSP_FILE_HDR_5, *PDSP_FILE_HDR_5;

typedef struct _DSP_IMAGE_INFO {
- long coff_date; // Date/time when DSP Coff image was built.
- long begin_offset; // Offset in file where image begins.
- long end_offset; // Offset in file where image begins.
- long run_address; // On chip Start address of DSP code.
- long image_size; // Size of image.
- long version; // Embedded version # of DSP code.
+ u32 coff_date; // Date/time when DSP Coff image was built.
+ u32 begin_offset; // Offset in file where image begins.
+ u32 end_offset; // Offset in file where image begins.
+ u32 run_address; // On chip Start address of DSP code.
+ u32 image_size; // Size of image.
+ u32 version; // Embedded version # of DSP code.
} __attribute__ ((packed)) DSP_IMAGE_INFO, *PDSP_IMAGE_INFO;

typedef struct _DSP_IMAGE_INFO_V6 {
- long coff_date; // Date/time when DSP Coff image was built.
- long begin_offset; // Offset in file where image begins.
- long end_offset; // Offset in file where image begins.
- long run_address; // On chip Start address of DSP code.
- long image_size; // Size of image.
- long version; // Embedded version # of DSP code.
+ u32 coff_date; // Date/time when DSP Coff image was built.
+ u32 begin_offset; // Offset in file where image begins.
+ u32 end_offset; // Offset in file where image begins.
+ u32 run_address; // On chip Start address of DSP code.
+ u32 image_size; // Size of image.
+ u32 version; // Embedded version # of DSP code.
unsigned short checksum; // Dsp File checksum
unsigned short pad1;
} __attribute__ ((packed)) DSP_IMAGE_INFO_V6, *PDSP_IMAGE_INFO_V6;
@@ -846,8 +846,8 @@ int card_download(struct net_device *dev, const u8 *pFileStart, UINT FileLength)
break;

case STATE_DONE_DWNLD:
- if (((UINT) (pUcFile) - (UINT) pFileStart) >=
- (UINT) FileLength) {
+ if (((unsigned long) (pUcFile) - (unsigned long) pFileStart) >=
+ (unsigned long) FileLength) {
uiState = STATE_DONE_FILE;
break;
}
@@ -901,11 +901,11 @@ int card_download(struct net_device *dev, const u8 *pFileStart, UINT FileLength)
&info->prov_list);
// Move to next entry if available
pUcFile =
- (UCHAR *) ((UINT) pUcFile +
- (UINT) ((usHdrLength + 1) & 0xFFFFFFFE) + sizeof(PSEUDO_HDR));
- if ((UINT) (pUcFile) -
- (UINT) (pFileStart) >=
- (UINT) FileLength) {
+ (UCHAR *) ((unsigned long) pUcFile +
+ (unsigned long) ((usHdrLength + 1) & 0xFFFFFFFE) + sizeof(PSEUDO_HDR));
+ if ((unsigned long) (pUcFile) -
+ (unsigned long) (pFileStart) >=
+ (unsigned long) FileLength) {
uiState =
STATE_DONE_FILE;
}
--
1.7.1


2011-03-23 10:27:21

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] staging: ft1000-pcmcia: Fix ft1000_dnld() to work also on 64bit architectures.

On 03/23/2011 08:08 AM, Marek Belisko wrote:
> From: pixo <pixo@pixo-LIFEBOOK-E8310.(none)>
>
> Firmware file needs to be read by 4bytes also on 64 bit architectures.
> Change long type to u32 and also extend checking. Tested on 32 and also
> 64 bit architectures.
>
> Signed-off-by: Stano Lanci <[email protected]>
> Signed-off-by: Marek Belisko <[email protected]>
>
> Tested-by: Stano Lanci <[email protected]>
> ---
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c | 78 ++++++++++----------
> 1 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
> index b0729fc..fb375ea 100644
> --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
> +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
> @@ -95,47 +95,47 @@ void put_request_value(struct net_device *dev, long lvalue);
> USHORT hdr_checksum(PPSEUDO_HDR pHdr);
>
> typedef struct _DSP_FILE_HDR {
> - long build_date;
> - long dsp_coff_date;
> - long loader_code_address;
> - long loader_code_size;
> - long loader_code_end;
> - long dsp_code_address;
> - long dsp_code_size;
> - long dsp_code_end;
> - long reserved[8];
> + u32 build_date;
> + u32 dsp_coff_date;
> + u32 loader_code_address;
> + u32 loader_code_size;
> + u32 loader_code_end;
> + u32 dsp_code_address;
> + u32 dsp_code_size;
> + u32 dsp_code_end;
> + u32 reserved[8];
> } __attribute__ ((packed)) DSP_FILE_HDR, *PDSP_FILE_HDR;

As you also changed signedness, did you check (or do you know) that the
values cannot be negative?

Or at least the change doesn't affect code flow?

regards,
--
js

2011-03-23 10:36:42

by Belisko Marek

[permalink] [raw]
Subject: Re: [PATCH] staging: ft1000-pcmcia: Fix ft1000_dnld() to work also on 64bit architectures.

On Wed, Mar 23, 2011 at 11:27 AM, Jiri Slaby <[email protected]> wrote:
> On 03/23/2011 08:08 AM, Marek Belisko wrote:
>> From: pixo <pixo@pixo-LIFEBOOK-E8310.(none)>
>>
>> Firmware file needs to be read by 4bytes also on 64 bit architectures.
>> Change long type to u32 and also extend checking. Tested on 32 and also
>> 64 bit architectures.
>>
>> Signed-off-by: Stano Lanci <[email protected]>
>> Signed-off-by: Marek Belisko <[email protected]>
>>
>> Tested-by: Stano Lanci <[email protected]>
>> ---
>>  drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c |   78 ++++++++++----------
>>  1 files changed, 39 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
>> index b0729fc..fb375ea 100644
>> --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
>> +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_dnld.c
>> @@ -95,47 +95,47 @@ void put_request_value(struct net_device *dev, long lvalue);
>>  USHORT hdr_checksum(PPSEUDO_HDR pHdr);
>>
>>  typedef struct _DSP_FILE_HDR {
>> -     long build_date;
>> -     long dsp_coff_date;
>> -     long loader_code_address;
>> -     long loader_code_size;
>> -     long loader_code_end;
>> -     long dsp_code_address;
>> -     long dsp_code_size;
>> -     long dsp_code_end;
>> -     long reserved[8];
>> +     u32  build_date;
>> +     u32  dsp_coff_date;
>> +     u32  loader_code_address;
>> +     u32  loader_code_size;
>> +     u32  loader_code_end;
>> +     u32  dsp_code_address;
>> +     u32  dsp_code_size;
>> +     u32  dsp_code_end;
>> +     u32  reserved[8];
>>  } __attribute__ ((packed)) DSP_FILE_HDR, *PDSP_FILE_HDR;
>
> As you also changed signedness, did you check (or do you know) that the
> values cannot be negative?
No structures represent FW part and non of values could be negative.
This is bug from old implementation.
>
> Or at least the change doesn't affect code flow?
>
> regards,
> --
> js
>

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