Re: [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2024-07-10 at 12:23 +0200, Jan Beulich wrote:
> On 03.07.2024 12:42, Oleksii Kurochko wrote:
> > From: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> > 
> > Arm's setup.c contains a collection of functions for parsing memory
> > map
> > and other boot information from a device tree. Since these routines
> > are
> > generally useful on any architecture that supports device tree
> > booting,
> > move them into xen/common/device-tree.
> > 
> > Suggested-by: Julien Grall <julien@xxxxxxx>
> > Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> > Changes in V5:
> >  - add xen/include/xen/bootfdt.h to MAINTAINERS file.
> >  - drop message "Early device tree parsing and".
> >  - After rebase on top of the current staging the following changes
> > were done:
> >    - init bootinfo variable in <common/device-tree/bootinfo.c> with
> > BOOTINFO_INIT;
> >    - update the code of dt_unreserved_regions():
> >        CONFIG_STATIC_SHM related changes and getting of
> > reserved_mem
> >        bootinfo_get_shmem() ??
> >    - update the code of meminfo_overlap_check():
> >        add check ( INVALID_PADDR == bank_start ) to if case.
> >    - update the code of check_reserved_regions_overlap():
> >        CONFIG_STATIC_SHM related changes.
> >    - struct bootinfo was updated ( CONFIG_STATIC_SHM changes )
> >    - add shared_meminfo ( because of CONFIG_STATIC_SHM )
> >    - struct struct membanks was update with __struct group so
> > <xen/kernel> is
> >      neeeded to be included in bootfdt.h
> >    - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, BOOTINFO_INIT to
> > generic bootfdt.h
> >    - bootinfo_get_reserved_mem(), bootinfo_get_mem(),
> > bootinfo_get_acpi(),
> >      bootinfo_get_shmem() and bootinfo_get_shmem_extra() were moved
> > to xen/bootfdt.h
> >  - s/arm32/CONFIG_SEPARATE_XENHEAP/
> >  - add inclusion of <xen/macros.h> because there are function in
> > <xen/bootfdt.h> which
> >    are using container_of().
> 
> Just to mention it: This is confusing. The series is tagged "v1". I
> understand
> you took Shawn's work, which had already undergone revisions. But
> then imo you
> want to at least clarify how your v1 relates to his v4 or v5, i.e.
> then making
> clear to the reader whether all of the changes above were actually
> done by you
> on top of an earlier v4, or whether you took the earlier v5 verbatim.
That is why I wrote "Changes in v5" to show that these changes were
done on top of v4 version of Shawn's series, so what is mentioned in v5
here it is what was done by me.

I'll reword that and probably I shouldn't drop "Changes in v4,3,2,1"
from Shawn's patch.

~ Oleksii