Re: [XEN][PATCH v3 1/1] Update libfdt to v1.6.1

[Vikram Garhwal <fnu.vikram@xxxxxxxxxx>]

On Mon, Nov 08, 2021 at 10:37:31AM +0000, Julien Grall wrote:
Hi Julien,
> Hi Vikram,
> 
> On 05/11/2021 21:28, Vikram Garhwal wrote:
> >Update libfdt to v1.6.1 of libfdt taken from git://github.com/dgibson/dtc.
> >This update is done to support device tree overlays.
> >
> >A few minor changes are done to make it compatible with Xen:
> >     fdt_overlay.c: overlay_fixup_phandle()
> >
> >         Replace strtoul() with simple_strtoul() as strtoul() is not 
> > available in
> >         Xen lib and included lib.h.
> >
> >         Change char *endptr to const char *endptr.
> 
> I would clarify this is a requirement to use simple_strtoul().
Will add it in next version.
> 
> [...]
> 
> >diff --git a/xen/include/xen/libfdt/libfdt_env.h 
> >b/xen/include/xen/libfdt/libfdt_env.h
> >index 035bf75..03380d5 100644
> >--- a/xen/include/xen/libfdt/libfdt_env.h
> >+++ b/xen/include/xen/libfdt/libfdt_env.h
> >@@ -1,22 +1,98 @@
> >-#ifndef _LIBFDT_ENV_H
> >-#define _LIBFDT_ENV_H
> >+/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
> >+#ifndef LIBFDT_ENV_H
> >+#define LIBFDT_ENV_H
> >+/*
> >+ * libfdt - Flat Device Tree manipulation
> >+ * Copyright (C) 2006 David Gibson, IBM Corporation.
> >+ * Copyright 2012 Kim Phillips, Freescale Semiconductor.
> >+ */
> >+#include <xen/config.h>
> 
> The build system should always include <xen/config.h> automagically. So can
> you clarify why this is necessary?
You are right. This was something got carried over from v1.4. I can build 
without this.
Will remove it in v4 patch.
> 
> >  #include <xen/types.h>
> >  #include <xen/string.h>
> >  #include <asm/byteorder.h>
> >+#include <xen/stdbool.h>
> 
> We usually order the headers so <xen/...> are first and then <asm/...>. They
> are then ordered alphabetically within themself.
> 
> But it is not clear why you are adding <xen/stdbool.h> as AFAICT it is
> already going to be included by existing headers. Did you see an error
> without including it?
Perhaps my build was messed up then. I tried just now and can build without it.
Will remove this one too in v4.

Thanks for corrections!

Regards,
Vikram
> 
> Cheers,
> 
> -- 
> Julien Grall