Thread (15 messages) 15 messages, 4 authors, 2015-01-06

Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration

From: Rob Herring <hidden>
Date: 2015-01-02 20:46:15
Also in: linux-arm-kernel, linux-pci, lkml

On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri [off-list ref] wrote:
Rob,

See my response below. Arnd and Will, please review this as well.

On 12/26/2014 02:33 PM, Rob Herring wrote:
quoted
On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri[off-list ref]
wrote:
quoted
Add of_pci_dma_configure() to allow updating the dma configuration
of the pci device using the configuration from DT of the parent of
the root bridge device.

Signed-off-by: Murali Karicheri<redacted>
---
  drivers/of/of_pci.c    |   73
++++++++++++++++++++++++++++++++++++++++++++++++
  include/linux/of_pci.h |   12 ++++++++
  2 files changed, 85 insertions(+)
[...]
quoted
quoted
+               coherent = of_dma_is_coherent(parent_np);
+               dev_dbg(dev, "device is%sdma coherent\n",
+                       coherent ? " " : " not ");
+
+               arch_setup_dma_ops(dev, dma_addr, size, NULL, coherent);

This is the same code as of_dma_configure. The only difference I see
is which node ptr is passed to of_dma_get_range. You need to make that
a function param of of_dma_configure.

of_dma_configure also has iommu handling now. You will probably need
something similar for PCI in that you setup an iommu based on the root
bus DT properties.
Initially I had the same idea to re-use the existing function
of_dma_configure() for this. I wanted to defer this until we have an
agreement on the changes required for the subject functionality. My quick
review of the code suggestio this would require additional API changes as
below. I did a quick test of the changes and it works for Keystone, but need
to be reviewed by everyone as I touch the IOMMU functionality here and I
don't have a platform with IOMMU. Need test by someone to make sure I don't
break anything.
The IOMMU changes look trivial. We may want to address the comment in
of_iommu_configure about parent nodes. We should be sure these changes
work with how we would do searching parent nodes.
Here are the changes required to implement this suggestion.

1. Move the of_dma_configure() to drivers/of/device.c (include the API in
of_device.h) and make it global (using proper EXPORT macro). Otherwise, we
will have to include of_platform.h in drivers/of/of_pci.c assuming the
prototype is defined in of_platform.h which doesn't look appropriate to me.
Would require following additional include files in drivers/of/device.c as
well.

+#include <linux/of_address.h>
+#include <linux/of_iommu.h>
+#include <linux/dma-mapping.h>
Okay.
quoted hunk ↗ jump to hunk
2. drivers/iommu/of_iommu.c. This is needed so that of_iommu_configure() can
take confuguration from the root bus DT as you have suggested.

-struct iommu_ops *of_iommu_configure(struct device *dev)
+struct iommu_ops *of_iommu_configure(struct device *dev, struct device_node
*node)
 {
        struct of_phandle_args iommu_spec;
        struct device_node *np;
@@ -145,7 +145,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev)
         * See the `Notes:' section of
         * Documentation/devicetree/bindings/iommu/iommu.txt
         */
-       while (!of_parse_phandle_with_args(dev->of_node, "iommus",
+       while (!of_parse_phandle_with_args(node, "iommus",
                                           "#iommu-cells", idx,
                                           &iommu_spec)) {
Seems safe enough.
3. drivers/of/of_pci.c. The existing function (added in this patch) will
make call to of_dma_configure() as

parent_np = of_get_pci_root_bridge_parent(pci_dev);
of_dma_configure(dev, parent_np);
Okay.
4. drivers/of/platform.c. Add a wrapper function of_platform_dma_configure()
that calls of_dma_configure() as
of_dma_configure(dev, dev->of_node). All existing calls converted to this
wrapper.
There's only a few callers of of_dma_configure, so I don't think this
is necessary. The only thing platform bus specific is who is calling
the function.

Rob
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help