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

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

Hi Oleksii,

On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> diff --git a/xen/arch/ppc/include/asm/Makefile 
> b/xen/arch/ppc/include/asm/Makefile
> index ece7fa66dd..df4c1ebb08 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +generic-y += device.h
>  generic-y += paging.h
>  generic-y += vm_event.h
> diff --git a/xen/arch/ppc/include/asm/device.h 
> b/xen/arch/ppc/include/asm/device.h
> deleted file mode 100644
> index 8253e61d51..0000000000
> --- a/xen/arch/ppc/include/asm/device.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_DEVICE_H__
> -#define __ASM_PPC_DEVICE_H__
> -
> -enum device_type
> -{
> -    DEV_DT,
> -    DEV_PCI,
> -};
> -
> -struct device {
> -    enum device_type type;
> -#ifdef CONFIG_HAS_DEVICE_TREE
> -    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> -#endif
> -};
> -
> -enum device_class
> -{
> -    DEVICE_SERIAL,
> -    DEVICE_IOMMU,
> -    DEVICE_PCI_HOSTBRIDGE,
> -    /* Use for error */
> -    DEVICE_UNKNOWN,
> -};
> -
> -struct device_desc {
> -    /* Device name */
> -    const char *name;
> -    /* Device class */
> -    enum device_class class;
> -    /* List of devices supported by this driver */
> -    const struct dt_device_match *dt_match;
> -    /*
> -     * Device initialization.
> -     *
> -     * -EAGAIN is used to indicate that device probing is deferred.
> -     */
> -    int (*init)(struct dt_device_node *dev, const void *data);
> -};
> -
> -typedef struct device device_t;
> -
> -#define DT_DEVICE_START(name_, namestr_, class_)                    \
> -static const struct device_desc __dev_desc_##name_ __used           \
> -__section(".dev.info") = {                                          \
> -    .name = namestr_,                                               \
> -    .class = class_,                                                \
> -
> -#define DT_DEVICE_END                                               \
> -};
> -
> -#endif /* __ASM_PPC_DEVICE_H__ */
> diff --git a/xen/arch/ppc/include/asm/irq.h b/xen/arch/ppc/include/asm/irq.h
> index 5c37d0cf25..49193fddff 100644
> --- 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

I realize that you were likely following PPC's device.h which also
checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
check this conditional in PPC code at all -- we will always have
HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
this will ever not be the case.

Unless Jan (or someone else) disagrees, I'd rather this conditional be
dropped inside of PPC code.

>  #include <xen/device_tree.h>
> +#endif
>  #include <public/device_tree_defs.h>
>  
>  /* TODO */
> @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu *v)
>      BUG_ON("unimplemented");
>  }
>  
> +#ifdef CONFIG_HAS_DEVICE_TREE

Ditto.

>  static inline int platform_get_irq(const struct dt_device_node *device, int 
> index)
>  {
>      BUG_ON("unimplemented");
>  }
> +#endif
>  
>  #endif /* __ASM_PPC_IRQ_H__ */

Thanks,
Shawn