On Thu, Oct 01, 2020 at 12:46:14PM +0200, Thierry Reding wrote: > > > > - /* > > > > - * This is a bit of a hack. Ideally we'd want to simply return this > > > > - * value. However the IOMMU registration process will attempt to add > > > > - * all devices to the IOMMU when bus_set_iommu() is called. In order > > > > - * not to rely on global variables to track the IOMMU instance, we > > > > - * set it here so that it can be looked up from the .probe_device() > > > > - * callback via the IOMMU device's .drvdata field. > > > > - */ > > > > - mc->smmu = smmu; > > > > > > I don't think this is going to work. I distinctly remember putting this > > > here because we needed access to this before ->probe_device() had been > > > called for any of the devices. > > > > Do you remember which exact part of code needs to access mc->smmu > > before ->probe_device() is called? > > > > What I understood is that IOMMU core didn't allow ERR_PTR(-ENODEV) > > return value from ->probe_device(), previously ->add_device(), to > > carry on when you added this code/driver: > > commit 8918465163171322c77a19d5258a95f56d89d2e4 > > Author: Thierry Reding > > Date: Wed Apr 16 09:24:44 2014 +0200 > > memory: Add NVIDIA Tegra memory controller support > > > > ..until the core had a change one year later: > > commit 38667f18900afe172a4fe44279b132b4140f920f > > Author: Joerg Roedel > > Date: Mon Jun 29 10:16:08 2015 +0200 > > iommu: Ignore -ENODEV errors from add_device call-back > > > > As my commit message of this change states, ->probe_device() will > > be called in from both bus_set_iommu() and really_probe() of each > > device through of_iommu_configure() -- the later one initializes > > an fwspec by polling the iommus property in the IOMMU core, same > > as what we do here in tegra-smmu. If this works, we can probably > > drop the hack here and get rid of tegra_smmu_configure(). > > Looking at this a bit more, I notice that tegra_smmu_configure() does a > lot of what's already done during of_iommu_configure(), so it'd indeed > be nice if we could somehow get rid of that. However, like I said, I do > recall that for DMA/IOMMU we need this prior to ->probe_device(), so it > isn't clear to me if we can do that. > > So I think in order to make progress we need to check that dropping this > does indeed still work when we enable DMA/IOMMU (and the preliminary > patches to pass 1:1 mappings via reserved-memory regions). If so, I > think it should be safe to remove this. I am attaching a patch that works with both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA. Would it be possible for you to give a test? The implementation of getting mc->smmu is using a parent_driver as I was asking you in the other reply. Yet, it could let us give it a try.