Re: [Xen-devel] [PATCH v2 06/12] xen/arm: Introduce a generic way to describe device

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 16 Jan 2015, Julien Grall wrote:
> Currently, Xen is supporting PCI and Platform device (based on Device Tree).
> 
> While Xen only supports Platform device on ARM, Xen will gain support of
> PCI soon.
> 
> Some drivers, such as IOMMU drivers, may handle PCI and platform device in
> the same way. Only few lines of code differs.
> 
> Rather than requesting to provide 2 set of functions (one for PCI and
> one for platform device), introduce a generic structure "device" which
> is embedded in each specialized device.
> 
> As x86 only supports PCI, introduce a new type device_t which will be an
> alias to pci_dev for this architecture. It will avoid to add a new field
> for this place.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

It looks OK to me


>     Changes in v2:
>         - As x86 will only support PCI, only introduce the generic
>         device on ARM
>         - Introduce a typedef device_t to be used in common code
>         - Drop the PCI code for ARM for now. It will be reintroduced
>         when PCI support will be added
>         - s#asm/device.h#xen/device.h# is not anymore needed
> ---
>  xen/common/device.c           | 21 +++++++++++++++++++++
>  xen/common/device_tree.c      |  3 +++
>  xen/include/asm-arm/device.h  | 26 ++++++++++++++++++++++++++
>  xen/include/asm-x86/device.h  | 25 +++++++++++++++++++++++++
>  xen/include/xen/device_tree.h | 13 +++++++++++++
>  xen/include/xen/iommu.h       |  1 +
>  xen/include/xen/pci.h         |  1 +
>  7 files changed, 90 insertions(+)
>  create mode 100644 xen/common/device.c
>  create mode 100644 xen/include/asm-x86/device.h
> 
> diff --git a/xen/common/device.c b/xen/common/device.c
> new file mode 100644
> index 0000000..3450f20
> --- /dev/null
> +++ b/xen/common/device.c
> @@ -0,0 +1,21 @@
> +#include <xen/types.h>
> +#include <xen/device.h>
> +
> +void device_initialize(struct device *dev, enum device_type type)
> +{
> +    dev->type = type;
> +
> +#ifdef HAS_DEVICE_TREE
> +    if ( type == DEV_DT )
> +        dev->of_node = dev_to_dt(dev);
> +#endif
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 34a1b9e..d1c716f 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1454,6 +1454,9 @@ static unsigned long __init unflatten_dt_node(const 
> void *fdt,
>              ((char *)pp->value)[sz - 1] = 0;
>              dt_dprintk("fixed up name for %s -> %s\n", pathp,
>                         (char *)pp->value);
> +            /* Generic device initialization */
> +            np->dev.type = DEV_DT;
> +            np->dev.of_node = np;
>          }
>      }
>      if ( allnextpp )
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index 72a9028..2fe4280 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -2,8 +2,34 @@
>  #define __ASM_ARM_DEVICE_H
>  
>  #include <xen/init.h>
> +
> +enum device_type
> +{
> +    DEV_DT,
> +};
> +
> +struct dev_archdata {
> +    void *iommu;    /* IOMMU private data */
> +};
> +
> +/* struct device - The basic device structure */
> +struct device
> +{
> +    enum device_type type;
> +#ifdef HAS_DEVICE_TREE
> +    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> +#endif
> +    struct dev_archdata archdata;
> +};
> +
> +typedef struct device device_t;
> +
>  #include <xen/device_tree.h>
>  
> +/* TODO: Correctly implement dev_is_pci when PCI will be supported on ARM */
> +#define dev_is_pci(dev) ((void)(dev), 0)
> +#define dev_is_dt(dev)  ((dev->type == DEV_DT)
> +
>  enum device_match
>  {
>      DEVICE_SERIAL,
> diff --git a/xen/include/asm-x86/device.h b/xen/include/asm-x86/device.h
> new file mode 100644
> index 0000000..a016112
> --- /dev/null
> +++ b/xen/include/asm-x86/device.h
> @@ -0,0 +1,25 @@
> +#ifndef __ASM_X86_DEVICE_H
> +#define __ASM_X86_DEVICE_H
> +
> +#include <xen/pci.h>
> +
> +/*
> + * x86 is only supported PCI. Therefore it's possible to directly use
> + * pci_dev to avoid adding new field.
> + */
> +
> +typedef struct pci_dev device_t;
> +
> +#define dev_is_pci(dev) ((void)(dev), 1)
> +#define pci_to_dev(pci) (pci)
> +
> +#endif /* __ASM_X86_DEVICE_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 6502369..c8a0375 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -11,7 +11,9 @@
>  #define __XEN_DEVICE_TREE_H__
>  
>  #include <asm/byteorder.h>
> +#include <asm/device.h>
>  #include <public/xen.h>
> +#include <xen/kernel.h>
>  #include <xen/init.h>
>  #include <xen/string.h>
>  #include <xen/types.h>
> @@ -80,8 +82,19 @@ struct dt_device_node {
>      /* IOMMU specific fields */
>      bool is_protected;
>      struct list_head domain_list;
> +
> +    struct device dev;
>  };
>  
> +#define dt_to_dev(dt_node)  (&(dt_node)->dev)
> +
> +static inline struct dt_device_node *dev_to_dt(struct device *dev)
> +{
> +    ASSERT(dev->type == DEV_DT);
> +
> +    return container_of(dev, struct dt_device_node, dev);
> +}
> +
>  #define MAX_PHANDLE_ARGS 16
>  struct dt_phandle_args {
>      struct dt_device_node *np;
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 8eb764a..ecb2627 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -25,6 +25,7 @@
>  #include <xen/pci.h>
>  #include <public/hvm/ioreq.h>
>  #include <public/domctl.h>
> +#include <asm/device.h>
>  #include <asm/iommu.h>
>  
>  extern bool_t iommu_enable, iommu_enabled;
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 5f295f3..3988ee68 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -13,6 +13,7 @@
>  #include <xen/irq.h>
>  #include <xen/pci_regs.h>
>  #include <xen/pfn.h>
> +#include <asm/device.h>
>  #include <asm/pci.h>
>  
>  /*
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel