Re: [PATCH v3 25/25] xen/arm: split domain_build.c

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Stefano Stabellini <stefanos@xilinx.com>,
	andrii_anisov@epam.com, xen-devel@lists.xen.org
Subject: Re: [PATCH v3 25/25] xen/arm: split domain_build.c
Date: Mon, 13 Aug 2018 15:29:53 +0100	[thread overview]
Message-ID: <a5331c07-7f2a-dfee-6c7d-a3b3e6e21eb2@arm.com> (raw)
In-Reply-To: <1533079688-9541-25-git-send-email-sstabellini@kernel.org>

Hi,

On 01/08/18 00:28, Stefano Stabellini wrote:
> domain_build.c is too large.
> 
> Move all the ACPI specific device tree generating functions from
> domain_build.c to acpi/acpi_dt_build.c.

The directory is called "acpi" so there is no point to duplicate in the 
filename.

Also, looking at the code moved, the name does not seem to be correct. 
Indeed you also generate ACPI tables. A better name for this file would 
be domain_build.c

> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> ---
>   xen/arch/arm/acpi/Makefile        |   1 +
>   xen/arch/arm/acpi/acpi_dt_build.c | 591 ++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/acpi/acpi_dt_build.h |  32 +++
>   xen/arch/arm/domain_build.c       | 585 +------------------------------------
>   4 files changed, 629 insertions(+), 580 deletions(-)
>   create mode 100644 xen/arch/arm/acpi/acpi_dt_build.c
>   create mode 100644 xen/arch/arm/acpi/acpi_dt_build.h
> 
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> index 23963f8..ac0804b 100644
> --- a/xen/arch/arm/acpi/Makefile
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -1,2 +1,3 @@
>   obj-y += lib.o
> +obj-y += acpi_dt_build.o
>   obj-y += boot.init.o
> diff --git a/xen/arch/arm/acpi/acpi_dt_build.c b/xen/arch/arm/acpi/acpi_dt_build.c
> new file mode 100644
> index 0000000..7e12d64
> --- /dev/null
> +++ b/xen/arch/arm/acpi/acpi_dt_build.c
> @@ -0,0 +1,591 @@

Missing copyright headers here.

> +#include <xen/mm.h>
> +#include <xen/sched.h>
> +#include <xen/acpi.h>
> +#include <xen/event.h>
> +#include <xen/iocap.h>
> +#include <xen/device_tree.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <xen/irq.h>
> +#include <asm/irq.h>

Do we really need xen/irq.h and asm/irq.h?

> +#include <acpi/actables.h>
> +#include "acpi_dt_build.h"
> +#include "../kernel.h"

Urgh, that's a call to move kernel.h in asm-arm/.

> diff --git a/xen/arch/arm/acpi/acpi_dt_build.h b/xen/arch/arm/acpi/acpi_dt_build.h
> new file mode 100644
> index 0000000..08e7aab
> --- /dev/null
> +++ b/xen/arch/arm/acpi/acpi_dt_build.h
> @@ -0,0 +1,32 @@
> +#ifndef __ARCH_ARM_ACPI_ACPI_DT_BUILD_H__
> +#define __ARCH_ARM_ACPI_ACPI_DT_BUILD_H__
> +
> +#include <xen/sched.h>
> +#include "../kernel.h"

See above.

> +
> +int map_irq_to_domain(struct domain *d, unsigned int irq,
> +                      bool need_mapping, const char *devname);
> +int make_chosen_node(const struct kernel_info *kinfo);
> +void evtchn_allocate(struct domain *d);

Those one should be moved in an header domain_build.h in asm-arm.

> +
> +#ifndef CONFIG_ACPI
> +static inline int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> +{
> +    /* Only booting with ACPI will hit here */
> +    BUG();
> +    return -EINVAL;
> +}
> +#else
> +int prepare_acpi(struct domain *d, struct kernel_info *kinfo);
> +#endif

This one should go in asm-arm/acpi.h.

So this header is not necessary anymore.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel