Re: [PATCH v6 2/6] iommu/tegra-smmu: Rename struct tegra_smmu_group_soc *soc to *group_soc
From: Thierry Reding <hidden>
Date: 2021-10-07 16:50:58
Also in:
linux-iommu, lkml
On Mon, Sep 13, 2021 at 06:38:54PM -0700, Nicolin Chen wrote:
There are both tegra_smmu_soc and tegra_smmu_group_soc using "soc" for their pointer instances. This patch renames the one of struct tegra_smmu_group_soc from "soc" to "group_soc" to distinguish it. Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
I think the context makes it clear which one this is. The "soc" field in struct tegra_smmu_group clearly refers to the group SoC data, whereas the "soc" field in struct tegra_smmu refers to the SMMU SoC data.
quoted hunk ↗ jump to hunk
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 6ebae635d3aa..a32ed347e25d 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c@@ -22,7 +22,7 @@ struct tegra_smmu_group { struct list_head list; struct tegra_smmu *smmu; - const struct tegra_smmu_group_soc *soc; + const struct tegra_smmu_group_soc *group_soc; struct iommu_group *grp; unsigned int swgroup; };@@ -870,7 +870,7 @@ static struct iommu_device *tegra_smmu_probe_device(struct device *dev) static void tegra_smmu_release_device(struct device *dev) {} static const struct tegra_smmu_group_soc * -tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup) +tegra_smmu_find_group_soc(struct tegra_smmu *smmu, unsigned int swgroup)
This one might be okay to disambiguate, but even here I think this isn't really necessary. It's already clear from the return value what's being returned.
quoted hunk ↗ jump to hunk
{ unsigned int i, j;@@ -896,19 +896,20 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct tegra_smmu *smmu = dev_iommu_priv_get(dev); - const struct tegra_smmu_group_soc *soc; + const struct tegra_smmu_group_soc *group_soc; unsigned int swgroup = fwspec->ids[0]; struct tegra_smmu_group *group; struct iommu_group *grp; /* Find group_soc associating with swgroup */ - soc = tegra_smmu_find_group(smmu, swgroup); + group_soc = tegra_smmu_find_group_soc(smmu, swgroup); mutex_lock(&smmu->lock); /* Find existing iommu_group associating with swgroup or group_soc */ list_for_each_entry(group, &smmu->groups, list) - if ((group->swgroup == swgroup) || (soc && group->soc == soc)) { + if ((group->swgroup == swgroup) || + (group_soc && group->group_soc == group_soc)) { grp = iommu_group_ref_get(group->grp); mutex_unlock(&smmu->lock); return grp;@@ -921,9 +922,9 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) } INIT_LIST_HEAD(&group->list); + group->group_soc = group_soc; group->swgroup = swgroup; group->smmu = smmu; - group->soc = soc;
As another example, it's pretty evident that group->soc refers to the group SoC data rather than the SMMU SoC data. The latter can be obtained from group->smmu->soc, which again is enough context to make it clear what this is. So I don't think this makes things any clearer. It only makes the names more redundant and awkward to write. Thierry
Attachments
- signature.asc [application/pgp-signature] 833 bytes