Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info

["Jaggi, Manish" <Manish.Jaggi@caviumnetworks.com>]

Regards,
Manish Jaggi

________________________________________
From: Julien Grall <julien.grall@linaro.org>
Sent: Friday, March 27, 2015 6:29 PM
To: Jaggi, Manish; Xen Devel; Stefano Stabellini; Ian Campbell; Prasun.kapoor@cavium.com; Kumar, Vijaya
Subject: Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info

Hi Manish,

On 27/03/15 07:20, Manish Jaggi wrote:
> arm_smmu_xen_device is not an intuitive name for a datastructure which
> represents
> device->archdata.iommu. Rename arm_smmu_xen_device with device_iommu_info

device_iommu_info is not more intuitive... At least arm_smmu_xen_device
shows that it's a specific Xen structure and not coming from the Linux
drivers.

[manish] But that is not a valid reason for a non intuitive naming. It is really hard to keep us readability of the code with arm_smmu_xen_device. It is not clear that it is referring to a device attached to smmu or smmu itself. There is another data structure arm_smmu_device as well. 

Please choose another name I can take it but arm_smmu_xen_device is really confusing

Regards,

> Signed-off-by: Manish Jaggi <manish.jaggi@caviumnetworks.com>
> ---
>  xen/drivers/passthrough/arm/smmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index a7a7da9..ab4f7a4 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -247,12 +247,12 @@ struct arm_smmu_xen_domain {
>   * that would require to move some hackery (dummy iommu_group) in a
> more generic
>   * place.
>   * */
> -struct arm_smmu_xen_device {
> +struct device_iommu_info {
>      struct iommu_domain *domain;
>      struct iommu_group *group;
>  };
>
> -#define dev_archdata(dev) ((struct arm_smmu_xen_device
> *)dev->archdata.iommu)
> +#define dev_archdata(dev) ((struct device_iommu_info
> *)dev->archdata.iommu)
>  #define dev_iommu_domain(dev) (dev_archdata(dev)->domain)
>  #define dev_iommu_group(dev) (dev_archdata(dev)->group)
>
> @@ -2574,7 +2574,7 @@ static int arm_smmu_assign_dev(struct domain *d,
> u8 devfn,
>      xen_domain = domain_hvm_iommu(d)->arch.priv;
>
>      if (!dev->archdata.iommu) {
> -        dev->archdata.iommu = xzalloc(struct arm_smmu_xen_device);
> +        dev->archdata.iommu = xzalloc(struct device_iommu_info);
>          if (!dev->archdata.iommu)
>              return -ENOMEM;
>      }


--
Julien Grall