All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
@ 2015-03-27  7:20 Manish Jaggi
  2015-03-27 12:59 ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Manish Jaggi @ 2015-03-27  7:20 UTC (permalink / raw)
  To: Xen Devel, Stefano Stabellini, Julien Grall, Ian Campbell,
	Prasun.kapoor, Kumar, Vijaya

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

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;
      }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
  2015-03-27  7:20 [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info Manish Jaggi
@ 2015-03-27 12:59 ` Julien Grall
  2015-03-27 13:21   ` Jaggi, Manish
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2015-03-27 12:59 UTC (permalink / raw)
  To: Manish Jaggi, Xen Devel, Stefano Stabellini, Ian Campbell,
	Prasun.kapoor, Kumar, Vijaya

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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
  2015-03-27 12:59 ` Julien Grall
@ 2015-03-27 13:21   ` Jaggi, Manish
  2015-03-27 13:35     ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Jaggi, Manish @ 2015-03-27 13:21 UTC (permalink / raw)
  To: Julien Grall, Xen Devel, Stefano Stabellini, Ian Campbell,
	Prasun.kapoor, Kumar, Vijaya



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
  2015-03-27 13:21   ` Jaggi, Manish
@ 2015-03-27 13:35     ` Julien Grall
  2015-03-27 18:00       ` Jaggi, Manish
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2015-03-27 13:35 UTC (permalink / raw)
  To: Jaggi, Manish, Xen Devel, Stefano Stabellini, Ian Campbell,
	Prasun.kapoor, Kumar, Vijaya

On 27/03/15 13:21, Jaggi, Manish wrote:
> 
> 
> Regards,
> Manish Jaggi

Could you please try to configure you email client correctly? It's
rather confusing the "regards, Manish Jaggi" at the beginning of the mail.

> ________________________________________
> 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. 

Did you read the comment explaining the structure arm_smmu_xen_device?
It's just above the definition.

"arm_smmu" is the prefix for any structure within this file.
"xen" means it's a structure added for Xen.
"device" means it's data stored for a device.

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

I won't choose a name myself for a name that I think valid...

If you really want to change the name, you have to put at least
arm_smmu_xen_ in the name.

> Regards,

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
  2015-03-27 13:35     ` Julien Grall
@ 2015-03-27 18:00       ` Jaggi, Manish
  2015-04-06 10:45         ` Manish Jaggi
  0 siblings, 1 reply; 6+ messages in thread
From: Jaggi, Manish @ 2015-03-27 18:00 UTC (permalink / raw)
  To: Julien Grall, Xen Devel, Stefano Stabellini, Ian Campbell,
	Prasun.kapoor, Kumar, Vijaya

________________________________
From: Julien Grall <julien.grall@linaro.org>
Sent: Friday, March 27, 2015 7:05 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

On 27/03/15 13:21, Jaggi, Manish wrote:
>
>
> Regards,
> Manish Jaggi

Could you please try to configure you email client correctly? It's
rather confusing the "regards, Manish Jaggi" at the beginning of the mail.

[manish] Fixed. Thanks for pointing out

> ________________________________________
> 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.

Did you read the comment explaining the structure arm_smmu_xen_device?
It's just above the definition.

"arm_smmu" is the prefix for any structure within this file.
"xen" means it's a structure added for Xen.
"device" means it's data stored for a device.

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

I won't choose a name myself for a name that I think valid...

If you really want to change the name, you have to put at least
arm_smmu_xen_ in the name.

[manish] what about device_archdata_priv, this is denoting what it is.

> Regards,

Regards,

--
Julien Grall

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info
  2015-03-27 18:00       ` Jaggi, Manish
@ 2015-04-06 10:45         ` Manish Jaggi
  0 siblings, 0 replies; 6+ messages in thread
From: Manish Jaggi @ 2015-04-06 10:45 UTC (permalink / raw)
  To: Jaggi, Manish, Julien Grall, Xen Devel, Stefano Stabellini,
	Ian Campbell, Prasun.kapoor, Kumar, Vijaya


On Friday 27 March 2015 11:30 PM, Jaggi, Manish wrote:
> ________________________________
> From: Julien Grall <julien.grall@linaro.org>
> Sent: Friday, March 27, 2015 7:05 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
>
> On 27/03/15 13:21, Jaggi, Manish wrote:
>>
>> Regards,
>> Manish Jaggi
> Could you please try to configure you email client correctly? It's
> rather confusing the "regards, Manish Jaggi" at the beginning of the mail.
>
> [manish] Fixed. Thanks for pointing out
>
>> ________________________________________
>> 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.
> Did you read the comment explaining the structure arm_smmu_xen_device?
> It's just above the definition.
>
> "arm_smmu" is the prefix for any structure within this file.
> "xen" means it's a structure added for Xen.
> "device" means it's data stored for a device.
>
>> Please choose another name I can take it but arm_smmu_xen_device is really confusing
> I won't choose a name myself for a name that I think valid...
>
> If you really want to change the name, you have to put at least
> arm_smmu_xen_ in the name.
>
> [manish] what about device_archdata_priv, this is denoting what it is.
>
>> Regards,
As per Ians mail in other thread, 
%s/arm_smmu_xen_device/arch_smm_xen_device/g is ok with you ?
> Regards,
>
> --
> Julien Grall

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-04-06 10:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27  7:20 [PATCH v1 1/3] xen/arm: smmu: Rename arm_smmu_xen_device with, device_iommu_info Manish Jaggi
2015-03-27 12:59 ` Julien Grall
2015-03-27 13:21   ` Jaggi, Manish
2015-03-27 13:35     ` Julien Grall
2015-03-27 18:00       ` Jaggi, Manish
2015-04-06 10:45         ` Manish Jaggi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.