On 02/15/2016 06:34 AM, Wolfgang Denk wrote:
> Dear William,
>
> In message <1455506732-22307-1-git-send-email-wco...@redhat.com> you wrote:
>>
>> Recent MIPS Linux kernels are using a 64-bit value for the load
>> address (0xffffffff80010000) for the Creator CI20 board kernel. When
>> this argument was passed to the mkimage program running on a 32-bit
>> machine such as the Creator CI20 board the load address was
>> incorrectly obtained from the first half of the argument, 0xffffffff
>> by the strtoul. The mkimage should be able to tolerate the longer,
>> 64-bit signed version of the arguments with the use of strtoull.
>
> Hm... I think we have multiple problems here.
Hi,
The write up in the patch wasn't as clear as it should have been. I
have revised the patch to try to make the reason for this patch
clearer. The MIP Linux kernel is using sign-extending 32-bit values
to 64-bit values, so the truncation of the strtoll values to 32-bits
is actually desired. The revised patch now passes the linux kernel
checkpatch.pl checks. I will be resending the patch in a moment.
-Will
>
> First, the old legacy uImage header usersd 32 bit data types for the
> addresses. This is a binary data structure, and there is no way to
> extend it for 64 bit addresses without breaking compatibility.
>
>
>> - params.addr = strtoul (*++argv, &ptr, 16);
>> + params.addr = strtoull (*++argv, &ptr, 16);
>
> I don't see what you win here... strtoull() returns unsigned long long,
> but params.addr is an unsigned int, so the value will be truncated to
> 32 bit. Are you sure this is what you want?
>
>> fprintf (stderr,
>> "%s: invalid load address %s\n",
>> @@ -146,7 +146,7 @@ int main(int argc, char **argv)
>> case 'e':
>> if (--argc <= 0)
>> usage ();
>> - params.ep = strtoul (*++argv, &ptr, 16);
>> + params.ep = strtoull (*++argv, &ptr, 16);
>
> Ditto...
>
>
> Also please note that your patch triggers a number of checkpatch
> warnings and an error, especially:
>
> WARNING: space prohibited between function name and open parenthesis '('
> #108: FILE: tools/mkimage.c:132:
> + params.addr = strtoull (*++argv, &ptr, 16);
>
> WARNING: space prohibited between function name and open parenthesis '('
> #117: FILE: tools/mkimage.c:149:
> + params.ep = strtoull (*++argv, &ptr, 16);
>
> ERROR: Missing Signed-off-by: line(s)
>
>
> Best regards,
>
> Wolfgang Denk
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
