Re: [PATCH V3 0/8] IOMMU probe deferral support

Will Deacon
2016-11-09T16:59:49Z

On Wed, Nov 09, 2016 at 11:54:20AM +0530, Sricharan wrote:
> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >> index 71ce4b6..a1d0b3c 100644
> >> --- a/drivers/iommu/arm-smmu.c
> >> +++ b/drivers/iommu/arm-smmu.c
> >> @@ -1516,8 +1516,10 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
> >> group = smmu->s2crs[idx].group;
> >> }
> >>
> >> - if (group)
> >> + if (group) {
> >> + iommu_group_get_by_id(iommu_group_id(group));
> >> return group;
> >
> >This might as well just be inline, i.e.:
> >
> > return iommu_group_get_by_id(iommu_group_id(group));
> >
> >It's a shame we have to go all round the houses when we have the group
> >right there, but this is probably the most expedient fix. I guess we can
> >extend the API with some sort of iommu_group_get(group) overload in
> >future if we really want to.
> >
>
> ok, i can send this fix separately then. Otherwise, Will was suggesting on the
> other thread that there should probably be a separate API to increment
> the group refcount or get the group from the existing aliasing device.
> As per me adding the api, looks like another option or post the above ?

I think adding a new function to the API is the way to go -- having code
like what you propose above littered around the drivers is hard to read and
pretty error-prone, since it looks like a NOP to people who aren't already
thinking about ref counts.

Will