[PATCH 0/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[PATCH 0/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

This patch set improves error handling and replaces the deprecated
of_clk_add_provider() function with of_clk_add_hw_provider() in the
Intel SoC-FPGA family clock drivers.

The patch set is based on top of the patch: "Fix memory leak in
socfpga_gate_init()" to avoid a conflict.

https://lore.kernel.org/all/20221123031622.63171-1-xiujianfeng@huawei.com/

Marco Pagani (6):
  clk: socfpga: use of_clk_add_hw_provider and improve error handling
  clk: socfpga: arria10: use of_clk_add_hw_provider and improve error
    handling
  clk: socfpga: use of_clk_add_hw_provider and improve error handling
  clk: socfpga: arria10: use of_clk_add_hw_provider and improve error
    handling
  clk: socfpga: use of_clk_add_hw_provider and improve error handling
  clk: socfpga: arria10: use of_clk_add_hw_provider and improve error
    handling

 drivers/clk/socfpga/clk-gate-a10.c   | 26 +++++++++++++++------
 drivers/clk/socfpga/clk-gate.c       | 35 +++++++++++++++++-----------
 drivers/clk/socfpga/clk-periph-a10.c | 22 ++++++++++-------
 drivers/clk/socfpga/clk-periph.c     | 26 ++++++++++++++++-----
 drivers/clk/socfpga/clk-pll-a10.c    | 30 +++++++++++++++++-------
 drivers/clk/socfpga/clk-pll.c        | 32 +++++++++++++++++--------
 6 files changed, 118 insertions(+), 53 deletions(-)

-- 
2.38.1

[PATCH 1/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The indentation of the init function parameters has been aligned
to match open parenthesis, as suggested by checkpatch, and the __init
macro moved before the function name, as specified in init.h.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-periph.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/socfpga/clk-periph.c b/drivers/clk/socfpga/clk-periph.c
index 43707e2d7248..6a4075147b9c 100644
--- a/drivers/clk/socfpga/clk-periph.c
+++ b/drivers/clk/socfpga/clk-periph.c
@@ -47,8 +47,8 @@ static const struct clk_ops periclk_ops = {
 	.get_parent = clk_periclk_get_parent,
 };
 
-static __init void __socfpga_periph_init(struct device_node *node,
-	const struct clk_ops *ops)
+static void __init __socfpga_periph_init(struct device_node *node,
+					 const struct clk_ops *ops)
 {
 	u32 reg;
 	struct clk_hw *hw_clk;
@@ -96,11 +96,25 @@ static __init void __socfpga_periph_init(struct device_node *node,
 	periph_clk->hw.hw.init = &init;
 	hw_clk = &periph_clk->hw.hw;
 
-	if (clk_hw_register(NULL, hw_clk)) {
-		kfree(periph_clk);
-		return;
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
+	}
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock provider for node:%s\n",
+		       clk_name);
+		goto err_of_clk_add_hw_provider;
 	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
+
+	return;
+
+err_of_clk_add_hw_provider:
+	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(periph_clk);
 }
 
 void __init socfpga_periph_init(struct device_node *node)
-- 
2.38.1

[PATCH 2/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The indentation of the init function parameters has been aligned
to match open parenthesis, as suggested by checkpatch, and the __init
macro moved before the function name, as specified in init.h.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-periph-a10.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/socfpga/clk-periph-a10.c b/drivers/clk/socfpga/clk-periph-a10.c
index b9cdde4b8441..64cc70b970b7 100644
--- a/drivers/clk/socfpga/clk-periph-a10.c
+++ b/drivers/clk/socfpga/clk-periph-a10.c
@@ -57,8 +57,8 @@ static const struct clk_ops periclk_ops = {
 	.get_parent = clk_periclk_get_parent,
 };
 
-static __init void __socfpga_periph_init(struct device_node *node,
-	const struct clk_ops *ops)
+static void __init __socfpga_periph_init(struct device_node *node,
+					 const struct clk_ops *ops)
 {
 	u32 reg;
 	struct clk_hw *hw_clk;
@@ -106,21 +106,25 @@ static __init void __socfpga_periph_init(struct device_node *node,
 
 	hw_clk = &periph_clk->hw.hw;
 
-	if (clk_hw_register(NULL, hw_clk)) {
-		kfree(periph_clk);
-		return;
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
 	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
-	if (rc < 0) {
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
 		pr_err("Could not register clock provider for node:%s\n",
 		       clk_name);
-		goto err_clk;
+		goto err_of_clk_add_hw_provider;
 	}
 
 	return;
 
-err_clk:
+err_of_clk_add_hw_provider:
 	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(periph_clk);
 }
 
 void __init socfpga_a10_periph_init(struct device_node *node)
-- 
2.38.1

[PATCH 3/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The err variable unnecessarily duplicates the functionality of the
rc variable, so it has been removed.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-gate.c | 35 +++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
index 0c18c55edf8c..32ccda960f28 100644
--- a/drivers/clk/socfpga/clk-gate.c
+++ b/drivers/clk/socfpga/clk-gate.c
@@ -126,17 +126,14 @@ void __init socfpga_gate_init(struct device_node *node)
 	struct clk_init_data init;
 	struct clk_ops *ops;
 	int rc;
-	int err;
 
 	socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL);
 	if (WARN_ON(!socfpga_clk))
 		return;
 
 	ops = kmemdup(&gateclk_ops, sizeof(gateclk_ops), GFP_KERNEL);
-	if (WARN_ON(!ops)) {
-		kfree(socfpga_clk);
-		return;
-	}
+	if (WARN_ON(!ops))
+		goto err_kmemdup;
 
 	rc = of_property_read_u32_array(node, "clk-gate", clk_gate, 2);
 	if (rc)
@@ -182,13 +179,25 @@ void __init socfpga_gate_init(struct device_node *node)
 
 	hw_clk = &socfpga_clk->hw.hw;
 
-	err = clk_hw_register(NULL, hw_clk);
-	if (err) {
-		kfree(ops);
-		kfree(socfpga_clk);
-		return;
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
 	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
-	if (WARN_ON(rc))
-		return;
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock provider for node:%s\n",
+		       clk_name);
+		goto err_of_clk_add_hw_provider;
+	}
+
+	return;
+
+err_of_clk_add_hw_provider:
+	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(ops);
+err_kmemdup:
+	kfree(socfpga_clk);
 }
-- 
2.38.1

[PATCH 4/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The indentation of the init function parameters has been aligned
to match open parenthesis as suggested by checkpatch.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-gate-a10.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/socfpga/clk-gate-a10.c b/drivers/clk/socfpga/clk-gate-a10.c
index 7cdf2f07c79b..06f129c160bc 100644
--- a/drivers/clk/socfpga/clk-gate-a10.c
+++ b/drivers/clk/socfpga/clk-gate-a10.c
@@ -40,7 +40,7 @@ static struct clk_ops gateclk_ops = {
 };
 
 static void __init __socfpga_gate_init(struct device_node *node,
-	const struct clk_ops *ops)
+				       const struct clk_ops *ops)
 {
 	u32 clk_gate[2];
 	u32 div_reg[3];
@@ -94,13 +94,25 @@ static void __init __socfpga_gate_init(struct device_node *node,
 	socfpga_clk->hw.hw.init = &init;
 	hw_clk = &socfpga_clk->hw.hw;
 
-	if (clk_hw_register(NULL, hw_clk)) {
-		kfree(socfpga_clk);
-		return;
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
 	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
-	if (WARN_ON(rc))
-		return;
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock provider for node:%s\n",
+		       clk_name);
+		goto err_of_clk_add_hw_provider;
+	}
+
+	return;
+
+err_of_clk_add_hw_provider:
+	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(socfpga_clk);
 }
 
 void __init socfpga_a10_gate_init(struct device_node *node)
-- 
2.38.1

[PATCH 5/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The return type of the init function has been changed to void since
the return value was not used, and the indentation of the parameters has
been aligned to match open parenthesis, as suggested by checkpatch.

The err variable has been renamed rc for consistency.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-pll.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
index 127cc849c5ee..9dcc1b2d2cc0 100644
--- a/drivers/clk/socfpga/clk-pll.c
+++ b/drivers/clk/socfpga/clk-pll.c
@@ -70,8 +70,8 @@ static const struct clk_ops clk_pll_ops = {
 	.get_parent = clk_pll_get_parent,
 };
 
-static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
-	const struct clk_ops *ops)
+static void __init __socfpga_pll_init(struct device_node *node,
+				      const struct clk_ops *ops)
 {
 	u32 reg;
 	struct clk_hw *hw_clk;
@@ -80,13 +80,13 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 	const char *parent_name[SOCFPGA_MAX_PARENTS];
 	struct clk_init_data init;
 	struct device_node *clkmgr_np;
-	int err;
+	int rc;
 
 	of_property_read_u32(node, "reg", &reg);
 
 	pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL);
 	if (WARN_ON(!pll_clk))
-		return NULL;
+		return;
 
 	clkmgr_np = of_find_compatible_node(NULL, NULL, "altr,clk-mgr");
 	clk_mgr_base_addr = of_iomap(clkmgr_np, 0);
@@ -108,13 +108,25 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 
 	hw_clk = &pll_clk->hw.hw;
 
-	err = clk_hw_register(NULL, hw_clk);
-	if (err) {
-		kfree(pll_clk);
-		return ERR_PTR(err);
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
 	}
-	of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
-	return hw_clk;
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock provider for node:%s\n",
+		       clk_name);
+		goto err_of_clk_add_hw_provider;
+	}
+
+	return;
+
+err_of_clk_add_hw_provider:
+	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(pll_clk);
 }
 
 void __init socfpga_pll_init(struct device_node *node)
-- 
2.38.1

[PATCH 6/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Marco Pagani]

The function of_clk_add_provider() has been deprecated, so use its
suggested replacement of_clk_add_hw_provider() instead.

Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
check its return value and do the error handling.

The return type of the init function has been changed to void since
the return value was not used, and the indentation of the parameters has
been aligned to match open parenthesis, as suggested by checkpatch.

Signed-off-by: Marco Pagani <marpagan@redhat.com>
---
 drivers/clk/socfpga/clk-pll-a10.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/socfpga/clk-pll-a10.c b/drivers/clk/socfpga/clk-pll-a10.c
index bee0f7da5b6e..b028f25c658a 100644
--- a/drivers/clk/socfpga/clk-pll-a10.c
+++ b/drivers/clk/socfpga/clk-pll-a10.c
@@ -63,8 +63,8 @@ static const struct clk_ops clk_pll_ops = {
 	.get_parent = clk_pll_get_parent,
 };
 
-static struct clk_hw * __init __socfpga_pll_init(struct device_node *node,
-	const struct clk_ops *ops)
+static void __init __socfpga_pll_init(struct device_node *node,
+				      const struct clk_ops *ops)
 {
 	u32 reg;
 	struct clk_hw *hw_clk;
@@ -73,13 +73,14 @@ static struct clk_hw * __init __socfpga_pll_init(struct device_node *node,
 	const char *parent_name[SOCFGPA_MAX_PARENTS];
 	struct clk_init_data init;
 	struct device_node *clkmgr_np;
+	int rc;
 	int i = 0;
 
 	of_property_read_u32(node, "reg", &reg);
 
 	pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL);
 	if (WARN_ON(!pll_clk))
-		return NULL;
+		return;
 
 	clkmgr_np = of_find_compatible_node(NULL, NULL, "altr,clk-mgr");
 	clk_mgr_a10_base_addr = of_iomap(clkmgr_np, 0);
@@ -103,12 +104,25 @@ static struct clk_hw * __init __socfpga_pll_init(struct device_node *node,
 	pll_clk->hw.bit_idx = SOCFPGA_PLL_EXT_ENA;
 	hw_clk = &pll_clk->hw.hw;
 
-	if (clk_hw_register(NULL, hw_clk)) {
-		kfree(pll_clk);
-		return NULL;
+	rc = clk_hw_register(NULL, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock:%s\n", clk_name);
+		goto err_clk_hw_register;
 	}
-	of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
-	return hw_clk;
+
+	rc = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw_clk);
+	if (rc) {
+		pr_err("Could not register clock provider for node:%s\n",
+		       clk_name);
+		goto err_of_clk_add_hw_provider;
+	}
+
+	return;
+
+err_of_clk_add_hw_provider:
+	clk_hw_unregister(hw_clk);
+err_clk_hw_register:
+	kfree(pll_clk);
 }
 
 void __init socfpga_a10_pll_init(struct device_node *node)
-- 
2.38.1

Re: [PATCH 0/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:07)
> This patch set improves error handling and replaces the deprecated
> of_clk_add_provider() function with of_clk_add_hw_provider() in the
> Intel SoC-FPGA family clock drivers.
> 
> The patch set is based on top of the patch: "Fix memory leak in
> socfpga_gate_init()" to avoid a conflict.
> 
> https://lore.kernel.org/all/20221123031622.63171-1-xiujianfeng@huawei.com/

I was waiting for Dinh to review this. I guess that won't happen so I'll
just go apply this.

Re: [PATCH 1/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:08)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The indentation of the init function parameters has been aligned
> to match open parenthesis, as suggested by checkpatch, and the __init
> macro moved before the function name, as specified in init.h.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next

Re: [PATCH 2/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:09)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The indentation of the init function parameters has been aligned
> to match open parenthesis, as suggested by checkpatch, and the __init
> macro moved before the function name, as specified in init.h.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next

Re: [PATCH 3/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:10)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The err variable unnecessarily duplicates the functionality of the
> rc variable, so it has been removed.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next

Re: [PATCH 4/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:11)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The indentation of the init function parameters has been aligned
> to match open parenthesis as suggested by checkpatch.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next

Re: [PATCH 5/6] clk: socfpga: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:12)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The return type of the init function has been changed to void since
> the return value was not used, and the indentation of the parameters has
> been aligned to match open parenthesis, as suggested by checkpatch.
> 
> The err variable has been renamed rc for consistency.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next

Re: [PATCH 6/6] clk: socfpga: arria10: use of_clk_add_hw_provider and improve error handling

[Stephen Boyd]

Quoting Marco Pagani (2022-12-09 07:29:13)
> The function of_clk_add_provider() has been deprecated, so use its
> suggested replacement of_clk_add_hw_provider() instead.
> 
> Since of_clk_add_hw_provider() can fail, like of_clk_add_provider(),
> check its return value and do the error handling.
> 
> The return type of the init function has been changed to void since
> the return value was not used, and the indentation of the parameters has
> been aligned to match open parenthesis, as suggested by checkpatch.
> 
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---

Applied to clk-next