[PATCH] clk: si5351: fail prepare when PLL reset times out

[PATCH] clk: si5351: fail prepare when PLL reset times out

[Pengpeng Hou]

si5351_clkout_prepare() clears the output powerdown bit and, when
requested, resets the parent PLL before enabling the output. The PLL
reset helper logs when the reset bit does not clear, but returns void,
so prepare still enables the output and reports success.

Make the reset helper return the poll error and propagate it from
.prepare. If the reset fails, restore the output powerdown bit before
returning the error.

Fixes: 5142cbcea324 ("clk: si5351: Wait for bit clear after PLL reset")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/clk/clk-si5351.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index e755db545..4e60261da 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -907,18 +907,18 @@ static int _si5351_clkout_set_disable_state(
 	return 0;
 }
 
-static void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
+static int _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
 {
 	u8 val = si5351_reg_read(drvdata, SI5351_CLK0_CTRL + num);
 	u8 mask = val & SI5351_CLK_PLL_SELECT ? SI5351_PLL_RESET_B :
-						       SI5351_PLL_RESET_A;
+							       SI5351_PLL_RESET_A;
 	unsigned int v;
 	int err;
 
 	switch (val & SI5351_CLK_INPUT_MASK) {
 	case SI5351_CLK_INPUT_XTAL:
 	case SI5351_CLK_INPUT_CLKIN:
-		return;  /* pll not used, no need to reset */
+		return 0;  /* pll not used, no need to reset */
 	}
 
 	si5351_reg_write(drvdata, SI5351_PLL_RESET, mask);
@@ -931,6 +931,8 @@ static void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num
 	dev_dbg(&drvdata->client->dev, "%s - %s: pll = %d\n",
 		__func__, clk_hw_get_name(&drvdata->clkout[num].hw),
 		(val & SI5351_CLK_PLL_SELECT) ? 1 : 0);
+
+	return err;
 }
 
 static int si5351_clkout_prepare(struct clk_hw *hw)
@@ -947,8 +949,18 @@ static int si5351_clkout_prepare(struct clk_hw *hw)
 	 * Do a pll soft reset on the parent pll -- needed to get a
 	 * deterministic phase relationship between the output clocks.
 	 */
-	if (pdata->clkout[hwdata->num].pll_reset)
-		_si5351_clkout_reset_pll(hwdata->drvdata, hwdata->num);
+	if (pdata->clkout[hwdata->num].pll_reset) {
+		int ret;
+
+		ret = _si5351_clkout_reset_pll(hwdata->drvdata, hwdata->num);
+		if (ret) {
+			si5351_set_bits(hwdata->drvdata,
+					SI5351_CLK0_CTRL + hwdata->num,
+					SI5351_CLK_POWERDOWN,
+					SI5351_CLK_POWERDOWN);
+			return ret;
+		}
+	}
 
 	si5351_set_bits(hwdata->drvdata, SI5351_OUTPUT_ENABLE_CTRL,
 			(1 << hwdata->num), 0);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] clk: si5351: fail prepare when PLL reset times out

[Brian Masney]

Hi Pengpeng,

Thanks for the patch.

On Tue, Jun 23, 2026 at 09:56:25PM +0800, Pengpeng Hou wrote:
> si5351_clkout_prepare() clears the output powerdown bit and, when
> requested, resets the parent PLL before enabling the output. The PLL
> reset helper logs when the reset bit does not clear, but returns void,
> so prepare still enables the output and reports success.
> 
> Make the reset helper return the poll error and propagate it from
> .prepare. If the reset fails, restore the output powerdown bit before
> returning the error.
> 
> Fixes: 5142cbcea324 ("clk: si5351: Wait for bit clear after PLL reset")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/clk/clk-si5351.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
> index e755db545..4e60261da 100644
> --- a/drivers/clk/clk-si5351.c
> +++ b/drivers/clk/clk-si5351.c
> @@ -907,18 +907,18 @@ static int _si5351_clkout_set_disable_state(
>  	return 0;
>  }
>  
> -static void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
> +static int _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
>  {
>  	u8 val = si5351_reg_read(drvdata, SI5351_CLK0_CTRL + num);
>  	u8 mask = val & SI5351_CLK_PLL_SELECT ? SI5351_PLL_RESET_B :
> -						       SI5351_PLL_RESET_A;
> +							       SI5351_PLL_RESET_A;

Whitespace change can be dropped.

With this fixed in a new version, feel free to add:

Reviewed-by: Brian Masney <bmasney@redhat.com>