Re: [PATCH v3 6/9] xen/common: Move Arm's bootfdt.c to common

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

Hi Julien,

On 3/21/24 12:50 PM, Julien Grall wrote:
> Hi Shawn,
> 
> On 14/03/2024 22:15, Shawn Anastasio wrote:
>> Move Arm's bootfdt.c to xen/common so that it can be used by other
>> device tree architectures like PPC and RISCV.
>>
>> Suggested-by: Julien Grall <julien@xxxxxxx>
>> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
>> Acked-by: Julien Grall <julien@xxxxxxx>
>> ---
>> Changes in v2:
>>    - Drop #if defined(CONFIG_ARM_EFI) now that efi_enabled is stubbed
>>
>>   MAINTAINERS                                    | 1 +
>>   xen/arch/arm/Makefile                          | 1 -
>>   xen/common/device-tree/Makefile                | 1 +
>>   xen/{arch/arm => common/device-tree}/bootfdt.c | 0
>>   4 files changed, 2 insertions(+), 1 deletion(-)
>>   rename xen/{arch/arm => common/device-tree}/bootfdt.c (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e85fbe6737..20fdec9ffa 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -251,6 +251,7 @@ S:    Supported
>>   L:    xen-devel@xxxxxxxxxxxxxxxxxxxx
>>   F:    docs/misc/arm/
>>   F:    xen/arch/arm/
>> +F:    xen/common/device-tree/bootfdt.c
>>   F:    xen/drivers/char/arm-uart.c
>>   F:    xen/drivers/char/cadence-uart.c
>>   F:    xen/drivers/char/exynos4210-uart.c
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 7b1350e2ef..9e1548378c 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -10,7 +10,6 @@ obj-$(CONFIG_TEE) += tee/
>>   obj-$(CONFIG_HAS_VPCI) += vpci.o
>>
>>   obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>> -obj-y += bootfdt.init.o
>>   obj-y += cpuerrata.o
>>   obj-y += cpufeature.o
>>   obj-y += decode.o
>> diff --git a/xen/common/device-tree/Makefile
>> b/xen/common/device-tree/Makefile
>> index c97b2bd88c..fa5beafd65 100644
>> --- a/xen/common/device-tree/Makefile
>> +++ b/xen/common/device-tree/Makefile
>> @@ -1 +1,2 @@
>> +obj-y += bootfdt.init.o
>>   obj-y += bootinfo.o
> 
> Looking at the names, it is not entirely clear what would be the
> differences between bootfdt and bootinfo. Should they just be one file?
> 

With the current split I've chosen, all functions pertaining to managing
the `struct bootinfo` data structure are contained within bootinfo.c and
all functions responsible for parsing the FDT on boot are in bootfdt.c.

This separation exists currently in the ARM tree, but the bootinfo
functions are contained in setup.c rather than a separate self-contained
bootinfo.c.

If you feel strongly that we would be better off with everything in a
single file, I'm not necessarily opposed to that, but I do think that
this split at least makes sense.

>> diff --git a/xen/arch/arm/bootfdt.c b/xen/common/device-tree/bootfdt.c
>> similarity index 100%
>> rename from xen/arch/arm/bootfdt.c
>> rename to xen/common/device-tree/bootfdt.c
>> -- 
>> 2.30.2
>>
> 
> Cheers,

Thanks,
Shawn