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

["Jaggi, Manish" <Manish.Jaggi@xxxxxxxxxxxxxxxxxx>]

________________________________
From: Julien Grall <julien.grall@xxxxxxxxxx>
Sent: Friday, March 27, 2015 7:05 PM
To: Jaggi, Manish; Xen Devel; Stefano Stabellini; Ian Campbell; 
Prasun.kapoor@xxxxxxxxxx; 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@xxxxxxxxxx>
> Sent: Friday, March 27, 2015 6:29 PM
> To: Jaggi, Manish; Xen Devel; Stefano Stabellini; Ian Campbell; 
> Prasun.kapoor@xxxxxxxxxx; 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

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