Re: [PATCH v5 13/17] spi: stm32: Simplify clock handling with devm_clk_get_enabled()

Amelie Delaunay
2026-03-19T22:43:22Z

On 3/19/26 03:04, Pei Xiao wrote:
> Replace devm_clk_get() followed by clk_prepare_enable() with
> devm_clk_get_enabled() for the clock. This removes the need for
> explicit clock enable and disable calls, as the managed API automatically
> handles clock disabling on device removal or probe failure.
>
> Remove the now-unnecessary clk_disable_unprepare() calls from the probe
> error paths and the remove callback. Also simplify error handling by
> using dev_err_probe().
>
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>

Reviewed-by: Amelie Delaunay <amelie.delaunay@foss.st.com>

> ---
> drivers/spi/spi-stm32.c | 62 +++++++++++------------------------------
> 1 file changed, 17 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
> index 8a7f5a10d4af..be88e62075af 100644
> --- a/drivers/spi/spi-stm32.c
> +++ b/drivers/spi/spi-stm32.c
> @@ -2360,25 +2360,20 @@ static int stm32_spi_probe(struct platform_device *pdev)
> int ret;
>
> cfg = of_device_get_match_data(&pdev->dev);
> - if (!cfg) {
> - dev_err(&pdev->dev, "Failed to get match data for platform\n");
> - return -ENODEV;
> - }
> + if (!cfg)
> + return dev_err_probe(&pdev->dev, -ENODEV,
> + "Failed to get match data for platform\n");
>
> device_mode = of_property_read_bool(np, "spi-slave");
> - if (!cfg->has_device_mode && device_mode) {
> - dev_err(&pdev->dev, "spi-slave not supported\n");
> - return -EPERM;
> - }
> + if (!cfg->has_device_mode && device_mode)
> + return dev_err_probe(&pdev->dev, -EPERM, "spi-slave not supported\n");
>
> if (device_mode)
> ctrl = devm_spi_alloc_target(&pdev->dev, sizeof(struct stm32_spi));
> else
> ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(struct stm32_spi));
> - if (!ctrl) {
> - dev_err(&pdev->dev, "spi controller allocation failed\n");
> - return -ENOMEM;
> - }
> + if (!ctrl)
> + return dev_err_probe(&pdev->dev, -ENOMEM, "spi controller allocation failed\n");
> platform_set_drvdata(pdev, ctrl);
>
> spi = spi_controller_get_devdata(ctrl);
> @@ -2409,32 +2404,18 @@ static int stm32_spi_probe(struct platform_device *pdev)
> return ret;
> }
>
> - spi->clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(spi->clk)) {
> - ret = PTR_ERR(spi->clk);
> - dev_err(&pdev->dev, "clk get failed: %d\n", ret);
> - return ret;
> - }
> + spi->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> + if (IS_ERR(spi->clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(spi->clk), "clk enabled failed\n");
>
> - ret = clk_prepare_enable(spi->clk);
> - if (ret) {
> - dev_err(&pdev->dev, "clk enable failed: %d\n", ret);
> - return ret;
> - }
> spi->clk_rate = clk_get_rate(spi->clk);
> - if (!spi->clk_rate) {
> - dev_err(&pdev->dev, "clk rate = 0\n");
> - ret = -EINVAL;
> - goto err_clk_disable;
> - }
> + if (!spi->clk_rate)
> + return dev_err_probe(&pdev->dev, -EINVAL, "clk rate = 0\n");
>
> rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> if (rst) {
> - if (IS_ERR(rst)) {
> - ret = dev_err_probe(&pdev->dev, PTR_ERR(rst),
> - "failed to get reset\n");
> - goto err_clk_disable;
> - }
> + if (IS_ERR(rst))
> + return dev_err_probe(&pdev->dev, PTR_ERR(rst), "failed to get reset\n");
>
> reset_control_assert(rst);
> udelay(2);
> @@ -2461,11 +2442,8 @@ static int stm32_spi_probe(struct platform_device *pdev)
> dev_dbg(spi->dev, "one message max size %d\n", spi->t_size_max);
>
> ret = spi->cfg->config(spi);
> - if (ret) {
> - dev_err(&pdev->dev, "controller configuration failed: %d\n",
> - ret);
> - goto err_clk_disable;
> - }
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "controller configuration failed: %d\n", ret);
>
> ctrl->auto_runtime_pm = true;
> ctrl->bus_num = pdev->id;
> @@ -2490,8 +2468,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "tx dma disabled\n");
> spi->dma_tx = NULL;
> } else {
> - dev_err_probe(&pdev->dev, ret, "failed to request tx dma channel\n");
> - goto err_clk_disable;
> + return dev_err_probe(&pdev->dev, ret, "failed to request tx dma channel\n");
> }
> } else {
> ctrl->dma_tx = spi->dma_tx;
> @@ -2579,8 +2556,6 @@ static int stm32_spi_probe(struct platform_device *pdev)
> err_dma_tx_release:
> if (spi->dma_tx)
> dma_release_channel(spi->dma_tx);
> -err_clk_disable:
> - clk_disable_unprepare(spi->clk);
>
> return ret;
> }
> @@ -2610,9 +2585,6 @@ static void stm32_spi_remove(struct platform_device *pdev)
> gen_pool_free(spi->sram_pool, (unsigned long)spi->sram_rx_buf,
> spi->sram_rx_buf_size);
>
> - clk_disable_unprepare(spi->clk);
> -
> -
> pinctrl_pm_select_sleep_state(&pdev->dev);
> }
>