All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper
@ 2022-11-06 15:46 Dario Binacchi
  2022-11-06 15:46 ` [PATCH v3 2/2] clk: ti: dra7-atl: don't allocate `parent_names' variable Dario Binacchi
  2022-11-09  8:15 ` [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Tony Lindgren
  0 siblings, 2 replies; 6+ messages in thread
From: Dario Binacchi @ 2022-11-06 15:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: michael, Amarula patchwork, Dario Binacchi, Allison Randal,
	Greg Kroah-Hartman, Liang He, Michael Turquette, Stephen Boyd,
	Tero Kristo, Thomas Gleixner, Tony Lindgren, linux-clk,
	linux-omap

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(+)

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,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] clk: ti: dra7-atl: don't allocate `parent_names' variable
  2022-11-06 15:46 [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Dario Binacchi
@ 2022-11-06 15:46 ` Dario Binacchi
  2022-11-09  8:15 ` [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Tony Lindgren
  1 sibling, 0 replies; 6+ messages in thread
From: Dario Binacchi @ 2022-11-06 15:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: michael, Amarula patchwork, Dario Binacchi, kernel test robot,
	Allison Randal, Greg Kroah-Hartman, Miaoqian Lin,
	Michael Turquette, Stephen Boyd, Tero Kristo, Thomas Gleixner,
	Tony Lindgren, linux-clk, linux-omap

The `parent_names' variable was freed also in case of kzalloc() error.
Instead of modifying the code to perform a proper memory release, I
decided to fix the bug by not allocating memory.
Since only one parent name is referenced, it is not necessary to
allocate this variable at runtime and therefore you can avoid calling
the kzalloc() function. This simplifies the code (even calls to kfree
can be removed) and improves the performance of the routine.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Reported-by: kernel test robot <lkp@intel.com>

---

Changes in v3:
- Add the "clk: ti: add of_ti_clk_register() helper" patch to the series.
- Use a variable of type struct clk_parent_data to furthermore simplify
  the code.
- Update the commit message.

Changes in v2:
- Fix compiling error
- Add kernel test robot's Reported-by tag.

 drivers/clk/ti/clk-dra7-atl.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/ti/clk-dra7-atl.c b/drivers/clk/ti/clk-dra7-atl.c
index ff4d6a951681..373561626ec4 100644
--- a/drivers/clk/ti/clk-dra7-atl.c
+++ b/drivers/clk/ti/clk-dra7-atl.c
@@ -163,8 +163,8 @@ static const struct clk_ops atl_clk_ops = {
 static void __init of_dra7_atl_clock_setup(struct device_node *node)
 {
 	struct dra7_atl_desc *clk_hw = NULL;
+	struct clk_parent_data pdata = { .index = 0 };
 	struct clk_init_data init = { NULL };
-	const char **parent_names = NULL;
 	const char *name;
 	struct clk *clk;
 
@@ -188,24 +188,15 @@ static void __init of_dra7_atl_clock_setup(struct device_node *node)
 		goto cleanup;
 	}
 
-	parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
-
-	if (!parent_names)
-		goto cleanup;
-
-	parent_names[0] = of_clk_get_parent_name(node, 0);
-
-	init.parent_names = parent_names;
-
-	clk = ti_clk_register(NULL, &clk_hw->hw, name);
+	init.parent_data = &pdata;
 
+	clk = of_ti_clk_register(node, &clk_hw->hw, name);
 	if (!IS_ERR(clk)) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		kfree(parent_names);
 		return;
 	}
+
 cleanup:
-	kfree(parent_names);
 	kfree(clk_hw);
 }
 CLK_OF_DECLARE(dra7_atl_clock, "ti,dra7-atl-clock", of_dra7_atl_clock_setup);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper
  2022-11-06 15:46 [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Dario Binacchi
  2022-11-06 15:46 ` [PATCH v3 2/2] clk: ti: dra7-atl: don't allocate `parent_names' variable Dario Binacchi
@ 2022-11-09  8:15 ` Tony Lindgren
  2022-11-09  8:17   ` Tony Lindgren
  1 sibling, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2022-11-09  8:15 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: linux-kernel, michael, Amarula patchwork, Allison Randal,
	Greg Kroah-Hartman, Liang He, Michael Turquette, Stephen Boyd,
	Tero Kristo, Thomas Gleixner, linux-clk, linux-omap

* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper
  2022-11-09  8:15 ` [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Tony Lindgren
@ 2022-11-09  8:17   ` Tony Lindgren
  2022-11-09  8:38     ` Dario Binacchi
  0 siblings, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2022-11-09  8:17 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: linux-kernel, michael, Amarula patchwork, Allison Randal,
	Greg Kroah-Hartman, Liang He, Michael Turquette, Stephen Boyd,
	Tero Kristo, Thomas Gleixner, linux-clk, linux-omap

* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper
  2022-11-09  8:17   ` Tony Lindgren
@ 2022-11-09  8:38     ` Dario Binacchi
  2022-11-09  8:52       ` Tony Lindgren
  0 siblings, 1 reply; 6+ messages in thread
From: Dario Binacchi @ 2022-11-09  8:38 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-kernel, michael, Amarula patchwork, Allison Randal,
	Greg Kroah-Hartman, Liang He, Michael Turquette, Stephen Boyd,
	Tero Kristo, Thomas Gleixner, linux-clk, linux-omap

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



-- 

Dario Binacchi

Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper
  2022-11-09  8:38     ` Dario Binacchi
@ 2022-11-09  8:52       ` Tony Lindgren
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2022-11-09  8:52 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: linux-kernel, michael, Amarula patchwork, Allison Randal,
	Greg Kroah-Hartman, Liang He, Michael Turquette, Stephen Boyd,
	Tero Kristo, Thomas Gleixner, linux-clk, linux-omap

* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-09  8:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-06 15:46 [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Dario Binacchi
2022-11-06 15:46 ` [PATCH v3 2/2] clk: ti: dra7-atl: don't allocate `parent_names' variable Dario Binacchi
2022-11-09  8:15 ` [PATCH v3 1/2] clk: ti: add of_ti_clk_register() helper Tony Lindgren
2022-11-09  8:17   ` Tony Lindgren
2022-11-09  8:38     ` Dario Binacchi
2022-11-09  8:52       ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.