diff mbox series

[v1] clk: Move struct clk_core to use struct fwnode_handle

Message ID 20210209170952.49794-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State Changes Requested, archived
Headers show
Series [v1] clk: Move struct clk_core to use struct fwnode_handle | expand

Commit Message

Andy Shevchenko Feb. 9, 2021, 5:09 p.m. UTC
fwnode is an abstraction on the different types of firmware nodes.
In order to allow clocks to be linked with any type of such node,
start a conversion to the struct fwnode_handle instead of being
stuck with struct device_node.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/clk/clk.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Stephen Boyd Feb. 9, 2021, 11:59 p.m. UTC | #1
Quoting Andy Shevchenko (2021-02-09 09:09:52)
> fwnode is an abstraction on the different types of firmware nodes.
> In order to allow clocks to be linked with any type of such node,
> start a conversion to the struct fwnode_handle instead of being
> stuck with struct device_node.

Is ACPI going to support clk hardware? We're "stuck" with device nodes
mostly because there isn't a clk framework for ACPI.
Andy Shevchenko Feb. 10, 2021, 11:01 a.m. UTC | #2
On Tue, Feb 09, 2021 at 03:59:05PM -0800, Stephen Boyd wrote:
> Quoting Andy Shevchenko (2021-02-09 09:09:52)
> > fwnode is an abstraction on the different types of firmware nodes.
> > In order to allow clocks to be linked with any type of such node,
> > start a conversion to the struct fwnode_handle instead of being
> > stuck with struct device_node.
> 
> Is ACPI going to support clk hardware? We're "stuck" with device nodes
> mostly because there isn't a clk framework for ACPI.

Here I'm not talking about ACPI vs. DT vs. anything, the pure motivation is to
make less divergence of standalone OF vs. fwnode (see IRQ domain APIs, for
example, which allows to use them in regmap IRQ APIs).
Stephen Boyd Feb. 11, 2021, 2:25 a.m. UTC | #3
Quoting Andy Shevchenko (2021-02-10 03:01:53)
> On Tue, Feb 09, 2021 at 03:59:05PM -0800, Stephen Boyd wrote:
> > Quoting Andy Shevchenko (2021-02-09 09:09:52)
> > > fwnode is an abstraction on the different types of firmware nodes.
> > > In order to allow clocks to be linked with any type of such node,
> > > start a conversion to the struct fwnode_handle instead of being
> > > stuck with struct device_node.
> > 
> > Is ACPI going to support clk hardware? We're "stuck" with device nodes
> > mostly because there isn't a clk framework for ACPI.
> 
> Here I'm not talking about ACPI vs. DT vs. anything, the pure motivation is to
> make less divergence of standalone OF vs. fwnode (see IRQ domain APIs, for
> example, which allows to use them in regmap IRQ APIs).
> 

I thought the fwnode changes in IRQ domain APIs was to work across both
ACPI and DT. Please tell me more!
Andy Shevchenko Feb. 11, 2021, 10:37 a.m. UTC | #4
On Wed, Feb 10, 2021 at 06:25:31PM -0800, Stephen Boyd wrote:
> Quoting Andy Shevchenko (2021-02-10 03:01:53)
> > On Tue, Feb 09, 2021 at 03:59:05PM -0800, Stephen Boyd wrote:
> > > Quoting Andy Shevchenko (2021-02-09 09:09:52)
> > > > fwnode is an abstraction on the different types of firmware nodes.
> > > > In order to allow clocks to be linked with any type of such node,
> > > > start a conversion to the struct fwnode_handle instead of being
> > > > stuck with struct device_node.
> > > 
> > > Is ACPI going to support clk hardware? We're "stuck" with device nodes
> > > mostly because there isn't a clk framework for ACPI.
> > 
> > Here I'm not talking about ACPI vs. DT vs. anything, the pure motivation is to
> > make less divergence of standalone OF vs. fwnode (see IRQ domain APIs, for
> > example, which allows to use them in regmap IRQ APIs).
> > 
> 
> I thought the fwnode changes in IRQ domain APIs was to work across both
> ACPI and DT. Please tell me more!

I wish I could dig this out from the commit
f110711a6053 ("irqdomain: Convert irqdomain-%3Eof_node to fwnode")
but it kept silent what the motivation of doing that.

For me the fwnode API brings an agnostic interface which is good thing to have
and makes it easier to be used by other providers (you know that we have swnode
besides ACPI and DT, right?).

