This patch add an important capability to PCI driver on Keystone. I hope to
have this merged to the upstream branch so that it is available for v3.20.
Also would like thank everyone for the contribution.
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/[email protected]/msg790244.html
[1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
Change history:
v6 - Rebased to v3.19-v7
- Addressed some minor comments about node name and DT size validation.
- Pulled out 8/8 of v5 and plan to send a patch for enhancing
of_dma_configure() to use size to calculate dma mask.
- Added Acks from reviewers.
v5 - moved the dma_mask update in device from ARM specific API to
of_dma_configure to allow this across other architecture as well
- improved sanity check for DT dma-range size in of_dma_configure()
- moved API to get parent bridge device to PCI (host-bridge.c)
v4 - moved size adjustments in of_iommu_configure() to a separate patch
- consistent node name comment from Rob
- patch 6 added for dma_mask adjustment and iommu mapping size
limiting.
v3 - addressed comments to re-use of_dma_configure() for PCI
- To help re-use, change of_iommu_configure() function argument
- Move of_dma_configure to of/device.c
- Limit the of_iommu_configure to non pci devices
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.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Acked-by: Murali Karicheri <[email protected]>
Murali Karicheri (7):
of: iommu: add ptr to OF node arg to of_iommu_configure()
of: move of_dma_configure() to device.c to help re-use
of: fix size when dma-range is not used
PCI: add helper functions pci_get[put]_host_bridge_device()
of/pci: add of_pci_dma_configure() update dma configuration
PCI: update dma configuration from DT
arm: dma-mapping: limit iommu mapping size
arch/arm/mm/dma-mapping.c | 7 +++++
drivers/iommu/of_iommu.c | 10 ++++--
drivers/of/device.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
drivers/of/of_pci.c | 19 ++++++++++++
drivers/of/platform.c | 58 ++---------------------------------
drivers/pci/host-bridge.c | 14 +++++++++
drivers/pci/probe.c | 2 ++
include/linux/of_device.h | 2 ++
include/linux/of_iommu.h | 6 ++--
include/linux/of_pci.h | 5 +++
include/linux/pci.h | 3 ++
11 files changed, 140 insertions(+), 60 deletions(-)
--
1.7.9.5
Function of_iommu_configure() is called from of_dma_configure() to
setup iommu ops using DT property. This API is currently used for
platform devices for which DMA configuration (including iommu ops)
may come from device's parent. To extend this functionality for PCI
devices, this API need to take a parent node ptr as an argument
instead of assuming device's parent. This is needed since for PCI, the
dma configuration may be defined in the DT node of the root bus bridge's
parent device. Currently only dma-range is used for PCI and iommu is not
supported. So return error if the device is PCI.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
drivers/iommu/of_iommu.c | 10 ++++++++--
drivers/of/platform.c | 2 +-
include/linux/of_iommu.h | 6 ++++--
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index af1dc6a..39c2c0c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
return ops;
}
-struct iommu_ops *of_iommu_configure(struct device *dev)
+struct iommu_ops *of_iommu_configure(struct device *dev,
+ struct device_node *master_np)
{
struct of_phandle_args iommu_spec;
struct device_node *np;
struct iommu_ops *ops = NULL;
int idx = 0;
+ if (dev_is_pci(dev)) {
+ dev_err(dev, "iommu is currently not supported for PCI\n");
+ return NULL;
+ }
+
/*
* 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",
+ while (!of_parse_phandle_with_args(master_np, "iommus",
"#iommu-cells", idx,
&iommu_spec)) {
np = iommu_spec.np;
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b0d50d7..d3f3988 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -196,7 +196,7 @@ static void of_dma_configure(struct device *dev)
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");
- iommu = of_iommu_configure(dev);
+ iommu = of_iommu_configure(dev, dev->of_node);
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 16c7554..ffbe470 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -12,7 +12,8 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
size_t *size);
extern void of_iommu_init(void);
-extern struct iommu_ops *of_iommu_configure(struct device *dev);
+extern struct iommu_ops *of_iommu_configure(struct device *dev,
+ struct device_node *master_np);
#else
@@ -24,7 +25,8 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
}
static inline void of_iommu_init(void) { }
-static inline struct iommu_ops *of_iommu_configure(struct device *dev)
+static inline struct iommu_ops *of_iommu_configure(struct device *dev,
+ struct device_node *master_np)
{
return NULL;
}
--
1.7.9.5
Move of_dma_configure() to device.c so that same function can be re-used
for PCI devices to obtain DMA configuration from DT. Also add a second
argument so that for PCI, DT node of root bus host bridge can be used to
obtain the DMA configuration for the slave PCI device.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
drivers/of/device.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 58 ++------------------------------------------
include/linux/of_device.h | 2 ++
3 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 46d6c75c..2de320d 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -2,6 +2,9 @@
#include <linux/kernel.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/of_iommu.h>
+#include <linux/dma-mapping.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
@@ -66,6 +69,62 @@ int of_device_add(struct platform_device *ofdev)
return device_add(&ofdev->dev);
}
+/**
+ * of_dma_configure - Setup DMA configuration
+ * @dev: Device to apply DMA configuration
+ * @np: ptr to of node having dma configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly.
+ *
+ * In case if platform code need to use own special DMA configuration,it
+ * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event
+ * to fix up DMA configuration.
+ */
+void of_dma_configure(struct device *dev, struct device_node *np)
+{
+ u64 dma_addr, paddr, size;
+ int ret;
+ bool coherent;
+ unsigned long offset;
+ struct iommu_ops *iommu;
+
+ /*
+ * 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(np, &dma_addr, &paddr, &size);
+ if (ret < 0) {
+ dma_addr = offset = 0;
+ size = dev->coherent_dma_mask;
+ } 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(np);
+ dev_dbg(dev, "device is%sdma coherent\n",
+ coherent ? " " : " not ");
+
+ iommu = of_iommu_configure(dev, np);
+ dev_dbg(dev, "device is%sbehind an iommu\n",
+ iommu ? " " : " not ");
+
+ arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+}
+EXPORT_SYMBOL_GPL(of_dma_configure);
+
int of_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index d3f3988..cbee18d 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -19,7 +19,6 @@
#include <linux/slab.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
-#include <linux/of_iommu.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
@@ -150,59 +149,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
}
EXPORT_SYMBOL(of_device_alloc);
-/**
- * of_dma_configure - Setup DMA configuration
- * @dev: Device to apply DMA configuration
- *
- * Try to get devices's DMA configuration from DT and update it
- * accordingly.
- *
- * In case if platform code need to use own special DMA configuration,it
- * can use Platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE event
- * to fix up DMA configuration.
- */
-static void of_dma_configure(struct device *dev)
-{
- u64 dma_addr, paddr, size;
- int ret;
- bool coherent;
- unsigned long offset;
- struct iommu_ops *iommu;
-
- /*
- * 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(dev->of_node, &dma_addr, &paddr, &size);
- if (ret < 0) {
- dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
- } 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(dev->of_node);
- dev_dbg(dev, "device is%sdma coherent\n",
- coherent ? " " : " not ");
-
- iommu = of_iommu_configure(dev, dev->of_node);
- dev_dbg(dev, "device is%sbehind an iommu\n",
- iommu ? " " : " not ");
-
- arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
-}
-
static void of_dma_deconfigure(struct device *dev)
{
arch_teardown_dma_ops(dev);
@@ -236,7 +182,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
- of_dma_configure(&dev->dev);
+ of_dma_configure(&dev->dev, dev->dev.of_node);
if (of_device_add(dev) != 0) {
of_dma_deconfigure(&dev->dev);
@@ -299,7 +245,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev_set_name(&dev->dev, "%s", bus_id);
else
of_device_make_bus_id(&dev->dev);
- of_dma_configure(&dev->dev);
+ of_dma_configure(&dev->dev, dev->dev.of_node);
/* Allow the HW Peripheral ID to be overridden */
prop = of_get_property(node, "arm,primecell-periphid", NULL);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index ef37021..c661496 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -53,6 +53,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}
+void of_dma_configure(struct device *dev, struct device_node *np);
#else /* CONFIG_OF */
static inline int of_driver_match_device(struct device *dev,
@@ -90,6 +91,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
{
return NULL;
}
+void of_dma_configure(struct device *dev, struct device_node *np) { }
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
--
1.7.9.5
Fix the dma-range size when the DT attribute is missing. i.e set size to
dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
code to check invalid values of size configured in DT and log error.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
drivers/of/device.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2de320d..314c8a9 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -105,9 +105,24 @@ void of_dma_configure(struct device *dev, struct device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
+ size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
+
+ /*
+ * Add a work around to treat the size as mask + 1 in case
+ * it is defined in DT as a mask.
+ */
+ if (size & 1) {
+ dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
+ size);
+ size = size + 1;
+ }
+
+ if (!size) {
+ dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
+ return;
+ }
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
--
1.7.9.5
Add a helper function to get/put the root bus's host bridge device.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
drivers/pci/host-bridge.c | 14 ++++++++++++++
include/linux/pci.h | 3 +++
2 files changed, 17 insertions(+)
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 0e5f3c9..f58e05b 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -23,6 +23,20 @@ static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
return to_pci_host_bridge(root_bus->bridge);
}
+struct device *pci_get_host_bridge_device(struct pci_dev *dev)
+{
+ struct pci_bus *root_bus = find_pci_root_bus(dev->bus);
+ struct device *bridge = root_bus->bridge;
+
+ kobject_get(&bridge->kobj);
+ return bridge;
+}
+
+void pci_put_host_bridge_device(struct device *dev)
+{
+ kobject_put(&dev->kobj);
+}
+
void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
void (*release_fn)(struct pci_host_bridge *),
void *release_data)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9603094..d677c66 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -513,6 +513,9 @@ static inline struct pci_dev *pci_upstream_bridge(struct pci_dev *dev)
return dev->bus->self;
}
+struct device *pci_get_host_bridge_device(struct pci_dev *dev);
+void pci_put_host_bridge_device(struct device *dev);
+
#ifdef CONFIG_PCI_MSI
static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev)
{
--
1.7.9.5
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. Use the newly added APIs
pci_get/put_host_bridge_device() for implementing this.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
drivers/of/of_pci.c | 19 +++++++++++++++++++
include/linux/of_pci.h | 5 +++++
2 files changed, 24 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 88471d3..0f1dd0b 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -2,6 +2,7 @@
#include <linux/export.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_pci.h>
#include <linux/slab.h>
@@ -229,6 +230,24 @@ parse_failed:
return err;
}
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+
+/**
+ * of_pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the pci device
+ *
+ * Function to update PCI devices's DMA configuration using the same
+ * info from the OF node of root host bridge's parent.
+ */
+void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+ struct device *dev = &pci_dev->dev;
+ struct device *bridge = pci_get_host_bridge_device(pci_dev);
+
+ of_dma_configure(dev, bridge->parent->of_node);
+ pci_put_host_bridge_device(bridge);
+}
+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..8f1741f 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -16,6 +16,7 @@ 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);
+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 +51,10 @@ of_get_pci_domain_nr(struct device_node *node)
{
return -1;
}
+
+static inline void of_pci_dma_configure(struct pci_dev *pci_dev)
+{
+}
#endif
#if defined(CONFIG_OF_ADDRESS)
--
1.7.9.5
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.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
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
arm_iommu_create_mapping() has size parameter of size_t and
arm_setup_iommu_dma_ops() can take a value higher than that
when this is called from the of code. So limit the size to
SIZE_MAX.
Cc: Joerg Roedel <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Signed-off-by: Murali Karicheri <[email protected]>
---
arch/arm/mm/dma-mapping.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index a673c7f..b05d907 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2027,6 +2027,13 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
if (!iommu)
return false;
+ /*
+ * currently arm_iommu_create_mapping() takes a max of size_t
+ * for size param. So check this limit for now.
+ */
+ if (size > SIZE_MAX)
+ return false;
+
mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
if (IS_ERR(mapping)) {
pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
--
1.7.9.5
Hi Arnd & Bjorn,
I believe this is ready to be merged so that it becomes available in
v3.20. Not sure which subsystem will pick this up. Could you respond?
I am working on another patch to address the size based dma mask
calculation as suggested in this thread and don't want to delay merge of
this series because of that for reason I have mentioned below in the
cover letter. So if this looks good, please merge this to the
appropriate subsystem.
Thanks a lot for the review and comments.
Murali
On 02/05/2015 04:52 PM, Murali Karicheri wrote:
> This patch add an important capability to PCI driver on Keystone. I hope to
> have this merged to the upstream branch so that it is available for v3.20.
> Also would like thank everyone for the contribution.
>
> 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/[email protected]/msg790244.html
> [1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
>
> Change history:
> v6 - Rebased to v3.19-v7
> - Addressed some minor comments about node name and DT size validation.
> - Pulled out 8/8 of v5 and plan to send a patch for enhancing
> of_dma_configure() to use size to calculate dma mask.
> - Added Acks from reviewers.
> v5 - moved the dma_mask update in device from ARM specific API to
> of_dma_configure to allow this across other architecture as well
> - improved sanity check for DT dma-range size in of_dma_configure()
> - moved API to get parent bridge device to PCI (host-bridge.c)
> v4 - moved size adjustments in of_iommu_configure() to a separate patch
> - consistent node name comment from Rob
> - patch 6 added for dma_mask adjustment and iommu mapping size
> limiting.
> v3 - addressed comments to re-use of_dma_configure() for PCI
> - To help re-use, change of_iommu_configure() function argument
> - Move of_dma_configure to of/device.c
> - Limit the of_iommu_configure to non pci devices
> 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.
>
> Cc: Joerg Roedel<[email protected]>
> Cc: Grant Likely<[email protected]>
> Cc: Rob Herring<[email protected]>
> Cc: Bjorn Helgaas<[email protected]>
> Cc: Will Deacon<[email protected]>
> Cc: Russell King<[email protected]>
> Cc: Arnd Bergmann<[email protected]>
> Cc: Suravee Suthikulpanit<[email protected]>
>
> Acked-by: Bjorn Helgaas<[email protected]>
> Acked-by: Murali Karicheri<[email protected]>
>
> Murali Karicheri (7):
> of: iommu: add ptr to OF node arg to of_iommu_configure()
> of: move of_dma_configure() to device.c to help re-use
> of: fix size when dma-range is not used
> PCI: add helper functions pci_get[put]_host_bridge_device()
> of/pci: add of_pci_dma_configure() update dma configuration
> PCI: update dma configuration from DT
> arm: dma-mapping: limit iommu mapping size
>
> arch/arm/mm/dma-mapping.c | 7 +++++
> drivers/iommu/of_iommu.c | 10 ++++--
> drivers/of/device.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/of_pci.c | 19 ++++++++++++
> drivers/of/platform.c | 58 ++---------------------------------
> drivers/pci/host-bridge.c | 14 +++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/of_device.h | 2 ++
> include/linux/of_iommu.h | 6 ++--
> include/linux/of_pci.h | 5 +++
> include/linux/pci.h | 3 ++
> 11 files changed, 140 insertions(+), 60 deletions(-)
>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Thu, Feb 05, 2015 at 09:52:55PM +0000, Murali Karicheri wrote:
> Fix the dma-range size when the DT attribute is missing. i.e set size to
> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
> code to check invalid values of size configured in DT and log error.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Suravee Suthikulpanit <[email protected]>
>
> Signed-off-by: Murali Karicheri <[email protected]>
> ---
> drivers/of/device.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 2de320d..314c8a9 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -105,9 +105,24 @@ void of_dma_configure(struct device *dev, struct device_node *np)
> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> if (ret < 0) {
> dma_addr = offset = 0;
> - size = dev->coherent_dma_mask;
> + size = dev->coherent_dma_mask + 1;
> } else {
> offset = PFN_DOWN(paddr - dma_addr);
> +
> + /*
> + * Add a work around to treat the size as mask + 1 in case
> + * it is defined in DT as a mask.
> + */
> + if (size & 1) {
> + dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> + size);
> + size = size + 1;
> + }
> +
> + if (!size) {
> + dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> + return;
> + }
Would it make sense to set coherent_dma_mask to 0 here to make this more
noticeable? It can be done together with the mask calculation from size.
--
Catalin
On 02/06/2015 09:38 AM, Catalin Marinas wrote:
> On Thu, Feb 05, 2015 at 09:52:55PM +0000, Murali Karicheri wrote:
>> Fix the dma-range size when the DT attribute is missing. i.e set size to
>> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
>> code to check invalid values of size configured in DT and log error.
>>
>> Cc: Joerg Roedel<[email protected]>
>> Cc: Grant Likely<[email protected]>
>> Cc: Rob Herring<[email protected]>
>> Cc: Bjorn Helgaas<[email protected]>
>> Cc: Will Deacon<[email protected]>
>> Cc: Russell King<[email protected]>
>> Cc: Arnd Bergmann<[email protected]>
>> Cc: Suravee Suthikulpanit<[email protected]>
>>
>> Signed-off-by: Murali Karicheri<[email protected]>
>> ---
>> drivers/of/device.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 2de320d..314c8a9 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -105,9 +105,24 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>> ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
>> if (ret< 0) {
>> dma_addr = offset = 0;
>> - size = dev->coherent_dma_mask;
>> + size = dev->coherent_dma_mask + 1;
>> } else {
>> offset = PFN_DOWN(paddr - dma_addr);
>> +
>> + /*
>> + * Add a work around to treat the size as mask + 1 in case
>> + * it is defined in DT as a mask.
>> + */
>> + if (size& 1) {
>> + dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
>> + size);
>> + size = size + 1;
>> + }
>> +
>> + if (!size) {
>> + dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
>> + return;
>> + }
>
> Would it make sense to set coherent_dma_mask to 0 here to make this more
> noticeable? It can be done together with the mask calculation from size.
>
I guess you are the following in the code.
if (!size) {
dev->coherent_dma_mask = 0;
dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
return;
}
Not sure how this is going to help and how this get handled by the
caller and subsequent logic. Probably it will cause probe to fail, with
some helpful error code. Need to try this out. Will do before sending my
size based dma mask patch later today.
Murali
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Fri, Feb 06, 2015 at 02:54:23PM +0000, Murali Karicheri wrote:
> On 02/06/2015 09:38 AM, Catalin Marinas wrote:
> > On Thu, Feb 05, 2015 at 09:52:55PM +0000, Murali Karicheri wrote:
> >> Fix the dma-range size when the DT attribute is missing. i.e set size to
> >> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
> >> code to check invalid values of size configured in DT and log error.
> >>
> >> Cc: Joerg Roedel<[email protected]>
> >> Cc: Grant Likely<[email protected]>
> >> Cc: Rob Herring<[email protected]>
> >> Cc: Bjorn Helgaas<[email protected]>
> >> Cc: Will Deacon<[email protected]>
> >> Cc: Russell King<[email protected]>
> >> Cc: Arnd Bergmann<[email protected]>
> >> Cc: Suravee Suthikulpanit<[email protected]>
> >>
> >> Signed-off-by: Murali Karicheri<[email protected]>
> >> ---
> >> drivers/of/device.c | 17 ++++++++++++++++-
> >> 1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/of/device.c b/drivers/of/device.c
> >> index 2de320d..314c8a9 100644
> >> --- a/drivers/of/device.c
> >> +++ b/drivers/of/device.c
> >> @@ -105,9 +105,24 @@ void of_dma_configure(struct device *dev, struct device_node *np)
> >> ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
> >> if (ret< 0) {
> >> dma_addr = offset = 0;
> >> - size = dev->coherent_dma_mask;
> >> + size = dev->coherent_dma_mask + 1;
> >> } else {
> >> offset = PFN_DOWN(paddr - dma_addr);
> >> +
> >> + /*
> >> + * Add a work around to treat the size as mask + 1 in case
> >> + * it is defined in DT as a mask.
> >> + */
> >> + if (size& 1) {
> >> + dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> >> + size);
> >> + size = size + 1;
> >> + }
> >> +
> >> + if (!size) {
> >> + dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> >> + return;
> >> + }
> >
> > Would it make sense to set coherent_dma_mask to 0 here to make this more
> > noticeable? It can be done together with the mask calculation from size.
>
> I guess you are the following in the code.
>
> if (!size) {
> dev->coherent_dma_mask = 0;
> dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> return;
> }
>
> Not sure how this is going to help and how this get handled by the
> caller and subsequent logic. Probably it will cause probe to fail, with
> some helpful error code.
Not sure how it will fail, maybe the driver figures out that DMA isn't
available and say something or switch to PIO. I guess you can leave it
as 32-bit mask by default for now even in case of size == 0.
BTW, since pci_device_add() already sets coherent_dma_mask, you could
add another check at the beginning of of_dma_configure():
if (!dev->coherent_dma_mask)
dev->coherent_dma_mask = DMA_BIT_MASK(32);
That's more of a nitpick as the values are both the same.
--
Catalin
On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
> This patch add an important capability to PCI driver on Keystone. I hope to
> have this merged to the upstream branch so that it is available for v3.20.
It's very late for 3.20 and the code hasn't been in linux-next at all
(but it's not me who's merging this code), unless you can squeeze it in
as a bug-fix.
But the good part is that there is more time to fix the dma mask setting
as well ;).
> 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.
For the series, you can add:
Reviewed-by: Catalin Marinas <[email protected]>
On 02/06/2015 10:15 AM, Catalin Marinas wrote:
> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>> This patch add an important capability to PCI driver on Keystone. I hope to
>> have this merged to the upstream branch so that it is available for v3.20.
> It's very late for 3.20 and the code hasn't been in linux-next at all
> (but it's not me who's merging this code), unless you can squeeze it in
> as a bug-fix.
This is in fact a bug fix as PCI on Keystone is broken without this.
> But the good part is that there is more time to fix the dma mask setting
> as well ;).
>
>> 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.
> For the series, you can add:
>
> Reviewed-by: Catalin Marinas<[email protected]>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri <[email protected]> wrote:
> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>
>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>
>>> This patch add an important capability to PCI driver on Keystone. I hope
>>> to
>>> have this merged to the upstream branch so that it is available for
>>> v3.20.
>>
>> It's very late for 3.20 and the code hasn't been in linux-next at all
>> (but it's not me who's merging this code), unless you can squeeze it in
>> as a bug-fix.
>
> This is in fact a bug fix as PCI on Keystone is broken without this.
Oh, sorry, I didn't realize that this was so urgent. I guess I read
"this adds an important capability" in the cover letter and concluded
that it was new functionality.
Anyway, if it's broken, presumably PCI on Keystone *did* work at one
point. Can you identify the commit that broke it and requires these
fixes, so we can figure out how far the fixes need to be backported?
If I merge it, I would like to get into my for-linus branch and get a
little time in -next before asking Linus to pull it. The merge window
is a little wrinkle there -- I don't like to add new things to the mix
during the window. But if it's an important fix we can still get it
in before the final v3.20.
Bjorn
On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]> wrote:
>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>
>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>
>>>> This patch add an important capability to PCI driver on Keystone. I hope
>>>> to
>>>> have this merged to the upstream branch so that it is available for
>>>> v3.20.
>>>
>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>> (but it's not me who's merging this code), unless you can squeeze it in
>>> as a bug-fix.
>>
>> This is in fact a bug fix as PCI on Keystone is broken without this.
>
> Oh, sorry, I didn't realize that this was so urgent. I guess I read
> "this adds an important capability" in the cover letter and concluded
> that it was new functionality.
Bjorn,
Thanks for responding.
Let me give you some context on this without which my explanation won't
be complete. For using PCI driver on Keystone, I had submitted patches
related to machine and DTS to the arm mailing list to enable the driver
on Keystone. Subsequenty one of the patch from my series was Nack-ed
and I was asked to implememented in a different way and started this
series. You can refer to the discussion of this at
http://www.gossamer-threads.com/lists/linux/kernel/2024591
The PCI driver enablement on Keystone is still a working in progress and
I am trying to get it fully functional on the upstream. Another missing
piece is the SerDes phy driver patch. We have started working on the
other part (SerDes phy driver) already as the initial one was not
accepted. So it is fine if we are too late for the v3.20 merge window to
merge this series and this can be applied to the next branch for v3.21.
>
> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
> point. Can you identify the commit that broke it and requires these
> fixes, so we can figure out how far the fixes need to be backported?
>
I am trying to get this driver enabled on Keystone by adding the missing
pieces as described above. So I guess we don't have to back port
anything here.
> If I merge it, I would like to get into my for-linus branch and get a
> little time in -next before asking Linus to pull it. The merge window
> is a little wrinkle there -- I don't like to add new things to the mix
> during the window. But if it's an important fix we can still get it
> in before the final v3.20.
Please apply this to next branch for v3.21. It currently apply cleanly
to v3.19-rc7. If you want me rebase to another branch, let me know I can
apply and send you an updated patch.
Thanks
Murali
>
> Bjorn
--
Murali Karicheri
Linux Kernel, Texas Instruments
On 02/06/2015 10:12 AM, Catalin Marinas wrote:
> On Fri, Feb 06, 2015 at 02:54:23PM +0000, Murali Karicheri wrote:
>> On 02/06/2015 09:38 AM, Catalin Marinas wrote:
>>> On Thu, Feb 05, 2015 at 09:52:55PM +0000, Murali Karicheri wrote:
>>>> Fix the dma-range size when the DT attribute is missing. i.e set size to
>>>> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
>>>> code to check invalid values of size configured in DT and log error.
>>>>
>>>> Cc: Joerg Roedel<[email protected]>
>>>> Cc: Grant Likely<[email protected]>
>>>> Cc: Rob Herring<[email protected]>
>>>> Cc: Bjorn Helgaas<[email protected]>
>>>> Cc: Will Deacon<[email protected]>
>>>> Cc: Russell King<[email protected]>
>>>> Cc: Arnd Bergmann<[email protected]>
>>>> Cc: Suravee Suthikulpanit<[email protected]>
>>>>
>>>> Signed-off-by: Murali Karicheri<[email protected]>
>>>> ---
>>>> drivers/of/device.c | 17 ++++++++++++++++-
>>>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>>>> index 2de320d..314c8a9 100644
>>>> --- a/drivers/of/device.c
>>>> +++ b/drivers/of/device.c
>>>> @@ -105,9 +105,24 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>>>> ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
>>>> if (ret< 0) {
>>>> dma_addr = offset = 0;
>>>> - size = dev->coherent_dma_mask;
>>>> + size = dev->coherent_dma_mask + 1;
>>>> } else {
>>>> offset = PFN_DOWN(paddr - dma_addr);
>>>> +
>>>> + /*
>>>> + * Add a work around to treat the size as mask + 1 in case
>>>> + * it is defined in DT as a mask.
>>>> + */
>>>> + if (size& 1) {
>>>> + dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
>>>> + size);
>>>> + size = size + 1;
>>>> + }
>>>> +
>>>> + if (!size) {
>>>> + dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
>>>> + return;
>>>> + }
>>>
>>> Would it make sense to set coherent_dma_mask to 0 here to make this more
>>> noticeable? It can be done together with the mask calculation from size.
>>
>> I guess you are the following in the code.
>>
>> if (!size) {
>> dev->coherent_dma_mask = 0;
>> dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
>> return;
>> }
>>
>> Not sure how this is going to help and how this get handled by the
>> caller and subsequent logic. Probably it will cause probe to fail, with
>> some helpful error code.
>
> Not sure how it will fail, maybe the driver figures out that DMA isn't
> available and say something or switch to PIO. I guess you can leave it
> as 32-bit mask by default for now even in case of size == 0.
>
> BTW, since pci_device_add() already sets coherent_dma_mask, you could
> add another check at the beginning of of_dma_configure():
>
> if (!dev->coherent_dma_mask)
> dev->coherent_dma_mask = DMA_BIT_MASK(32);
>
> That's more of a nitpick as the values are both the same.
>
Catalin,
Just posted a patch for size based dma mask with title "of: calculate
masks of the device based on dma-range size". Please review and comment
--
Murali Karicheri
Linux Kernel, Texas Instruments
Sorry for delay response. I have also tested this on AMD Seattle
platform w/ PCI Generic Host Controller, and I can see that the PCI
endpoint devices are getting proper dma_map_ops as set in the host bridge.
<Tested-by>: Suravee Suthikulpanit <[email protected]>
Thanks,
Suravee
On 02/06/2015 05:52 AM, Murali Karicheri wrote:
> This patch add an important capability to PCI driver on Keystone. I hope to
> have this merged to the upstream branch so that it is available for v3.20.
> Also would like thank everyone for the contribution.
>
> 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/[email protected]/msg790244.html
> [1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
>
> Change history:
> v6 - Rebased to v3.19-v7
> - Addressed some minor comments about node name and DT size validation.
> - Pulled out 8/8 of v5 and plan to send a patch for enhancing
> of_dma_configure() to use size to calculate dma mask.
> - Added Acks from reviewers.
> v5 - moved the dma_mask update in device from ARM specific API to
> of_dma_configure to allow this across other architecture as well
> - improved sanity check for DT dma-range size in of_dma_configure()
> - moved API to get parent bridge device to PCI (host-bridge.c)
> v4 - moved size adjustments in of_iommu_configure() to a separate patch
> - consistent node name comment from Rob
> - patch 6 added for dma_mask adjustment and iommu mapping size
> limiting.
> v3 - addressed comments to re-use of_dma_configure() for PCI
> - To help re-use, change of_iommu_configure() function argument
> - Move of_dma_configure to of/device.c
> - Limit the of_iommu_configure to non pci devices
> 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.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Suravee Suthikulpanit <[email protected]>
>
> Acked-by: Bjorn Helgaas <[email protected]>
> Acked-by: Murali Karicheri <[email protected]>
>
> Murali Karicheri (7):
> of: iommu: add ptr to OF node arg to of_iommu_configure()
> of: move of_dma_configure() to device.c to help re-use
> of: fix size when dma-range is not used
> PCI: add helper functions pci_get[put]_host_bridge_device()
> of/pci: add of_pci_dma_configure() update dma configuration
> PCI: update dma configuration from DT
> arm: dma-mapping: limit iommu mapping size
>
> arch/arm/mm/dma-mapping.c | 7 +++++
> drivers/iommu/of_iommu.c | 10 ++++--
> drivers/of/device.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/of_pci.c | 19 ++++++++++++
> drivers/of/platform.c | 58 ++---------------------------------
> drivers/pci/host-bridge.c | 14 +++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/of_device.h | 2 ++
> include/linux/of_iommu.h | 6 ++--
> include/linux/of_pci.h | 5 +++
> include/linux/pci.h | 3 ++
> 11 files changed, 140 insertions(+), 60 deletions(-)
>
On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
> This patch add an important capability to PCI driver on Keystone. I hope to
> have this merged to the upstream branch so that it is available for v3.20.
> Also would like thank everyone for the contribution.
>
> 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/[email protected]/msg790244.html
> [1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
>
> Change history:
> v6 - Rebased to v3.19-v7
> - Addressed some minor comments about node name and DT size validation.
> - Pulled out 8/8 of v5 and plan to send a patch for enhancing
> of_dma_configure() to use size to calculate dma mask.
> - Added Acks from reviewers.
This series looks fine to me:
Acked-by: Will Deacon <[email protected]>
Will
On 02/09/2015 12:23 AM, Suravee Suthikulpanit wrote:
> Sorry for delay response. I have also tested this on AMD Seattle
> platform w/ PCI Generic Host Controller, and I can see that the PCI
> endpoint devices are getting proper dma_map_ops as set in the host
> bridge.
>
> <Tested-by>: Suravee Suthikulpanit <[email protected]>
Suravee,
Thanks for testing this patch.
Murali
>
> Thanks,
> Suravee
>
> On 02/06/2015 05:52 AM, Murali Karicheri wrote:
>> This patch add an important capability to PCI driver on Keystone. I
>> hope to
>> have this merged to the upstream branch so that it is available for
>> v3.20.
>> Also would like thank everyone for the contribution.
>>
>> 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/[email protected]/msg790244.html
>> [1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
>>
>> Change history:
>> v6 - Rebased to v3.19-v7
>> - Addressed some minor comments about node name and DT size
>> validation.
>> - Pulled out 8/8 of v5 and plan to send a patch for enhancing
>> of_dma_configure() to use size to calculate dma mask.
>> - Added Acks from reviewers.
>> v5 - moved the dma_mask update in device from ARM specific API to
>> of_dma_configure to allow this across other architecture as
>> well
>> - improved sanity check for DT dma-range size in
>> of_dma_configure()
>> - moved API to get parent bridge device to PCI (host-bridge.c)
>> v4 - moved size adjustments in of_iommu_configure() to a separate
>> patch
>> - consistent node name comment from Rob
>> - patch 6 added for dma_mask adjustment and iommu mapping size
>> limiting.
>> v3 - addressed comments to re-use of_dma_configure() for PCI
>> - To help re-use, change of_iommu_configure() function argument
>> - Move of_dma_configure to of/device.c
>> - Limit the of_iommu_configure to non pci devices
>> 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.
>>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Suravee Suthikulpanit <[email protected]>
>>
>> Acked-by: Bjorn Helgaas <[email protected]>
>> Acked-by: Murali Karicheri <[email protected]>
>>
>> Murali Karicheri (7):
>> of: iommu: add ptr to OF node arg to of_iommu_configure()
>> of: move of_dma_configure() to device.c to help re-use
>> of: fix size when dma-range is not used
>> PCI: add helper functions pci_get[put]_host_bridge_device()
>> of/pci: add of_pci_dma_configure() update dma configuration
>> PCI: update dma configuration from DT
>> arm: dma-mapping: limit iommu mapping size
>>
>> arch/arm/mm/dma-mapping.c | 7 +++++
>> drivers/iommu/of_iommu.c | 10 ++++--
>> drivers/of/device.c | 74
>> +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/of/of_pci.c | 19 ++++++++++++
>> drivers/of/platform.c | 58 ++---------------------------------
>> drivers/pci/host-bridge.c | 14 +++++++++
>> drivers/pci/probe.c | 2 ++
>> include/linux/of_device.h | 2 ++
>> include/linux/of_iommu.h | 6 ++--
>> include/linux/of_pci.h | 5 +++
>> include/linux/pci.h | 3 ++
>> 11 files changed, 140 insertions(+), 60 deletions(-)
>>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On 02/06/2015 01:36 PM, Murali Karicheri wrote:
> On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
>> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]>
>> wrote:
>>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>>
>>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>>
>>>>> This patch add an important capability to PCI driver on Keystone. I
>>>>> hope
>>>>> to
>>>>> have this merged to the upstream branch so that it is available for
>>>>> v3.20.
>>>>
>>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>>> (but it's not me who's merging this code), unless you can squeeze it in
>>>> as a bug-fix.
>>>
>>> This is in fact a bug fix as PCI on Keystone is broken without this.
>>
>> Oh, sorry, I didn't realize that this was so urgent. I guess I read
>> "this adds an important capability" in the cover letter and concluded
>> that it was new functionality.
> Bjorn,
>
> Thanks for responding.
>
> Let me give you some context on this without which my explanation won't
> be complete. For using PCI driver on Keystone, I had submitted patches
> related to machine and DTS to the arm mailing list to enable the driver
> on Keystone. Subsequenty one of the patch from my series was Nack-ed and
> I was asked to implememented in a different way and started this series.
> You can refer to the discussion of this at
>
> http://www.gossamer-threads.com/lists/linux/kernel/2024591
>
> The PCI driver enablement on Keystone is still a working in progress and
> I am trying to get it fully functional on the upstream. Another missing
> piece is the SerDes phy driver patch. We have started working on the
> other part (SerDes phy driver) already as the initial one was not
> accepted. So it is fine if we are too late for the v3.20 merge window to
> merge this series and this can be applied to the next branch for v3.21.
>
>>
>> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
>> point. Can you identify the commit that broke it and requires these
>> fixes, so we can figure out how far the fixes need to be backported?
>>
>
> I am trying to get this driver enabled on Keystone by adding the missing
> pieces as described above. So I guess we don't have to back port
> anything here.
>
>> If I merge it, I would like to get into my for-linus branch and get a
>> little time in -next before asking Linus to pull it. The merge window
>> is a little wrinkle there -- I don't like to add new things to the mix
>> during the window. But if it's an important fix we can still get it
>> in before the final v3.20.
>
> Please apply this to next branch for v3.21. It currently apply cleanly
> to v3.19-rc7. If you want me rebase to another branch, let me know I can
> apply and send you an updated patch.
>
Bjorn, Arnd,
I am assuming, Bjorn is going to merge this to his next branch for
v3.21. If not, it might have to be merged through the arm soc? There are
a couple of Tested-by and Acked-by received after v7. Do you want me to
post v8 with these updated in the patches?
Murali
> Thanks
>
> Murali
>
>>
>> Bjorn
>
>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On 02/11/2015 11:54 AM, Murali Karicheri wrote:
> On 02/06/2015 01:36 PM, Murali Karicheri wrote:
>> On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
>>> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]>
>>> wrote:
>>>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>>>
>>>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>>>
>>>>>> This patch add an important capability to PCI driver on Keystone. I
>>>>>> hope
>>>>>> to
>>>>>> have this merged to the upstream branch so that it is available for
>>>>>> v3.20.
>>>>>
>>>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>>>> (but it's not me who's merging this code), unless you can squeeze
>>>>> it in
>>>>> as a bug-fix.
>>>>
>>>> This is in fact a bug fix as PCI on Keystone is broken without this.
>>>
>>> Oh, sorry, I didn't realize that this was so urgent. I guess I read
>>> "this adds an important capability" in the cover letter and concluded
>>> that it was new functionality.
>> Bjorn,
>>
>> Thanks for responding.
>>
>> Let me give you some context on this without which my explanation won't
>> be complete. For using PCI driver on Keystone, I had submitted patches
>> related to machine and DTS to the arm mailing list to enable the driver
>> on Keystone. Subsequenty one of the patch from my series was Nack-ed and
>> I was asked to implememented in a different way and started this series.
>> You can refer to the discussion of this at
>>
>> http://www.gossamer-threads.com/lists/linux/kernel/2024591
>>
>> The PCI driver enablement on Keystone is still a working in progress and
>> I am trying to get it fully functional on the upstream. Another missing
>> piece is the SerDes phy driver patch. We have started working on the
>> other part (SerDes phy driver) already as the initial one was not
>> accepted. So it is fine if we are too late for the v3.20 merge window to
>> merge this series and this can be applied to the next branch for v3.21.
>>
>>>
>>> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
>>> point. Can you identify the commit that broke it and requires these
>>> fixes, so we can figure out how far the fixes need to be backported?
>>>
>>
>> I am trying to get this driver enabled on Keystone by adding the missing
>> pieces as described above. So I guess we don't have to back port
>> anything here.
>>
>>> If I merge it, I would like to get into my for-linus branch and get a
>>> little time in -next before asking Linus to pull it. The merge window
>>> is a little wrinkle there -- I don't like to add new things to the mix
>>> during the window. But if it's an important fix we can still get it
>>> in before the final v3.20.
>>
>> Please apply this to next branch for v3.21. It currently apply cleanly
>> to v3.19-rc7. If you want me rebase to another branch, let me know I can
>> apply and send you an updated patch.
>>
> Bjorn, Arnd,
>
> I am assuming, Bjorn is going to merge this to his next branch for
> v3.21. If not, it might have to be merged through the arm soc? There are
> a couple of Tested-by and Acked-by received after v7. Do you want me to
> post v8 with these updated in the patches?
FYI.
These are the updates.
Series was
1) Tested-by: Suravee Suthikulpanit <[email protected]>
(on AMD Seattle platform w/ PCI Generic Host controller)
2) Acked-by: Will Deacon <[email protected]>
3) Reviewed-by: Catalin Marinas <[email protected]>
If you want to send a updated series with these, please let me know.
Thanks and regards,
Murali
>
> Murali
>
>> Thanks
>>
>> Murali
>>
>>>
>>> Bjorn
>>
>>
>
>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On 02/11/2015 11:58 AM, Murali Karicheri wrote:
> On 02/11/2015 11:54 AM, Murali Karicheri wrote:
>> On 02/06/2015 01:36 PM, Murali Karicheri wrote:
>>> On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
>>>> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]>
>>>> wrote:
>>>>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>>>>
>>>>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>>>>
>>>>>>> This patch add an important capability to PCI driver on Keystone. I
>>>>>>> hope
>>>>>>> to
>>>>>>> have this merged to the upstream branch so that it is available for
>>>>>>> v3.20.
>>>>>>
>>>>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>>>>> (but it's not me who's merging this code), unless you can squeeze
>>>>>> it in
>>>>>> as a bug-fix.
>>>>>
>>>>> This is in fact a bug fix as PCI on Keystone is broken without this.
>>>>
>>>> Oh, sorry, I didn't realize that this was so urgent. I guess I read
>>>> "this adds an important capability" in the cover letter and concluded
>>>> that it was new functionality.
>>> Bjorn,
>>>
>>> Thanks for responding.
>>>
>>> Let me give you some context on this without which my explanation won't
>>> be complete. For using PCI driver on Keystone, I had submitted patches
>>> related to machine and DTS to the arm mailing list to enable the driver
>>> on Keystone. Subsequenty one of the patch from my series was Nack-ed and
>>> I was asked to implememented in a different way and started this series.
>>> You can refer to the discussion of this at
>>>
>>> http://www.gossamer-threads.com/lists/linux/kernel/2024591
>>>
>>> The PCI driver enablement on Keystone is still a working in progress and
>>> I am trying to get it fully functional on the upstream. Another missing
>>> piece is the SerDes phy driver patch. We have started working on the
>>> other part (SerDes phy driver) already as the initial one was not
>>> accepted. So it is fine if we are too late for the v3.20 merge window to
>>> merge this series and this can be applied to the next branch for v3.21.
>>>
>>>>
>>>> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
>>>> point. Can you identify the commit that broke it and requires these
>>>> fixes, so we can figure out how far the fixes need to be backported?
>>>>
>>>
>>> I am trying to get this driver enabled on Keystone by adding the missing
>>> pieces as described above. So I guess we don't have to back port
>>> anything here.
>>>
>>>> If I merge it, I would like to get into my for-linus branch and get a
>>>> little time in -next before asking Linus to pull it. The merge window
>>>> is a little wrinkle there -- I don't like to add new things to the mix
>>>> during the window. But if it's an important fix we can still get it
>>>> in before the final v3.20.
>>>
>>> Please apply this to next branch for v3.21. It currently apply cleanly
>>> to v3.19-rc7. If you want me rebase to another branch, let me know I can
>>> apply and send you an updated patch.
>>>
>> Bjorn, Arnd,
>>
>> I am assuming, Bjorn is going to merge this to his next branch for
>> v3.21. If not, it might have to be merged through the arm soc? There are
>> a couple of Tested-by and Acked-by received after v7. Do you want me to
>> post v8 with these updated in the patches?
> FYI.
>
> These are the updates.
> Series was
>
> 1) Tested-by: Suravee Suthikulpanit <[email protected]>
> (on AMD Seattle platform w/ PCI Generic Host controller)
> 2) Acked-by: Will Deacon <[email protected]>
> 3) Reviewed-by: Catalin Marinas <[email protected]>
>
Bjorn,
I haven't seen a reply from my above email. As soon as you are ready to
pull this to v4.1 next (originally requested for v3.21 as per above
email)branch, please let me know and I can send you an updated version
rebased to your next branch and with the above acks.
Thanks and regards,
Murali
> If you want to send a updated series with these, please let me know.
>
> Thanks and regards,
>
> Murali
>>
>> Murali
>>
>>> Thanks
>>>
>>> Murali
>>>
>>>>
>>>> Bjorn
>>>
>>>
>>
>>
>
>
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Mon, Feb 23, 2015 at 4:08 PM, Murali Karicheri <[email protected]> wrote:
> On 02/11/2015 11:58 AM, Murali Karicheri wrote:
>>
>> On 02/11/2015 11:54 AM, Murali Karicheri wrote:
>>>
>>> On 02/06/2015 01:36 PM, Murali Karicheri wrote:
>>>>
>>>> On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
>>>>>
>>>>> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]>
>>>>> wrote:
>>>>>>
>>>>>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> This patch add an important capability to PCI driver on Keystone. I
>>>>>>>> hope
>>>>>>>> to
>>>>>>>> have this merged to the upstream branch so that it is available for
>>>>>>>> v3.20.
>>>>>>>
>>>>>>>
>>>>>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>>>>>> (but it's not me who's merging this code), unless you can squeeze
>>>>>>> it in
>>>>>>> as a bug-fix.
>>>>>>
>>>>>>
>>>>>> This is in fact a bug fix as PCI on Keystone is broken without this.
>>>>>
>>>>>
>>>>> Oh, sorry, I didn't realize that this was so urgent. I guess I read
>>>>> "this adds an important capability" in the cover letter and concluded
>>>>> that it was new functionality.
>>>>
>>>> Bjorn,
>>>>
>>>> Thanks for responding.
>>>>
>>>> Let me give you some context on this without which my explanation won't
>>>> be complete. For using PCI driver on Keystone, I had submitted patches
>>>> related to machine and DTS to the arm mailing list to enable the driver
>>>> on Keystone. Subsequenty one of the patch from my series was Nack-ed and
>>>> I was asked to implememented in a different way and started this series.
>>>> You can refer to the discussion of this at
>>>>
>>>> http://www.gossamer-threads.com/lists/linux/kernel/2024591
>>>>
>>>> The PCI driver enablement on Keystone is still a working in progress and
>>>> I am trying to get it fully functional on the upstream. Another missing
>>>> piece is the SerDes phy driver patch. We have started working on the
>>>> other part (SerDes phy driver) already as the initial one was not
>>>> accepted. So it is fine if we are too late for the v3.20 merge window to
>>>> merge this series and this can be applied to the next branch for v3.21.
>>>>
>>>>>
>>>>> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
>>>>> point. Can you identify the commit that broke it and requires these
>>>>> fixes, so we can figure out how far the fixes need to be backported?
>>>>>
>>>>
>>>> I am trying to get this driver enabled on Keystone by adding the missing
>>>> pieces as described above. So I guess we don't have to back port
>>>> anything here.
>>>>
>>>>> If I merge it, I would like to get into my for-linus branch and get a
>>>>> little time in -next before asking Linus to pull it. The merge window
>>>>> is a little wrinkle there -- I don't like to add new things to the mix
>>>>> during the window. But if it's an important fix we can still get it
>>>>> in before the final v3.20.
>>>>
>>>>
>>>> Please apply this to next branch for v3.21. It currently apply cleanly
>>>> to v3.19-rc7. If you want me rebase to another branch, let me know I can
>>>> apply and send you an updated patch.
>>>>
>>> Bjorn, Arnd,
>>>
>>> I am assuming, Bjorn is going to merge this to his next branch for
>>> v3.21. If not, it might have to be merged through the arm soc? There are
>>> a couple of Tested-by and Acked-by received after v7. Do you want me to
>>> post v8 with these updated in the patches?
>>
>> FYI.
>>
>> These are the updates.
>> Series was
>>
>> 1) Tested-by: Suravee Suthikulpanit <[email protected]>
>> (on AMD Seattle platform w/ PCI Generic Host controller)
>> 2) Acked-by: Will Deacon <[email protected]>
>> 3) Reviewed-by: Catalin Marinas <[email protected]>
>>
> Bjorn,
>
> I haven't seen a reply from my above email. As soon as you are ready to pull
> this to v4.1 next (originally requested for v3.21 as per above email)branch,
> please let me know and I can send you an updated version rebased to your
> next branch and with the above acks.
My "next" and "for-linus" branches will be based on v4.0-rc1, so
that's the ideal base for patches. I don't expect any significant
changes that would require a rebase, so unless something in your
patches themselves has changed since you last posted them, you don't
need to repost them just for a rebase.
Bjorn
On 02/23/2015 05:15 PM, Bjorn Helgaas wrote:
> On Mon, Feb 23, 2015 at 4:08 PM, Murali Karicheri<[email protected]> wrote:
>> On 02/11/2015 11:58 AM, Murali Karicheri wrote:
>>>
>>> On 02/11/2015 11:54 AM, Murali Karicheri wrote:
>>>>
>>>> On 02/06/2015 01:36 PM, Murali Karicheri wrote:
>>>>>
>>>>> On 02/06/2015 12:53 PM, Bjorn Helgaas wrote:
>>>>>>
>>>>>> On Fri, Feb 6, 2015 at 9:28 AM, Murali Karicheri<[email protected]>
>>>>>> wrote:
>>>>>>>
>>>>>>> On 02/06/2015 10:15 AM, Catalin Marinas wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Feb 05, 2015 at 09:52:52PM +0000, Murali Karicheri wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This patch add an important capability to PCI driver on Keystone. I
>>>>>>>>> hope
>>>>>>>>> to
>>>>>>>>> have this merged to the upstream branch so that it is available for
>>>>>>>>> v3.20.
>>>>>>>>
>>>>>>>>
>>>>>>>> It's very late for 3.20 and the code hasn't been in linux-next at all
>>>>>>>> (but it's not me who's merging this code), unless you can squeeze
>>>>>>>> it in
>>>>>>>> as a bug-fix.
>>>>>>>
>>>>>>>
>>>>>>> This is in fact a bug fix as PCI on Keystone is broken without this.
>>>>>>
>>>>>>
>>>>>> Oh, sorry, I didn't realize that this was so urgent. I guess I read
>>>>>> "this adds an important capability" in the cover letter and concluded
>>>>>> that it was new functionality.
>>>>>
>>>>> Bjorn,
>>>>>
>>>>> Thanks for responding.
>>>>>
>>>>> Let me give you some context on this without which my explanation won't
>>>>> be complete. For using PCI driver on Keystone, I had submitted patches
>>>>> related to machine and DTS to the arm mailing list to enable the driver
>>>>> on Keystone. Subsequenty one of the patch from my series was Nack-ed and
>>>>> I was asked to implememented in a different way and started this series.
>>>>> You can refer to the discussion of this at
>>>>>
>>>>> http://www.gossamer-threads.com/lists/linux/kernel/2024591
>>>>>
>>>>> The PCI driver enablement on Keystone is still a working in progress and
>>>>> I am trying to get it fully functional on the upstream. Another missing
>>>>> piece is the SerDes phy driver patch. We have started working on the
>>>>> other part (SerDes phy driver) already as the initial one was not
>>>>> accepted. So it is fine if we are too late for the v3.20 merge window to
>>>>> merge this series and this can be applied to the next branch for v3.21.
>>>>>
>>>>>>
>>>>>> Anyway, if it's broken, presumably PCI on Keystone *did* work at one
>>>>>> point. Can you identify the commit that broke it and requires these
>>>>>> fixes, so we can figure out how far the fixes need to be backported?
>>>>>>
>>>>>
>>>>> I am trying to get this driver enabled on Keystone by adding the missing
>>>>> pieces as described above. So I guess we don't have to back port
>>>>> anything here.
>>>>>
>>>>>> If I merge it, I would like to get into my for-linus branch and get a
>>>>>> little time in -next before asking Linus to pull it. The merge window
>>>>>> is a little wrinkle there -- I don't like to add new things to the mix
>>>>>> during the window. But if it's an important fix we can still get it
>>>>>> in before the final v3.20.
>>>>>
>>>>>
>>>>> Please apply this to next branch for v3.21. It currently apply cleanly
>>>>> to v3.19-rc7. If you want me rebase to another branch, let me know I can
>>>>> apply and send you an updated patch.
>>>>>
>>>> Bjorn, Arnd,
>>>>
>>>> I am assuming, Bjorn is going to merge this to his next branch for
>>>> v3.21. If not, it might have to be merged through the arm soc? There are
>>>> a couple of Tested-by and Acked-by received after v7. Do you want me to
>>>> post v8 with these updated in the patches?
>>>
>>> FYI.
>>>
>>> These are the updates.
>>> Series was
>>>
>>> 1) Tested-by: Suravee Suthikulpanit<[email protected]>
>>> (on AMD Seattle platform w/ PCI Generic Host controller)
>>> 2) Acked-by: Will Deacon<[email protected]>
>>> 3) Reviewed-by: Catalin Marinas<[email protected]>
>>>
>> Bjorn,
>>
>> I haven't seen a reply from my above email. As soon as you are ready to pull
>> this to v4.1 next (originally requested for v3.21 as per above email)branch,
>> please let me know and I can send you an updated version rebased to your
>> next branch and with the above acks.
>
> My "next" and "for-linus" branches will be based on v4.0-rc1, so
> that's the ideal base for patches. I don't expect any significant
> changes that would require a rebase, so unless something in your
> patches themselves has changed since you last posted them, you don't
> need to repost them just for a rebase.
>
Bjorn,
Thanks for the response.
Nothing from my side except for the above acks/tested-by/reviewed-by
Murali
> Bjorn
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Thu, Feb 05, 2015 at 04:52:58PM -0500, Murali Karicheri wrote:
> 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.
Hi Murali,
I'm guessing this is the patch that actually fixes things for Keystone.
And it looks like it should also fix things for other hardware that has
similar characteristics. So I'd like to include some higher-level text
about that here. For example:
This fixes DMA on systems where DMA addresses are a constant offset
from CPU physical addresses.
(I don't know exactly how these patches all fit together, so that's
probably not accurate, but that's the *sort* of thing I'd like to include.)
If that actually *is* what's going on, I have to wonder why this isn't
implemented as a very simple IOMMU instead of adding dma_pfn_offset,
which is present on all arches but only used on ARM. In some sense that
offset is parallel but incompatible with an IOMMU: they both translate DMA
addresses into system RAM addresses.
I know you're not adding this, and I assume somebody explored that option
and rejected it for good reasons. I just missed that discussion.
> This patch use the new helper function of_pci_dma_configure() to update
> the device dma configuration.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Suravee Suthikulpanit <[email protected]>
>
> Acked-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Murali Karicheri <[email protected]>
> ---
> 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 linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On 02/24/2015 08:53 PM, Bjorn Helgaas wrote:
> On Thu, Feb 05, 2015 at 04:52:58PM -0500, Murali Karicheri wrote:
>> 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.
>
> Hi Murali,
>
> I'm guessing this is the patch that actually fixes things for Keystone.
>
Yes, but depends on other patches in the series.
> And it looks like it should also fix things for other hardware that has
> similar characteristics.
This originally started as a patch for Keystone, but modified to apply
for other platforms as well. Tested by one another platform AFAIK, by
Suravee Suthikulpanit on AMD Seattle platform w/ PCI Generic Host
Controller. See the Tested-By from him against this series.
So I'd like to include some higher-level text
> about that here. For example:
>
> This fixes DMA on systems where DMA addresses are a constant offset
> from CPU physical addresses.
>
That is one fix. It also update dma configuration for the device
such as dma operations through a call to of_dma_configure() in 5/7.
You may add the above description in 5/7. I didn't add it in the
description because of_dma_configure() API call takes care of it already.
> (I don't know exactly how these patches all fit together, so that's
> probably not accurate, but that's the *sort* of thing I'd like to include.)
>
> If that actually *is* what's going on, I have to wonder why this isn't
> implemented as a very simple IOMMU instead of adding dma_pfn_offset,
> which is present on all arches but only used on ARM. In some sense that
> offset is parallel but incompatible with an IOMMU: they both translate DMA
> addresses into system RAM addresses.
I don't have much history on any previous discussion on the subject you
are referring to. I assume it would have happened when
of_dma_configure() was first introduced. On Keystone, we don't have
IOMMU support and dma_pfn_offset is needed to translate DMA address to
System RAM address and vice-versa. So this has to be supported for
Keystone. There can be enhancement for IOMMU with out impacting this
feature for Keystone.
I know that Will Deacon is working on updates for IOMMU which is
partially touched by my series (1/7). Probably this can be a question
when that patches comes up for review or could be a seperate discussion
topic.
I think it is better this series is applied to the next branch for v4.1
as soon as possible so that others can add features on top of this
without breaking the function for Keystone.
I am looking forward for the merge of this series to the next branch at
the earliest.
Thanks.
Murali
>
> I know you're not adding this, and I assume somebody explored that option
> and rejected it for good reasons. I just missed that discussion.
>
>> This patch use the new helper function of_pci_dma_configure() to update
>> the device dma configuration.
>>
>> Cc: Joerg Roedel<[email protected]>
>> Cc: Grant Likely<[email protected]>
>> Cc: Rob Herring<[email protected]>
>> Cc: Will Deacon<[email protected]>
>> Cc: Russell King<[email protected]>
>> Cc: Arnd Bergmann<[email protected]>
>> Cc: Suravee Suthikulpanit<[email protected]>
>>
>> Acked-by: Bjorn Helgaas<[email protected]>
>> Signed-off-by: Murali Karicheri<[email protected]>
>> ---
>> 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 linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Wednesday 25 February 2015 11:03:02 Murali Karicheri wrote:
>
> > (I don't know exactly how these patches all fit together, so that's
> > probably not accurate, but that's the *sort* of thing I'd like to include.)
> >
> > If that actually *is* what's going on, I have to wonder why this isn't
> > implemented as a very simple IOMMU instead of adding dma_pfn_offset,
> > which is present on all arches but only used on ARM. In some sense that
> > offset is parallel but incompatible with an IOMMU: they both translate DMA
> > addresses into system RAM addresses.
>
> I don't have much history on any previous discussion on the subject you
> are referring to. I assume it would have happened when
> of_dma_configure() was first introduced. On Keystone, we don't have
> IOMMU support and dma_pfn_offset is needed to translate DMA address to
> System RAM address and vice-versa. So this has to be supported for
> Keystone. There can be enhancement for IOMMU with out impacting this
> feature for Keystone.
The direction we are taking with IOMMU in general is opposite to what Bjorn
is suggesting: I believe what he wants to say is that we should use the
traditional approach of having a specialized dma_map_ops implementation
for this, just like we do for IOMMU implementations on x86, IA64 or PowerPC.
However, with the recent explosion of IOMMU implementations on ARM, we
are moving towards having a single dma_map_ops structure for all of them,
and that structure does not work with the keystone hardware. Instead,
the normal ARM dma_map_ops have been changed to handle the offset,
which is the same thing we do on PowerPC.
Arnd
On 02/25/2015 11:09 AM, Arnd Bergmann wrote:
> On Wednesday 25 February 2015 11:03:02 Murali Karicheri wrote:
>>
>>> (I don't know exactly how these patches all fit together, so that's
>>> probably not accurate, but that's the *sort* of thing I'd like to include.)
>>>
>>> If that actually *is* what's going on, I have to wonder why this isn't
>>> implemented as a very simple IOMMU instead of adding dma_pfn_offset,
>>> which is present on all arches but only used on ARM. In some sense that
>>> offset is parallel but incompatible with an IOMMU: they both translate DMA
>>> addresses into system RAM addresses.
>>
>> I don't have much history on any previous discussion on the subject you
>> are referring to. I assume it would have happened when
>> of_dma_configure() was first introduced. On Keystone, we don't have
>> IOMMU support and dma_pfn_offset is needed to translate DMA address to
>> System RAM address and vice-versa. So this has to be supported for
>> Keystone. There can be enhancement for IOMMU with out impacting this
>> feature for Keystone.
>
> The direction we are taking with IOMMU in general is opposite to what Bjorn
> is suggesting: I believe what he wants to say is that we should use the
> traditional approach of having a specialized dma_map_ops implementation
> for this, just like we do for IOMMU implementations on x86, IA64 or PowerPC.
>
> However, with the recent explosion of IOMMU implementations on ARM, we
> are moving towards having a single dma_map_ops structure for all of them,
> and that structure does not work with the keystone hardware. Instead,
> the normal ARM dma_map_ops have been changed to handle the offset,
> which is the same thing we do on PowerPC.
>
Arnd,
Thanks for the clarification.
Murali
> Arnd
--
Murali Karicheri
Linux Kernel, Texas Instruments
On Thu, Feb 05, 2015 at 04:52:52PM -0500, Murali Karicheri wrote:
> This patch add an important capability to PCI driver on Keystone. I hope to
> have this merged to the upstream branch so that it is available for v3.20.
> Also would like thank everyone for the contribution.
>
> 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/[email protected]/msg790244.html
> [1] : http://www.gossamer-threads.com/lists/linux/kernel/2024591
>
> Change history:
> v6 - Rebased to v3.19-v7
> - Addressed some minor comments about node name and DT size validation.
> - Pulled out 8/8 of v5 and plan to send a patch for enhancing
> of_dma_configure() to use size to calculate dma mask.
> - Added Acks from reviewers.
> v5 - moved the dma_mask update in device from ARM specific API to
> of_dma_configure to allow this across other architecture as well
> - improved sanity check for DT dma-range size in of_dma_configure()
> - moved API to get parent bridge device to PCI (host-bridge.c)
> v4 - moved size adjustments in of_iommu_configure() to a separate patch
> - consistent node name comment from Rob
> - patch 6 added for dma_mask adjustment and iommu mapping size
> limiting.
> v3 - addressed comments to re-use of_dma_configure() for PCI
> - To help re-use, change of_iommu_configure() function argument
> - Move of_dma_configure to of/device.c
> - Limit the of_iommu_configure to non pci devices
> 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.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Suravee Suthikulpanit <[email protected]>
>
> Acked-by: Bjorn Helgaas <[email protected]>
> Acked-by: Murali Karicheri <[email protected]>
>
> Murali Karicheri (7):
> of: iommu: add ptr to OF node arg to of_iommu_configure()
> of: move of_dma_configure() to device.c to help re-use
> of: fix size when dma-range is not used
> PCI: add helper functions pci_get[put]_host_bridge_device()
> of/pci: add of_pci_dma_configure() update dma configuration
> PCI: update dma configuration from DT
> arm: dma-mapping: limit iommu mapping size
>
> arch/arm/mm/dma-mapping.c | 7 +++++
> drivers/iommu/of_iommu.c | 10 ++++--
> drivers/of/device.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/of_pci.c | 19 ++++++++++++
> drivers/of/platform.c | 58 ++---------------------------------
> drivers/pci/host-bridge.c | 14 +++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/of_device.h | 2 ++
> include/linux/of_iommu.h | 6 ++--
> include/linux/of_pci.h | 5 +++
> include/linux/pci.h | 3 ++
> 11 files changed, 140 insertions(+), 60 deletions(-)
Applied with additional acks/tested-by to pci/iommu for v4.1, thanks!
Bjorn