[PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

The of_dma_is_coherent() helper parses the given DT device
node to see if the "dma-coherent" property is supported and
returns true or false accordingly.

Cc: Russell King <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <redacted>
Cc: Grant Likely <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Santosh Shilimkar <redacted>
---
 drivers/dma/of-dma.c   |   22 ++++++++++++++++++++++
 include/linux/of_dma.h |    5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 9b51768..c5958d1 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c

@@ -304,3 +304,25 @@ out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_dma_get_range);
+
+/**
+ * of_dma_is_coherent - Check if device is coherent
+ * @np:	device node
+ *
+ * It returns true if "dma-coherent" property was found
+ * for this device in DT.
+ */
+bool of_dma_is_coherent(struct device_node *np)
+{
+	struct device_node *node = np;
+
+	while (node) {
+		if (of_property_read_bool(node, "dma-coherent")) {
+			of_node_put(node);
+			return true;
+		}
+		node = of_get_next_parent(node);
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_dma_is_coherent);

diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index f04171a..6191f02 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h

@@ -44,6 +44,7 @@ extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 
 extern int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
 		phys_addr_t *paddr, phys_addr_t *size);
+extern bool of_dma_is_coherent(struct device_node *np);
 #else
 static inline int of_dma_controller_register(struct device_node *np,
 		struct dma_chan *(*of_dma_xlate)

@@ -74,6 +75,10 @@ static inline int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
 {
 	return -ENODEV;
 }
+static inline bool of_dma_is_coherent(struct device_node *np)
+{
+	return false;
+}
 #endif
 
 #endif /* __LINUX_OF_DMA_H */

-- 
1.7.9.5

On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:

+
+/**
+ * of_dma_is_coherent - Check if device is coherent
+ * @np:        device node
+ *
+ * It returns true if "dma-coherent" property was found
+ * for this device in DT.
+ */
+bool of_dma_is_coherent(struct device_node *np)
+{
+       struct device_node *node = np;
+
+       while (node) {
+               if (of_property_read_bool(node, "dma-coherent")) {
+                       of_node_put(node);
+                       return true;
+               }
+               node = of_get_next_parent(node);
+       }
+       return false;
+}
+EXPORT_SYMBOL_GPL(of_dma_is_coherent);

This won't work on architectures that are always coherent and
did not need 'dma-coherent' properties before, such as IBM
Power servers.

That said, I think the property makes sense, and we already have
platforms using it (highbank is the one I'm aware of).

We probably need ways to override this function in both ways:
"always coherent" (powerpc, x86), and "never coherent" (keystone
without LPAE) from platform code, and it would be nice to put
either option into DT in a global location as well. We may have
to go through a few iterations of this patch to get the best
algorithm, but I think the interface is good at least.

	Arnd

On Friday 28 February 2014 04:39 AM, Arnd Bergmann wrote:

On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:

quoted

+
+/**
+ * of_dma_is_coherent - Check if device is coherent
+ * @np:        device node
+ *
+ * It returns true if "dma-coherent" property was found
+ * for this device in DT.
+ */
+bool of_dma_is_coherent(struct device_node *np)
+{
+       struct device_node *node = np;
+
+       while (node) {
+               if (of_property_read_bool(node, "dma-coherent")) {
+                       of_node_put(node);
+                       return true;
+               }
+               node = of_get_next_parent(node);
+       }
+       return false;
+}
+EXPORT_SYMBOL_GPL(of_dma_is_coherent);

This won't work on architectures that are always coherent and
did not need 'dma-coherent' properties before, such as IBM
Power servers.

That said, I think the property makes sense, and we already have
platforms using it (highbank is the one I'm aware of).

We probably need ways to override this function in both ways:
"always coherent" (powerpc, x86), and "never coherent" (keystone
without LPAE) from platform code, and it would be nice to put
either option into DT in a global location as well. We may have
to go through a few iterations of this patch to get the best
algorithm, but I think the interface is good at least.

Probably we should discuss bit more next week at connect. The
current 'dma-coherent' is a per device property. For arch's
which are always coherent, the per device property doesn't make
sense.

BTW, the current users of this API is only ARM32 bit port
and if this satisfies the ARM platforms, we should get
this in kernel and then address other cases on need
basis.

Regards,
Santosh

On Fri, Feb 28, 2014 at 3:39 AM, Arnd Bergmann [off-list ref] wrote:

On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:

quoted

+
+/**
+ * of_dma_is_coherent - Check if device is coherent
+ * @np:        device node
+ *
+ * It returns true if "dma-coherent" property was found
+ * for this device in DT.
+ */
+bool of_dma_is_coherent(struct device_node *np)
+{
+       struct device_node *node = np;
+
+       while (node) {
+               if (of_property_read_bool(node, "dma-coherent")) {
+                       of_node_put(node);
+                       return true;
+               }
+               node = of_get_next_parent(node);
+       }
+       return false;
+}
+EXPORT_SYMBOL_GPL(of_dma_is_coherent);

This won't work on architectures that are always coherent and
did not need 'dma-coherent' properties before, such as IBM
Power servers.

That said, I think the property makes sense, and we already have
platforms using it (highbank is the one I'm aware of).

We probably need ways to override this function in both ways:
"always coherent" (powerpc, x86), and "never coherent" (keystone
without LPAE) from platform code, and it would be nice to put
either option into DT in a global location as well. We may have
to go through a few iterations of this patch to get the best
algorithm, but I think the interface is good at least.

I know Will D was not a fan of this property. Primarily I believe
because you may need to describe more than just a boolean in more
complex bus topologies.

Effectively, highbank is always coherent. It was only PCI that is
non-coherent, but I can safely say PCI will never be enabled at this
point. There are no designs with PCI beyond 1 or 2 validation boards
(total boards, not designs), and getting PCI to work was quite hacky
due to only a 1MB window. The other masters are programmable, but only
the coherent path is used as the non-coherent path actually has some
issues. I had expected the opposite believing the ACP port would
actually have issues which is also why I made it configurable.

Rob