public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] debug warning
@ 2015-07-15 20:02 York Sun
  2015-07-15 20:23 ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2015-07-15 20:02 UTC (permalink / raw)
  To: u-boot

Simon,

Did it happen to you with this warning?

lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
    debug("addr=%08lx, size=%08x\n",
    ^

I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
phys_addr_t, and fdt_size_t to phys_size_t. What do you say?

York

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] debug warning
  2015-07-15 20:02 [U-Boot] debug warning York Sun
@ 2015-07-15 20:23 ` Albert ARIBAUD
  2015-07-15 20:25   ` York Sun
  0 siblings, 1 reply; 6+ messages in thread
From: Albert ARIBAUD @ 2015-07-15 20:23 UTC (permalink / raw)
  To: u-boot

Hello York,

On Wed, 15 Jul 2015 13:02:07 -0700, York Sun <yorksun@freescale.com>
wrote:
> Simon,
> 
> Did it happen to you with this warning?
> 
> lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
> int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
>     debug("addr=%08lx, size=%08x\n",
>     ^
> 
> I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
> statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
> phys_addr_t, and fdt_size_t to phys_size_t. What do you say?

I say there is no reason to change a type just because a printf format
specifier is wrong for it when building for 64-bit.

Is there a rationale apart from the format specifier error?

If not, then What should be done is fix the specifier so that it is
correct in both 32 and 64 bits.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] debug warning
  2015-07-15 20:23 ` Albert ARIBAUD
@ 2015-07-15 20:25   ` York Sun
  2015-07-15 20:29     ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2015-07-15 20:25 UTC (permalink / raw)
  To: u-boot



On 07/15/2015 01:23 PM, Albert ARIBAUD wrote:
> Hello York,
> 
> On Wed, 15 Jul 2015 13:02:07 -0700, York Sun <yorksun@freescale.com>
> wrote:
>> Simon,
>>
>> Did it happen to you with this warning?
>>
>> lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
>> int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
>>     debug("addr=%08lx, size=%08x\n",
>>     ^
>>
>> I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
>> statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
>> phys_addr_t, and fdt_size_t to phys_size_t. What do you say?
> 
> I say there is no reason to change a type just because a printf format
> specifier is wrong for it when building for 64-bit.
> 
> Is there a rationale apart from the format specifier error?
> 
> If not, then What should be done is fix the specifier so that it is
> correct in both 32 and 64 bits.
> 

Albert,

Let me send a RFC patch so you can comment on it.

York

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] debug warning
  2015-07-15 20:25   ` York Sun
@ 2015-07-15 20:29     ` Simon Glass
  2015-07-15 20:32       ` York Sun
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2015-07-15 20:29 UTC (permalink / raw)
  To: u-boot

Hi York,

On 15 July 2015 at 14:25, York Sun <yorksun@freescale.com> wrote:
>
>
> On 07/15/2015 01:23 PM, Albert ARIBAUD wrote:
>> Hello York,
>>
>> On Wed, 15 Jul 2015 13:02:07 -0700, York Sun <yorksun@freescale.com>
>> wrote:
>>> Simon,
>>>
>>> Did it happen to you with this warning?
>>>
>>> lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
>>> int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
>>>     debug("addr=%08lx, size=%08x\n",
>>>     ^
>>>
>>> I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
>>> statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
>>> phys_addr_t, and fdt_size_t to phys_size_t. What do you say?
>>
>> I say there is no reason to change a type just because a printf format
>> specifier is wrong for it when building for 64-bit.
>>
>> Is there a rationale apart from the format specifier error?
>>
>> If not, then What should be done is fix the specifier so that it is
>> correct in both 32 and 64 bits.
>>
>
> Albert,
>
> Let me send a RFC patch so you can comment on it.

I'm not sure how to do what Albert is suggesting.

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] debug warning
  2015-07-15 20:29     ` Simon Glass
