[PATCH 7/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error
From: tfiga@chromium.org (Tomasz Figa)
Date: 2016-08-12 07:46:06
Also in:
linux-iommu
Hi, On Tue, Aug 9, 2016 at 7:49 AM, Sricharan R [off-list ref] wrote:
+ + if (ops->add_device) + ops = ops->add_device(dev) ? ops : NULL;
Patch description fails to mention anything about this change. Also it looks slightly incorrect to lose the error condition here. I think we should at least print some error message here and tell the user that we are falling back to non-IOMMU setup.
quoted hunk ↗ jump to hunk
of_node_put(np); idx++;@@ -200,7 +213,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, err_put_node: of_node_put(np); - return NULL; + return ops; } void __init of_iommu_init(void)@@ -211,7 +224,7 @@ void __init of_iommu_init(void) for_each_matching_node_and_match(np, matches, &match) { const of_iommu_init_fn init_fn = match->data; - if (init_fn(np)) + if (init_fn && init_fn(np))
When is it possible to have NULL init_fn?
quoted hunk ↗ jump to hunk
pr_err("Failed to initialise IOMMU %s\n", of_node_full_name(np)); }diff --git a/drivers/of/device.c b/drivers/of/device.c index e1fad50..92e02dc 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c@@ -149,6 +149,8 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) coherent ? " " : " not "); iommu = of_iommu_configure(dev, np); + if (IS_ERR(iommu)) + return PTR_ERR(iommu);
nit: Would be good to add a blank line here for improved readability.
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");Best regards, Tomasz