Hi Masahiro,
On 15 December 2014 at 18:47, Masahiro YAMADA <yamad...@jp.panasonic.com> wrote:
> Simon,
>
>
> 2014-12-02 5:06 GMT+09:00 Simon Glass <s...@chromium.org>:
>> Hi Masahiro,
>>
>> On 26 November 2014 at 00:45, Masahiro Yamada <yamad...@jp.panasonic.com>
>> wrote:
>>> Hi Tom, Simon, other developers,
>>>
>>>
>>>
>>> Since commit 0d296cc2d3b8 (Provide option to avoid defining a custom
>>> version of uintptr_t)
>>> and commit 4166ecb247a1 (Add some standard headers external code might
>>> need),
>>> I have been wondering if they were right decisions.
>>>
>>>
>>>
>>> As arch/arm/include/asm/types.h of Linux Kernel says,
>>> using 'stdint.h' is not feasible.
>>>
>>> -------------------------------------->8-------------------------------------
>>> /*
>>> * The C99 types uintXX_t that are usually defined in 'stdint.h' are not as
>>> * unambiguous on ARM as you would expect. For the types below, there is a
>>> * difference on ARM between GCC built for bare metal ARM, GCC built for
>>> glibc
>>> * and the kernel itself, which results in build errors if you try to build
>>> with
>>> * -ffreestanding and include 'stdint.h' (such as when you include
>>> 'arm_neon.h'
>>> * in order to use NEON intrinsics)
>>> *
>>> * As the typedefs for these types in 'stdint.h' are based on builtin
>>> defines
>>> * supplied by GCC, we can tweak these to align with the kernel's idea of
>>> those
>>> * types, so 'linux/types.h' and 'stdint.h' can be safely included from the
>>> same
>>> * source file (provided that -ffreestanding is used).
>>> *
>>> * int32_t uint32_t uintptr_t
>>> * bare metal GCC long unsigned long unsigned int
>>> * glibc GCC int unsigned int unsigned int
>>> * kernel int unsigned int unsigned long
>>> */
>>> --------------------------------------8<----------------------------------------
>>>
>>
>> To me this doesn't matter. I feel that int32_t should probably be int
>> on 32-bit ARM, but actually so long as it is consistent then it is
>> fine if it is long. In fact so long as these types are defined
>> consistently, everything works.
>
> Gabe Black and you broke the consistency.
> See my explanation below.
>
>
>
>
>>>
>>> Actually, the kernel never includes <stdint.h> except host programs.
>>>
>>>
>>> Commit 0d296cc2d3b8 introduced "USE_STDINT", but it causes type conflicts
>>> depending on which GCC is used.
>>>
>>>
>>> With ARM bare metal GCC,
>>>
>>> yamada@beagle:~/workspace/u-boot$ make omap3_beagle_defconfig all
>>> CROSS_COMPILE=arm-none-eabi- USE_STDINT=1
>>> #
>>> # configuration written to .config
>>> #
>>> #
>>> # configuration written to spl/.config
>>> #
>>> scripts/kconfig/conf --silentoldconfig Kconfig
>>> scripts/kconfig/conf --silentoldconfig Kconfig
>>> CHK include/config.h
>>> GEN include/autoconf.mk
>>> GEN include/autoconf.mk.dep
>>> GEN spl/include/autoconf.mk
>>> CHK include/config/uboot.release
>>> CHK include/generated/version_autogenerated.h
>>> CHK include/generated/timestamp_autogenerated.h
>>> UPD include/generated/timestamp_autogenerated.h
>>> CC lib/asm-offsets.s
>>> In file included from
>>> /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint.h:5:0,
>>> from include/compiler.h:117,
>>> from include/image.h:19,
>>> from include/common.h:85,
>>> from lib/asm-offsets.c:15:
>>> /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:40:24:
>>> error: conflicting types for 'int32_t'
>>> include/linux/types.h:99:17: note: previous declaration of 'int32_t' was
>>> here
>>> /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:52:25:
>>> error: conflicting types for 'uint32_t'
>>> include/linux/types.h:105:17: note: previous declaration of 'uint32_t' was
>>> here
>>> make[2]: *** [lib/asm-offsets.s] Error 1
>>> make[1]: *** [prepare0] Error 2
>>> make: *** [__build_one_by_one] Error 2
>>>
>>>
>>
>> While toolchain is this? I don't see this problem - it seems broken.
>
> Not broken at all.
> I downloaded it from Mentor Graphics
> http://www.mentor.com/embedded-software/codesourcery
>
> This is a bare-metal compiler.
> If you do not understand, you should read arch/arm/include/asm/types.h
> of Linux Kernel once again.
>
OK, so perhaps for that compiler you cannot use USE_STDINT=1? What
does it define for the conflicting type?
>
>
> Until commit 0d296cc2d, U-Boot has used the hard-coded defines like Linux.
Well it still does, only that it also now has the *option* of using stdint.h.
>
> In my understaing, we should only use ILP32 and LP64 compilers.
>
>
> short int long longlong pointer
> ILP32 (32bit system) 16 32 32 64 32
> LP64 (64bit Unix-like system) 16 32 64 64 64
>
>
> Whether it is 32bit or 64bit system, we can hard-code
>
> u32/uint32_t as unsigned int
> u64/uint64_t as unsigned long long
> uintptr_t as unsigned long
>
> We do not need to refer to compiler-provided headers.
>
> Moreover, we __should not__ refer to compiler-provided <stdint.h>.
>
>
> Including <stdint.h> means that we use uint32_t defined by the compiler.
> It is "unsigned int" on some compilers, and "unsigned long" on
> some compilers.
I don't seem to have that problem, or at least I have not noticed it
with printf().
>
> We still have the hard-coded uint32_t define in <linux/types.h>
> This causes the type conflict error I showed above.
>
> OK, we can fix it, but horrible problems still remain.
>
> If we depends on <stdint.h>, we do not know if uint32_t is defined
> as "unsigned int" or "unsigned long".
>
> To print out 32bit variables, we would always have to use PRId32.
> Do you want to modify all the printf() printing 32bit variables
> just to make them unreadable?
>
Ick, I have not seen this. Can you given an example of where this happens?
>
>
>>> OK, we can fix "int32_t" and "uint32_t", but it still seems strange to see
>>> that
>>> "uint32_t" is defined as "unsigned long", whereas "u32" is defined as
>>> "unsigned int".
>>>
>>> If so, must we fix "u32", "s32", ... all the fixed-width typedefs ?
>>>
>>> I notice including <linux/types.h> in the U-Boot source tree and
>>> <stdint.h> provided by your compiler at the same time is a nightmare.
>>>
>>> If we lean toward <stdint.h>, we must ban <linux/types.h>,
>>> but <stdint.h> is not available all the time.
>>> For example, kernel.org tool-chains do not provide <stdint.h>.
>>>
>>> Maybe we can drastically re-write <linux/types.h> and friends
>>> to resolve the type conflicts, but I do not think we should not drift apart
>>> from the kernel
>>> because we have borrowed many source files from Linux.
>>
>> So far I don't see a big problem.
>
> Horrible problem!
>
>> I feel that, were stdint.h available
>> earlier, then types.h might not have been written and we would just
>> use stdint.h. Presumably stdint.h has been created to fix these sorts
>> of problems, and in fact for new projects, they would not define their
>> own types.
>
> What is missing is how to pass the variable width nicely to printf()/scanf().
>
> printf("foo =" PRId32 "\n", foo_32);
> printf("bar =" PRId64 "\n", bar_64);
>
> This is ridiculous. Extremely unreadable.
>
>
> I want something like this:
> printf("foo = %32x \n", foo_32);
> printf("bar =" %64x \n", bar_64);
>
>
> If this had been provided, Linux and U-Boot might have adopted <stdint.h>,
> but in fact, PRI* is an awful workaround.
Agreed it's not very readable but so long as it only applies to 64-bit
values and only to printf() it doesn't see bad to me. Provided we
resolve what you have brought up above.
>
>> IMO in time the kernel and U-Boot might move to stdint.h, and having
>> it as an option at the moment helps us understand the issues. It does
>> not break any builds.
>
> I double it.
> Using <stdint.h> is a misjudge. It will mess up printf() everywhere.
>
> Linux Kernel has guideline how to print each variable type.
> Documentation/printk-formats.txt
>
>
>> I could be wrong though, time will tell. We should keep an eye on it.
>
> If we keep wrong code in the code base, someone might continue development
> base on it, which is also wrong.
>
> In order not to lose our time, wrong code should be immediately removed.
How do you explain stdint.h? So many projects use it - it is now part
of the C99 standard. Has the world gone mad?
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
