Hi Tomaz,

quoted

+               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.

    Ok, will have to improve the patch description to add this and will
    fix the error value as well. Will also add the remove_device during
     deconfigure as well.

quoted

                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?

   ya wrong, should not be needed.
 

quoted

                        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.

   ok, will add.

Regards,
 Sricharan