Re: [RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration
On Thu, May 14, 2015 at 6:00 PM, Laurent Pinchart
laurent.pinchart+rene...@ideasonboard.com wrote:
The of_configure_dma() function configures both the DMA masks and ops.
Moving DMA ops configuration to probe time would thus also delay
configuration of the DMA masks, which might not be safe. To avoid issues
split the configuration in two to allow keeping masks configuration at
device add time and move ops configuration to device probe time.
Why is it not safe? The probe is deferred before drivers' probe happens.
Rob
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
---
drivers/of/device.c | 48
++-
drivers/of/of_pci.c | 3 ++-
drivers/of/platform.c | 6 --
include/linux/of_device.h | 11 +--
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f1b84f464fe1..3cb3f78a6d13 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
}
/**
- * of_dma_configure - Setup DMA configuration
+ * of_dma_configure - Setup DMA masks and offset
* @dev: Device to apply DMA configuration
* @np:Pointer to OF node having DMA configuration
*
@@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+void of_dma_configure_masks(struct device *dev, struct device_node *np)
{
- u64 dma_addr, paddr, size;
- int ret;
- bool coherent;
+ u64 dma_addr, paddr, size, range_mask;
unsigned long offset;
- struct iommu_ops *iommu;
+ int ret;
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
@@ -105,9 +103,10 @@ 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 + 1;
+ range_mask = dev-coherent_dma_mask + 1;
+ offset = 0;
} else {
+ range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, dma_pfn_offset(%#08lx)\n, offset);
}
@@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
* Limit coherent and dma mask based on size and default mask
* set by the driver.
*/
- dev-coherent_dma_mask = min(dev-coherent_dma_mask,
-DMA_BIT_MASK(ilog2(dma_addr + size)));
- *dev-dma_mask = min((*dev-dma_mask),
-DMA_BIT_MASK(ilog2(dma_addr + size)));
+ dev-coherent_dma_mask = min(dev-coherent_dma_mask, range_mask);
+ *dev-dma_mask = min((*dev-dma_mask), range_mask);
+}
+EXPORT_SYMBOL_GPL(of_dma_configure_masks);
+
+/**
+ * of_dma_configure_ops - Setup DMA operations
+ * @dev: Device to apply DMA configuration
+ * @np:Pointer to OF node having DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly.
+ */
+int of_dma_configure_ops(struct device *dev, struct device_node *np)
+{
+ u64 dma_addr, paddr, size;
+ struct iommu_ops *iommu;
+ bool coherent;
+ int ret;
+
+ ret = of_dma_get_range(np, dma_addr, paddr, size);
+ if (ret 0) {
+ dma_addr = 0;
+ size = dev-coherent_dma_mask + 1;
+ }
coherent = of_dma_is_coherent(np);
dev_dbg(dev, device is%sdma coherent\n,
@@ -132,8 +152,10 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
iommu ? : not );
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(of_dma_configure);
+EXPORT_SYMBOL_GPL(of_dma_configure_ops);
/**
* of_dma_deconfigure - Clean up DMA configuration
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5b6494..c65803da2067 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -132,7 +132,8 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
if (!bridge-parent)
return;
- of_dma_configure(dev, bridge-parent-of_node);
+ of_dma_configure_masks(dev, bridge-parent-of_node);
+ of_dma_configure_ops(dev, bridge-parent-of_node);
pci_put_host_bridge_device(bridge);
}
EXPORT_SYMBOL_GPL(of_pci_dma_configure);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7a660c79ff84..9a29f09b7723 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -177,7 +177,8 @@ static struct
Re: [RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration
On Fri, May 15, 2015 at 12:00:06AM +0100, Laurent Pinchart wrote:
The of_configure_dma() function configures both the DMA masks and ops.
Moving DMA ops configuration to probe time would thus also delay
configuration of the DMA masks, which might not be safe. To avoid issues
split the configuration in two to allow keeping masks configuration at
device add time and move ops configuration to device probe time.
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
---
drivers/of/device.c | 48
++-
drivers/of/of_pci.c | 3 ++-
drivers/of/platform.c | 6 --
include/linux/of_device.h | 11 +--
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f1b84f464fe1..3cb3f78a6d13 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
}
/**
- * of_dma_configure - Setup DMA configuration
+ * of_dma_configure - Setup DMA masks and offset
* @dev: Device to apply DMA configuration
* @np: Pointer to OF node having DMA configuration
*
@@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+void of_dma_configure_masks(struct device *dev, struct device_node *np)
{
- u64 dma_addr, paddr, size;
- int ret;
- bool coherent;
+ u64 dma_addr, paddr, size, range_mask;
unsigned long offset;
- struct iommu_ops *iommu;
+ int ret;
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
@@ -105,9 +103,10 @@ 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 + 1;
+ range_mask = dev-coherent_dma_mask + 1;
Hmm, do we still want this '+ 1'...
+ offset = 0;
} else {
+ range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, dma_pfn_offset(%#08lx)\n, offset);
}
@@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
* Limit coherent and dma mask based on size and default mask
* set by the driver.
*/
- dev-coherent_dma_mask = min(dev-coherent_dma_mask,
- DMA_BIT_MASK(ilog2(dma_addr + size)));
- *dev-dma_mask = min((*dev-dma_mask),
- DMA_BIT_MASK(ilog2(dma_addr + size)));
+ dev-coherent_dma_mask = min(dev-coherent_dma_mask, range_mask);
... if we now compare directly with the mask here? In fact, this setting
of the coherent_dma_mask could just belong in the else clause, no?
+ *dev-dma_mask = min((*dev-dma_mask), range_mask);
Similar comment here, but we need to do this for both cases I guess.
+}
+EXPORT_SYMBOL_GPL(of_dma_configure_masks);
+
+/**
+ * of_dma_configure_ops - Setup DMA operations
+ * @dev: Device to apply DMA configuration
+ * @np: Pointer to OF node having DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
Weird spelling.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration
The of_configure_dma() function configures both the DMA masks and ops.
Moving DMA ops configuration to probe time would thus also delay
configuration of the DMA masks, which might not be safe. To avoid issues
split the configuration in two to allow keeping masks configuration at
device add time and move ops configuration to device probe time.
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
---
drivers/of/device.c | 48 ++-
drivers/of/of_pci.c | 3 ++-
drivers/of/platform.c | 6 --
include/linux/of_device.h | 11 +--
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f1b84f464fe1..3cb3f78a6d13 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
}
/**
- * of_dma_configure - Setup DMA configuration
+ * of_dma_configure - Setup DMA masks and offset
* @dev: Device to apply DMA configuration
* @np:Pointer to OF node having DMA configuration
*
@@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+void of_dma_configure_masks(struct device *dev, struct device_node *np)
{
- u64 dma_addr, paddr, size;
- int ret;
- bool coherent;
+ u64 dma_addr, paddr, size, range_mask;
unsigned long offset;
- struct iommu_ops *iommu;
+ int ret;
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
@@ -105,9 +103,10 @@ 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 + 1;
+ range_mask = dev-coherent_dma_mask + 1;
+ offset = 0;
} else {
+ range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, dma_pfn_offset(%#08lx)\n, offset);
}
@@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
* Limit coherent and dma mask based on size and default mask
* set by the driver.
*/
- dev-coherent_dma_mask = min(dev-coherent_dma_mask,
-DMA_BIT_MASK(ilog2(dma_addr + size)));
- *dev-dma_mask = min((*dev-dma_mask),
-DMA_BIT_MASK(ilog2(dma_addr + size)));
+ dev-coherent_dma_mask = min(dev-coherent_dma_mask, range_mask);
+ *dev-dma_mask = min((*dev-dma_mask), range_mask);
+}
+EXPORT_SYMBOL_GPL(of_dma_configure_masks);
+
+/**
+ * of_dma_configure_ops - Setup DMA operations
+ * @dev: Device to apply DMA configuration
+ * @np:Pointer to OF node having DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly.
+ */
+int of_dma_configure_ops(struct device *dev, struct device_node *np)
+{
+ u64 dma_addr, paddr, size;
+ struct iommu_ops *iommu;
+ bool coherent;
+ int ret;
+
+ ret = of_dma_get_range(np, dma_addr, paddr, size);
+ if (ret 0) {
+ dma_addr = 0;
+ size = dev-coherent_dma_mask + 1;
+ }
coherent = of_dma_is_coherent(np);
dev_dbg(dev, device is%sdma coherent\n,
@@ -132,8 +152,10 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
iommu ? : not );
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(of_dma_configure);
+EXPORT_SYMBOL_GPL(of_dma_configure_ops);
/**
* of_dma_deconfigure - Clean up DMA configuration
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5b6494..c65803da2067 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -132,7 +132,8 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
if (!bridge-parent)
return;
- of_dma_configure(dev, bridge-parent-of_node);
+ of_dma_configure_masks(dev, bridge-parent-of_node);
+ of_dma_configure_ops(dev, bridge-parent-of_node);
pci_put_host_bridge_device(bridge);
}
EXPORT_SYMBOL_GPL(of_pci_dma_configure);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7a660c79ff84..9a29f09b7723 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -177,7 +177,8 @@ 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, dev-dev.of_node);
+ of_dma_configure_masks(dev-dev, dev-dev.of_node);
+ of_dma_configure_ops(dev-dev,
