On Thu, Oct 01, 2020 at 12:23:16PM +0200, Thierry Reding wrote: > > >>>>>> It looks to me like the only reason why you need this new global API is > > > >>>>>> because PCI devices may not have a device tree node with a phandle to > > > >>>>>> the IOMMU. However, SMMU support for PCI will only be enabled if the > > > >>>>>> root complex has an iommus property, right? In that case, can't we > > > >>>>>> simply do something like this: > > > >>>>>> > > > >>>>>> if (dev_is_pci(dev)) > > > >>>>>> np = find_host_bridge(dev)->of_node; > > > >>>>>> else > > > >>>>>> np = dev->of_node; > > I personally am not a fan of adding a path for PCI device either, > > since PCI/IOMMU cores could have taken care of it while the same > > path can't be used for other buses. > > There's already plenty of other drivers that do something similar to > this. Take a look at the arm-smmu driver, for example, which seems to be > doing exactly the same thing to finding the right device tree node to > look at (see dev_get_dev_node() in drivers/iommu/arm-smmu/arm-smmu.c). Hmm..okay..that is quite convincing then... > > If we can't come to an agreement on globalizing mc pointer, would > > it be possible to pass tegra_mc_driver through tegra_smmu_probe() > > so we can continue to use driver_find_device_by_fwnode() as v1? > > > > v1: https://lkml.org/lkml/2020/9/26/68 > > tegra_smmu_probe() already takes a struct tegra_mc *. Did you mean > tegra_smmu_probe_device()? I don't think we can do that because it isn't I was saying to have a global parent_driver pointer: similar to my v1, yet rather than "extern" the tegra_mc_driver, we pass it through egra_smmu_probe() and store it in a static global value so as to call tegra_smmu_get_by_fwnode() in ->probe_device(). Though I agree that creating a global device pointer (mc) might be controversial, yet having a global parent_driver pointer may not be against the rule, considering that it is common in iommu drivers to call driver_find_device_by_fwnode in probe_device(). > known at that point whether MC really is the SMMU. That's in fact the > whole reason why we have to go through this whole dance of iterating > over the iommus entries to find the SMMU. Hmm..I don't quite get the meaning of: "it isn't known at that point whether MC really is the SMMU". Are you saying the stage of bus_set_iommu()? So because at that point either SMMU probe() or MC probe() hasn't finished yet? Thanks!
