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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3343 bytes --]

On Fri, May 31, 2013 at 11:48:47AM +1000, Peter Crosthwaite wrote:
> Hi David,
> 
> On Wed, May 29, 2013 at 7:31 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> > On Wed, May 29, 2013 at 10:14:19AM +0100, Peter Maydell wrote:
> >> On 29 May 2013 09:18, David Gibson <david@gibson.dropbear.id.au> wrote:
> >> > On Mon, May 27, 2013 at 02:20:57PM +1000, peter.crosthwaite@xilinx.com wrote:
> >> >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >> >> 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 <kim.phillips@freescale.com>
> Date:   Wed Nov 28 17:33:01 2012 -0600
> 
>     dtc/libfdt: introduce fdt types for annotation by endian
>     checkers

Ah, that one.  Basically, we decided that the benefit gained was worth
the pain of the required change to libfdt_env.h

> > 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?

I concur.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]