I would like to re-use clk-gpio in ACPI based environment and I found it quite
difficult without changing a lot of things in clk framework which is tighten to
OF by nails. This is not good from design perspective and makes my journey
miserable. Of course if clk is against this, I would live with copy'n'paste
approach — no hard feelings. Just let me know.
Stephen Boyd Feb. 11, 2021, 7:09 p.m. UTC | #5
Quoting Andy Shevchenko (2021-02-11 02:37:52)
> On Wed, Feb 10, 2021 at 06:25:31PM -0800, Stephen Boyd wrote:
> > Quoting Andy Shevchenko (2021-02-10 03:01:53)
> > > On Tue, Feb 09, 2021 at 03:59:05PM -0800, Stephen Boyd wrote:
> > > > Quoting Andy Shevchenko (2021-02-09 09:09:52)
> > > > > fwnode is an abstraction on the different types of firmware nodes.
> > > > > In order to allow clocks to be linked with any type of such node,
> > > > > start a conversion to the struct fwnode_handle instead of being
> > > > > stuck with struct device_node.
> > > > 
> > > > Is ACPI going to support clk hardware? We're "stuck" with device nodes
> > > > mostly because there isn't a clk framework for ACPI.
> > > 
> > > Here I'm not talking about ACPI vs. DT vs. anything, the pure motivation is to
> > > make less divergence of standalone OF vs. fwnode (see IRQ domain APIs, for
> > > example, which allows to use them in regmap IRQ APIs).
> > > 
> > 
> > I thought the fwnode changes in IRQ domain APIs was to work across both
> > ACPI and DT. Please tell me more!
> 
> I wish I could dig this out from the commit
> f110711a6053 ("irqdomain: Convert irqdomain-%3Eof_node to fwnode")
> but it kept silent what the motivation of doing that.
> 
> For me the fwnode API brings an agnostic interface which is good thing to have
> and makes it easier to be used by other providers (you know that we have swnode
> besides ACPI and DT, right?).
> 
> I would like to re-use clk-gpio in ACPI based environment and I found it quite
> difficult without changing a lot of things in clk framework which is tighten to
> OF by nails. This is not good from design perspective and makes my journey
> miserable. Of course if clk is against this, I would live with copy'n'paste
> approach — no hard feelings. Just let me know.
> 

Please add this motivation to the commit text. In particular how it will
help the clk-gpio code be used on an ACPI environment. Or maybe even
send it along with the clk-gpio changes that you've made.
diff mbox series

Patch

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3d751ae5bc70..dd8e11e4312d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@ 
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/sched.h>
 #include <linux/clkdev.h>
 
@@ -59,7 +60,7 @@  struct clk_core {
 	struct clk_hw		*hw;
 	struct module		*owner;
 	struct device		*dev;
-	struct device_node	*of_node;
+	struct fwnode_handle	*fwnode;
 	struct clk_core		*parent;
 	struct clk_parent_map	*parents;
 	u8			num_parents;
@@ -396,7 +397,7 @@  static struct clk_core *clk_core_get(struct clk_core *core, u8 p_index)
 	struct clk_hw *hw = ERR_PTR(-ENOENT);
 	struct device *dev = core->dev;
 	const char *dev_id = dev ? dev_name(dev) : NULL;
-	struct device_node *np = core->of_node;
+	struct device_node *np = to_of_node(core->fwnode);
 	struct of_phandle_args clkspec;
 
 	if (np && (name || index >= 0) &&
@@ -3189,7 +3190,7 @@  static void possible_parent_show(struct seq_file *s, struct clk_core *core,
 		seq_printf(s, "<%s>(fw)", core->parents[i].fw_name);
 	else if (core->parents[i].index >= 0)
 		seq_puts(s,
-			 of_clk_get_parent_name(core->of_node,
+			 of_clk_get_parent_name(to_of_node(core->fwnode),
 						core->parents[i].index));
 	else
 		seq_puts(s, "(missing)");
@@ -3814,7 +3815,7 @@  static void clk_core_free_parent_map(struct clk_core *core)
 }
 
 static struct clk *
-__clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
+__clk_register(struct device *dev, struct fwnode_handle *fwnode, struct clk_hw *hw)
 {
 	int ret;
 	struct clk_core *core;
@@ -3848,7 +3849,7 @@  __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	if (dev && pm_runtime_enabled(dev))
 		core->rpm_enabled = true;
 	core->dev = dev;
-	core->of_node = np;
+	core->fwnode = fwnode;
 	if (dev && dev->driver)
 		core->owner = dev->driver->owner;
 	core->hw = hw;
@@ -3906,18 +3907,18 @@  __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
  * @dev->parent if dev doesn't have a device node, or NULL if neither
  * @dev or @dev->parent have a device node.
  */
-static struct device_node *dev_or_parent_of_node(struct device *dev)
+static struct fwnode_handle *dev_or_parent_fwnode(struct device *dev)
 {
-	struct device_node *np;
+	struct fwnode_handle *fwnode;
 
 	if (!dev)
 		return NULL;
 
-	np = dev_of_node(dev);
-	if (!np)
-		np = dev_of_node(dev->parent);
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		fwnode = dev_fwnode(dev->parent);
 
-	return np;
+	return fwnode;
 }
 
 /**
@@ -3935,7 +3936,7 @@  static struct device_node *dev_or_parent_of_node(struct device *dev)
  */
 struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 {
-	return __clk_register(dev, dev_or_parent_of_node(dev), hw);
+	return __clk_register(dev, dev_or_parent_fwnode(dev), hw);
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
@@ -3951,8 +3952,7 @@  EXPORT_SYMBOL_GPL(clk_register);
  */
 int clk_hw_register(struct device *dev, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_or_parent_of_node(dev),
-			       hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_or_parent_fwnode(dev), hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
 
@@ -3969,7 +3969,7 @@  EXPORT_SYMBOL_GPL(clk_hw_register);
  */
 int of_clk_hw_register(struct device_node *node, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(__clk_register(NULL, node, hw));
+	return PTR_ERR_OR_ZERO(__clk_register(NULL, of_fwnode_handle(node), hw));
 }
 EXPORT_SYMBOL_GPL(of_clk_hw_register);