Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.11.2023 15:13, Oleksii Kurochko wrote:
> Arm, PPC and RISC-V use the same device.h thereby device.h
> was moved to asm-generic. Arm's device.h was taken as a base with
> the following changes:
>  - #ifdef PCI related things.
>  - #ifdef ACPI related things.
>  - Rename #ifdef guards.
>  - Add SPDX tag.
>  - #ifdef CONFIG_HAS_DEVICE_TREE related things.
>  - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
> 
> Also Arm and PPC are switched to asm-generic version of device.h
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in V4:
>  - Updated the commit message
>  - Switched Arm and PPC to asm-generic version of device.h
>  - Replaced HAS_PCI with CONFIG_HAS_PCI
>  - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
>  - Updated the commit message ( remove a note with question about
>    if device.h should be in asm-generic or not )
>  - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
>  - Rationalized usage of CONFIG_HAS_* in device.h
>  - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> ---
> Changes in V3:
>  - ifdef device tree related things.
>  - update the commit message
> ---
> Changes in V2:
>       - take ( as common ) device.h from Arm as PPC and RISC-V use it as a 
> base.
>       - #ifdef PCI related things.
>       - #ifdef ACPI related things.
>       - rename DEVICE_GIC to DEVIC_IC.
>       - rename #ifdef guards.
>       - switch Arm and PPC to generic device.h
>       - add SPDX tag
>       - update the commit message
> 
> ---
>  xen/arch/arm/device.c                         |  15 ++-
>  xen/arch/arm/domain_build.c                   |   2 +-
>  xen/arch/arm/gic-v2.c                         |   4 +-
>  xen/arch/arm/gic-v3.c                         |   6 +-
>  xen/arch/arm/gic.c                            |   4 +-
>  xen/arch/arm/include/asm/Makefile             |   1 +
>  xen/arch/ppc/include/asm/Makefile             |   1 +
>  xen/arch/ppc/include/asm/device.h             |  53 --------
>  xen/arch/ppc/include/asm/irq.h                |   4 +
>  .../asm => include/asm-generic}/device.h      | 125 +++++++++++-------
>  xen/include/headers++.chk.new                 |   0
>  11 files changed, 106 insertions(+), 109 deletions(-)
>  delete mode 100644 xen/arch/ppc/include/asm/device.h
>  rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
>  create mode 100644 xen/include/headers++.chk.new

Stray new file, presumably because of a missing entry in .gitignore?

Overall I think there are too many changes done all in one go here.
But it's mostly Arm which is affected, so I'll leave judging about that
to the Arm maintainers.

> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
>  #define __ASM_PPC_IRQ_H__
>  
>  #include <xen/lib.h>
> +#ifdef CONFIG_HAS_DEVICE_TREE
>  #include <xen/device_tree.h>
> +#endif
>  #include <public/device_tree_defs.h>

Why would this #ifdef not cover the public header as well? (Otherwise I'd
be inclined to ask that the conditional be moved inside that header.)

Jan