[PATCH] clk: si5341: disable input clock on probe failure and remove

[PATCH] clk: si5341: disable input clock on probe failure and remove

[Myeonghun Pak]

si5341_clk_select_active_input() prepares and enables the selected
input clock, but the driver never disables it. The devm-managed clock
lookup only releases the clock handle; it does not unwind the
prepare/enable count.

Track the input clock that was successfully prepared and disable it
from both the post-selection probe error path and the remove path.

Fixes: 3044a860fd09 ("clk: Add Si5341/Si5340 driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/clk/clk-si5341.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index 2499b77..14f80a5 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -74,6 +74,7 @@ struct clk_si5341 {
 	struct clk_si5341_output clk[SI5341_MAX_NUM_OUTPUTS];
 	struct clk *input_clk[SI5341_NUM_INPUTS];
 	const char *input_clk_name[SI5341_NUM_INPUTS];
+	int prepared_input;
 	const u16 *reg_output_offset;
 	const u16 *reg_rdiv_offset;
 	u64 freq_vco; /* 13500–14256 MHz */
@@ -1463,9 +1464,20 @@ static int si5341_clk_select_active_input(struct clk_si5341 *data)
 	if (err < 0)
 		return err;
 
+	data->prepared_input = res;
+
 	return res;
 }
 
+static void si5341_clk_disable_active_input(struct clk_si5341 *data)
+{
+	if (data->prepared_input < 0)
+		return;
+
+	clk_disable_unprepare(data->input_clk[data->prepared_input]);
+	data->prepared_input = -1;
+}
+
 static ssize_t input_present_show(struct device *dev,
 				  struct device_attribute *attr,
 				  char *buf)
@@ -1571,6 +1583,7 @@ static int si5341_probe(struct i2c_client *client)
 	if (!data)
 		return -ENOMEM;
 
+	data->prepared_input = -1;
 	data->i2c_client = client;
 
 	/* Must be done before otherwise touching hardware */
@@ -1803,6 +1816,7 @@ static int si5341_probe(struct i2c_client *client)
 
 cleanup:
 	if (err) {
+		si5341_clk_disable_active_input(data);
 		for (i = 0; i < SI5341_MAX_NUM_OUTPUTS; ++i) {
 			if (data->clk[i].vddo_reg)
 				regulator_disable(data->clk[i].vddo_reg);
@@ -1818,6 +1832,8 @@ static void si5341_remove(struct i2c_client *client)
 
 	sysfs_remove_files(&client->dev.kobj, si5341_attributes);
 
+	si5341_clk_disable_active_input(data);
+
 	for (i = 0; i < SI5341_MAX_NUM_OUTPUTS; ++i) {
 		if (data->clk[i].vddo_reg)
 			regulator_disable(data->clk[i].vddo_reg);
-- 
2.50.0

Re: [PATCH] clk: si5341: disable input clock on probe failure and remove

[Mike Looijmans]

On 6/26/26 11:33, Myeonghun Pak wrote:
> si5341_clk_select_active_input() prepares and enables the selected
> input clock, but the driver never disables it. The devm-managed clock
> lookup only releases the clock handle; it does not unwind the
> prepare/enable count.
>
> Track the input clock that was successfully prepared and disable it
> from both the post-selection probe error path and the remove path.
>
> Fixes: 3044a860fd09 ("clk: Add Si5341/Si5340 driver")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>   drivers/clk/clk-si5341.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
> index 2499b77..14f80a5 100644
> --- a/drivers/clk/clk-si5341.c
> +++ b/drivers/clk/clk-si5341.c
> @@ -74,6 +74,7 @@ struct clk_si5341 {
>   	struct clk_si5341_output clk[SI5341_MAX_NUM_OUTPUTS];
>   	struct clk *input_clk[SI5341_NUM_INPUTS];
>   	const char *input_clk_name[SI5341_NUM_INPUTS];
> +	int prepared_input;

I don't think you need this here. At the points where you want to disable the 
clock, you already know that the clock has been enabled. For example, you 
cannot reach the "remove" function without having it enabled. So in remove, 
just disable the input clock unconditionally.


>   	const u16 *reg_output_offset;
>   	const u16 *reg_rdiv_offset;
>   	u64 freq_vco; /* 13500–14256 MHz */
> @@ -1463,9 +1464,20 @@ static int si5341_clk_select_active_input(struct clk_si5341 *data)
>   	if (err < 0)
>   		return err;
>   
> +	data->prepared_input = res;
> +
>   	return res;
>   }
>   
> +static void si5341_clk_disable_active_input(struct clk_si5341 *data)
> +{
> +	if (data->prepared_input < 0)
> +		return;
> +
> +	clk_disable_unprepare(data->input_clk[data->prepared_input]);
> +	data->prepared_input = -1;
> +}
> +
>   static ssize_t input_present_show(struct device *dev,
>   				  struct device_attribute *attr,
>   				  char *buf)
> @@ -1571,6 +1583,7 @@ static int si5341_probe(struct i2c_client *client)
>   	if (!data)
>   		return -ENOMEM;
>   
> +	data->prepared_input = -1;
>   	data->i2c_client = client;
>   
>   	/* Must be done before otherwise touching hardware */
> @@ -1803,6 +1816,7 @@ static int si5341_probe(struct i2c_client *client)
>   
>   cleanup:
>   	if (err) {
> +		si5341_clk_disable_active_input(data);
>   		for (i = 0; i < SI5341_MAX_NUM_OUTPUTS; ++i) {
>   			if (data->clk[i].vddo_reg)
>   				regulator_disable(data->clk[i].vddo_reg);
> @@ -1818,6 +1832,8 @@ static void si5341_remove(struct i2c_client *client)
>   
>   	sysfs_remove_files(&client->dev.kobj, si5341_attributes);
>   
> +	si5341_clk_disable_active_input(data);
> +
>   	for (i = 0; i < SI5341_MAX_NUM_OUTPUTS; ++i) {
>   		if (data->clk[i].vddo_reg)
>   			regulator_disable(data->clk[i].vddo_reg);

--

Mike.

[PATCH v2] clk: si5341: disable input clock on probe failure and remove

[Myeonghun Pak]

si5341_clk_select_active_input() prepares and enables the selected
input clock, but the driver never disables it. devm_clk_get() only
releases the clock handle; it does not unwind the prepare/enable count.

Register a devm cleanup action for the exact input clock after
clk_prepare_enable() succeeds. This unwinds the prepare/enable reference
on both probe failure after input selection and driver detach, without
having to re-read the current input parent later.

Fixes: 3044a860fd09 ("clk: Add Si5341/Si5340 driver")
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_add_action_or_reset() for the prepared input clock.
- Drop the prepared_input state and manual cleanup calls.

 drivers/clk/clk-si5341.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index 2499b77..2c36757 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -1426,6 +1426,11 @@ static int si5341_initialize_pll(struct clk_si5341 *data)
 			SI5341_PLL_M_NUM, m_num, m_den);
 }
 
+static void si5341_clk_disable_unprepare(void *clk)
+{
+	clk_disable_unprepare(clk);
+}
+
 static int si5341_clk_select_active_input(struct clk_si5341 *data)
 {
 	int res;
@@ -1463,6 +1468,12 @@ static int si5341_clk_select_active_input(struct clk_si5341 *data)
 	if (err < 0)
 		return err;
 
+	err = devm_add_action_or_reset(&data->i2c_client->dev,
+				       si5341_clk_disable_unprepare,
+				       data->input_clk[res]);
+	if (err < 0)
+		return err;
+
 	return res;
 }
 
-- 
2.50.0