Re: [PATCH 1/3] serial: mxs-auart: rework clock handling in mxs_get_clks and probe

[Frank Li]

On Tue, Jun 02, 2026 at 07:58:55PM -0700, Rosen Penev wrote:
> Use devm_clk_get_enabled for the AHB clock so its enable/disable
> lifetime is managed by the driver model. Move the mod clock
> (clk) prepare_enable out of mxs_get_clks and into probe so that
> clk_set_rate is called while the clock is still disabled, avoiding
> CLK_SET_RATE_GATE failures. Clean up the error labels accordingly.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  drivers/tty/serial/mxs-auart.c | 47 ++++++++++++----------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 697318dbb146..1390fa000a5b 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1470,34 +1470,22 @@ static int mxs_get_clks(struct mxs_auart_port *s,
>  		return PTR_ERR(s->clk);
>  	}
>
> -	s->clk_ahb = devm_clk_get(s->dev, "ahb");
> +	s->clk_ahb = devm_clk_get_enabled(s->dev, "ahb");
>  	if (IS_ERR(s->clk_ahb)) {
>  		dev_err(s->dev, "Failed to get \"ahb\" clk\n");
>  		return PTR_ERR(s->clk_ahb);
>  	}
>
> -	err = clk_prepare_enable(s->clk_ahb);
> -	if (err) {
> -		dev_err(s->dev, "Failed to enable ahb_clk!\n");
> -		return err;
> -	}
> -
> +	/*
> +	 * Set mod clock rate while it is still disabled so
> +	 * CLK_SET_RATE_GATE does not cause clk_set_rate to fail.
> +	 * The mod clock will be enabled in mxs_auart_startup()
> +	 * and in probe after mxs_get_clks returns.
> +	 */
>  	err = clk_set_rate(s->clk, clk_get_rate(s->clk_ahb));
> -	if (err) {
> +	if (err)
>  		dev_err(s->dev, "Failed to set rate!\n");
> -		goto disable_clk_ahb;
> -	}
>
> -	err = clk_prepare_enable(s->clk);
> -	if (err) {
> -		dev_err(s->dev, "Failed to enable clk!\n");
> -		goto disable_clk_ahb;
> -	}
> -
> -	return 0;
> -
> -disable_clk_ahb:
> -	clk_disable_unprepare(s->clk_ahb);
>  	return err;
>  }
>
> @@ -1604,17 +1592,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	ret = clk_prepare_enable(s->clk);
> +	if (ret)
> +		return ret;
> +

why not direct enable clock when get?
	s->clk = devm_clk_get(s->dev, "mod");

Frank
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r) {
>  		ret = -ENXIO;
> -		goto out_disable_clks;
> +		goto out_disable_clk;
>  	}
>
>  	s->port.mapbase = r->start;
>  	s->port.membase = ioremap(r->start, resource_size(r));
>  	if (!s->port.membase) {
>  		ret = -ENOMEM;
> -		goto out_disable_clks;
> +		goto out_disable_clk;
>  	}
>  	s->port.ops = &mxs_auart_ops;
>  	s->port.iotype = UPIO_MEM;
> @@ -1681,11 +1673,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  out_iounmap:
>  	iounmap(s->port.membase);
>
> -out_disable_clks:
> -	if (is_asm9260_auart(s)) {
> -		clk_disable_unprepare(s->clk);
> -		clk_disable_unprepare(s->clk_ahb);
> -	}
> +out_disable_clk:
> +	clk_disable_unprepare(s->clk);
>  	return ret;
>  }
>
> @@ -1697,10 +1686,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
>  	auart_port[pdev->id] = NULL;
>  	mxs_auart_free_gpio_irq(s);
>  	iounmap(s->port.membase);
> -	if (is_asm9260_auart(s)) {
> +	if (is_asm9260_auart(s))
>  		clk_disable_unprepare(s->clk);
> -		clk_disable_unprepare(s->clk_ahb);
> -	}
>  }
>
>  static struct platform_driver mxs_auart_driver = {
> --
> 2.54.0
>