subject:"\[U\-Boot\] \[RFC PATCH v4 2\/3\] common\: Convert ulong to phys_addr

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

2016-02-23 Thread Simon Glass

Hi York,

On 23 February 2016 at 13:36, york sun  wrote:
> On 02/16/2016 08:01 AM, Simon Glass wrote:
>> Hi York,
>>
>> On 12 February 2016 at 13:59, York Sun  wrote:
>>> When dealing with image addresses, ulong has been used. Some files
>>> are used by both host and target. It is OK for the target, but not
>>> always enough for host tools including mkimage. This patch replaces
>>> "ulong" with "phys_addr_t" to make sure addresses are correct for
>>> both the target and the host.
>>>
>>> This issue was found on 32-bit host when compiling for 64-bit target
>>> to support images with address higher than 32-bit space.
>>>
>>> Signed-off-by: York Sun 
>>>
>>> ---
>>>
>>> Changes in v4:
>>>   New patch, separated from fixing FIT image.
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>  arch/powerpc/lib/bootm.c |4 ++--
>>>  cmd/ximg.c   |9 +
>>>  common/bootm.c   |   21 +++--
>>>  common/bootm_os.c|   14 --
>>>  common/image-android.c   |6 +++---
>>>  common/image-fdt.c   |   16 
>>>  common/image-fit.c   |   27 ++-
>>>  common/image.c   |   17 ++---
>>>  include/bootm.h  |6 +++---
>>>  include/image.h  |   30 ++
>>>  tools/default_image.c|2 +-
>>>  11 files changed, 83 insertions(+), 69 deletions(-)
>>>
>>> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
>>> index ef15e7a..794382a 100644
>>> --- a/arch/powerpc/lib/bootm.c
>>> +++ b/arch/powerpc/lib/bootm.c
>>> @@ -47,7 +47,7 @@ static void boot_jump_linux(bootm_headers_t *images)
>>>  #endif
>>>
>>> kernel = (void (*)(bd_t *, ulong, ulong, ulong,
>>> -  ulong, ulong, ulong))images->ep;
>>> +  ulong, ulong, ulong))(uintptr_t)images->ep;
>>> debug ("## Transferring control to Linux (at address %08lx) ...\n",
>>> (ulong)kernel);
>>>
>>> @@ -335,7 +335,7 @@ void boot_jump_vxworks(bootm_headers_t *images)
>>> WATCHDOG_RESET();
>>>
>>> ((void (*)(void *, ulong, ulong, ulong,
>>> -   ulong, ulong, ulong))images->ep)(images->ft_addr,
>>> +   ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,
>>> 0, 0, EPAPR_MAGIC, getenv_bootm_mapsize(), 0, 0);
>>>  }
>>>  #endif
>>> diff --git a/cmd/ximg.c b/cmd/ximg.c
>>> index d033c15..70d6d14 100644
>>> --- a/cmd/ximg.c
>>> +++ b/cmd/ximg.c
>>> @@ -33,7 +33,8 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char 
>>> * const argv[])
>>>  {
>>> ulong   addr = load_addr;
>>> ulong   dest = 0;
>>> -   ulong   data, len;
>>> +   phys_addr_t data;
>>> +   ulong   len;
>>> int verify;
>>> int part = 0;
>>>  #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
>>> @@ -173,7 +174,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, 
>>> char * const argv[])
>>> return 1;
>>> }
>>>
>>> -   data = (ulong)fit_data;
>>> +   data = (phys_addr_t)(uintptr_t)fit_data;
>>> len = (ulong)fit_len;
>>> break;
>>>  #endif
>>> @@ -205,14 +206,14 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, 
>>> char * const argv[])
>>> }
>>>  #else  /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
>>> printf("   Loading part %d ... ", part);
>>> -   memmove((char *) dest, (char *)data, len);
>>> +   memmove((char *)dest, (char *)(uintptr_t)data, len);
>>>  #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
>>> break;
>>>  #ifdef CONFIG_GZIP
>>> case IH_COMP_GZIP:
>>> printf("   Uncompressing part %d ... ", part);
>>> if (gunzip((void *) dest, unc_len,
>>> -  (uchar *) data, ) != 0) {
>>> +  (uchar *)(uintptr_t)data, ) != 0) {
>>
>> The uintptr_t cast presumably defeats the warning. Is the intention to
>> convert a 64-bit value to 32-bit?
>
> Yes. Before this patch, all these addresses are ulong. It is enough to hold 
> the
> addresses _used_ for images on 32- and 64-bit targets. Please note, the key 
> here
> is the address used. On system with 36 or more bits for physical addresses, 
> the
> phys_addr_t is "unsigned long long". If the core is in 32-bit, the images are
> actually put in 32-bit space. So it is safe to case the 64-bit phys_addr_t to 
> a
> 32-bit pointer.
>
> The idea behind this change is to make the host tool (such as mkimge) to 
> support
> 64-bit address, even on a 32-bit host. My solution is to always use 64-bit
> variable on host. I didn't find a better way to deal with this issue.
>
>>
>>

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

2016-02-23 Thread york sun

On 02/16/2016 08:01 AM, Simon Glass wrote:
> Hi York,
> 
> On 12 February 2016 at 13:59, York Sun  wrote:
>> When dealing with image addresses, ulong has been used. Some files
>> are used by both host and target. It is OK for the target, but not
>> always enough for host tools including mkimage. This patch replaces
>> "ulong" with "phys_addr_t" to make sure addresses are correct for
>> both the target and the host.
>>
>> This issue was found on 32-bit host when compiling for 64-bit target
>> to support images with address higher than 32-bit space.
>>
>> Signed-off-by: York Sun 
>>
>> ---
>>
>> Changes in v4:
>>   New patch, separated from fixing FIT image.
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>  arch/powerpc/lib/bootm.c |4 ++--
>>  cmd/ximg.c   |9 +
>>  common/bootm.c   |   21 +++--
>>  common/bootm_os.c|   14 --
>>  common/image-android.c   |6 +++---
>>  common/image-fdt.c   |   16 
>>  common/image-fit.c   |   27 ++-
>>  common/image.c   |   17 ++---
>>  include/bootm.h  |6 +++---
>>  include/image.h  |   30 ++
>>  tools/default_image.c|2 +-
>>  11 files changed, 83 insertions(+), 69 deletions(-)
>>
>> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
>> index ef15e7a..794382a 100644
>> --- a/arch/powerpc/lib/bootm.c
>> +++ b/arch/powerpc/lib/bootm.c
>> @@ -47,7 +47,7 @@ static void boot_jump_linux(bootm_headers_t *images)
>>  #endif
>>
>> kernel = (void (*)(bd_t *, ulong, ulong, ulong,
>> -  ulong, ulong, ulong))images->ep;
>> +  ulong, ulong, ulong))(uintptr_t)images->ep;
>> debug ("## Transferring control to Linux (at address %08lx) ...\n",
>> (ulong)kernel);
>>
>> @@ -335,7 +335,7 @@ void boot_jump_vxworks(bootm_headers_t *images)
>> WATCHDOG_RESET();
>>
>> ((void (*)(void *, ulong, ulong, ulong,
>> -   ulong, ulong, ulong))images->ep)(images->ft_addr,
>> +   ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,
>> 0, 0, EPAPR_MAGIC, getenv_bootm_mapsize(), 0, 0);
>>  }
>>  #endif
>> diff --git a/cmd/ximg.c b/cmd/ximg.c
>> index d033c15..70d6d14 100644
>> --- a/cmd/ximg.c
>> +++ b/cmd/ximg.c
>> @@ -33,7 +33,8 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char 
>> * const argv[])
>>  {
>> ulong   addr = load_addr;
>> ulong   dest = 0;
>> -   ulong   data, len;
>> +   phys_addr_t data;
>> +   ulong   len;
>> int verify;
>> int part = 0;
>>  #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
>> @@ -173,7 +174,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, 
>> char * const argv[])
>> return 1;
>> }
>>
>> -   data = (ulong)fit_data;
>> +   data = (phys_addr_t)(uintptr_t)fit_data;
>> len = (ulong)fit_len;
>> break;
>>  #endif
>> @@ -205,14 +206,14 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, 
>> char * const argv[])
>> }
>>  #else  /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
>> printf("   Loading part %d ... ", part);
>> -   memmove((char *) dest, (char *)data, len);
>> +   memmove((char *)dest, (char *)(uintptr_t)data, len);
>>  #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
>> break;
>>  #ifdef CONFIG_GZIP
>> case IH_COMP_GZIP:
>> printf("   Uncompressing part %d ... ", part);
>> if (gunzip((void *) dest, unc_len,
>> -  (uchar *) data, ) != 0) {
>> +  (uchar *)(uintptr_t)data, ) != 0) {
> 
> The uintptr_t cast presumably defeats the warning. Is the intention to
> convert a 64-bit value to 32-bit?

Yes. Before this patch, all these addresses are ulong. It is enough to hold the
addresses _used_ for images on 32- and 64-bit targets. Please note, the key here
is the address used. On system with 36 or more bits for physical addresses, the
phys_addr_t is "unsigned long long". If the core is in 32-bit, the images are
actually put in 32-bit space. So it is safe to case the 64-bit phys_addr_t to a
32-bit pointer.

The idea behind this change is to make the host tool (such as mkimge) to support
64-bit address, even on a 32-bit host. My solution is to always use 64-bit
variable on host. I didn't find a better way to deal with this issue.

> 
> Is it true that sizeof(phys_addr_t) is always sizeof(ulong) on the target?

No. On some arch (eg arm, mips, powerpc), phys_addr_t can be either "unsigned
long long", or "unsigned long".

> 
>>

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

2016-02-16 Thread Simon Glass

Hi York,

On 12 February 2016 at 13:59, York Sun  wrote:
> When dealing with image addresses, ulong has been used. Some files
> are used by both host and target. It is OK for the target, but not
> always enough for host tools including mkimage. This patch replaces
> "ulong" with "phys_addr_t" to make sure addresses are correct for
> both the target and the host.
>
> This issue was found on 32-bit host when compiling for 64-bit target
> to support images with address higher than 32-bit space.
>
> Signed-off-by: York Sun 
>
> ---
>
> Changes in v4:
>   New patch, separated from fixing FIT image.
>
> Changes in v3: None
> Changes in v2: None
>
>  arch/powerpc/lib/bootm.c |4 ++--
>  cmd/ximg.c   |9 +
>  common/bootm.c   |   21 +++--
>  common/bootm_os.c|   14 --
>  common/image-android.c   |6 +++---
>  common/image-fdt.c   |   16 
>  common/image-fit.c   |   27 ++-
>  common/image.c   |   17 ++---
>  include/bootm.h  |6 +++---
>  include/image.h  |   30 ++
>  tools/default_image.c|2 +-
>  11 files changed, 83 insertions(+), 69 deletions(-)
>
> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
> index ef15e7a..794382a 100644
> --- a/arch/powerpc/lib/bootm.c
> +++ b/arch/powerpc/lib/bootm.c
> @@ -47,7 +47,7 @@ static void boot_jump_linux(bootm_headers_t *images)
>  #endif
>
> kernel = (void (*)(bd_t *, ulong, ulong, ulong,
> -  ulong, ulong, ulong))images->ep;
> +  ulong, ulong, ulong))(uintptr_t)images->ep;
> debug ("## Transferring control to Linux (at address %08lx) ...\n",
> (ulong)kernel);
>
> @@ -335,7 +335,7 @@ void boot_jump_vxworks(bootm_headers_t *images)
> WATCHDOG_RESET();
>
> ((void (*)(void *, ulong, ulong, ulong,
> -   ulong, ulong, ulong))images->ep)(images->ft_addr,
> +   ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,
> 0, 0, EPAPR_MAGIC, getenv_bootm_mapsize(), 0, 0);
>  }
>  #endif
> diff --git a/cmd/ximg.c b/cmd/ximg.c
> index d033c15..70d6d14 100644
> --- a/cmd/ximg.c
> +++ b/cmd/ximg.c
> @@ -33,7 +33,8 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * 
> const argv[])
>  {
> ulong   addr = load_addr;
> ulong   dest = 0;
> -   ulong   data, len;
> +   phys_addr_t data;
> +   ulong   len;
> int verify;
> int part = 0;
>  #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
> @@ -173,7 +174,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char 
> * const argv[])
> return 1;
> }
>
> -   data = (ulong)fit_data;
> +   data = (phys_addr_t)(uintptr_t)fit_data;
> len = (ulong)fit_len;
> break;
>  #endif
> @@ -205,14 +206,14 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, 
> char * const argv[])
> }
>  #else  /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
> printf("   Loading part %d ... ", part);
> -   memmove((char *) dest, (char *)data, len);
> +   memmove((char *)dest, (char *)(uintptr_t)data, len);
>  #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
> break;
>  #ifdef CONFIG_GZIP
> case IH_COMP_GZIP:
> printf("   Uncompressing part %d ... ", part);
> if (gunzip((void *) dest, unc_len,
> -  (uchar *) data, ) != 0) {
> +  (uchar *)(uintptr_t)data, ) != 0) {

The uintptr_t cast presumably defeats the warning. Is the intention to
convert a 64-bit value to 32-bit?

Is it true that sizeof(phys_addr_t) is always sizeof(ulong) on the target?

> puts("GUNZIP ERROR - image not loaded\n");
> return 1;
> }
> diff --git a/common/bootm.c b/common/bootm.c
> index 99d574d..785858c 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -43,7 +43,7 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
>char * const argv[], bootm_headers_t 
> *images,
> -  ulong *os_data, ulong *os_len);
> +  phys_addr_t *os_data, ulong *os_len);
>
>  #ifdef CONFIG_LMB
>  static void boot_start_lmb(bootm_headers_t *images)
> @@ -325,9 +325,9 @@ static int handle_decomp_error(int comp_type, size_t 
> uncomp_size,
> return BOOTM_ERR_RESET;
>  }
>
> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> -

