Re: [RFC v2 1/2] arm,smmu: switch to using iommu_fwspec functions

[Brian Woods <brian.woods@xxxxxxxxxx>]

On Wed, Jul 22, 2020 at 09:47:43AM -0700, Stefano Stabellini wrote:
> On Tue, 21 Jul 2020, Brian Woods wrote:
> > Modify the smmu driver so that it uses the iommu_fwspec helper
> > functions.  This means both ARM IOMMU drivers will both use the
> > iommu_fwspec helper functions.
> > 
> > Signed-off-by: Brian Woods <brian.woods@xxxxxxxxxx>
> 
> [...]
> 
> > @@ -1924,14 +1924,21 @@ static int arm_smmu_add_device(struct device *dev)
> >                     ret = -ENOMEM;
> >                     goto out_put_group;
> >             }
> > +           cfg->fwspec = kzalloc(sizeof(struct iommu_fwspec), GFP_KERNEL);
> > +           if (!cfg->fwspec) {
> > +                   kfree(cfg);
> > +                   ret = -ENOMEM;
> > +                   goto out_put_group;
> > +           }
> > +           iommu_fwspec_init(dev, smmu->dev);
> 
> Normally the fwspec structure is initialized in
> xen/drivers/passthrough/device_tree.c:iommu_add_dt_device. However here
> we are trying to use it with the legacy bindings, that of course don't
> initialize in iommu_add_dt_device because they are different.
> 
> So I imagine this is the reason why we have to initialize iommu_fwspec here
> indepdendently from iommu_add_dt_device.
> 
> However, why are we allocating the struct iommu_fwspec twice?
> 
> We are calling kzalloc, then iommu_fwspec_init is calling xzalloc.
> 
> Similarly, we are storing the pointer to struct iommu_fwspec in
> cfg->fwspec but actually there is already a pointer to struct
> iommu_fwspec in struct device (the one set by dev_iommu_fwspec_set.)
> 
> Do we actually need both?

Sorry for taking so long.

Hrm, I've been looking for why I created two fwspecs and I'm not sure
why... It's pretty late, but later this morning I'll try some things
and try and remove it.

Brian