[PATCH v2] clk: palmas: Balance external-control clock prepare

[PATCH v2] clk: palmas: Balance external-control clock prepare

[Myeonghun Pak]

palmas_clks_init_configure() prepares the clock when an external control
pin is configured.  The current driver only drops that prepare reference
when external control configuration fails.

If provider registration fails after that point, or if the driver is later
removed, the prepare reference remains held.

Use devm_add_action_or_reset() to manage the prepare reference.  Use
devm_of_clk_add_hw_provider() to manage provider registration.  This
balances the prepare reference on probe failure and driver removal.

Fixes: 942d1d674931 ("clk: Add driver for Palmas clk32kg and clk32kgaudio clocks")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Use devm_of_clk_add_hw_provider() for provider cleanup.
- Manage the external-control clk_prepare() with devm_add_action_or_reset().
- Remove the manual provider remove callback.

 drivers/clk/clk-palmas.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c
index 39049f62db..fdbf8445eb 100644
--- a/drivers/clk/clk-palmas.c
+++ b/drivers/clk/clk-palmas.c
@@ -194,6 +194,13 @@ static void palmas_clks_get_clk_data(struct platform_device *pdev,
 	cinfo->ext_control_pin = prop;
 }
 
+static void palmas_clks_unprepare_ext_control(void *data)
+{
+	struct palmas_clock_info *cinfo = data;
+
+	clk_unprepare(cinfo->hw.clk);
+}
+
 static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
 {
 	int ret;
@@ -214,13 +221,18 @@ static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
 			return ret;
 		}
 
+		ret = devm_add_action_or_reset(cinfo->dev,
+					       palmas_clks_unprepare_ext_control,
+					       cinfo);
+		if (ret)
+			return ret;
+
 		ret = palmas_ext_control_req_config(cinfo->palmas,
 					cinfo->clk_desc->sleep_reqstr_id,
 					cinfo->ext_control_pin, true);
 		if (ret < 0) {
 			dev_err(cinfo->dev, "Ext config for %s failed, %d\n",
 				cinfo->clk_desc->clk_name, ret);
-			clk_unprepare(cinfo->hw.clk);
 			return ret;
 		}
 	}
@@ -230,7 +242,6 @@ static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
 static int palmas_clks_probe(struct platform_device *pdev)
 {
 	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
-	struct device_node *node = pdev->dev.of_node;
 	const struct palmas_clks_of_match_data *match_data;
 	struct palmas_clock_info *cinfo;
 	int ret;
@@ -264,24 +275,19 @@ static int palmas_clks_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = of_clk_add_hw_provider(node, of_clk_hw_simple_get, &cinfo->hw);
+	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_simple_get,
+					  &cinfo->hw);
 	if (ret < 0)
 		dev_err(&pdev->dev, "Fail to add clock driver, %d\n", ret);
 	return ret;
 }
 
-static void palmas_clks_remove(struct platform_device *pdev)
-{
-	of_clk_del_provider(pdev->dev.of_node);
-}
-
 static struct platform_driver palmas_clks_driver = {
 	.driver = {
 		.name = "palmas-clk",
 		.of_match_table = palmas_clks_of_match,
 	},
 	.probe = palmas_clks_probe,
-	.remove = palmas_clks_remove,
 };
 
 module_platform_driver(palmas_clks_driver);
-- 
2.47.1

Re: [PATCH v2] clk: palmas: Balance external-control clock prepare

[Brian Masney]

Hi Myeonghun,

On Wed, Jun 24, 2026 at 04:05:42PM +0900, Myeonghun Pak wrote:
> palmas_clks_init_configure() prepares the clock when an external control
> pin is configured.  The current driver only drops that prepare reference
> when external control configuration fails.
> 
> If provider registration fails after that point, or if the driver is later
> removed, the prepare reference remains held.
> 
> Use devm_add_action_or_reset() to manage the prepare reference.  Use
> devm_of_clk_add_hw_provider() to manage provider registration.  This
> balances the prepare reference on probe failure and driver removal.
> 
> Fixes: 942d1d674931 ("clk: Add driver for Palmas clk32kg and clk32kgaudio clocks")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

Please split these into two separate patches. Otherwise, it looks good
to me.

Brian


> ---
> Changes in v2:
> - Use devm_of_clk_add_hw_provider() for provider cleanup.
> - Manage the external-control clk_prepare() with devm_add_action_or_reset().
> - Remove the manual provider remove callback.
> 
>  drivers/clk/clk-palmas.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c
> index 39049f62db..fdbf8445eb 100644
> --- a/drivers/clk/clk-palmas.c
> +++ b/drivers/clk/clk-palmas.c
> @@ -194,6 +194,13 @@ static void palmas_clks_get_clk_data(struct platform_device *pdev,
>  	cinfo->ext_control_pin = prop;
>  }
>  
> +static void palmas_clks_unprepare_ext_control(void *data)
> +{
> +	struct palmas_clock_info *cinfo = data;
> +
> +	clk_unprepare(cinfo->hw.clk);
> +}
> +
>  static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
>  {
>  	int ret;
> @@ -214,13 +221,18 @@ static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
>  			return ret;
>  		}
>  
> +		ret = devm_add_action_or_reset(cinfo->dev,
> +					       palmas_clks_unprepare_ext_control,
> +					       cinfo);
> +		if (ret)
> +			return ret;
> +
>  		ret = palmas_ext_control_req_config(cinfo->palmas,
>  					cinfo->clk_desc->sleep_reqstr_id,
>  					cinfo->ext_control_pin, true);
>  		if (ret < 0) {
>  			dev_err(cinfo->dev, "Ext config for %s failed, %d\n",
>  				cinfo->clk_desc->clk_name, ret);
> -			clk_unprepare(cinfo->hw.clk);
>  			return ret;
>  		}
>  	}
> @@ -230,7 +242,6 @@ static int palmas_clks_init_configure(struct palmas_clock_info *cinfo)
>  static int palmas_clks_probe(struct platform_device *pdev)
>  {
>  	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
> -	struct device_node *node = pdev->dev.of_node;
>  	const struct palmas_clks_of_match_data *match_data;
>  	struct palmas_clock_info *cinfo;
>  	int ret;
> @@ -264,24 +275,19 @@ static int palmas_clks_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = of_clk_add_hw_provider(node, of_clk_hw_simple_get, &cinfo->hw);
> +	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_simple_get,
> +					  &cinfo->hw);
>  	if (ret < 0)
>  		dev_err(&pdev->dev, "Fail to add clock driver, %d\n", ret);
>  	return ret;
>  }
>  
> -static void palmas_clks_remove(struct platform_device *pdev)
> -{
> -	of_clk_del_provider(pdev->dev.of_node);
> -}
> -
>  static struct platform_driver palmas_clks_driver = {
>  	.driver = {
>  		.name = "palmas-clk",
>  		.of_match_table = palmas_clks_of_match,
>  	},
>  	.probe = palmas_clks_probe,
> -	.remove = palmas_clks_remove,
>  };
>  
>  module_platform_driver(palmas_clks_driver);
> -- 
> 2.47.1