[U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

2016-02-13 Thread York Sun

When dealing with image addresses, ulong has been used. Some files
are used by both host and target. It is OK for the target, but not
always enough for host tools including mkimage. This patch replaces
"ulong" with "phys_addr_t" to make sure addresses are correct for
both the target and the host.

This issue was found on 32-bit host when compiling for 64-bit target
to support images with address higher than 32-bit space.

Signed-off-by: York Sun 

---

Changes in v4:
  New patch, separated from fixing FIT image.

Changes in v3: None
Changes in v2: None

 arch/powerpc/lib/bootm.c |4 ++--
 cmd/ximg.c   |9 +
 common/bootm.c   |   21 +++--
 common/bootm_os.c|   14 --
 common/image-android.c   |6 +++---
 common/image-fdt.c   |   16 
 common/image-fit.c   |   27 ++-
 common/image.c   |   17 ++---
 include/bootm.h  |6 +++---
 include/image.h  |   30 ++
 tools/default_image.c|2 +-
 11 files changed, 83 insertions(+), 69 deletions(-)

diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
index ef15e7a..794382a 100644
--- a/arch/powerpc/lib/bootm.c
+++ b/arch/powerpc/lib/bootm.c
@@ -47,7 +47,7 @@ static void boot_jump_linux(bootm_headers_t *images)
 #endif
 
kernel = (void (*)(bd_t *, ulong, ulong, ulong,
-  ulong, ulong, ulong))images->ep;
+  ulong, ulong, ulong))(uintptr_t)images->ep;
debug ("## Transferring control to Linux (at address %08lx) ...\n",
(ulong)kernel);
 
