Re: [Qemu-devel] [PATCH v2 1/1] configure: dtc: Probe for libfdt_env.h

[Peter Crosthwaite]

Hi David,

On Wed, May 29, 2013 at 7:31 PM, David Gibson
<address@hidden> wrote:
> On Wed, May 29, 2013 at 10:14:19AM +0100, Peter Maydell wrote:
>> On 29 May 2013 09:18, David Gibson <address@hidden> wrote:
>> > On Mon, May 27, 2013 at 02:20:57PM +1000, address@hidden wrote:
>> >> From: Peter Crosthwaite <address@hidden>
>> >> Currently QEMU provides a local clone of the file libfdt_env.h in
>> >> /include. This file is supposed to come with the libfdt package and is
>> >
>> > So, this patch might be the right thing anyway, but the statement
>> > above is entirely false.  libfdt_env.h is explicitly intended to be
>> > supplied by the build environment embedding libfdt.  The version
>> > provided in the libfdt package is a example version which will do
>> > (although it might not be ideal) for userspace builds with a
>> > sufficiently normal C library.
>>
>> (You're upstream for libfdt, yes?)
>
> More or less, yes.
>
>> If you're expecting that build environments provide a libfdt_env.h
>> then shouldn't you be avoiding making breaking changes to libfdt
>> that require build environments to fix their libfdt_env.h?
>> This came up in the first place because we no longer build with
>> new libfdt...
>
> I missed that discussion, what exactly was the problem?

In file included from /usr/local/include/libfdt.h:55,
                 from
/home/peterc/Petalogix/Internal/plgx_src/qemu-arm-microblazeel/device_tree.c:28:
/usr/local/include/fdt.h:7: error: expected specifier-qualifier-list
before ‘fdt32_t’
/usr/local/include/fdt.h:26: error: expected specifier-qualifier-list
before ‘fdt64_t’
/usr/local/include/fdt.h:31: error: expected specifier-qualifier-list
before ‘fdt32_t’
/usr/local/include/fdt.h:36: error: expected specifier-qualifier-list
before ‘fdt32_t’

The type fdt32_t (and friends) is undefined.

Heres the patch DTC side that introduces these types.

commit feafcd972cb744750a65728440c99526e6199a6d
Author: Kim Phillips <address@hidden>
Date:   Wed Nov 28 17:33:01 2012 -0600

    dtc/libfdt: introduce fdt types for annotation by endian checkers

    Projects such as linux and u-boot run sparse on libfdt.  libfdt
    contains the notion of endianness via usage of endian conversion
    functions such as fdt32_to_cpu.  As such, in order to pass endian
    checks, libfdt has to annotate its fdt variables such that sparse
    can warn when mixing bitwise and regular integers.  This patch adds
    these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
    defines), includes the bitwise annotation.

    Signed-off-by: Kim Phillips <address@hidden>
    Acked-by: David Gibson <address@hidden>


> Then again, although we certainly
> want to keep changes which require updates to libfdt_env.h rare, I'm
> not going to rule out extensions to libfdt which add new (minor)
> external dependencies, and therefore changes to libfdt_env.h.
>
> That said, I did put my correction a bit too strongly.  While
> libfdt_env.h is notionally always provided by the surrounding
> environment, if the packaged one works for your environment, it's
> probably a good idea to use it.

I think we should be able to consider QEMU as normal enough to use
prepacked DTC and just fix the commit message?

Regards,
peter

  That should be the case for almost
> all userspace builds - providing a custom libfdt_env.h is more
> intended for kernels, bootloaders and other build environments with
> peculiar constraints.
>
>> (conversely, if we're supposed to update our libfdt_env.h as we
>> go along, is there a changelog that documents needed changes
>> and what needs to be present for compatibility with older versions
>> of libfdt?)
>
> Heh.  There definitely should be a document covering what needs to be
> provided by libfdt_env.h, but alas there is not.
>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson