diff mbox

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

Message ID 1441369343-4638-8-git-send-email-thomas@wytron.com.tw
State Superseded
Delegated to: Thomas Chou
Headers show

Commit Message

Thomas Chou Sept. 4, 2015, 12:22 p.m. UTC
Show fdt blob address in board info command.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 common/cmd_bdinfo.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Marek Vasut Sept. 4, 2015, 2:01 p.m. UTC | #1
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 <thomas@wytron.com.tw>
> ---
>  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
Simon Glass Sept. 4, 2015, 2:10 p.m. UTC | #2
On 4 September 2015 at 06:22, Thomas Chou <thomas@wytron.com.tw> wrote:
> Show fdt blob address in board info command.
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>  common/cmd_bdinfo.c | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>
Thomas Chou Sept. 5, 2015, 2:54 a.m. UTC | #3
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
Marek Vasut Sept. 5, 2015, 12:51 p.m. UTC | #4
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
Thomas Chou Sept. 6, 2015, 11:31 a.m. UTC | #5
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
Thomas Chou Sept. 6, 2015, 1:20 p.m. UTC | #6
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
Marek Vasut Sept. 6, 2015, 2:27 p.m. UTC | #7
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
Simon Glass Sept. 8, 2015, 3:55 a.m. UTC | #8
Hi,

On 6 September 2015 at 08:27, Marek Vasut <marex@denx.de> 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
Marek Vasut Sept. 8, 2015, 10:10 a.m. UTC | #9
On Tuesday, September 08, 2015 at 05:55:59 AM, Simon Glass wrote:
> Hi,
> 
> On 6 September 2015 at 08:27, Marek Vasut <marex@denx.de> 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
Thomas Chou Sept. 9, 2015, 4:26 a.m. UTC | #10
On 09/04/2015 08:22 PM, Thomas Chou wrote:
> Show fdt blob address in board info command.
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>   common/cmd_bdinfo.c | 3 +++
>   1 file changed, 3 insertions(+)

Applied to u-boot-nios2.
diff mbox

Patch

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;
 }