diff mbox series

[1/4] device property: Convert device_{dma_supported,get_dma_attr} to fwnode

Message ID 20220206091643.276833-2-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Shovel firmware specific code to appropriate locations | expand

Commit Message

Sakari Ailus Feb. 6, 2022, 9:16 a.m. UTC
Make the device_dma_supported and device_get_dma_attr functions to use the
fwnode ops, and move the implementation to ACPI and OF frameworks.

Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 14 ++++++++++++++
 drivers/base/property.c | 25 ++++---------------------
 drivers/of/property.c   | 17 +++++++++++++++++
 include/linux/fwnode.h  |  3 +++
 4 files changed, 38 insertions(+), 21 deletions(-)

Comments

Andy Shevchenko Feb. 7, 2022, 12:20 p.m. UTC | #1
On Sun, Feb 06, 2022 at 11:16:40AM +0200, Sakari Ailus wrote:
> Make the device_dma_supported and device_get_dma_attr functions to use the
> fwnode ops, and move the implementation to ACPI and OF frameworks.
> 
> Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")

As of today: ffa743d3f33b, but might be changed in case of rebase happens in
the future.
Rob Herring Feb. 11, 2022, 3:55 p.m. UTC | #2
On Sun, Feb 06, 2022 at 11:16:40AM +0200, Sakari Ailus wrote:
> Make the device_dma_supported and device_get_dma_attr functions to use the
> fwnode ops, and move the implementation to ACPI and OF frameworks.
> 
> Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")

Is this some new convention? What's wrong with 'base-commit' and 
shouldn't it be below the '---'?

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c | 14 ++++++++++++++
>  drivers/base/property.c | 25 ++++---------------------
>  drivers/of/property.c   | 17 +++++++++++++++++
>  include/linux/fwnode.h  |  3 +++
>  4 files changed, 38 insertions(+), 21 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>
Andy Shevchenko Feb. 11, 2022, 4:18 p.m. UTC | #3
On Fri, Feb 11, 2022 at 09:55:22AM -0600, Rob Herring wrote:
> On Sun, Feb 06, 2022 at 11:16:40AM +0200, Sakari Ailus wrote:
> > Make the device_dma_supported and device_get_dma_attr functions to use the
> > fwnode ops, and move the implementation to ACPI and OF frameworks.
> > 
> > Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")
> 
> Is this some new convention?

% git log --oneline --no-merges --grep Depends-on | wc -l
83

Or I misunderstood your question?

> What's wrong with 'base-commit' and 
> shouldn't it be below the '---'?

There is no guarantee with the SHA to be the same in either cases, it can be
filled later with a proper one.
Sakari Ailus Feb. 14, 2022, 1:31 p.m. UTC | #4
Hi Rob,

On Fri, Feb 11, 2022 at 09:55:22AM -0600, Rob Herring wrote:
> On Sun, Feb 06, 2022 at 11:16:40AM +0200, Sakari Ailus wrote:
> > Make the device_dma_supported and device_get_dma_attr functions to use the
> > fwnode ops, and move the implementation to ACPI and OF frameworks.
> > 
> > Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")
> 
> Is this some new convention? What's wrong with 'base-commit' and 
> shouldn't it be below the '---'?

I guess that could be possible, too. There are different practices it
seems.

I'll add commit id in v2 (or drop the tag if the dependent patch hits the
linux-acpi tree soonish).

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/acpi/property.c | 14 ++++++++++++++
> >  drivers/base/property.c | 25 ++++---------------------
> >  drivers/of/property.c   | 17 +++++++++++++++++
> >  include/linux/fwnode.h  |  3 +++
> >  4 files changed, 38 insertions(+), 21 deletions(-)
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks!
Rob Herring March 8, 2022, 3:50 p.m. UTC | #5
On Fri, Feb 11, 2022 at 10:19 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Feb 11, 2022 at 09:55:22AM -0600, Rob Herring wrote:
> > On Sun, Feb 06, 2022 at 11:16:40AM +0200, Sakari Ailus wrote:
> > > Make the device_dma_supported and device_get_dma_attr functions to use the
> > > fwnode ops, and move the implementation to ACPI and OF frameworks.
> > >
> > > Depends-on: ("device property: Don't split fwnode_get_irq*() APIs in the code")
> >
> > Is this some new convention?
>
> % git log --oneline --no-merges --grep Depends-on | wc -l
> 83

With 10k+ commits per cycle that's not really compelling.

> Or I misunderstood your question?

% git grep 'Depends-on' | wc -l
0

What I mean is where's the documentation for using this? Or even a discussion?

Rob
diff mbox series

Patch

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index d0986bda2964..1541b318ba46 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1253,6 +1253,17 @@  static bool acpi_fwnode_device_is_available(const struct fwnode_handle *fwnode)
 	return acpi_device_is_present(to_acpi_device_node(fwnode));
 }
 
