* [PATCH v2 0/2] PCI: get DMA configuration from parent device
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux, linux-arm-kernel, linux-kernel, arnd, will.deacon,
grant.likely, robh+dt, devicetree, bhelgaas, linux-pci
Cc: Murali Karicheri
PCI devices on Keystone doesn't have correct dma_pfn_offset set. This patch
add capability to set the dma configuration such as dma-mask, dma_pfn_offset,
and dma ops etc using the information from DT. The prior RFCs and discussions
are available at [1] and [2] below.
[2] : https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg790244.html
[1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
Change history:
v2 - update size to coherent_dma_mask + 1 if dma-range info is missing
- also check the np for null.
v1 - updates based on the comments against initial RFC.
- Added a helper function to get the OF node of the parent
- Added an API in of_pci.c to update DMA configuration of the pci
device.
Murali Karicheri (2):
of/pci: add of_pci_dma_configure() update dma configuration
PCI: update dma configuration from DT
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 2 ++
include/linux/of_pci.h | 12 ++++++++
3 files changed, 87 insertions(+)
--
1.7.9.5
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 0/2] PCI: get DMA configuration from parent device
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux, linux-arm-kernel, linux-kernel, arnd, will.deacon,
grant.likely, robh+dt, devicetree, bhelgaas, linux-pci
Cc: Murali Karicheri
PCI devices on Keystone doesn't have correct dma_pfn_offset set. This patch
add capability to set the dma configuration such as dma-mask, dma_pfn_offset,
and dma ops etc using the information from DT. The prior RFCs and discussions
are available at [1] and [2] below.
[2] : https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg790244.html
[1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
Change history:
v2 - update size to coherent_dma_mask + 1 if dma-range info is missing
- also check the np for null.
v1 - updates based on the comments against initial RFC.
- Added a helper function to get the OF node of the parent
- Added an API in of_pci.c to update DMA configuration of the pci
device.
Murali Karicheri (2):
of/pci: add of_pci_dma_configure() update dma configuration
PCI: update dma configuration from DT
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 2 ++
include/linux/of_pci.h | 12 ++++++++
3 files changed, 87 insertions(+)
--
1.7.9.5
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 0/2] PCI: get DMA configuration from parent device
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux-arm-kernel
PCI devices on Keystone doesn't have correct dma_pfn_offset set. This patch
add capability to set the dma configuration such as dma-mask, dma_pfn_offset,
and dma ops etc using the information from DT. The prior RFCs and discussions
are available at [1] and [2] below.
[2] : https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg790244.html
[1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
Change history:
v2 - update size to coherent_dma_mask + 1 if dma-range info is missing
- also check the np for null.
v1 - updates based on the comments against initial RFC.
- Added a helper function to get the OF node of the parent
- Added an API in of_pci.c to update DMA configuration of the pci
device.
Murali Karicheri (2):
of/pci: add of_pci_dma_configure() update dma configuration
PCI: update dma configuration from DT
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 2 ++
include/linux/of_pci.h | 12 ++++++++
3 files changed, 87 insertions(+)
--
1.7.9.5
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2014-12-24 22:11 ` Murali Karicheri
(?)
@ 2014-12-24 22:11 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux, linux-arm-kernel, linux-kernel, arnd, will.deacon,
grant.likely, robh+dt, devicetree, bhelgaas, linux-pci
Cc: Murali Karicheri
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 <m-karicheri2@ti.com>
---
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_pci.h | 12 ++++++++
2 files changed, 85 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 88471d3..6d43f59 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -229,6 +229,79 @@ parse_failed:
return err;
}
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+
+/**
+ * of_get_pci_root_bridge_parent - Helper function to get the OF node of
+ * the root bridge's parent
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * This function will traverse the bus up to the root bus starting with
+ * the child and return the of node ptr to bridge device's parent device.
+ */
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ struct pci_bus *bus = dev->bus;
+ struct device *bridge;
+
+ while (!pci_is_root_bus(bus))
+ bus = bus->parent;
+ bridge = bus->bridge;
+
+ return bridge->parent->of_node;
+}
+EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
+
+/**
+ * of_pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * Try to get PCI devices's DMA configuration from DT and update it
+ * accordingly. This is similar to of_dma_configure() in of/platform.c
+ */
+void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+ struct device *dev = &pci_dev->dev;
+ u64 dma_addr, paddr, size;
+ struct device_node *parent_np;
+ unsigned long offset;
+ bool coherent;
+ int ret;
+
+ parent_np = of_get_pci_root_bridge_parent(pci_dev);
+
+ if (parent_np) {
+ /*
+ * Set default dma-mask to 32 bit. Drivers are expected to setup
+ * the correct supported dma_mask.
+ */
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ /*
+ * Set it to coherent_dma_mask by default if the architecture
+ * code has not set it.
+ */
+ if (!dev->dma_mask)
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
+ if (ret < 0) {
+ dma_addr = offset = 0;
+ size = dev->coherent_dma_mask + 1;
+ } else {
+ offset = PFN_DOWN(paddr - dma_addr);
+ dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
+ }
+ dev->dma_pfn_offset = offset;
+
+ 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);
+ }
+}
+EXPORT_SYMBOL_GPL(of_pci_dma_configure);
+
#endif /* CONFIG_OF_ADDRESS */
#ifdef CONFIG_PCI_MSI
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index ce0e5ab..0465a2a 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -16,6 +16,8 @@ int of_pci_get_devfn(struct device_node *np);
int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
int of_get_pci_domain_nr(struct device_node *node);
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev);
+void of_pci_dma_configure(struct pci_dev *pci_dev);
#else
static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
{
@@ -50,6 +52,16 @@ of_get_pci_domain_nr(struct device_node *node)
{
return -1;
}
+
+static inline struct device_node
+*of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ return NULL;
+}
+
+static inline void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+}
#endif
#if defined(CONFIG_OF_ADDRESS)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux, linux-arm-kernel, linux-kernel, arnd, will.deacon,
grant.likely, robh+dt, devicetree, bhelgaas, linux-pci
Cc: Murali Karicheri
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 <m-karicheri2@ti.com>
---
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_pci.h | 12 ++++++++
2 files changed, 85 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 88471d3..6d43f59 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -229,6 +229,79 @@ parse_failed:
return err;
}
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+
+/**
+ * of_get_pci_root_bridge_parent - Helper function to get the OF node of
+ * the root bridge's parent
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * This function will traverse the bus up to the root bus starting with
+ * the child and return the of node ptr to bridge device's parent device.
+ */
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ struct pci_bus *bus = dev->bus;
+ struct device *bridge;
+
+ while (!pci_is_root_bus(bus))
+ bus = bus->parent;
+ bridge = bus->bridge;
+
+ return bridge->parent->of_node;
+}
+EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
+
+/**
+ * of_pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * Try to get PCI devices's DMA configuration from DT and update it
+ * accordingly. This is similar to of_dma_configure() in of/platform.c
+ */
+void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+ struct device *dev = &pci_dev->dev;
+ u64 dma_addr, paddr, size;
+ struct device_node *parent_np;
+ unsigned long offset;
+ bool coherent;
+ int ret;
+
+ parent_np = of_get_pci_root_bridge_parent(pci_dev);
+
+ if (parent_np) {
+ /*
+ * Set default dma-mask to 32 bit. Drivers are expected to setup
+ * the correct supported dma_mask.
+ */
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ /*
+ * Set it to coherent_dma_mask by default if the architecture
+ * code has not set it.
+ */
+ if (!dev->dma_mask)
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
+ if (ret < 0) {
+ dma_addr = offset = 0;
+ size = dev->coherent_dma_mask + 1;
+ } else {
+ offset = PFN_DOWN(paddr - dma_addr);
+ dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
+ }
+ dev->dma_pfn_offset = offset;
+
+ 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);
+ }
+}
+EXPORT_SYMBOL_GPL(of_pci_dma_configure);
+
#endif /* CONFIG_OF_ADDRESS */
#ifdef CONFIG_PCI_MSI
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index ce0e5ab..0465a2a 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -16,6 +16,8 @@ int of_pci_get_devfn(struct device_node *np);
int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
int of_get_pci_domain_nr(struct device_node *node);
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev);
+void of_pci_dma_configure(struct pci_dev *pci_dev);
#else
static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
{
@@ -50,6 +52,16 @@ of_get_pci_domain_nr(struct device_node *node)
{
return -1;
}
+
+static inline struct device_node
+*of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ return NULL;
+}
+
+static inline void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+}
#endif
#if defined(CONFIG_OF_ADDRESS)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux-arm-kernel
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 <m-karicheri2@ti.com>
---
drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_pci.h | 12 ++++++++
2 files changed, 85 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 88471d3..6d43f59 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -229,6 +229,79 @@ parse_failed:
return err;
}
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+
+/**
+ * of_get_pci_root_bridge_parent - Helper function to get the OF node of
+ * the root bridge's parent
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * This function will traverse the bus up to the root bus starting with
+ * the child and return the of node ptr to bridge device's parent device.
+ */
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ struct pci_bus *bus = dev->bus;
+ struct device *bridge;
+
+ while (!pci_is_root_bus(bus))
+ bus = bus->parent;
+ bridge = bus->bridge;
+
+ return bridge->parent->of_node;
+}
+EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
+
+/**
+ * of_pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * Try to get PCI devices's DMA configuration from DT and update it
+ * accordingly. This is similar to of_dma_configure() in of/platform.c
+ */
+void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+ struct device *dev = &pci_dev->dev;
+ u64 dma_addr, paddr, size;
+ struct device_node *parent_np;
+ unsigned long offset;
+ bool coherent;
+ int ret;
+
+ parent_np = of_get_pci_root_bridge_parent(pci_dev);
+
+ if (parent_np) {
+ /*
+ * Set default dma-mask to 32 bit. Drivers are expected to setup
+ * the correct supported dma_mask.
+ */
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ /*
+ * Set it to coherent_dma_mask by default if the architecture
+ * code has not set it.
+ */
+ if (!dev->dma_mask)
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
+ if (ret < 0) {
+ dma_addr = offset = 0;
+ size = dev->coherent_dma_mask + 1;
+ } else {
+ offset = PFN_DOWN(paddr - dma_addr);
+ dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
+ }
+ dev->dma_pfn_offset = offset;
+
+ 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);
+ }
+}
+EXPORT_SYMBOL_GPL(of_pci_dma_configure);
+
#endif /* CONFIG_OF_ADDRESS */
#ifdef CONFIG_PCI_MSI
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index ce0e5ab..0465a2a 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -16,6 +16,8 @@ int of_pci_get_devfn(struct device_node *np);
int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
int of_get_pci_domain_nr(struct device_node *node);
+struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev);
+void of_pci_dma_configure(struct pci_dev *pci_dev);
#else
static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
{
@@ -50,6 +52,16 @@ of_get_pci_domain_nr(struct device_node *node)
{
return -1;
}
+
+static inline struct device_node
+*of_get_pci_root_bridge_parent(struct pci_dev *dev)
+{
+ return NULL;
+}
+
+static inline void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+}
#endif
#if defined(CONFIG_OF_ADDRESS)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 2/2] PCI: update dma configuration from DT
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux, linux-arm-kernel, linux-kernel, arnd, will.deacon,
grant.likely, robh+dt, devicetree, bhelgaas, linux-pci
Cc: Murali Karicheri
If there is a DT node available for the root bridge's parent device,
use the dma configuration from that device node. For example, keystone
PCI devices would require dma_pfn_offset to be set correctly in the
device structure of the pci device in order to have the correct dma mask.
The DT node will have dma-ranges defined for this. Also support using
the DT property dma-coherent to allow coherent DMA operation by the
PCI device.
This patch use the new helper function of_pci_dma_configure() to update
the device dma configuration.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
drivers/pci/probe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 23212f8..d7dcd6c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -6,6 +6,7 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/pci.h>
+#include <linux/of_pci.h>
#include <linux/pci_hotplug.h>
#include <linux/slab.h>
#include <linux/module.h>
@@ -1520,6 +1521,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
dev->dev.dma_mask = &dev->dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
dev->dev.coherent_dma_mask = 0xffffffffull;
+ of_pci_dma_configure(dev);
pci_set_dma_max_seg_size(dev, 65536);
pci_set_dma_seg_boundary(dev, 0xffffffff);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 2/2] PCI: update dma configuration from DT
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux-lFZ/pmaqli7XmaaqVzeoHQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
will.deacon-5wv7dgnIgG8, grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
linux-pci-u79uwXL29TY76Z2rM5mHXA
Cc: Murali Karicheri
If there is a DT node available for the root bridge's parent device,
use the dma configuration from that device node. For example, keystone
PCI devices would require dma_pfn_offset to be set correctly in the
device structure of the pci device in order to have the correct dma mask.
The DT node will have dma-ranges defined for this. Also support using
the DT property dma-coherent to allow coherent DMA operation by the
PCI device.
This patch use the new helper function of_pci_dma_configure() to update
the device dma configuration.
Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
---
drivers/pci/probe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 23212f8..d7dcd6c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -6,6 +6,7 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/pci.h>
+#include <linux/of_pci.h>
#include <linux/pci_hotplug.h>
#include <linux/slab.h>
#include <linux/module.h>
@@ -1520,6 +1521,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
dev->dev.dma_mask = &dev->dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
dev->dev.coherent_dma_mask = 0xffffffffull;
+ of_pci_dma_configure(dev);
pci_set_dma_max_seg_size(dev, 65536);
pci_set_dma_seg_boundary(dev, 0xffffffff);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 2/2] PCI: update dma configuration from DT
@ 2014-12-24 22:11 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2014-12-24 22:11 UTC (permalink / raw)
To: linux-arm-kernel
If there is a DT node available for the root bridge's parent device,
use the dma configuration from that device node. For example, keystone
PCI devices would require dma_pfn_offset to be set correctly in the
device structure of the pci device in order to have the correct dma mask.
The DT node will have dma-ranges defined for this. Also support using
the DT property dma-coherent to allow coherent DMA operation by the
PCI device.
This patch use the new helper function of_pci_dma_configure() to update
the device dma configuration.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
drivers/pci/probe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 23212f8..d7dcd6c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -6,6 +6,7 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/pci.h>
+#include <linux/of_pci.h>
#include <linux/pci_hotplug.h>
#include <linux/slab.h>
#include <linux/module.h>
@@ -1520,6 +1521,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
dev->dev.dma_mask = &dev->dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
dev->dev.coherent_dma_mask = 0xffffffffull;
+ of_pci_dma_configure(dev);
pci_set_dma_max_seg_size(dev, 65536);
pci_set_dma_seg_boundary(dev, 0xffffffff);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2014-12-24 22:11 ` Murali Karicheri
(?)
@ 2014-12-26 19:33 ` Rob Herring
-1 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2014-12-26 19:33 UTC (permalink / raw)
To: Murali Karicheri
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel,
Arnd Bergmann, Will Deacon, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> 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 <m-karicheri2@ti.com>
> ---
> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_pci.h | 12 ++++++++
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 88471d3..6d43f59 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -229,6 +229,79 @@ parse_failed:
> return err;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +/**
> + * of_get_pci_root_bridge_parent - Helper function to get the OF node of
> + * the root bridge's parent
I believe this has to be a one line description for DocBook.
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * This function will traverse the bus up to the root bus starting with
> + * the child and return the of node ptr to bridge device's parent device.
> + */
> +struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct device *bridge;
> +
> + while (!pci_is_root_bus(bus))
> + bus = bus->parent;
> + bridge = bus->bridge;
> +
> + return bridge->parent->of_node;
> +}
> +EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
> +
> +/**
> + * of_pci_dma_configure - Setup DMA configuration
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * Try to get PCI devices's DMA configuration from DT and update it
> + * accordingly. This is similar to of_dma_configure() in of/platform.c
> + */
> +void of_pci_dma_configure(struct pci_dev *pci_dev)
> +{
> + struct device *dev = &pci_dev->dev;
> + u64 dma_addr, paddr, size;
> + struct device_node *parent_np;
> + unsigned long offset;
> + bool coherent;
> + int ret;
> +
> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
> +
> + if (parent_np) {
Save a level of indentation and do:
if (!parent_np)
return;
> + /*
> + * Set default dma-mask to 32 bit. Drivers are expected to setup
> + * the correct supported dma_mask.
> + */
> + dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +
> + /*
> + * Set it to coherent_dma_mask by default if the architecture
> + * code has not set it.
> + */
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> +
> + ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
> + if (ret < 0) {
> + dma_addr = offset = 0;
> + size = dev->coherent_dma_mask + 1;
> + } else {
> + offset = PFN_DOWN(paddr - dma_addr);
> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> + }
> + dev->dma_pfn_offset = offset;
> +
> + 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.
Rob
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2014-12-26 19:33 ` Rob Herring
0 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2014-12-26 19:33 UTC (permalink / raw)
To: Murali Karicheri
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel,
Arnd Bergmann, Will Deacon, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> 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 <m-karicheri2@ti.com>
> ---
> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_pci.h | 12 ++++++++
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 88471d3..6d43f59 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -229,6 +229,79 @@ parse_failed:
> return err;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +/**
> + * of_get_pci_root_bridge_parent - Helper function to get the OF node of
> + * the root bridge's parent
I believe this has to be a one line description for DocBook.
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * This function will traverse the bus up to the root bus starting with
> + * the child and return the of node ptr to bridge device's parent device.
> + */
> +struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct device *bridge;
> +
> + while (!pci_is_root_bus(bus))
> + bus = bus->parent;
> + bridge = bus->bridge;
> +
> + return bridge->parent->of_node;
> +}
> +EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
> +
> +/**
> + * of_pci_dma_configure - Setup DMA configuration
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * Try to get PCI devices's DMA configuration from DT and update it
> + * accordingly. This is similar to of_dma_configure() in of/platform.c
> + */
> +void of_pci_dma_configure(struct pci_dev *pci_dev)
> +{
> + struct device *dev = &pci_dev->dev;
> + u64 dma_addr, paddr, size;
> + struct device_node *parent_np;
> + unsigned long offset;
> + bool coherent;
> + int ret;
> +
> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
> +
> + if (parent_np) {
Save a level of indentation and do:
if (!parent_np)
return;
> + /*
> + * Set default dma-mask to 32 bit. Drivers are expected to setup
> + * the correct supported dma_mask.
> + */
> + dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +
> + /*
> + * Set it to coherent_dma_mask by default if the architecture
> + * code has not set it.
> + */
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> +
> + ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
> + if (ret < 0) {
> + dma_addr = offset = 0;
> + size = dev->coherent_dma_mask + 1;
> + } else {
> + offset = PFN_DOWN(paddr - dma_addr);
> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> + }
> + dev->dma_pfn_offset = offset;
> +
> + 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.
Rob
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2014-12-26 19:33 ` Rob Herring
0 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2014-12-26 19:33 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> 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 <m-karicheri2@ti.com>
> ---
> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_pci.h | 12 ++++++++
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 88471d3..6d43f59 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -229,6 +229,79 @@ parse_failed:
> return err;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +/**
> + * of_get_pci_root_bridge_parent - Helper function to get the OF node of
> + * the root bridge's parent
I believe this has to be a one line description for DocBook.
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * This function will traverse the bus up to the root bus starting with
> + * the child and return the of node ptr to bridge device's parent device.
> + */
> +struct device_node *of_get_pci_root_bridge_parent(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct device *bridge;
> +
> + while (!pci_is_root_bus(bus))
> + bus = bus->parent;
> + bridge = bus->bridge;
> +
> + return bridge->parent->of_node;
> +}
> +EXPORT_SYMBOL_GPL(of_get_pci_root_bridge_parent);
> +
> +/**
> + * of_pci_dma_configure - Setup DMA configuration
> + * @dev: ptr to pci_dev struct of the pci device
> + *
> + * Try to get PCI devices's DMA configuration from DT and update it
> + * accordingly. This is similar to of_dma_configure() in of/platform.c
> + */
> +void of_pci_dma_configure(struct pci_dev *pci_dev)
> +{
> + struct device *dev = &pci_dev->dev;
> + u64 dma_addr, paddr, size;
> + struct device_node *parent_np;
> + unsigned long offset;
> + bool coherent;
> + int ret;
> +
> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
> +
> + if (parent_np) {
Save a level of indentation and do:
if (!parent_np)
return;
> + /*
> + * Set default dma-mask to 32 bit. Drivers are expected to setup
> + * the correct supported dma_mask.
> + */
> + dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +
> + /*
> + * Set it to coherent_dma_mask by default if the architecture
> + * code has not set it.
> + */
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> +
> + ret = of_dma_get_range(parent_np, &dma_addr, &paddr, &size);
> + if (ret < 0) {
> + dma_addr = offset = 0;
> + size = dev->coherent_dma_mask + 1;
> + } else {
> + offset = PFN_DOWN(paddr - dma_addr);
> + dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> + }
> + dev->dma_pfn_offset = offset;
> +
> + 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.
Rob
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2014-12-26 19:33 ` Rob Herring
(?)
@ 2015-01-02 17:20 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 17:20 UTC (permalink / raw)
To: Rob Herring, Arnd Bergmann, Will Deacon
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel,
Grant Likely, Rob Herring, devicetree, Bjorn Helgaas, linux-pci
Rob,
See my response below. Arnd and Will, please review this as well.
On 12/26/2014 02:33 PM, Rob Herring wrote:
> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> 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<m-karicheri2@ti.com>
>> ---
>> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of_pci.h | 12 ++++++++
>> 2 files changed, 85 insertions(+)
>>
----------------------CUT--------------------------------------------
>> + unsigned long offset;
>> + bool coherent;
>> + int ret;
>> +
>> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
>> +
>> + if (parent_np) {
>
> Save a level of indentation and do:
>
> if (!parent_np)
> return;
Agree. This will not be needed if I go with changes proposed by your
next comment below.
>
>> + /*
-------CUT------------------------------------------------------
>> +
>> + 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.
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>
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)) {
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);
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.
If the above looks good, I can post v3 of the patch with these changes.
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 17:20 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 17:20 UTC (permalink / raw)
To: Rob Herring, Arnd Bergmann, Will Deacon
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel,
Grant Likely, Rob Herring, devicetree, Bjorn Helgaas, linux-pci
Rob,
See my response below. Arnd and Will, please review this as well.
On 12/26/2014 02:33 PM, Rob Herring wrote:
> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> 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<m-karicheri2@ti.com>
>> ---
>> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of_pci.h | 12 ++++++++
>> 2 files changed, 85 insertions(+)
>>
----------------------CUT--------------------------------------------
>> + unsigned long offset;
>> + bool coherent;
>> + int ret;
>> +
>> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
>> +
>> + if (parent_np) {
>
> Save a level of indentation and do:
>
> if (!parent_np)
> return;
Agree. This will not be needed if I go with changes proposed by your
next comment below.
>
>> + /*
-------CUT------------------------------------------------------
>> +
>> + 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.
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>
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)) {
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);
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.
If the above looks good, I can post v3 of the patch with these changes.
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 17:20 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 17:20 UTC (permalink / raw)
To: linux-arm-kernel
Rob,
See my response below. Arnd and Will, please review this as well.
On 12/26/2014 02:33 PM, Rob Herring wrote:
> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> 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<m-karicheri2@ti.com>
>> ---
>> drivers/of/of_pci.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of_pci.h | 12 ++++++++
>> 2 files changed, 85 insertions(+)
>>
----------------------CUT--------------------------------------------
>> + unsigned long offset;
>> + bool coherent;
>> + int ret;
>> +
>> + parent_np = of_get_pci_root_bridge_parent(pci_dev);
>> +
>> + if (parent_np) {
>
> Save a level of indentation and do:
>
> if (!parent_np)
> return;
Agree. This will not be needed if I go with changes proposed by your
next comment below.
>
>> + /*
-------CUT------------------------------------------------------
>> +
>> + 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.
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>
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)) {
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);
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.
If the above looks good, I can post v3 of the patch with these changes.
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-02 17:20 ` Murali Karicheri
(?)
@ 2015-01-02 20:45 ` Rob Herring
-1 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2015-01-02 20:45 UTC (permalink / raw)
To: Murali Karicheri
Cc: Arnd Bergmann, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> Rob,
>
> See my response below. Arnd and Will, please review this as well.
>
> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>
>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>> wrote:
>>>
>>> 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<m-karicheri2@ti.com>
>>> ---
>>> drivers/of/of_pci.c | 73
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of_pci.h | 12 ++++++++
>>> 2 files changed, 85 insertions(+)
>>>
[...]
>>> + 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.
> 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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 20:45 ` Rob Herring
0 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2015-01-02 20:45 UTC (permalink / raw)
To: Murali Karicheri
Cc: Arnd Bergmann, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> Rob,
>
> See my response below. Arnd and Will, please review this as well.
>
> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>
>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>> wrote:
>>>
>>> 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<m-karicheri2@ti.com>
>>> ---
>>> drivers/of/of_pci.c | 73
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of_pci.h | 12 ++++++++
>>> 2 files changed, 85 insertions(+)
>>>
[...]
>>> + 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.
> 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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 20:45 ` Rob Herring
0 siblings, 0 replies; 49+ messages in thread
From: Rob Herring @ 2015-01-02 20:45 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> Rob,
>
> See my response below. Arnd and Will, please review this as well.
>
> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>
>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>> wrote:
>>>
>>> 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<m-karicheri2@ti.com>
>>> ---
>>> drivers/of/of_pci.c | 73
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of_pci.h | 12 ++++++++
>>> 2 files changed, 85 insertions(+)
>>>
[...]
>>> + 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.
> 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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-02 17:20 ` Murali Karicheri
(?)
@ 2015-01-02 20:57 ` Arnd Bergmann
-1 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-02 20:57 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
> 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.
>
> 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>
Yes, sounds good.
> 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)) {
Right.
> 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);
With dev = &pci_dev->dev, I assume?
> 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 are only two callers, I don't think we need a wrapper for it,
just change the calling conventions along with step 2.
> If the above looks good, I can post v3 of the patch with these changes.
With that one minor change, sounds perfect to me. The same can also
be used by other bus types if we ever need it.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 20:57 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-02 20:57 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
> 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.
>
> 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>
Yes, sounds good.
> 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)) {
Right.
> 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);
With dev = &pci_dev->dev, I assume?
> 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 are only two callers, I don't think we need a wrapper for it,
just change the calling conventions along with step 2.
> If the above looks good, I can post v3 of the patch with these changes.
With that one minor change, sounds perfect to me. The same can also
be used by other bus types if we ever need it.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 20:57 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-02 20:57 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
> 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.
>
> 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>
Yes, sounds good.
> 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)) {
Right.
> 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);
With dev = &pci_dev->dev, I assume?
> 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 are only two callers, I don't think we need a wrapper for it,
just change the calling conventions along with step 2.
> If the above looks good, I can post v3 of the patch with these changes.
With that one minor change, sounds perfect to me. The same can also
be used by other bus types if we ever need it.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-02 20:57 ` Arnd Bergmann
(?)
(?)
@ 2015-01-02 21:12 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 21:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/02/2015 03:57 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
>> 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.
>>
>> 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>
>
> Yes, sounds good.
>
>> 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)) {
>
> Right.
>
>> 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);
>
> With dev =&pci_dev->dev, I assume?
Yes
>
>> 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 are only two callers, I don't think we need a wrapper for it,
> just change the calling conventions along with step 2.
Ok. That is what Rob's comment as well.
>
>> If the above looks good, I can post v3 of the patch with these changes.
>
> With that one minor change, sounds perfect to me. The same can also
> be used by other bus types if we ever need it.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 21:12 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 21:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Bjorn Helgaas,
linux-pci-u79uwXL29TY76Z2rM5mHXA
On 01/02/2015 03:57 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
>> 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.
>>
>> 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>
>
> Yes, sounds good.
>
>> 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)) {
>
> Right.
>
>> 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);
>
> With dev =&pci_dev->dev, I assume?
Yes
>
>> 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 are only two callers, I don't think we need a wrapper for it,
> just change the calling conventions along with step 2.
Ok. That is what Rob's comment as well.
>
>> If the above looks good, I can post v3 of the patch with these changes.
>
> With that one minor change, sounds perfect to me. The same can also
> be used by other bus types if we ever need it.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 21:12 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 21:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/02/2015 03:57 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
>> 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.
>>
>> 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>
>
> Yes, sounds good.
>
>> 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)) {
>
> Right.
>
>> 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);
>
> With dev =&pci_dev->dev, I assume?
Yes
>
>> 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 are only two callers, I don't think we need a wrapper for it,
> just change the calling conventions along with step 2.
Ok. That is what Rob's comment as well.
>
>> If the above looks good, I can post v3 of the patch with these changes.
>
> With that one minor change, sounds perfect to me. The same can also
> be used by other bus types if we ever need it.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 21:12 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 21:12 UTC (permalink / raw)
To: linux-arm-kernel
On 01/02/2015 03:57 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 12:20:53 Murali Karicheri wrote:
>> 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.
>>
>> 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>
>
> Yes, sounds good.
>
>> 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)) {
>
> Right.
>
>> 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);
>
> With dev =&pci_dev->dev, I assume?
Yes
>
>> 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 are only two callers, I don't think we need a wrapper for it,
> just change the calling conventions along with step 2.
Ok. That is what Rob's comment as well.
>
>> If the above looks good, I can post v3 of the patch with these changes.
>
> With that one minor change, sounds perfect to me. The same can also
> be used by other bus types if we ever need it.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-02 20:45 ` Rob Herring
(?)
@ 2015-01-02 22:33 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 22:33 UTC (permalink / raw)
To: Rob Herring, Will Deacon
Cc: Arnd Bergmann, Russell King - ARM Linux, linux-arm-kernel,
linux-kernel, Grant Likely, Rob Herring, devicetree,
Bjorn Helgaas, linux-pci
On 01/02/2015 03:45 PM, Rob Herring wrote:
> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> Rob,
>>
>> See my response below. Arnd and Will, please review this as well.
>>
>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>
>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>> wrote:
>>>>
>>>> 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<m-karicheri2@ti.com>
>>>> ---
>>>> drivers/of/of_pci.c | 73
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/of_pci.h | 12 ++++++++
>>>> 2 files changed, 85 insertions(+)
>>>>
>
> [...]
>
>>>> + 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.
I have no experience with IOMMU and may not offer much help here as I
originally wrote above. Will Deacon has added this API and probably able
to offer some help in this discussion.
Will Deacon,
Any comment?
Looking at the iommu documentation and of_iommu.c, I get a feeling that
this API is not really used at present as there are no callers of
of_iommu_set_ops() and I assume this is a WIP. I believe the way it is
expected to work is to have the iommu driver of the master IOMMU devices
call of_iommu_set_ops(). The device node of this master IOMMU device is
specified as a phandle in the OF node of the device (various bus devices
such as platform, PCI etc). This allow to retrieve the iommu ops though
the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
gut feeling is that for PCI devices, as there are no DT node, the root
bus node may specify iommus phandle to IOMMU master OF nodes.
W.r.t your comment "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",
I believe, the parent node search itself should work the same way in the
case of PCI as with platform bus case. PCI's case, we are providing the
OF node of the root bus host bridge. Why should this be any different in
terms of search?
I see a potential issue with dma-ranges as described in the notes below.
As noted below the usage of dma-range for iommu is to be determined. For
keystone, the of_iommu_configure() always return false as we don't use
the iommu. But don't know if this has any consequences for other
platforms. Or I got your questions wrong. Any help here from others on
the list?
========================================================================
One possible extension to the above is to use an "iommus" property along
with a "dma-ranges" property in a bus device node (such as PCI host
bridges). This can be useful to describe how children on the bus relate
to the IOMMU if they are not explicitly listed in the device tree (e.g.
PCI devices). However, the requirements of that use-case haven't been
fully determined yet. Implementing this is therefore not recommended
without further discussion and extension of this binding.
=========================================================================
The code is
struct iommu_ops *of_iommu_configure(struct device *dev)
{
struct of_phandle_args iommu_spec;
struct device_node *np;
struct iommu_ops *ops = NULL;
int idx = 0;
/*
* 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(dev->of_node, "iommus",
"#iommu-cells", idx,
&iommu_spec)) {
np = iommu_spec.np;
ops = of_iommu_get_ops(np);
if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
goto err_put_node;
of_node_put(np);
idx++;
}
return ops;
err_put_node:
of_node_put(np);
return NULL;
}
>> 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.
Ok.
>
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 22:33 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 22:33 UTC (permalink / raw)
To: Rob Herring, Will Deacon
Cc: Arnd Bergmann, Russell King - ARM Linux, linux-arm-kernel,
linux-kernel, Grant Likely, Rob Herring, devicetree,
Bjorn Helgaas, linux-pci
On 01/02/2015 03:45 PM, Rob Herring wrote:
> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> Rob,
>>
>> See my response below. Arnd and Will, please review this as well.
>>
>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>
>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>> wrote:
>>>>
>>>> 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<m-karicheri2@ti.com>
>>>> ---
>>>> drivers/of/of_pci.c | 73
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/of_pci.h | 12 ++++++++
>>>> 2 files changed, 85 insertions(+)
>>>>
>
> [...]
>
>>>> + 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.
I have no experience with IOMMU and may not offer much help here as I
originally wrote above. Will Deacon has added this API and probably able
to offer some help in this discussion.
Will Deacon,
Any comment?
Looking at the iommu documentation and of_iommu.c, I get a feeling that
this API is not really used at present as there are no callers of
of_iommu_set_ops() and I assume this is a WIP. I believe the way it is
expected to work is to have the iommu driver of the master IOMMU devices
call of_iommu_set_ops(). The device node of this master IOMMU device is
specified as a phandle in the OF node of the device (various bus devices
such as platform, PCI etc). This allow to retrieve the iommu ops though
the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
gut feeling is that for PCI devices, as there are no DT node, the root
bus node may specify iommus phandle to IOMMU master OF nodes.
W.r.t your comment "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",
I believe, the parent node search itself should work the same way in the
case of PCI as with platform bus case. PCI's case, we are providing the
OF node of the root bus host bridge. Why should this be any different in
terms of search?
I see a potential issue with dma-ranges as described in the notes below.
As noted below the usage of dma-range for iommu is to be determined. For
keystone, the of_iommu_configure() always return false as we don't use
the iommu. But don't know if this has any consequences for other
platforms. Or I got your questions wrong. Any help here from others on
the list?
========================================================================
One possible extension to the above is to use an "iommus" property along
with a "dma-ranges" property in a bus device node (such as PCI host
bridges). This can be useful to describe how children on the bus relate
to the IOMMU if they are not explicitly listed in the device tree (e.g.
PCI devices). However, the requirements of that use-case haven't been
fully determined yet. Implementing this is therefore not recommended
without further discussion and extension of this binding.
=========================================================================
The code is
struct iommu_ops *of_iommu_configure(struct device *dev)
{
struct of_phandle_args iommu_spec;
struct device_node *np;
struct iommu_ops *ops = NULL;
int idx = 0;
/*
* 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(dev->of_node, "iommus",
"#iommu-cells", idx,
&iommu_spec)) {
np = iommu_spec.np;
ops = of_iommu_get_ops(np);
if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
goto err_put_node;
of_node_put(np);
idx++;
}
return ops;
err_put_node:
of_node_put(np);
return NULL;
}
>> 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.
Ok.
>
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-02 22:33 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-02 22:33 UTC (permalink / raw)
To: linux-arm-kernel
On 01/02/2015 03:45 PM, Rob Herring wrote:
> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>> Rob,
>>
>> See my response below. Arnd and Will, please review this as well.
>>
>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>
>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>> wrote:
>>>>
>>>> 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<m-karicheri2@ti.com>
>>>> ---
>>>> drivers/of/of_pci.c | 73
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/of_pci.h | 12 ++++++++
>>>> 2 files changed, 85 insertions(+)
>>>>
>
> [...]
>
>>>> + 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.
I have no experience with IOMMU and may not offer much help here as I
originally wrote above. Will Deacon has added this API and probably able
to offer some help in this discussion.
Will Deacon,
Any comment?
Looking at the iommu documentation and of_iommu.c, I get a feeling that
this API is not really used at present as there are no callers of
of_iommu_set_ops() and I assume this is a WIP. I believe the way it is
expected to work is to have the iommu driver of the master IOMMU devices
call of_iommu_set_ops(). The device node of this master IOMMU device is
specified as a phandle in the OF node of the device (various bus devices
such as platform, PCI etc). This allow to retrieve the iommu ops though
the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
gut feeling is that for PCI devices, as there are no DT node, the root
bus node may specify iommus phandle to IOMMU master OF nodes.
W.r.t your comment "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",
I believe, the parent node search itself should work the same way in the
case of PCI as with platform bus case. PCI's case, we are providing the
OF node of the root bus host bridge. Why should this be any different in
terms of search?
I see a potential issue with dma-ranges as described in the notes below.
As noted below the usage of dma-range for iommu is to be determined. For
keystone, the of_iommu_configure() always return false as we don't use
the iommu. But don't know if this has any consequences for other
platforms. Or I got your questions wrong. Any help here from others on
the list?
========================================================================
One possible extension to the above is to use an "iommus" property along
with a "dma-ranges" property in a bus device node (such as PCI host
bridges). This can be useful to describe how children on the bus relate
to the IOMMU if they are not explicitly listed in the device tree (e.g.
PCI devices). However, the requirements of that use-case haven't been
fully determined yet. Implementing this is therefore not recommended
without further discussion and extension of this binding.
=========================================================================
The code is
struct iommu_ops *of_iommu_configure(struct device *dev)
{
struct of_phandle_args iommu_spec;
struct device_node *np;
struct iommu_ops *ops = NULL;
int idx = 0;
/*
* 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(dev->of_node, "iommus",
"#iommu-cells", idx,
&iommu_spec)) {
np = iommu_spec.np;
ops = of_iommu_get_ops(np);
if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
goto err_put_node;
of_node_put(np);
idx++;
}
return ops;
err_put_node:
of_node_put(np);
return NULL;
}
>> 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.
Ok.
>
> Rob
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-03 21:37 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-03 21:37 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
It's complicated :(
> Looking at the iommu documentation and of_iommu.c, I get a feeling that
> this API is not really used at present as there are no callers of
> of_iommu_set_ops() and I assume this is a WIP.
Right, but we have patches for some iommu drivers based on this API,
and we should migrate all of them eventually.
> I believe the way it is
> expected to work is to have the iommu driver of the master IOMMU devices
> call of_iommu_set_ops(). The device node of this master IOMMU device is
> specified as a phandle in the OF node of the device (various bus devices
> such as platform, PCI etc). This allow to retrieve the iommu ops though
> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
> gut feeling is that for PCI devices, as there are no DT node, the root
> bus node may specify iommus phandle to IOMMU master OF nodes.
Yes, but we also need to pass a PCI device specific identifier along
with the root bus node, because some iommu drivers take the PCI
bus/device/function number into account for creating per-function
i/o page tables.
> W.r.t your comment "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",
>
> I believe, the parent node search itself should work the same way in the
> case of PCI as with platform bus case. PCI's case, we are providing the
> OF node of the root bus host bridge. Why should this be any different in
> terms of search?
>
> I see a potential issue with dma-ranges as described in the notes below.
> As noted below the usage of dma-range for iommu is to be determined. For
> keystone, the of_iommu_configure() always return false as we don't use
> the iommu. But don't know if this has any consequences for other
> platforms. Or I got your questions wrong. Any help here from others on
> the list?
>
> ========================================================================
> One possible extension to the above is to use an "iommus" property along
> with a "dma-ranges" property in a bus device node (such as PCI host
> bridges). This can be useful to describe how children on the bus relate
> to the IOMMU if they are not explicitly listed in the device tree (e.g.
> PCI devices). However, the requirements of that use-case haven't been
> fully determined yet. Implementing this is therefore not recommended
> without further discussion and extension of this binding.
> =========================================================================
This probably won't ever apply to PCI devices, so let's ignore it for now.
For the moment (and for PCI), we should assume that we either configure
an iommu directly or we use dma-ranges if no iommu is in use.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-03 21:37 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-03 21:37 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Bjorn Helgaas,
linux-pci-u79uwXL29TY76Z2rM5mHXA
On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
It's complicated :(
> Looking at the iommu documentation and of_iommu.c, I get a feeling that
> this API is not really used at present as there are no callers of
> of_iommu_set_ops() and I assume this is a WIP.
Right, but we have patches for some iommu drivers based on this API,
and we should migrate all of them eventually.
> I believe the way it is
> expected to work is to have the iommu driver of the master IOMMU devices
> call of_iommu_set_ops(). The device node of this master IOMMU device is
> specified as a phandle in the OF node of the device (various bus devices
> such as platform, PCI etc). This allow to retrieve the iommu ops though
> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
> gut feeling is that for PCI devices, as there are no DT node, the root
> bus node may specify iommus phandle to IOMMU master OF nodes.
Yes, but we also need to pass a PCI device specific identifier along
with the root bus node, because some iommu drivers take the PCI
bus/device/function number into account for creating per-function
i/o page tables.
> W.r.t your comment "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",
>
> I believe, the parent node search itself should work the same way in the
> case of PCI as with platform bus case. PCI's case, we are providing the
> OF node of the root bus host bridge. Why should this be any different in
> terms of search?
>
> I see a potential issue with dma-ranges as described in the notes below.
> As noted below the usage of dma-range for iommu is to be determined. For
> keystone, the of_iommu_configure() always return false as we don't use
> the iommu. But don't know if this has any consequences for other
> platforms. Or I got your questions wrong. Any help here from others on
> the list?
>
> ========================================================================
> One possible extension to the above is to use an "iommus" property along
> with a "dma-ranges" property in a bus device node (such as PCI host
> bridges). This can be useful to describe how children on the bus relate
> to the IOMMU if they are not explicitly listed in the device tree (e.g.
> PCI devices). However, the requirements of that use-case haven't been
> fully determined yet. Implementing this is therefore not recommended
> without further discussion and extension of this binding.
> =========================================================================
This probably won't ever apply to PCI devices, so let's ignore it for now.
For the moment (and for PCI), we should assume that we either configure
an iommu directly or we use dma-ranges if no iommu is in use.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-03 21:37 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-03 21:37 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
It's complicated :(
> Looking at the iommu documentation and of_iommu.c, I get a feeling that
> this API is not really used at present as there are no callers of
> of_iommu_set_ops() and I assume this is a WIP.
Right, but we have patches for some iommu drivers based on this API,
and we should migrate all of them eventually.
> I believe the way it is
> expected to work is to have the iommu driver of the master IOMMU devices
> call of_iommu_set_ops(). The device node of this master IOMMU device is
> specified as a phandle in the OF node of the device (various bus devices
> such as platform, PCI etc). This allow to retrieve the iommu ops though
> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
> gut feeling is that for PCI devices, as there are no DT node, the root
> bus node may specify iommus phandle to IOMMU master OF nodes.
Yes, but we also need to pass a PCI device specific identifier along
with the root bus node, because some iommu drivers take the PCI
bus/device/function number into account for creating per-function
i/o page tables.
> W.r.t your comment "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",
>
> I believe, the parent node search itself should work the same way in the
> case of PCI as with platform bus case. PCI's case, we are providing the
> OF node of the root bus host bridge. Why should this be any different in
> terms of search?
>
> I see a potential issue with dma-ranges as described in the notes below.
> As noted below the usage of dma-range for iommu is to be determined. For
> keystone, the of_iommu_configure() always return false as we don't use
> the iommu. But don't know if this has any consequences for other
> platforms. Or I got your questions wrong. Any help here from others on
> the list?
>
> ========================================================================
> One possible extension to the above is to use an "iommus" property along
> with a "dma-ranges" property in a bus device node (such as PCI host
> bridges). This can be useful to describe how children on the bus relate
> to the IOMMU if they are not explicitly listed in the device tree (e.g.
> PCI devices). However, the requirements of that use-case haven't been
> fully determined yet. Implementing this is therefore not recommended
> without further discussion and extension of this binding.
> =========================================================================
This probably won't ever apply to PCI devices, so let's ignore it for now.
For the moment (and for PCI), we should assume that we either configure
an iommu directly or we use dma-ranges if no iommu is in use.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-03 21:37 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-03 21:37 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
It's complicated :(
> Looking at the iommu documentation and of_iommu.c, I get a feeling that
> this API is not really used at present as there are no callers of
> of_iommu_set_ops() and I assume this is a WIP.
Right, but we have patches for some iommu drivers based on this API,
and we should migrate all of them eventually.
> I believe the way it is
> expected to work is to have the iommu driver of the master IOMMU devices
> call of_iommu_set_ops(). The device node of this master IOMMU device is
> specified as a phandle in the OF node of the device (various bus devices
> such as platform, PCI etc). This allow to retrieve the iommu ops though
> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
> gut feeling is that for PCI devices, as there are no DT node, the root
> bus node may specify iommus phandle to IOMMU master OF nodes.
Yes, but we also need to pass a PCI device specific identifier along
with the root bus node, because some iommu drivers take the PCI
bus/device/function number into account for creating per-function
i/o page tables.
> W.r.t your comment "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",
>
> I believe, the parent node search itself should work the same way in the
> case of PCI as with platform bus case. PCI's case, we are providing the
> OF node of the root bus host bridge. Why should this be any different in
> terms of search?
>
> I see a potential issue with dma-ranges as described in the notes below.
> As noted below the usage of dma-range for iommu is to be determined. For
> keystone, the of_iommu_configure() always return false as we don't use
> the iommu. But don't know if this has any consequences for other
> platforms. Or I got your questions wrong. Any help here from others on
> the list?
>
> ========================================================================
> One possible extension to the above is to use an "iommus" property along
> with a "dma-ranges" property in a bus device node (such as PCI host
> bridges). This can be useful to describe how children on the bus relate
> to the IOMMU if they are not explicitly listed in the device tree (e.g.
> PCI devices). However, the requirements of that use-case haven't been
> fully determined yet. Implementing this is therefore not recommended
> without further discussion and extension of this binding.
> =========================================================================
This probably won't ever apply to PCI devices, so let's ignore it for now.
For the moment (and for PCI), we should assume that we either configure
an iommu directly or we use dma-ranges if no iommu is in use.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-03 21:37 ` Arnd Bergmann
(?)
(?)
@ 2015-01-05 20:06 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 20:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> It's complicated :(
>
>> Looking at the iommu documentation and of_iommu.c, I get a feeling that
>> this API is not really used at present as there are no callers of
>> of_iommu_set_ops() and I assume this is a WIP.
>
> Right, but we have patches for some iommu drivers based on this API,
> and we should migrate all of them eventually.
>
>> I believe the way it is
>> expected to work is to have the iommu driver of the master IOMMU devices
>> call of_iommu_set_ops(). The device node of this master IOMMU device is
>> specified as a phandle in the OF node of the device (various bus devices
>> such as platform, PCI etc). This allow to retrieve the iommu ops though
>> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
>> gut feeling is that for PCI devices, as there are no DT node, the root
>> bus node may specify iommus phandle to IOMMU master OF nodes.
>
> Yes, but we also need to pass a PCI device specific identifier along
> with the root bus node, because some iommu drivers take the PCI
> bus/device/function number into account for creating per-function
> i/o page tables.
>
>> W.r.t your comment "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",
>>
>> I believe, the parent node search itself should work the same way in the
>> case of PCI as with platform bus case. PCI's case, we are providing the
>> OF node of the root bus host bridge. Why should this be any different in
>> terms of search?
>>
>> I see a potential issue with dma-ranges as described in the notes below.
>> As noted below the usage of dma-range for iommu is to be determined. For
>> keystone, the of_iommu_configure() always return false as we don't use
>> the iommu. But don't know if this has any consequences for other
>> platforms. Or I got your questions wrong. Any help here from others on
>> the list?
>>
>> ========================================================================
>> One possible extension to the above is to use an "iommus" property along
>> with a "dma-ranges" property in a bus device node (such as PCI host
>> bridges). This can be useful to describe how children on the bus relate
>> to the IOMMU if they are not explicitly listed in the device tree (e.g.
>> PCI devices). However, the requirements of that use-case haven't been
>> fully determined yet. Implementing this is therefore not recommended
>> without further discussion and extension of this binding.
>> =========================================================================
>
> This probably won't ever apply to PCI devices, so let's ignore it for now.
> For the moment (and for PCI), we should assume that we either configure
> an iommu directly or we use dma-ranges if no iommu is in use.
>
Thanks Arnd,
I will post v3 of the patch with what is agreed before in my response
and I understand there is no additional change required based on this
particular discussion about iommu. Right?
Murali
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 20:06 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 20:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree, Russell King - ARM Linux, linux-pci, Will Deacon,
linux-kernel, Bjorn Helgaas, Rob Herring, Grant Likely,
linux-arm-kernel
On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> It's complicated :(
>
>> Looking at the iommu documentation and of_iommu.c, I get a feeling that
>> this API is not really used at present as there are no callers of
>> of_iommu_set_ops() and I assume this is a WIP.
>
> Right, but we have patches for some iommu drivers based on this API,
> and we should migrate all of them eventually.
>
>> I believe the way it is
>> expected to work is to have the iommu driver of the master IOMMU devices
>> call of_iommu_set_ops(). The device node of this master IOMMU device is
>> specified as a phandle in the OF node of the device (various bus devices
>> such as platform, PCI etc). This allow to retrieve the iommu ops though
>> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
>> gut feeling is that for PCI devices, as there are no DT node, the root
>> bus node may specify iommus phandle to IOMMU master OF nodes.
>
> Yes, but we also need to pass a PCI device specific identifier along
> with the root bus node, because some iommu drivers take the PCI
> bus/device/function number into account for creating per-function
> i/o page tables.
>
>> W.r.t your comment "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",
>>
>> I believe, the parent node search itself should work the same way in the
>> case of PCI as with platform bus case. PCI's case, we are providing the
>> OF node of the root bus host bridge. Why should this be any different in
>> terms of search?
>>
>> I see a potential issue with dma-ranges as described in the notes below.
>> As noted below the usage of dma-range for iommu is to be determined. For
>> keystone, the of_iommu_configure() always return false as we don't use
>> the iommu. But don't know if this has any consequences for other
>> platforms. Or I got your questions wrong. Any help here from others on
>> the list?
>>
>> ========================================================================
>> One possible extension to the above is to use an "iommus" property along
>> with a "dma-ranges" property in a bus device node (such as PCI host
>> bridges). This can be useful to describe how children on the bus relate
>> to the IOMMU if they are not explicitly listed in the device tree (e.g.
>> PCI devices). However, the requirements of that use-case haven't been
>> fully determined yet. Implementing this is therefore not recommended
>> without further discussion and extension of this binding.
>> =========================================================================
>
> This probably won't ever apply to PCI devices, so let's ignore it for now.
> For the moment (and for PCI), we should assume that we either configure
> an iommu directly or we use dma-ranges if no iommu is in use.
>
Thanks Arnd,
I will post v3 of the patch with what is agreed before in my response
and I understand there is no additional change required based on this
particular discussion about iommu. Right?
Murali
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 20:06 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 20:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> It's complicated :(
>
>> Looking at the iommu documentation and of_iommu.c, I get a feeling that
>> this API is not really used at present as there are no callers of
>> of_iommu_set_ops() and I assume this is a WIP.
>
> Right, but we have patches for some iommu drivers based on this API,
> and we should migrate all of them eventually.
>
>> I believe the way it is
>> expected to work is to have the iommu driver of the master IOMMU devices
>> call of_iommu_set_ops(). The device node of this master IOMMU device is
>> specified as a phandle in the OF node of the device (various bus devices
>> such as platform, PCI etc). This allow to retrieve the iommu ops though
>> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
>> gut feeling is that for PCI devices, as there are no DT node, the root
>> bus node may specify iommus phandle to IOMMU master OF nodes.
>
> Yes, but we also need to pass a PCI device specific identifier along
> with the root bus node, because some iommu drivers take the PCI
> bus/device/function number into account for creating per-function
> i/o page tables.
>
>> W.r.t your comment "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",
>>
>> I believe, the parent node search itself should work the same way in the
>> case of PCI as with platform bus case. PCI's case, we are providing the
>> OF node of the root bus host bridge. Why should this be any different in
>> terms of search?
>>
>> I see a potential issue with dma-ranges as described in the notes below.
>> As noted below the usage of dma-range for iommu is to be determined. For
>> keystone, the of_iommu_configure() always return false as we don't use
>> the iommu. But don't know if this has any consequences for other
>> platforms. Or I got your questions wrong. Any help here from others on
>> the list?
>>
>> ========================================================================
>> One possible extension to the above is to use an "iommus" property along
>> with a "dma-ranges" property in a bus device node (such as PCI host
>> bridges). This can be useful to describe how children on the bus relate
>> to the IOMMU if they are not explicitly listed in the device tree (e.g.
>> PCI devices). However, the requirements of that use-case haven't been
>> fully determined yet. Implementing this is therefore not recommended
>> without further discussion and extension of this binding.
>> =========================================================================
>
> This probably won't ever apply to PCI devices, so let's ignore it for now.
> For the moment (and for PCI), we should assume that we either configure
> an iommu directly or we use dma-ranges if no iommu is in use.
>
Thanks Arnd,
I will post v3 of the patch with what is agreed before in my response
and I understand there is no additional change required based on this
particular discussion about iommu. Right?
Murali
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 20:06 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 20:06 UTC (permalink / raw)
To: linux-arm-kernel
On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> On Friday 02 January 2015 17:33:53 Murali Karicheri wrote:
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> It's complicated :(
>
>> Looking at the iommu documentation and of_iommu.c, I get a feeling that
>> this API is not really used at present as there are no callers of
>> of_iommu_set_ops() and I assume this is a WIP.
>
> Right, but we have patches for some iommu drivers based on this API,
> and we should migrate all of them eventually.
>
>> I believe the way it is
>> expected to work is to have the iommu driver of the master IOMMU devices
>> call of_iommu_set_ops(). The device node of this master IOMMU device is
>> specified as a phandle in the OF node of the device (various bus devices
>> such as platform, PCI etc). This allow to retrieve the iommu ops though
>> the of_iommu_configure() API and use it in arch_setup_dma_ops(). So my
>> gut feeling is that for PCI devices, as there are no DT node, the root
>> bus node may specify iommus phandle to IOMMU master OF nodes.
>
> Yes, but we also need to pass a PCI device specific identifier along
> with the root bus node, because some iommu drivers take the PCI
> bus/device/function number into account for creating per-function
> i/o page tables.
>
>> W.r.t your comment "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",
>>
>> I believe, the parent node search itself should work the same way in the
>> case of PCI as with platform bus case. PCI's case, we are providing the
>> OF node of the root bus host bridge. Why should this be any different in
>> terms of search?
>>
>> I see a potential issue with dma-ranges as described in the notes below.
>> As noted below the usage of dma-range for iommu is to be determined. For
>> keystone, the of_iommu_configure() always return false as we don't use
>> the iommu. But don't know if this has any consequences for other
>> platforms. Or I got your questions wrong. Any help here from others on
>> the list?
>>
>> ========================================================================
>> One possible extension to the above is to use an "iommus" property along
>> with a "dma-ranges" property in a bus device node (such as PCI host
>> bridges). This can be useful to describe how children on the bus relate
>> to the IOMMU if they are not explicitly listed in the device tree (e.g.
>> PCI devices). However, the requirements of that use-case haven't been
>> fully determined yet. Implementing this is therefore not recommended
>> without further discussion and extension of this binding.
>> =========================================================================
>
> This probably won't ever apply to PCI devices, so let's ignore it for now.
> For the moment (and for PCI), we should assume that we either configure
> an iommu directly or we use dma-ranges if no iommu is in use.
>
Thanks Arnd,
I will post v3 of the patch with what is agreed before in my response
and I understand there is no additional change required based on this
particular discussion about iommu. Right?
Murali
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-05 20:06 ` Murali Karicheri
(?)
@ 2015-01-05 22:26 ` Arnd Bergmann
-1 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-05 22:26 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> > Yes, but we also need to pass a PCI device specific identifier along
> > with the root bus node, because some iommu drivers take the PCI
> > bus/device/function number into account for creating per-function
> > i/o page tables.
> ...
> I will post v3 of the patch with what is agreed before in my response
> and I understand there is no additional change required based on this
> particular discussion about iommu. Right?
Actually regarding the bit I wrote above, it might be helpful to pass
the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
While this may or may not be sufficient, I think there is no question
about it being needed for the ARM SMMU with PCI, so we may as well add
it at the point when you touch the same lines already. In the platform
bus case, just pass zero here.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 22:26 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-05 22:26 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> > Yes, but we also need to pass a PCI device specific identifier along
> > with the root bus node, because some iommu drivers take the PCI
> > bus/device/function number into account for creating per-function
> > i/o page tables.
> ...
> I will post v3 of the patch with what is agreed before in my response
> and I understand there is no additional change required based on this
> particular discussion about iommu. Right?
Actually regarding the bit I wrote above, it might be helpful to pass
the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
While this may or may not be sufficient, I think there is no question
about it being needed for the ARM SMMU with PCI, so we may as well add
it at the point when you touch the same lines already. In the platform
bus case, just pass zero here.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 22:26 ` Arnd Bergmann
0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2015-01-05 22:26 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
> > Yes, but we also need to pass a PCI device specific identifier along
> > with the root bus node, because some iommu drivers take the PCI
> > bus/device/function number into account for creating per-function
> > i/o page tables.
> ...
> I will post v3 of the patch with what is agreed before in my response
> and I understand there is no additional change required based on this
> particular discussion about iommu. Right?
Actually regarding the bit I wrote above, it might be helpful to pass
the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
While this may or may not be sufficient, I think there is no question
about it being needed for the ARM SMMU with PCI, so we may as well add
it at the point when you touch the same lines already. In the platform
bus case, just pass zero here.
Arnd
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-05 22:26 ` Arnd Bergmann
(?)
@ 2015-01-05 23:35 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 23:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/05/2015 05:26 PM, Arnd Bergmann wrote:
> On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
>> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
>>> Yes, but we also need to pass a PCI device specific identifier along
>>> with the root bus node, because some iommu drivers take the PCI
>>> bus/device/function number into account for creating per-function
>>> i/o page tables.
>
>> ...
>
>> I will post v3 of the patch with what is agreed before in my response
>> and I understand there is no additional change required based on this
>> particular discussion about iommu. Right?
>
> Actually regarding the bit I wrote above, it might be helpful to pass
> the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
>
> While this may or may not be sufficient, I think there is no question
> about it being needed for the ARM SMMU with PCI, so we may as well add
> it at the point when you touch the same lines already. In the platform
> bus case, just pass zero here.
Arnd,
PCI_DEVID() is defined as
#define PCI_DEVID(bus, devfn) ((((u16)(bus)) << 8) | (devfn))
So PCI_DEVID of 0 is a valid PCI DEVID.
How about checking if the device is PCI in of_iommu_configure() using
dev_is_pci(dev) macro and return immediately for PCI? Need to include
pci.h in this file though.
Some of the iommu drivers already include this.
a0868495@ares-ubuntu:~/projects/linux-keystone$ grep -r pci.h drivers/iommu/
drivers/iommu/amd_iommu_v2.c:#include <linux/pci.h>
drivers/iommu/dmar.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_types.h:#include <linux/pci.h>
drivers/iommu/amd_iommu.c:#include <linux/pci.h>
drivers/iommu/fsl_pamu_domain.c:#include <sysdev/fsl_pci.h>
drivers/iommu/iommu.c:#include <linux/pci.h>
drivers/iommu/intel-iommu.c:#include <linux/pci.h>
drivers/iommu/intel_irq_remapping.c:#include <linux/pci.h>
drivers/iommu/arm-smmu.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_init.c:#include <linux/pci.h>
drivers/iommu/irq_remapping.c:#include <linux/pci.h>
This will allow us to re-visit this later for IOMMU support for PCI
without polluting the API.
Murali
>
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 23:35 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 23:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Will Deacon, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, Grant Likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/05/2015 05:26 PM, Arnd Bergmann wrote:
> On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
>> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
>>> Yes, but we also need to pass a PCI device specific identifier along
>>> with the root bus node, because some iommu drivers take the PCI
>>> bus/device/function number into account for creating per-function
>>> i/o page tables.
>
>> ...
>
>> I will post v3 of the patch with what is agreed before in my response
>> and I understand there is no additional change required based on this
>> particular discussion about iommu. Right?
>
> Actually regarding the bit I wrote above, it might be helpful to pass
> the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
>
> While this may or may not be sufficient, I think there is no question
> about it being needed for the ARM SMMU with PCI, so we may as well add
> it at the point when you touch the same lines already. In the platform
> bus case, just pass zero here.
Arnd,
PCI_DEVID() is defined as
#define PCI_DEVID(bus, devfn) ((((u16)(bus)) << 8) | (devfn))
So PCI_DEVID of 0 is a valid PCI DEVID.
How about checking if the device is PCI in of_iommu_configure() using
dev_is_pci(dev) macro and return immediately for PCI? Need to include
pci.h in this file though.
Some of the iommu drivers already include this.
a0868495@ares-ubuntu:~/projects/linux-keystone$ grep -r pci.h drivers/iommu/
drivers/iommu/amd_iommu_v2.c:#include <linux/pci.h>
drivers/iommu/dmar.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_types.h:#include <linux/pci.h>
drivers/iommu/amd_iommu.c:#include <linux/pci.h>
drivers/iommu/fsl_pamu_domain.c:#include <sysdev/fsl_pci.h>
drivers/iommu/iommu.c:#include <linux/pci.h>
drivers/iommu/intel-iommu.c:#include <linux/pci.h>
drivers/iommu/intel_irq_remapping.c:#include <linux/pci.h>
drivers/iommu/arm-smmu.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_init.c:#include <linux/pci.h>
drivers/iommu/irq_remapping.c:#include <linux/pci.h>
This will allow us to re-visit this later for IOMMU support for PCI
without polluting the API.
Murali
>
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-05 23:35 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-05 23:35 UTC (permalink / raw)
To: linux-arm-kernel
On 01/05/2015 05:26 PM, Arnd Bergmann wrote:
> On Monday 05 January 2015 15:06:57 Murali Karicheri wrote:
>> On 01/03/2015 04:37 PM, Arnd Bergmann wrote:
>>> Yes, but we also need to pass a PCI device specific identifier along
>>> with the root bus node, because some iommu drivers take the PCI
>>> bus/device/function number into account for creating per-function
>>> i/o page tables.
>
>> ...
>
>> I will post v3 of the patch with what is agreed before in my response
>> and I understand there is no additional change required based on this
>> particular discussion about iommu. Right?
>
> Actually regarding the bit I wrote above, it might be helpful to pass
> the PCI_DEVID() into both of_iommu_configure and of_dma_configure.
>
> While this may or may not be sufficient, I think there is no question
> about it being needed for the ARM SMMU with PCI, so we may as well add
> it at the point when you touch the same lines already. In the platform
> bus case, just pass zero here.
Arnd,
PCI_DEVID() is defined as
#define PCI_DEVID(bus, devfn) ((((u16)(bus)) << 8) | (devfn))
So PCI_DEVID of 0 is a valid PCI DEVID.
How about checking if the device is PCI in of_iommu_configure() using
dev_is_pci(dev) macro and return immediately for PCI? Need to include
pci.h in this file though.
Some of the iommu drivers already include this.
a0868495@ares-ubuntu:~/projects/linux-keystone$ grep -r pci.h drivers/iommu/
drivers/iommu/amd_iommu_v2.c:#include <linux/pci.h>
drivers/iommu/dmar.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_types.h:#include <linux/pci.h>
drivers/iommu/amd_iommu.c:#include <linux/pci.h>
drivers/iommu/fsl_pamu_domain.c:#include <sysdev/fsl_pci.h>
drivers/iommu/iommu.c:#include <linux/pci.h>
drivers/iommu/intel-iommu.c:#include <linux/pci.h>
drivers/iommu/intel_irq_remapping.c:#include <linux/pci.h>
drivers/iommu/arm-smmu.c:#include <linux/pci.h>
drivers/iommu/amd_iommu_init.c:#include <linux/pci.h>
drivers/iommu/irq_remapping.c:#include <linux/pci.h>
This will allow us to re-visit this later for IOMMU support for PCI
without polluting the API.
Murali
>
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-02 22:33 ` Murali Karicheri
(?)
@ 2015-01-06 19:50 ` Will Deacon
-1 siblings, 0 replies; 49+ messages in thread
From: Will Deacon @ 2015-01-06 19:50 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Arnd Bergmann, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, grant.likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
> On 01/02/2015 03:45 PM, Rob Herring wrote:
> > On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
> >> On 12/26/2014 02:33 PM, Rob Herring wrote:
> >>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
> >>>> + 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.
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
Sorry for the delay; I'm still catching up on email after the Christmas
break, but I *will* get around to looking at this. If you have a new version
to post based on the comments from Rob and Arnd, feel free to send that and
I'll look at that instead.
Will
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-06 19:50 ` Will Deacon
0 siblings, 0 replies; 49+ messages in thread
From: Will Deacon @ 2015-01-06 19:50 UTC (permalink / raw)
To: Murali Karicheri
Cc: Rob Herring, Arnd Bergmann, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, grant.likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
> On 01/02/2015 03:45 PM, Rob Herring wrote:
> > On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
> >> On 12/26/2014 02:33 PM, Rob Herring wrote:
> >>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
> >>>> + 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.
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
Sorry for the delay; I'm still catching up on email after the Christmas
break, but I *will* get around to looking at this. If you have a new version
to post based on the comments from Rob and Arnd, feel free to send that and
I'll look at that instead.
Will
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-06 19:50 ` Will Deacon
0 siblings, 0 replies; 49+ messages in thread
From: Will Deacon @ 2015-01-06 19:50 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
> On 01/02/2015 03:45 PM, Rob Herring wrote:
> > On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
> >> On 12/26/2014 02:33 PM, Rob Herring wrote:
> >>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
> >>>> + 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.
>
> I have no experience with IOMMU and may not offer much help here as I
> originally wrote above. Will Deacon has added this API and probably able
> to offer some help in this discussion.
>
> Will Deacon,
>
> Any comment?
Sorry for the delay; I'm still catching up on email after the Christmas
break, but I *will* get around to looking at this. If you have a new version
to post based on the comments from Rob and Arnd, feel free to send that and
I'll look at that instead.
Will
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
2015-01-06 19:50 ` Will Deacon
(?)
(?)
@ 2015-01-06 21:08 ` Murali Karicheri
-1 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-06 21:08 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Arnd Bergmann, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, grant.likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/06/2015 02:50 PM, Will Deacon wrote:
> On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
>> On 01/02/2015 03:45 PM, Rob Herring wrote:
>>> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>>>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>>>>> + 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.
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> Sorry for the delay; I'm still catching up on email after the Christmas
> break, but I *will* get around to looking at this. If you have a new version
> to post based on the comments from Rob and Arnd, feel free to send that and
> I'll look at that instead.
>
> Will
Will,
I will be posting v3 of the patch tomorrow and you can review that.
Regards,
Murali
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-06 21:08 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-06 21:08 UTC (permalink / raw)
To: Will Deacon
Cc: devicetree, Russell King - ARM Linux, Arnd Bergmann, linux-pci,
linux-kernel, Bjorn Helgaas, Rob Herring, grant.likely,
linux-arm-kernel
On 01/06/2015 02:50 PM, Will Deacon wrote:
> On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
>> On 01/02/2015 03:45 PM, Rob Herring wrote:
>>> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>>>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>>>>> + 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.
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> Sorry for the delay; I'm still catching up on email after the Christmas
> break, but I *will* get around to looking at this. If you have a new version
> to post based on the comments from Rob and Arnd, feel free to send that and
> I'll look at that instead.
>
> Will
Will,
I will be posting v3 of the patch tomorrow and you can review that.
Regards,
Murali
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-06 21:08 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-06 21:08 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Arnd Bergmann, Russell King - ARM Linux,
linux-arm-kernel, linux-kernel, grant.likely, Rob Herring,
devicetree, Bjorn Helgaas, linux-pci
On 01/06/2015 02:50 PM, Will Deacon wrote:
> On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
>> On 01/02/2015 03:45 PM, Rob Herring wrote:
>>> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>>>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>>>>> + 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.
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> Sorry for the delay; I'm still catching up on email after the Christmas
> break, but I *will* get around to looking at this. If you have a new version
> to post based on the comments from Rob and Arnd, feel free to send that and
> I'll look at that instead.
>
> Will
Will,
I will be posting v3 of the patch tomorrow and you can review that.
Regards,
Murali
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration
@ 2015-01-06 21:08 ` Murali Karicheri
0 siblings, 0 replies; 49+ messages in thread
From: Murali Karicheri @ 2015-01-06 21:08 UTC (permalink / raw)
To: linux-arm-kernel
On 01/06/2015 02:50 PM, Will Deacon wrote:
> On Fri, Jan 02, 2015 at 10:33:53PM +0000, Murali Karicheri wrote:
>> On 01/02/2015 03:45 PM, Rob Herring wrote:
>>> On Fri, Jan 2, 2015 at 11:20 AM, Murali Karicheri<m-karicheri2@ti.com> wrote:
>>>> On 12/26/2014 02:33 PM, Rob Herring wrote:
>>>>> On Wed, Dec 24, 2014 at 4:11 PM, Murali Karicheri<m-karicheri2@ti.com>
>>>>>> + 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.
>>
>> I have no experience with IOMMU and may not offer much help here as I
>> originally wrote above. Will Deacon has added this API and probably able
>> to offer some help in this discussion.
>>
>> Will Deacon,
>>
>> Any comment?
>
> Sorry for the delay; I'm still catching up on email after the Christmas
> break, but I *will* get around to looking at this. If you have a new version
> to post based on the comments from Rob and Arnd, feel free to send that and
> I'll look at that instead.
>
> Will
Will,
I will be posting v3 of the patch tomorrow and you can review that.
Regards,
Murali
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2015-01-06 21:10 UTC | newest]
Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-24 22:11 [PATCH v2 0/2] PCI: get DMA configuration from parent device Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
2014-12-24 22:11 ` [PATCH v2 1/2] of/pci: add of_pci_dma_configure() update dma configuration Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
2014-12-26 19:33 ` Rob Herring
2014-12-26 19:33 ` Rob Herring
2014-12-26 19:33 ` Rob Herring
2015-01-02 17:20 ` Murali Karicheri
2015-01-02 17:20 ` Murali Karicheri
2015-01-02 17:20 ` Murali Karicheri
2015-01-02 20:45 ` Rob Herring
2015-01-02 20:45 ` Rob Herring
2015-01-02 20:45 ` Rob Herring
2015-01-02 22:33 ` Murali Karicheri
2015-01-02 22:33 ` Murali Karicheri
2015-01-02 22:33 ` Murali Karicheri
2015-01-03 21:37 ` Arnd Bergmann
2015-01-03 21:37 ` Arnd Bergmann
2015-01-03 21:37 ` Arnd Bergmann
2015-01-03 21:37 ` Arnd Bergmann
2015-01-05 20:06 ` Murali Karicheri
2015-01-05 20:06 ` Murali Karicheri
2015-01-05 20:06 ` Murali Karicheri
2015-01-05 20:06 ` Murali Karicheri
2015-01-05 22:26 ` Arnd Bergmann
2015-01-05 22:26 ` Arnd Bergmann
2015-01-05 22:26 ` Arnd Bergmann
2015-01-05 23:35 ` Murali Karicheri
2015-01-05 23:35 ` Murali Karicheri
2015-01-05 23:35 ` Murali Karicheri
2015-01-06 19:50 ` Will Deacon
2015-01-06 19:50 ` Will Deacon
2015-01-06 19:50 ` Will Deacon
2015-01-06 21:08 ` Murali Karicheri
2015-01-06 21:08 ` Murali Karicheri
2015-01-06 21:08 ` Murali Karicheri
2015-01-06 21:08 ` Murali Karicheri
2015-01-02 20:57 ` Arnd Bergmann
2015-01-02 20:57 ` Arnd Bergmann
2015-01-02 20:57 ` Arnd Bergmann
2015-01-02 21:12 ` Murali Karicheri
2015-01-02 21:12 ` Murali Karicheri
2015-01-02 21:12 ` Murali Karicheri
2015-01-02 21:12 ` Murali Karicheri
2014-12-24 22:11 ` [PATCH v2 2/2] PCI: update dma configuration from DT Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
2014-12-24 22:11 ` Murali Karicheri
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.