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.wo...@xilinx.com>
>
> [...]
>
> > @@ -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
