From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jaggi, Manish" <Manish.Jaggi@caviumnetworks.com>
Subject: Re: [PATCH v1 1/3] xen/arm: smmu: Rename
 arm_smmu_xen_device with, device_iommu_info
Date: Fri, 27 Mar 2015 13:21:38 +0000
Message-ID: <1427462515927.43521@caviumnetworks.com>
References: <551504D8.1030102@caviumnetworks.com>,
	<5515543F.5090001@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5515543F.5090001@linaro.org>
Content-Language: en-US
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@linaro.org>, Xen Devel <xen-devel@lists.xen.org>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, "Prasun.kapoor@cavium.com" <Prasun.kapoor@cavium.com>, "Kumar, Vijaya" <Vijaya.Kumar@caviumnetworks.com>
List-Id: xen-devel@lists.xenproject.org



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