Re: [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs

[Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]

Hi Thierry,

Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Tue, 19 Nov 2013 11:25:07 +0100:

> From earlier discussions I thought the goal was to actually defer this
> until all nodes referred to by the iommus property were actually
> registered. The above only checks that the phandles can be resolved to
> valid struct device_node:s. That doesn't mean that an actual IOMMU has
> been registered for it, only that the devices have been created.

Currently "bus->iommu_ops" is set at the end of tegra_smmu_probe(). So
if "bus->iommu_ops" is set, it means that an iommu instance is
populated at that time.

> If you really only rely on dev->bus->iommu_ops to be present, then there
> is no need to go through the loop in the first place, since you have
> access to it immediately through the struct device that's passed into
> the function.

As mentioned in the above, "bus->iommu_ops" is set when an iommu
instance is populated. "iommus=" is used only to defer a device probe
until an iommu instance shows up although multiple IOMMUs are not
supported yet correctly as you pointed out.

> Furthermore, relying on dev->bus->iommu_ops will prevent multiple IOMMUs
> from being used at all, since only one IOMMU can register iommu_ops with
> the bus, right? So I think what we really need here is a way to resolve
> the IOMMU using a phandle and return the associated struct
> iommu_ops.

Multiple IOMMU support doesn't work right now. It needs to be
discussed a bit more.

> I also have some trouble understanding how the current IOMMU framework
> is supposed to work together with multiple IOMMUs for one device. The
> .add_device() callback seems to be missing crucial information to help
> decide whether the device to be added is actually one that it covers.
> Also with an of_iommu_attach() function, doesn't that become more or
> less redundant?

I understand what you meant. In the current tegra SMMU implementation,
iommu_ops is set when an iommu is populated so that we cannot use
iommu_ops before that time. That's why of_iommu_attach() was
introduced here. I couldn't set iommu_ops at drvier init yet because
we support SMMU and GART with a single image and the current IOMMU
framework assume that there's one IOMMU on the bus. If we set
iommu_ops at driver init, the latter one would overwrite it.