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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2024-07-11 at 11:21 +0200, Jan Beulich wrote:
> On 11.07.2024 11:00, Oleksii wrote:
> > 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.
> 
> Except that what you sent is v1, not v5.
> 
> > I'll reword that and probably I shouldn't drop "Changes in
> > v4,3,2,1"
> > from Shawn's patch.
> 
> I don't think you should drop anything. You want to clarify
> relationship
> of the version of your series with that of Shawn's earlier one. Or
> you'd
> want to continue numbering from what the previous series had, yet
> that
> may then also end up confusing if Shawn resumed work there.
Oh, I understood now what you meant. The Patch should be with the name:
[PATCH *v5* 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions
to common

I will update that. Thanks.

~ Oleksii