[PATCH 25/31] clk: mediatek: pll: Implement error handling in register API

Chen-Yu Tsai posted 31 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH 25/31] clk: mediatek: pll: Implement error handling in register API
Posted by Chen-Yu Tsai 2 years, 9 months ago
The pll clk type registration function does not stop or return errors
if any clk failed to be registered, nor does it implement an error
handling path. This may result in a partially working device if any
step failed.

Make the register function return proper error codes, and bail out if
errors occur. Proper cleanup, i.e. unregister any clks that were
successfully registered, and unmap the I/O space, is done in the new
error path.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-pll.c | 23 +++++++++++++++++++----
 drivers/clk/mediatek/clk-pll.h |  6 +++---
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
index 9698d1c97cd6..1dd15f560659 100644
--- a/drivers/clk/mediatek/clk-pll.c
+++ b/drivers/clk/mediatek/clk-pll.c
@@ -369,8 +369,9 @@ static void mtk_clk_unregister_pll(struct clk *clk)
 	kfree(pll);
 }
 
-void mtk_clk_register_plls(struct device_node *node,
-		const struct mtk_pll_data *plls, int num_plls, struct clk_onecell_data *clk_data)
+int mtk_clk_register_plls(struct device_node *node,
+			  const struct mtk_pll_data *plls, int num_plls,
+			  struct clk_onecell_data *clk_data)
 {
 	void __iomem *base;
 	int i;
@@ -379,7 +380,7 @@ void mtk_clk_register_plls(struct device_node *node,
 	base = of_iomap(node, 0);
 	if (!base) {
 		pr_err("%s(): ioremap failed\n", __func__);
-		return;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < num_plls; i++) {
@@ -389,11 +390,25 @@ void mtk_clk_register_plls(struct device_node *node,
 
 		if (IS_ERR(clk)) {
 			pr_err("Failed to register clk %s: %pe\n", pll->name, clk);
-			continue;
+			goto err;
 		}
 
 		clk_data->clks[pll->id] = clk;
 	}
+
+	return 0;
+
+err:
+	while (--i >= 0) {
+		const struct mtk_pll_data *pll = &plls[i];
+
+		mtk_clk_unregister_pll(clk_data->clks[pll->id]);
+		clk_data->clks[pll->id] = ERR_PTR(-ENOENT);
+	};
+
+	iounmap(base);
+
+	return PTR_ERR(clk);
 }
 EXPORT_SYMBOL_GPL(mtk_clk_register_plls);
 
diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h
index a889b1e472e7..bf06e44caef9 100644
--- a/drivers/clk/mediatek/clk-pll.h
+++ b/drivers/clk/mediatek/clk-pll.h
@@ -48,9 +48,9 @@ struct mtk_pll_data {
 	u8 pll_en_bit; /* Assume 0, indicates BIT(0) by default */
 };
 
-void mtk_clk_register_plls(struct device_node *node,
-			   const struct mtk_pll_data *plls, int num_plls,
-			   struct clk_onecell_data *clk_data);
+int mtk_clk_register_plls(struct device_node *node,
+			  const struct mtk_pll_data *plls, int num_plls,
+			  struct clk_onecell_data *clk_data);
 void mtk_clk_unregister_plls(const struct mtk_pll_data *plls, int num_plls,
 			     struct clk_onecell_data *clk_data);
 
-- 
2.35.0.rc0.227.g00780c9af4-goog

Re: [PATCH 25/31] clk: mediatek: pll: Implement error handling in register API
Posted by kernel test robot 2 years, 9 months ago
Hi Chen-Yu,

I love your patch! Perhaps something to improve:

[auto build test WARNING on clk/clk-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chen-Yu-Tsai/clk-mediatek-Cleanups-and-Improvements-Part-1/20220122-172158
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: h8300-randconfig-c003-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242043.D1yrOE9S-lkp@intel.com/config)
compiler: h8300-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cocci warnings: (new ones prefixed by >>)
>> drivers/clk/mediatek/clk-pll.c:407:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Re: [PATCH 25/31] clk: mediatek: pll: Implement error handling in register API
Posted by Miles Chen 2 years, 9 months ago
> The pll clk type registration function does not stop or return errors
> if any clk failed to be registered, nor does it implement an error
> handling path. This may result in a partially working device if any
> step failed.
> 
> Make the register function return proper error codes, and bail out if
> errors occur. Proper cleanup, i.e. unregister any clks that were
> successfully registered, and unmap the I/O space, is done in the new
> error path.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: Miles Chen <miles.chen@mediatek.com>
> ---
>  drivers/clk/mediatek/clk-pll.c | 23 +++++++++++++++++++----
>  drivers/clk/mediatek/clk-pll.h |  6 +++---
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
> index 9698d1c97cd6..1dd15f560659 100644
> --- a/drivers/clk/mediatek/clk-pll.c
> +++ b/drivers/clk/mediatek/clk-pll.c
> @@ -369,8 +369,9 @@ static void mtk_clk_unregister_pll(struct clk *clk)
>  	kfree(pll);
>  }
>  
> -void mtk_clk_register_plls(struct device_node *node,
> -		const struct mtk_pll_data *plls, int num_plls, struct clk_onecell_data *clk_data)
> +int mtk_clk_register_plls(struct device_node *node,
> +			  const struct mtk_pll_data *plls, int num_plls,
> +			  struct clk_onecell_data *clk_data)
>  {
>  	void __iomem *base;
>  	int i;
> @@ -379,7 +380,7 @@ void mtk_clk_register_plls(struct device_node *node,
>  	base = of_iomap(node, 0);
>  	if (!base) {
>  		pr_err("%s(): ioremap failed\n", __func__);
> -		return;
> +		return -EINVAL;
>  	}
>  
>  	for (i = 0; i < num_plls; i++) {
> @@ -389,11 +390,25 @@ void mtk_clk_register_plls(struct device_node *node,
>  
>  		if (IS_ERR(clk)) {
>  			pr_err("Failed to register clk %s: %pe\n", pll->name, clk);
> -			continue;
> +			goto err;
>  		}
>  
>  		clk_data->clks[pll->id] = clk;
>  	}
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0) {
> +		const struct mtk_pll_data *pll = &plls[i];
> +
> +		mtk_clk_unregister_pll(clk_data->clks[pll->id]);
> +		clk_data->clks[pll->id] = ERR_PTR(-ENOENT);
> +	};
> +
> +	iounmap(base);
> +
> +	return PTR_ERR(clk);
>  }
>  EXPORT_SYMBOL_GPL(mtk_clk_register_plls);
>  
> diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h
> index a889b1e472e7..bf06e44caef9 100644
> --- a/drivers/clk/mediatek/clk-pll.h
> +++ b/drivers/clk/mediatek/clk-pll.h
> @@ -48,9 +48,9 @@ struct mtk_pll_data {
>  	u8 pll_en_bit; /* Assume 0, indicates BIT(0) by default */
>  };
>  
> -void mtk_clk_register_plls(struct device_node *node,
> -			   const struct mtk_pll_data *plls, int num_plls,
> -			   struct clk_onecell_data *clk_data);
> +int mtk_clk_register_plls(struct device_node *node,
> +			  const struct mtk_pll_data *plls, int num_plls,
> +			  struct clk_onecell_data *clk_data);
>  void mtk_clk_unregister_plls(const struct mtk_pll_data *plls, int num_plls,
>  			     struct clk_onecell_data *clk_data);
>  
> -- 
> 2.35.0.rc0.227.g00780c9af4-goog


[PATCH] clk: mediatek: pll: fix semicolon.cocci warnings
Posted by kernel test robot 2 years, 9 months ago
From: kernel test robot <lkp@intel.com>

drivers/clk/mediatek/clk-pll.c:407:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---

url:    https://github.com/0day-ci/linux/commits/Chen-Yu-Tsai/clk-mediatek-Cleanups-and-Improvements-Part-1/20220122-172158
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago

 clk-pll.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/clk/mediatek/clk-pll.c
+++ b/drivers/clk/mediatek/clk-pll.c
@@ -404,7 +404,7 @@ err:
 
 		mtk_clk_unregister_pll(clk_data->clks[pll->id]);
 		clk_data->clks[pll->id] = ERR_PTR(-ENOENT);
-	};
+	}
 
 	iounmap(base);