@ 2015-07-15 20:32       ` York Sun
  2015-07-16  6:54         ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2015-07-15 20:32 UTC (permalink / raw)
  To: u-boot



On 07/15/2015 01:29 PM, Simon Glass wrote:
> Hi York,
> 
> On 15 July 2015 at 14:25, York Sun <yorksun@freescale.com> wrote:
>>
>>
>> On 07/15/2015 01:23 PM, Albert ARIBAUD wrote:
>>> Hello York,
>>>
>>> On Wed, 15 Jul 2015 13:02:07 -0700, York Sun <yorksun@freescale.com>
>>> wrote:
>>>> Simon,
>>>>
>>>> Did it happen to you with this warning?
>>>>
>>>> lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
>>>> int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
>>>>     debug("addr=%08lx, size=%08x\n",
>>>>     ^
>>>>
>>>> I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
>>>> statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
>>>> phys_addr_t, and fdt_size_t to phys_size_t. What do you say?
>>>
>>> I say there is no reason to change a type just because a printf format
>>> specifier is wrong for it when building for 64-bit.
>>>
>>> Is there a rationale apart from the format specifier error?
>>>
>>> If not, then What should be done is fix the specifier so that it is
>>> correct in both 32 and 64 bits.
>>>
>>
>> Albert,
>>
>> Let me send a RFC patch so you can comment on it.
> 
> I'm not sure how to do what Albert is suggesting.
> 

I only thought of two ways, using #ifdef or changing the type as my RFC patch
suggests.

York

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] debug warning
  2015-07-15 20:32       ` York Sun
@ 2015-07-16  6:54         ` Albert ARIBAUD
  0 siblings, 0 replies; 6+ messages in thread
From: Albert ARIBAUD @ 2015-07-16  6:54 UTC (permalink / raw)
  To: u-boot

Hello York,

On Wed, 15 Jul 2015 13:32:30 -0700, York Sun <yorksun@freescale.com>
wrote:
> 
> 
> On 07/15/2015 01:29 PM, Simon Glass wrote:
> > Hi York,
> > 
> > On 15 July 2015 at 14:25, York Sun <yorksun@freescale.com> wrote:
> >>
> >>
> >> On 07/15/2015 01:23 PM, Albert ARIBAUD wrote:
> >>> Hello York,
> >>>
> >>> On Wed, 15 Jul 2015 13:02:07 -0700, York Sun <yorksun@freescale.com>
> >>> wrote:
> >>>> Simon,
> >>>>
> >>>> Did it happen to you with this warning?
> >>>>
> >>>> lib/fdtdec.c:108:4: warning: format ?%x? expects argument of type ?unsigned
> >>>> int?, but argument 3 has type ?fdt_size_t? [-Wformat=]
> >>>>     debug("addr=%08lx, size=%08x\n",
> >>>>     ^
> >>>>
> >>>> I think when we have 64-bit physical address, as defined in fdtdec.h, this debug
> >>>> statement needs to be changed. I am thinking to change the typedef fdt_addr_t to
> >>>> phys_addr_t, and fdt_size_t to phys_size_t. What do you say?
> >>>
> >>> I say there is no reason to change a type just because a printf format
> >>> specifier is wrong for it when building for 64-bit.
> >>>
> >>> Is there a rationale apart from the format specifier error?
> >>>
> >>> If not, then What should be done is fix the specifier so that it is
> >>> correct in both 32 and 64 bits.
> >>>
> >>
> >> Albert,
> >>
> >> Let me send a RFC patch so you can comment on it.
> > 
> > I'm not sure how to do what Albert is suggesting.
> > 
> 
> I only thought of two ways, using #ifdef or changing the type as my RFC patch
> suggests.

There's a third one: changing the form of the format specifier
according to the target's definition of the variable -- you do that
once somewhere in a common header file and you're all set up everywhere
else.

See for instance how 'uint64_t' has such a format specifier PRIu64
#define-d in /usr/include/inttypes.h; whatever the definition of
uint64_t, you can do a printf with it as follows:

	uint64_t var;
	...
	printf("Value of var is " PRIu64 ".\n", var);

(I learned this about a year ago while working on code that had to run
on both a 64-bit Intel-core PC and a 32-bit ARM-core Beaglebone)

The same technique can be used here and I strongly prefer it oer
changing the type of the variable, because changing the type affects
the function of *all* code that uses the variable (thus potentially
modifying the board behavior), whereas changing the format specifier
affects *only* the printf() code (thus not changing the board behavior
except for debug code which, supposedly, it only *fixes*.

> York

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-07-16  6:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-15 20:02 [U-Boot] debug warning York Sun
2015-07-15 20:23 ` Albert ARIBAUD
2015-07-15 20:25   ` York Sun
2015-07-15 20:29     ` Simon Glass
2015-07-15 20:32       ` York Sun
2015-07-16  6:54         ` Albert ARIBAUD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox