Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Marek Vasut]

On Tuesday, September 08, 2015 at 05:55:59 AM, Simon Glass wrote:
> Hi,
> 
> On 6 September 2015 at 08:27, Marek Vasut  wrote:
> > On Sunday, September 06, 2015 at 03:20:21 PM, Thomas Chou wrote:
> >> Hi Marek,
> >> 
> >> >>> I think this will not work on 64bit machines.
> >> 
> >> I tried 64bits with
> >> 
> >>   print_lnum("fdt_blob", (u64)gd->fdt_blob);
> >> 
> >> But got this warning,
> >> 
> >>CC  common/cmd_bdinfo.o
> >> 
> >> common/cmd_bdinfo.c: In function 'do_bdinfo':
> >> common/cmd_bdinfo.c:180:25: warning: cast from pointer to integer of
> >> different size [-Wpointer-to-int-cast]
> >> 
> >>print_lnum("fdt_blob", (u64)gd->fdt_blob);
> >>
> >>   ^
> >> 
> >> include/asm-generic/global_data.h
> >> 
> >>   const void *fdt_blob;   /* Our device tree, NULL if none */
> >> 
> >> Actually, the print is guarded with CONFIG_NIOS2, so it is safe to use
> >> 32bits.
> >> 
> >> Or shall we invent a "print pointer address"?
> > 
> > Simon, Tom, what's your take on this ?
> 
> From what I can tell this code is not generic, and is built for NIOS
> only. So IMO the patch is fine and we don't need to worry about
> 64-bit.

Oh my $DEITY, we have one bdinfo implementation per architecture, all in
one ugly file. In that case, you're right, but this is truly horrible.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Thomas Chou]

On 09/04/2015 08:22 PM, Thomas Chou wrote:

Show fdt blob address in board info command.

Signed-off-by: Thomas Chou 
---
  common/cmd_bdinfo.c | 3 +++
  1 file changed, 3 insertions(+)


Applied to u-boot-nios2.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Simon Glass]

Hi,

On 6 September 2015 at 08:27, Marek Vasut  wrote:
> On Sunday, September 06, 2015 at 03:20:21 PM, Thomas Chou wrote:
>> Hi Marek,
>>
>> >>> I think this will not work on 64bit machines.
>>
>> I tried 64bits with
>>
>>   print_lnum("fdt_blob", (u64)gd->fdt_blob);
>>
>> But got this warning,
>>
>>CC  common/cmd_bdinfo.o
>> common/cmd_bdinfo.c: In function 'do_bdinfo':
>> common/cmd_bdinfo.c:180:25: warning: cast from pointer to integer of
>> different size [-Wpointer-to-int-cast]
>>print_lnum("fdt_blob", (u64)gd->fdt_blob);
>>   ^
>> include/asm-generic/global_data.h
>>   const void *fdt_blob;   /* Our device tree, NULL if none */
>>
>> Actually, the print is guarded with CONFIG_NIOS2, so it is safe to use
>> 32bits.
>>
>> Or shall we invent a "print pointer address"?
>
> Simon, Tom, what's your take on this ?

>From what I can tell this code is not generic, and is built for NIOS
only. So IMO the patch is fine and we don't need to worry about
64-bit.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Thomas Chou]

Hi Marek,


I think this will not work on 64bit machines.


I tried 64bits with

print_lnum("fdt_blob", (u64)gd->fdt_blob);

But got this warning,

  CC  common/cmd_bdinfo.o
common/cmd_bdinfo.c: In function 'do_bdinfo':
common/cmd_bdinfo.c:180:25: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]

  print_lnum("fdt_blob", (u64)gd->fdt_blob);
 ^
include/asm-generic/global_data.h
const void *fdt_blob;   /* Our device tree, NULL if none */

Actually, the print is guarded with CONFIG_NIOS2, so it is safe to use 
32bits.


Or shall we invent a "print pointer address"?

Best regards,
Thomas Chou
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Marek Vasut]

On Sunday, September 06, 2015 at 03:20:21 PM, Thomas Chou wrote:
> Hi Marek,
> 
> >>> I think this will not work on 64bit machines.
> 
> I tried 64bits with
> 
>   print_lnum("fdt_blob", (u64)gd->fdt_blob);
> 
> But got this warning,
> 
>CC  common/cmd_bdinfo.o
> common/cmd_bdinfo.c: In function 'do_bdinfo':
> common/cmd_bdinfo.c:180:25: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
>print_lnum("fdt_blob", (u64)gd->fdt_blob);
>   ^
> include/asm-generic/global_data.h
>   const void *fdt_blob;   /* Our device tree, NULL if none */
> 
> Actually, the print is guarded with CONFIG_NIOS2, so it is safe to use
> 32bits.
> 
> Or shall we invent a "print pointer address"?

Simon, Tom, what's your take on this ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Thomas Chou]

Hi Marek,

On 09/05/2015 08:51 PM, Marek Vasut wrote:

That's a common code you're modifying, so it has to be done right, sorry,
this patch cannot be accepted in this form.


I understand. I will use (u64) and print_lnum().

Best regards,
Thomas Chou
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Marek Vasut]

On Saturday, September 05, 2015 at 04:54:52 AM, Thomas Chou wrote:
> Hi Marek,

Hi,

> On 09/04/2015 10:01 PM, Marek Vasut wrote:
> >> +  print_num("fdt_blob", (ulong)gd->fdt_blob);
> > 
> > I think this will not work on 64bit machines.
> 
> I'd agree that it won't work on 64bits machines, but it is enough for a
> soft-core 32btis fpga processor. :)

That's a common code you're modifying, so it has to be done right, sorry,
this patch cannot be accepted in this form.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Thomas Chou]

Show fdt blob address in board info command.

Signed-off-by: Thomas Chou 
---
 common/cmd_bdinfo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index ed3b935..74ff229 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -176,6 +176,9 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 #endif
 
printf("baudrate= %u bps\n", gd->baudrate);
+#if defined(CONFIG_OF_CONTROL)
+   print_num("fdt_blob", (ulong)gd->fdt_blob);
+#endif
 
return 0;
 }
-- 
2.1.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Marek Vasut]

On Friday, September 04, 2015 at 02:22:22 PM, Thomas Chou wrote:
> Show fdt blob address in board info command.
> 
> Signed-off-by: Thomas Chou 
> ---
>  common/cmd_bdinfo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
> index ed3b935..74ff229 100644
> --- a/common/cmd_bdinfo.c
> +++ b/common/cmd_bdinfo.c
> @@ -176,6 +176,9 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[]) #endif
> 
>   printf("baudrate= %u bps\n", gd->baudrate);
> +#if defined(CONFIG_OF_CONTROL)
> + print_num("fdt_blob", (ulong)gd->fdt_blob);

I think this will not work on 64bit machines.

> +#endif
> 
>   return 0;
>  }

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Simon Glass]

On 4 September 2015 at 06:22, Thomas Chou  wrote:
> Show fdt blob address in board info command.
>
> Signed-off-by: Thomas Chou 
> ---
>  common/cmd_bdinfo.c | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] nios2: show fdt blob address in board info command

[Thomas Chou]

Hi Marek,

On 09/04/2015 10:01 PM, Marek Vasut wrote:

+   print_num("fdt_blob", (ulong)gd->fdt_blob);


I think this will not work on 64bit machines.


I'd agree that it won't work on 64bits machines, but it is enough for a 
soft-core 32btis fpga processor. :)


Best regards,
Thomas Chou
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot