[v3,1/2] clk: ti: add of_ti_clk_register() helper

Message ID 20221106154612.3474940-1-dario.binacchi@amarulasolutions.com
State New
Headers
Series [v3,1/2] clk: ti: add of_ti_clk_register() helper |

Commit Message

Dario Binacchi Nov. 6, 2022, 3:46 p.m. UTC
  The ti_clk_register() function is always called with the parameter of
type struct device set to NULL, since the functions from which it is
called always have a parameter of type struct device_node. Adding this
helper will allow you to register a TI clock to the common clock
framework by taking advantage of the facilities provided by the
struct device_node type.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

(no changes since v1)

 drivers/clk/ti/clk.c   | 30 ++++++++++++++++++++++++++++++
 drivers/clk/ti/clock.h |  2 ++
 2 files changed, 32 insertions(+)
  

Comments

Tony Lindgren Nov. 9, 2022, 8:15 a.m. UTC | #1
* Dario Binacchi <dario.binacchi@amarulasolutions.com> [221106 17:36]:
> The ti_clk_register() function is always called with the parameter of
> type struct device set to NULL, since the functions from which it is
> called always have a parameter of type struct device_node. Adding this
> helper will allow you to register a TI clock to the common clock
> framework by taking advantage of the facilities provided by the
> struct device_node type.

Makes sense to me.

Do you have a patch to make use of this I can test with?

Regards,

Tony
  
Tony Lindgren Nov. 9, 2022, 8:17 a.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [221109 08:06]:
> * Dario Binacchi <dario.binacchi@amarulasolutions.com> [221106 17:36]:
> > The ti_clk_register() function is always called with the parameter of
> > type struct device set to NULL, since the functions from which it is
> > called always have a parameter of type struct device_node. Adding this
> > helper will allow you to register a TI clock to the common clock
> > framework by taking advantage of the facilities provided by the
> > struct device_node type.
> 
> Makes sense to me.
> 
> Do you have a patch to make use of this I can test with?

I mean a patch to convert the ti_clk_register() callers to use this or
what's your plan?

Regards,

Tony
  
Dario Binacchi Nov. 9, 2022, 8:38 a.m. UTC | #3
Hi Tony,

On Wed, Nov 9, 2022 at 9:17 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Tony Lindgren <tony@atomide.com> [221109 08:06]:
> > * Dario Binacchi <dario.binacchi@amarulasolutions.com> [221106 17:36]:
> > > The ti_clk_register() function is always called with the parameter of
> > > type struct device set to NULL, since the functions from which it is
> > > called always have a parameter of type struct device_node. Adding this
> > > helper will allow you to register a TI clock to the common clock
> > > framework by taking advantage of the facilities provided by the
> > > struct device_node type.
> >
> > Makes sense to me.
> >
> > Do you have a patch to make use of this I can test with?
>
> I mean a patch to convert the ti_clk_register() callers to use this or
> what's your plan?

The first patch that calls this function is the second one in this
series "clk: ti: dra7-atl: don't allocate` parent_names' variable ".
Since I don't have the dra7 hardware, I have indirectly tested it on a
beaglebone (gate clock driver) board. To do this I also
had to add the of_ti_clk_register_omap_hw() helper. In the case of the
dra7-atl driver it was not necessary because the setup
function calls the ti_clk_register() directly.
If you think it makes sense, I can do 1 or more patches that replace
ti_clk_register() and ti_clk_register_omap_hw() with their
counterparts of_ti_clk_register[_omap_hw]. And I could test this
further series on the beaglebone board.

Thanks and regards,
Dario

>
> Regards,
>
> Tony
  
Tony Lindgren Nov. 9, 2022, 8:52 a.m. UTC | #4
* Dario Binacchi <dario.binacchi@amarulasolutions.com> [221109 08:28]:
> Hi Tony,
> 
> On Wed, Nov 9, 2022 at 9:17 AM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Tony Lindgren <tony@atomide.com> [221109 08:06]:
> > > * Dario Binacchi <dario.binacchi@amarulasolutions.com> [221106 17:36]:
> > > > The ti_clk_register() function is always called with the parameter of
> > > > type struct device set to NULL, since the functions from which it is
> > > > called always have a parameter of type struct device_node. Adding this
> > > > helper will allow you to register a TI clock to the common clock
> > > > framework by taking advantage of the facilities provided by the
> > > > struct device_node type.
> > >
> > > Makes sense to me.
> > >
> > > Do you have a patch to make use of this I can test with?
> >
> > I mean a patch to convert the ti_clk_register() callers to use this or
> > what's your plan?
> 
> The first patch that calls this function is the second one in this
> series "clk: ti: dra7-atl: don't allocate` parent_names' variable ".
> Since I don't have the dra7 hardware, I have indirectly tested it on a
> beaglebone (gate clock driver) board. To do this I also
> had to add the of_ti_clk_register_omap_hw() helper. In the case of the
> dra7-atl driver it was not necessary because the setup
> function calls the ti_clk_register() directly.
> If you think it makes sense, I can do 1 or more patches that replace
> ti_clk_register() and ti_clk_register_omap_hw() with their
> counterparts of_ti_clk_register[_omap_hw]. And I could test this
> further series on the beaglebone board.

Yeah if you can please post one more patch separately replacing the old
users that would be great.

Regards,

Tony
  

Patch

diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index 1dc2f15fb75b..e29b5c7c0dc8 100644
--- a/drivers/clk/ti/clk.c
+++ b/drivers/clk/ti/clk.c
@@ -560,6 +560,36 @@  int ti_clk_add_alias(struct device *dev, struct clk *clk, const char *con)
 	return 0;
 }
 
+/**
+ * of_ti_clk_register - register a TI clock to the common clock framework
+ * @node: device node for the clock
+ * @hw: hardware clock handle
+ * @con: connection ID for this clock
+ *
+ * Registers a TI clock to the common clock framework, and adds a clock
+ * alias for it. Returns a handle to the registered clock if successful,
+ * ERR_PTR value in failure.
+ */
+struct clk *of_ti_clk_register(struct device_node *node, struct clk_hw *hw,
+			       const char *con)
+{
+	struct clk *clk;
+	int ret;
+
+	ret = of_clk_hw_register(node, hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	clk = hw->clk;
+	ret = ti_clk_add_alias(NULL, clk, con);
+	if (ret) {
+		clk_unregister(clk);
+		return ERR_PTR(ret);
+	}
+
+	return clk;
+}
+
 /**
  * ti_clk_register - register a TI clock to the common clock framework
  * @dev: device for this clock
diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h
index 37ab53339a9b..a75fcf775de0 100644
--- a/drivers/clk/ti/clock.h
+++ b/drivers/clk/ti/clock.h
@@ -199,6 +199,8 @@  extern const struct omap_clkctrl_data dm816_clkctrl_data[];
 
 typedef void (*ti_of_clk_init_cb_t)(void *, struct device_node *);
 
+struct clk *of_ti_clk_register(struct device_node *node, struct clk_hw *hw,
+			       const char *con);
 struct clk *ti_clk_register(struct device *dev, struct clk_hw *hw,
 			    const char *con);
 struct clk *ti_clk_register_omap_hw(struct device *dev, struct clk_hw *hw,