+static bool acpi_fwnode_device_dma_supported(const struct fwnode_handle *fwnode)
+{
+	return acpi_dma_supported(to_acpi_device_node(fwnode));
+}
+
+static enum dev_dma_attr
+acpi_fwnode_device_get_dma_attr(const struct fwnode_handle *fwnode)
+{
+	return acpi_get_dma_attr(to_acpi_device_node(fwnode));
+}
+
 static bool acpi_fwnode_property_present(const struct fwnode_handle *fwnode,
 					 const char *propname)
 {
@@ -1384,6 +1395,9 @@  acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 	const struct fwnode_operations ops = {				\
 		.device_is_available = acpi_fwnode_device_is_available, \
 		.device_get_match_data = acpi_fwnode_device_get_match_data, \
+		.device_dma_supported =				\
+			acpi_fwnode_device_dma_supported,		\
+		.device_get_dma_attr = acpi_fwnode_device_get_dma_attr,	\
 		.property_present = acpi_fwnode_property_present,	\
 		.property_read_int_array =				\
 			acpi_fwnode_property_read_int_array,		\
diff --git a/drivers/base/property.c b/drivers/base/property.c
index c0e94cce9c29..09686e2e903e 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -823,33 +823,16 @@  EXPORT_SYMBOL_GPL(device_get_child_node_count);
 
 bool device_dma_supported(struct device *dev)
 {
-	const struct fwnode_handle *fwnode = dev_fwnode(dev);
-
-	/* For DT, this is always supported.
-	 * For ACPI, this depends on CCA, which
-	 * is determined by the acpi_dma_supported().
-	 */
-	if (is_of_node(fwnode))
-		return true;
-
-	return acpi_dma_supported(to_acpi_device_node(fwnode));
+	return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);
 }
 EXPORT_SYMBOL_GPL(device_dma_supported);
 
 enum dev_dma_attr device_get_dma_attr(struct device *dev)
 {
-	const struct fwnode_handle *fwnode = dev_fwnode(dev);
-	enum dev_dma_attr attr = DEV_DMA_NOT_SUPPORTED;
-
-	if (is_of_node(fwnode)) {
-		if (of_dma_is_coherent(to_of_node(fwnode)))
-			attr = DEV_DMA_COHERENT;
-		else
-			attr = DEV_DMA_NON_COHERENT;
-	} else
-		attr = acpi_get_dma_attr(to_acpi_device_node(fwnode));
+	if (!fwnode_has_op(dev_fwnode(dev), device_get_dma_attr))
+		return DEV_DMA_NOT_SUPPORTED;
 
-	return attr;
+	return fwnode_call_int_op(dev_fwnode(dev), device_get_dma_attr);
 }
 EXPORT_SYMBOL_GPL(device_get_dma_attr);
 
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 8e90071de6ed..676899566f7c 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -22,6 +22,7 @@ 
 #define pr_fmt(fmt)	"OF: " fmt
 
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/of_irq.h>
@@ -872,6 +873,20 @@  static bool of_fwnode_device_is_available(const struct fwnode_handle *fwnode)
 	return of_device_is_available(to_of_node(fwnode));
 }
 
+static bool of_fwnode_device_dma_supported(const struct fwnode_handle *fwnode)
+{
+	return true;
+}
+
+static enum dev_dma_attr
+of_fwnode_device_get_dma_attr(const struct fwnode_handle *fwnode)
+{
+	if (of_dma_is_coherent(to_of_node(fwnode)))
+		return DEV_DMA_COHERENT;
+	else
+		return DEV_DMA_NON_COHERENT;
+}
+
 static bool of_fwnode_property_present(const struct fwnode_handle *fwnode,
 				       const char *propname)
 {
@@ -1472,6 +1487,8 @@  const struct fwnode_operations of_fwnode_ops = {
 	.put = of_fwnode_put,
 	.device_is_available = of_fwnode_device_is_available,
 	.device_get_match_data = of_fwnode_device_get_match_data,
+	.device_dma_supported = of_fwnode_device_dma_supported,
+	.device_get_dma_attr = of_fwnode_device_get_dma_attr,
 	.property_present = of_fwnode_property_present,
 	.property_read_int_array = of_fwnode_property_read_int_array,
 	.property_read_string_array = of_fwnode_property_read_string_array,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 3a532ba66f6c..6f307f21fc65 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -113,6 +113,9 @@  struct fwnode_operations {
 	bool (*device_is_available)(const struct fwnode_handle *fwnode);
 	const void *(*device_get_match_data)(const struct fwnode_handle *fwnode,
 					     const struct device *dev);
+	bool (*device_dma_supported)(const struct fwnode_handle *fwnode);
+	enum dev_dma_attr
+	(*device_get_dma_attr)(const struct fwnode_handle *fwnode);
 	bool (*property_present)(const struct fwnode_handle *fwnode,
 				 const char *propname);
 	int (*property_read_int_array)(const struct fwnode_handle *fwnode,