@@ -335,7 +335,7 @@ void boot_jump_vxworks(bootm_headers_t *images)
WATCHDOG_RESET();
 
((void (*)(void *, ulong, ulong, ulong,
-   ulong, ulong, ulong))images->ep)(images->ft_addr,
+   ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,
0, 0, EPAPR_MAGIC, getenv_bootm_mapsize(), 0, 0);
 }
 #endif
diff --git a/cmd/ximg.c b/cmd/ximg.c
index d033c15..70d6d14 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -33,7 +33,8 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * 
const argv[])
 {
ulong   addr = load_addr;
ulong   dest = 0;
-   ulong   data, len;
+   phys_addr_t data;
+   ulong   len;
int verify;
int part = 0;
 #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
@@ -173,7 +174,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * 
const argv[])
return 1;
}
 
-   data = (ulong)fit_data;
+   data = (phys_addr_t)(uintptr_t)fit_data;
len = (ulong)fit_len;
break;
 #endif
@@ -205,14 +206,14 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char 
* const argv[])
}
 #else  /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
printf("   Loading part %d ... ", part);
-   memmove((char *) dest, (char *)data, len);
+   memmove((char *)dest, (char *)(uintptr_t)data, len);
 #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
break;
 #ifdef CONFIG_GZIP
case IH_COMP_GZIP:
printf("   Uncompressing part %d ... ", part);
if (gunzip((void *) dest, unc_len,
-  (uchar *) data, ) != 0) {
+  (uchar *)(uintptr_t)data, ) != 0) {
puts("GUNZIP ERROR - image not loaded\n");
return 1;
}
diff --git a/common/bootm.c b/common/bootm.c
index 99d574d..785858c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -43,7 +43,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
   char * const argv[], bootm_headers_t *images,
-  ulong *os_data, ulong *os_len);
+  phys_addr_t *os_data, ulong *os_len);
 
 #ifdef CONFIG_LMB
 static void boot_start_lmb(bootm_headers_t *images)
@@ -325,9 +325,9 @@ static int handle_decomp_error(int comp_type, size_t 
uncomp_size,
return BOOTM_ERR_RESET;
 }
 
-int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
-  void *load_buf, void *image_buf, ulong image_len,
-  uint unc_len, ulong *load_end)
+int bootm_decomp_image(int comp, phys_addr_t load, phys_addr_t image_start,
+  int type, void *load_buf, void *image_buf,
+  ulong image_len, uint unc_len, ulong *load_end)
 {
int ret = 0;
 
@@ -767,7 +767,7 @@ static image_header_t *image_get_kernel(ulong img_addr, int 
verify)
 
 /**
  * boot_get_kernel - find

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

Re: [U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

[U-Boot] [RFC PATCH v4 2/3] common: Convert ulong to phys_addr_t for image addresses

4 matches

Site Navigation

Mail list logo

Footer information