On 08/03/17 18:58, Jean-Philippe Brucker wrote: [...] >> static const struct iommu_ops >> -*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node *bridge_np) >> +*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np) >> { >> const struct iommu_ops *ops; >> struct of_phandle_args iommu_spec; >> + int err; >> >> /* >> * Start by tracing the RID alias down the PCI topology as >> @@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) >> * bus into the system beyond, and which IOMMU it ends up at. >> */ >> iommu_spec.np = NULL; >> - if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map", >> - "iommu-map-mask", &iommu_spec.np, iommu_spec.args)) >> - return NULL; >> + err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map", >> + "iommu-map-mask", &iommu_spec.np, >> + iommu_spec.args); >> + if (err) >> + return ERR_PTR(err); > > This change doesn't work with of_pci_map_rid when the PCI RC isn't behind > an IOMMU: > > map = of_get_property(np, map_name, &map_len); > if (!map) { > if (target) > return -ENODEV; > /* Otherwise, no map implies no translation */ > *id_out = rid; > return 0; > } > > Previously with no iommu-map, we returned -ENODEV but it was discarded by > of_pci_iommu_configure. Now it is propagated and the whole device probing > fails. Instead, maybe of_pci_map_rid should always return 0 if no > iommu-map, and the caller should check if *target is still NULL? Ah yes, Tomasz had found breakages with the "mmu-masters" binding before, and I'd already pushed out a fixup for this one[1], but I forgot that that discussion was all off-list (out of diplomatic concern that the breakage might have been intentional - it wasn't, honest!) Now that rc1 is out I should re-do that branch with v8 of this series plus the fixups folded in, unless Sricharan beats me to it. Thanks for the reminder, Robin. [1]:http://www.linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=0049a34e523506813995c05766f5e2c16d616354 > > Thanks, > Jean-Philippe