| Message ID | 1485188293-20263-2-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Sricharan, On 23.01.2017 17:18, Sricharan R wrote: > From: Robin Murphy <robin.murphy@arm.com> > > In preparation for some upcoming cleverness, rework the control flow in > of_iommu_configure() to minimise duplication and improve the propogation > of errors. It's also as good a time as any to switch over from the > now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic > IOMMU instance interface directly. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > drivers/iommu/of_iommu.c | 83 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 53 insertions(+), 30 deletions(-) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index 0f57ddc..ee49081 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, > } > EXPORT_SYMBOL_GPL(of_get_dma_window); > > +static const struct iommu_ops > +*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec) > +{ > + const struct iommu_ops *ops; > + struct fwnode_handle *fwnode = &iommu_spec->np->fwnode; > + int err; > + > + ops = iommu_get_instance(fwnode); > + if (!ops || !ops->of_xlate) > + return NULL; > + > + err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); > + if (err) > + return ERR_PTR(err); > + > + err = ops->of_xlate(dev, iommu_spec); > + if (err) > + return ERR_PTR(err); > + > + return ops; > +} > + > static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) > { > struct of_phandle_args *iommu_spec = data; > @@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) > } > > 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); > > - ops = of_iommu_get_ops(iommu_spec.np); > - if (!ops || !ops->of_xlate || > - iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) || > - ops->of_xlate(&pdev->dev, &iommu_spec)) > - ops = NULL; > + ops = of_iommu_xlate(&pdev->dev, &iommu_spec); > > of_node_put(iommu_spec.np); > return ops; > } > > -const struct iommu_ops *of_iommu_configure(struct device *dev, > - struct device_node *master_np) > +static const struct iommu_ops > +*of_platform_iommu_init(struct device *dev, struct device_node *np) > { > struct of_phandle_args iommu_spec; > - struct device_node *np; > const struct iommu_ops *ops = NULL; > int idx = 0; > > - if (dev_is_pci(dev)) > - return of_pci_iommu_configure(to_pci_dev(dev), master_np); > - > /* > * We don't currently walk up the tree looking for a parent IOMMU. > * See the `Notes:' section of > * Documentation/devicetree/bindings/iommu/iommu.txt > */ > - while (!of_parse_phandle_with_args(master_np, "iommus", > - "#iommu-cells", idx, > - &iommu_spec)) { > - np = iommu_spec.np; > - ops = of_iommu_get_ops(np); > - > - if (!ops || !ops->of_xlate || > - iommu_fwspec_init(dev, &np->fwnode, ops) || > - ops->of_xlate(dev, &iommu_spec)) > - goto err_put_node; > - > - of_node_put(np); > + while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", > + idx, &iommu_spec)) { > + ops = of_iommu_xlate(dev, &iommu_spec); > + of_node_put(iommu_spec.np); > idx++; > + if (IS_ERR_OR_NULL(ops)) > + break; > } > > return ops; > +} > + > +const struct iommu_ops *of_iommu_configure(struct device *dev, > + struct device_node *master_np) > +{ > + const struct iommu_ops *ops; > + > + if (!master_np) > + return NULL; > + > + if (dev_is_pci(dev)) > + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); I gave the whole patch set a try on ThunderX. really_probe() is failing on dma_configure()->of_pci_iommu_init() for each PCI device. of_pci_iommu_init() tries to setup firmware stuff via "iommu-map" but ThunderX is using legacy "mmu-masters" binding. We need to take care of that case too. > + else > + ops = of_platform_iommu_init(dev, master_np); > > -err_put_node: > - of_node_put(np); > - return NULL; > + return IS_ERR(ops) ? NULL : ops; > } > > static int __init of_iommu_init(void) > Thanks, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, On 25/01/17 17:17, Tomasz Nowicki wrote: > Hi Sricharan, > > On 23.01.2017 17:18, Sricharan R wrote: >> From: Robin Murphy <robin.murphy@arm.com> >> >> In preparation for some upcoming cleverness, rework the control flow in >> of_iommu_configure() to minimise duplication and improve the propogation >> of errors. It's also as good a time as any to switch over from the >> now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic >> IOMMU instance interface directly. >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> drivers/iommu/of_iommu.c | 83 >> +++++++++++++++++++++++++++++++----------------- >> 1 file changed, 53 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >> index 0f57ddc..ee49081 100644 >> --- a/drivers/iommu/of_iommu.c >> +++ b/drivers/iommu/of_iommu.c >> @@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const >> char *prefix, int index, >> } >> EXPORT_SYMBOL_GPL(of_get_dma_window); >> >> +static const struct iommu_ops >> +*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec) >> +{ >> + const struct iommu_ops *ops; >> + struct fwnode_handle *fwnode = &iommu_spec->np->fwnode; >> + int err; >> + >> + ops = iommu_get_instance(fwnode); >> + if (!ops || !ops->of_xlate) >> + return NULL; >> + >> + err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); >> + if (err) >> + return ERR_PTR(err); >> + >> + err = ops->of_xlate(dev, iommu_spec); >> + if (err) >> + return ERR_PTR(err); >> + >> + return ops; >> +} >> + >> static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) >> { >> struct of_phandle_args *iommu_spec = data; >> @@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, >> u16 alias, void *data) >> } >> >> 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); >> >> - ops = of_iommu_get_ops(iommu_spec.np); >> - if (!ops || !ops->of_xlate || >> - iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) || >> - ops->of_xlate(&pdev->dev, &iommu_spec)) >> - ops = NULL; >> + ops = of_iommu_xlate(&pdev->dev, &iommu_spec); >> >> of_node_put(iommu_spec.np); >> return ops; >> } >> >> -const struct iommu_ops *of_iommu_configure(struct device *dev, >> - struct device_node *master_np) >> +static const struct iommu_ops >> +*of_platform_iommu_init(struct device *dev, struct device_node *np) >> { >> struct of_phandle_args iommu_spec; >> - struct device_node *np; >> const struct iommu_ops *ops = NULL; >> int idx = 0; >> >> - if (dev_is_pci(dev)) >> - return of_pci_iommu_configure(to_pci_dev(dev), master_np); >> - >> /* >> * We don't currently walk up the tree looking for a parent IOMMU. >> * See the `Notes:' section of >> * Documentation/devicetree/bindings/iommu/iommu.txt >> */ >> - while (!of_parse_phandle_with_args(master_np, "iommus", >> - "#iommu-cells", idx, >> - &iommu_spec)) { >> - np = iommu_spec.np; >> - ops = of_iommu_get_ops(np); >> - >> - if (!ops || !ops->of_xlate || >> - iommu_fwspec_init(dev, &np->fwnode, ops) || >> - ops->of_xlate(dev, &iommu_spec)) >> - goto err_put_node; >> - >> - of_node_put(np); >> + while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", >> + idx, &iommu_spec)) { >> + ops = of_iommu_xlate(dev, &iommu_spec); >> + of_node_put(iommu_spec.np); >> idx++; >> + if (IS_ERR_OR_NULL(ops)) >> + break; >> } >> >> return ops; >> +} >> + >> +const struct iommu_ops *of_iommu_configure(struct device *dev, >> + struct device_node *master_np) >> +{ >> + const struct iommu_ops *ops; >> + >> + if (!master_np) >> + return NULL; >> + >> + if (dev_is_pci(dev)) >> + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); > > I gave the whole patch set a try on ThunderX. really_probe() is failing > on dma_configure()->of_pci_iommu_init() for each PCI device. When you say "failing", do you mean cleanly, or with a crash? I've managed to hit __of_match_node() dereferencing NULL from of_iommu_xlate() in a horribly complicated chain of events, which I'm trying to figure out now, and I wonder if the two might be related. > of_pci_iommu_init() tries to setup firmware stuff via "iommu-map" but > ThunderX is using legacy "mmu-masters" binding. We need to take care of > that case too. That is by design. The "mmu-masters" binding is ARM-SMMU-specific and will continue to provide the same level of DMA ops support it always has (for which there really are practical implementation reasons, it's not just an arbitrary "encourage people to move to the generic bindings" decision). Robin. >> + else >> + ops = of_platform_iommu_init(dev, master_np); >> >> -err_put_node: >> - of_node_put(np); >> - return NULL; >> + return IS_ERR(ops) ? NULL : ops; >> } >> >> static int __init of_iommu_init(void) >> > > Thanks, > Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Robin, On 25.01.2017 18:35, Robin Murphy wrote: > Hi Tomasz, > > On 25/01/17 17:17, Tomasz Nowicki wrote: >> Hi Sricharan, >> >> On 23.01.2017 17:18, Sricharan R wrote: >>> From: Robin Murphy <robin.murphy@arm.com> >>> >>> In preparation for some upcoming cleverness, rework the control flow in >>> of_iommu_configure() to minimise duplication and improve the propogation >>> of errors. It's also as good a time as any to switch over from the >>> now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic >>> IOMMU instance interface directly. >>> >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>> --- >>> drivers/iommu/of_iommu.c | 83 >>> +++++++++++++++++++++++++++++++----------------- >>> 1 file changed, 53 insertions(+), 30 deletions(-) >>> >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >>> index 0f57ddc..ee49081 100644 >>> --- a/drivers/iommu/of_iommu.c >>> +++ b/drivers/iommu/of_iommu.c >>> @@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const >>> char *prefix, int index, >>> } >>> EXPORT_SYMBOL_GPL(of_get_dma_window); >>> >>> +static const struct iommu_ops >>> +*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec) >>> +{ >>> + const struct iommu_ops *ops; >>> + struct fwnode_handle *fwnode = &iommu_spec->np->fwnode; >>> + int err; >>> + >>> + ops = iommu_get_instance(fwnode); >>> + if (!ops || !ops->of_xlate) >>> + return NULL; >>> + >>> + err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); >>> + if (err) >>> + return ERR_PTR(err); >>> + >>> + err = ops->of_xlate(dev, iommu_spec); >>> + if (err) >>> + return ERR_PTR(err); >>> + >>> + return ops; >>> +} >>> + >>> static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) >>> { >>> struct of_phandle_args *iommu_spec = data; >>> @@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, >>> u16 alias, void *data) >>> } >>> >>> 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); >>> >>> - ops = of_iommu_get_ops(iommu_spec.np); >>> - if (!ops || !ops->of_xlate || >>> - iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) || >>> - ops->of_xlate(&pdev->dev, &iommu_spec)) >>> - ops = NULL; >>> + ops = of_iommu_xlate(&pdev->dev, &iommu_spec); >>> >>> of_node_put(iommu_spec.np); >>> return ops; >>> } >>> >>> -const struct iommu_ops *of_iommu_configure(struct device *dev, >>> - struct device_node *master_np) >>> +static const struct iommu_ops >>> +*of_platform_iommu_init(struct device *dev, struct device_node *np) >>> { >>> struct of_phandle_args iommu_spec; >>> - struct device_node *np; >>> const struct iommu_ops *ops = NULL; >>> int idx = 0; >>> >>> - if (dev_is_pci(dev)) >>> - return of_pci_iommu_configure(to_pci_dev(dev), master_np); >>> - >>> /* >>> * We don't currently walk up the tree looking for a parent IOMMU. >>> * See the `Notes:' section of >>> * Documentation/devicetree/bindings/iommu/iommu.txt >>> */ >>> - while (!of_parse_phandle_with_args(master_np, "iommus", >>> - "#iommu-cells", idx, >>> - &iommu_spec)) { >>> - np = iommu_spec.np; >>> - ops = of_iommu_get_ops(np); >>> - >>> - if (!ops || !ops->of_xlate || >>> - iommu_fwspec_init(dev, &np->fwnode, ops) || >>> - ops->of_xlate(dev, &iommu_spec)) >>> - goto err_put_node; >>> - >>> - of_node_put(np); >>> + while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", >>> + idx, &iommu_spec)) { >>> + ops = of_iommu_xlate(dev, &iommu_spec); >>> + of_node_put(iommu_spec.np); >>> idx++; >>> + if (IS_ERR_OR_NULL(ops)) >>> + break; >>> } >>> >>> return ops; >>> +} >>> + >>> +const struct iommu_ops *of_iommu_configure(struct device *dev, >>> + struct device_node *master_np) >>> +{ >>> + const struct iommu_ops *ops; >>> + >>> + if (!master_np) >>> + return NULL; >>> + >>> + if (dev_is_pci(dev)) >>> + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); >> I gave the whole patch set a try on ThunderX. really_probe() is failing >> on dma_configure()->of_pci_iommu_init() for each PCI device. > When you say "failing", do you mean cleanly, or with a crash? I've > managed to hit __of_match_node() dereferencing NULL from > of_iommu_xlate() in a horribly complicated chain of events, which I'm > trying to figure out now, and I wonder if the two might be related. > Cleanly, of_pci_iommu_init()->of_pci_map_rid() can't find "iommu-map" property for my case. Probably not related to your crash. Thanks, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Robin, [..] >>> +const struct iommu_ops *of_iommu_configure(struct device *dev, >>> + struct device_node *master_np) >>> +{ >>> + const struct iommu_ops *ops; >>> + >>> + if (!master_np) >>> + return NULL; >>> + >>> + if (dev_is_pci(dev)) >>> + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); >> >> I gave the whole patch set a try on ThunderX. really_probe() is failing >> on dma_configure()->of_pci_iommu_init() for each PCI device. > >When you say "failing", do you mean cleanly, or with a crash? I've >managed to hit __of_match_node() dereferencing NULL from >of_iommu_xlate() in a horribly complicated chain of events, which I'm >trying to figure out now, and I wonder if the two might be related. Sorry that there is crash still. __of_match_node seems to checking for NULL arguments , feels like some invalid pointer was passed in. Is there any particular sequence to try for this ? Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/01/17 18:00, Sricharan wrote: > Hi Robin, > > [..] > >>>> +const struct iommu_ops *of_iommu_configure(struct device *dev, >>>> + struct device_node *master_np) >>>> +{ >>>> + const struct iommu_ops *ops; >>>> + >>>> + if (!master_np) >>>> + return NULL; >>>> + >>>> + if (dev_is_pci(dev)) >>>> + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); >>> >>> I gave the whole patch set a try on ThunderX. really_probe() is failing >>> on dma_configure()->of_pci_iommu_init() for each PCI device. >> >> When you say "failing", do you mean cleanly, or with a crash? I've >> managed to hit __of_match_node() dereferencing NULL from >> of_iommu_xlate() in a horribly complicated chain of events, which I'm >> trying to figure out now, and I wonder if the two might be related. > > Sorry that there is crash still. __of_match_node seems to checking > for NULL arguments , feels like some invalid pointer was passed in. > Is there any particular sequence to try for this ? Ah, I did figure it out - it wasn't actually a NULL dereference, but an unmapped address. Turns out __iommu_of_table is in initdata, so any driver probing after init, connected to an unprobed IOMMU (in this case disabled in DT), trips over trying to match the now-freed table. I'm working on the fix - technically the bug's in my patch (#2) anyway ;) Robin. > > Regards, > Sricharan > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Robin, >> [..] >> >>>>> +const struct iommu_ops *of_iommu_configure(struct device *dev, >>>>> + struct device_node *master_np) >>>>> +{ >>>>> + const struct iommu_ops *ops; >>>>> + >>>>> + if (!master_np) >>>>> + return NULL; >>>>> + >>>>> + if (dev_is_pci(dev)) >>>>> + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); >>>> >>>> I gave the whole patch set a try on ThunderX. really_probe() is failing >>>> on dma_configure()->of_pci_iommu_init() for each PCI device. >>> >>> When you say "failing", do you mean cleanly, or with a crash? I've >>> managed to hit __of_match_node() dereferencing NULL from >>> of_iommu_xlate() in a horribly complicated chain of events, which I'm >>> trying to figure out now, and I wonder if the two might be related. >> >> Sorry that there is crash still. __of_match_node seems to checking >> for NULL arguments , feels like some invalid pointer was passed in. >> Is there any particular sequence to try for this ? > >Ah, I did figure it out - it wasn't actually a NULL dereference, but an >unmapped address. Turns out __iommu_of_table is in initdata, so any >driver probing after init, connected to an unprobed IOMMU (in this case >disabled in DT), trips over trying to match the now-freed table. I'm >working on the fix - technically the bug's in my patch (#2) anyway ;) > Ok, thanks for bringing this out. There is also an issue that Sinan has mentioned while testing the ACPI hotplug path, probably its related to the above, not sure. I will try to check more on that in the meanwhile. Then, taking your fix and fixing the hotplug case i will do one more repost. Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 0f57ddc..ee49081 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, } EXPORT_SYMBOL_GPL(of_get_dma_window); +static const struct iommu_ops +*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec) +{ + const struct iommu_ops *ops; + struct fwnode_handle *fwnode = &iommu_spec->np->fwnode; + int err; + + ops = iommu_get_instance(fwnode); + if (!ops || !ops->of_xlate) + return NULL; + + err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); + if (err) + return ERR_PTR(err); + + err = ops->of_xlate(dev, iommu_spec); + if (err) + return ERR_PTR(err); + + return ops; +} + static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) { struct of_phandle_args *iommu_spec = data; @@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data) } 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); - ops = of_iommu_get_ops(iommu_spec.np); - if (!ops || !ops->of_xlate || - iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) || - ops->of_xlate(&pdev->dev, &iommu_spec)) - ops = NULL; + ops = of_iommu_xlate(&pdev->dev, &iommu_spec); of_node_put(iommu_spec.np); return ops; } -const struct iommu_ops *of_iommu_configure(struct device *dev, - struct device_node *master_np) +static const struct iommu_ops +*of_platform_iommu_init(struct device *dev, struct device_node *np) { struct of_phandle_args iommu_spec; - struct device_node *np; const struct iommu_ops *ops = NULL; int idx = 0; - if (dev_is_pci(dev)) - return of_pci_iommu_configure(to_pci_dev(dev), master_np); - /* * We don't currently walk up the tree looking for a parent IOMMU. * See the `Notes:' section of * Documentation/devicetree/bindings/iommu/iommu.txt */ - while (!of_parse_phandle_with_args(master_np, "iommus", - "#iommu-cells", idx, - &iommu_spec)) { - np = iommu_spec.np; - ops = of_iommu_get_ops(np); - - if (!ops || !ops->of_xlate || - iommu_fwspec_init(dev, &np->fwnode, ops) || - ops->of_xlate(dev, &iommu_spec)) - goto err_put_node; - - of_node_put(np); + while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", + idx, &iommu_spec)) { + ops = of_iommu_xlate(dev, &iommu_spec); + of_node_put(iommu_spec.np); idx++; + if (IS_ERR_OR_NULL(ops)) + break; } return ops; +} + +const struct iommu_ops *of_iommu_configure(struct device *dev, + struct device_node *master_np) +{ + const struct iommu_ops *ops; + + if (!master_np) + return NULL; + + if (dev_is_pci(dev)) + ops = of_pci_iommu_init(to_pci_dev(dev), master_np); + else + ops = of_platform_iommu_init(dev, master_np); -err_put_node: - of_node_put(np); - return NULL; + return IS_ERR(ops) ? NULL : ops; } static int __init of_iommu_init(void)