On 11/21/2013 06:15 AM, Grant Likely wrote: > On Tue, 19 Nov 2013 11:33:06 +0200, Hiroshi Doyu <hdoyu@xxxxxxxxxx> wrote: >> IOMMU devices on the bus need to be poplulated first, then iommu >> master devices are done later. >> >> With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify >> whether a device can be an iommu msater or not. If a device can, we'll >> defer to populate that device till an iommu device is populated. Once >> an iommu device is populated, "dev->bus->iommu_ops" is set in the >> bus. Then, those defered iommu master devices are populated and >> configured for IOMMU with help of the already populated iommu device >> via iommu_ops->add_device(). Multiple IOMMUs can be listed on this >> "iommus" binding so that a device can have multiple IOMMUs attached. >> >> Signed-off-by: Hiroshi Doyu <hdoyu@xxxxxxxxxx> >> --- >> v5: >> Use "iommus=" binding instread of arm,smmu's "#stream-id-cells". >> >> v4: >> This is newly added, and the successor of the following RFC: >> [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach() >> http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html >> --- >> drivers/base/dd.c | 5 +++++ >> drivers/iommu/of_iommu.c | 22 ++++++++++++++++++++++ >> include/linux/of_iommu.h | 7 +++++++ >> 3 files changed, 34 insertions(+) >> >> diff --git a/drivers/base/dd.c b/drivers/base/dd.c >> index 35fa368..6e892d4 100644 >> --- a/drivers/base/dd.c >> +++ b/drivers/base/dd.c >> @@ -25,6 +25,7 @@ >> #include <linux/async.h> >> #include <linux/pm_runtime.h> >> #include <linux/pinctrl/devinfo.h> >> +#include <linux/of_iommu.h> >> >> #include "base.h" >> #include "power/power.h" >> @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv) >> >> dev->driver = drv; >> >> + ret = of_iommu_attach(dev); >> + if (ret) >> + goto probe_failed; >> + >> /* If using pinctrl, bind pins now before probing */ >> ret = pinctrl_bind_pins(dev); >> if (ret) > > I'm very concerned about this approach. Hooking into the core probe > path for things like this is not going to scale well. I'm not thrilled > with the pinctrl hook being here either, but that is already merged. :-( > Also, hooking in here is going affect every single device device driver > probe path, and a large number of them are never, ever, going to have > iommu interactions. > > There needs to be a less invasive way of doing what you want. I still > feel like the individual device drivers themselves need to be aware that > they might be hooking through an IOMMU. Or, if they are hooking through > a bus like PCIe, then have the PCIe bus defer creating child devices > until the IOMMU is configured and in place. I general though, couldn't any MMIO on-SoC device potentially be affected by an IOMMU? The point of putting the call to of_iommu_attach() here rather than inside individual driver's probe() is to avoid requiring "every" driver having to paste more boiler-plate into probe(). Perhaps what we need is a flag in struct device to indicate that the driver/device is MMIO, and hence potentially affected by an IOMMU. would the following work better for you? + if (drv->is_mmio) { // let's bikeshed on the field name:-) + ret = of_iommu_attach(dev); + if (ret) + goto probe_failed; + } I've often thought that struct device_driver (or similar) should declare the set of resources that a device needs (e.g. a list of GPIO names, regulator names, etc.), so that the driver core can parse all these standard properties from DT/... before calling the custom probe() function for all the non-standard stuff. This "is_mmio" flag would fit into that model well. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
