[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 3/3] xen/arm: smmu: Renaming struct iommu_domain *domain to, struct iommu_domain *iommu_domain



On Fri, 27 Mar 2015, Jaggi, Manish wrote:
> From: Julien Grall <julien.grall@xxxxxxxxxx>
> Sent: Friday, March 27, 2015 6:34 PM
> To: Jaggi, Manish; Xen Devel; Prasun.kapoor@xxxxxxxxxx; Kumar, Vijaya; Ian 
> Campbell; Stefano Stabellini
> Subject: Re: [PATCH v1 3/3] xen/arm: smmu: Renaming struct iommu_domain 
> *domain to, struct iommu_domain *iommu_domain
> 
> Hi manish,
> 
> On 27/03/15 07:24, Manish Jaggi wrote:
> > It is good for code readability as there are many structures ending with
> > the name domain.
> > Also a code like this one is now easy to understand with the rename
> > old: dev_iommu_domain(dev) = domain;
> > new: dev_iommu_domain(dev) = iommu_domain;
> [manish] Did u see this line
> >
> > Also in current code struct smmu_domain pointer variable name is always
> > smmu_domain.
> > The change is on the same lines
> 
> You are modifying the code from Linux just for your own comprehension.
> And we are trying to not diverge from a specific Linux commit in order
> to easily backport patch.
> 
> [manish] please rethink on nack. There are so many data structures ending in 
> _domain we need to provide proper naming.

We are trying to stay as close as possible to the Linux SMMU driver so
that we can easily import future updates of the driver into Xen. Although
it is true that the current naming is not great, changing the naming
scheme would make future driver updates much harder.

If it helps we could add a couple of comments on top of the structs in
smmu.c to explain the meaning of the fields, like:


/* iommu_domain, not to be confused with a Xen domain */

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.