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

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

On Friday 27 March 2015 11:30 PM, Jaggi, Manish wrote:
> ________________________________
> 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,
As per Ians mail in other thread, 
%s/arm_smmu_xen_device/arch_smm_xen_device/g is ok with you ?
> Regards,
>
> --
> Julien Grall

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