1. Patchew
  2. linux
  3. View series

[PATCH V2] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()

Pei Xiao posted 1 patch 2 months, 1 week ago 
drivers/spi/spi-zynq-qspi.c | 42 ++++++-------------------------------
1 file changed, 6 insertions(+), 36 deletions(-)
[PATCH V2] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
Posted by Pei Xiao 2 months, 1 week ago 
Replace devm_clk_get() followed by clk_prepare_enable() with
devm_clk_get_enabled() for both "pclk" and "ref_clk". This removes
the need for explicit clock enable and disable calls, as the managed
API automatically disables the clocks on device removal or probe
failure.

Remove the now-unnecessary clk_disable_unprepare() calls from the
probe error paths and the remove callback. Simplify error handling
by jumping directly to the remove_ctlr label.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changlog in v2: remove clk enable in setup_op function
---
 drivers/spi/spi-zynq-qspi.c | 42 ++++++-------------------------------
 1 file changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index 5232483c4a3a..af252500195c 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -381,21 +381,10 @@ static int zynq_qspi_setup_op(struct spi_device *spi)
 {
 	struct spi_controller *ctlr = spi->controller;
 	struct zynq_qspi *qspi = spi_controller_get_devdata(ctlr);
-	int ret;
 
 	if (ctlr->busy)
 		return -EBUSY;
 
-	ret = clk_enable(qspi->refclk);
-	if (ret)
-		return ret;
-
-	ret = clk_enable(qspi->pclk);
-	if (ret) {
-		clk_disable(qspi->refclk);
-		return ret;
-	}
-
 	zynq_qspi_write(qspi, ZYNQ_QSPI_ENABLE_OFFSET,
 			ZYNQ_QSPI_ENABLE_ENABLE_MASK);
 
@@ -661,7 +650,7 @@ static int zynq_qspi_probe(struct platform_device *pdev)
 		goto remove_ctlr;
 	}
 
-	xqspi->pclk = devm_clk_get(&pdev->dev, "pclk");
+	xqspi->pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
 	if (IS_ERR(xqspi->pclk)) {
 		dev_err(&pdev->dev, "pclk clock not found.\n");
 		ret = PTR_ERR(xqspi->pclk);
@@ -670,36 +659,24 @@ static int zynq_qspi_probe(struct platform_device *pdev)
 
 	init_completion(&xqspi->data_completion);
 
-	xqspi->refclk = devm_clk_get(&pdev->dev, "ref_clk");
+	xqspi->refclk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
 	if (IS_ERR(xqspi->refclk)) {
 		dev_err(&pdev->dev, "ref_clk clock not found.\n");
 		ret = PTR_ERR(xqspi->refclk);
 		goto remove_ctlr;
 	}
 
-	ret = clk_prepare_enable(xqspi->pclk);
-	if (ret) {
-		dev_err(&pdev->dev, "Unable to enable APB clock.\n");
-		goto remove_ctlr;
-	}
-
-	ret = clk_prepare_enable(xqspi->refclk);
-	if (ret) {
-		dev_err(&pdev->dev, "Unable to enable device clock.\n");
-		goto clk_dis_pclk;
-	}
-
 	xqspi->irq = platform_get_irq(pdev, 0);
 	if (xqspi->irq < 0) {
 		ret = xqspi->irq;
-		goto clk_dis_all;
+		goto remove_ctlr;
 	}
 	ret = devm_request_irq(&pdev->dev, xqspi->irq, zynq_qspi_irq,
 			       0, pdev->name, xqspi);
 	if (ret != 0) {
 		ret = -ENXIO;
 		dev_err(&pdev->dev, "request_irq failed\n");
-		goto clk_dis_all;
+		goto remove_ctlr;
 	}
 
 	ret = of_property_read_u32(np, "num-cs",
@@ -709,7 +686,7 @@ static int zynq_qspi_probe(struct platform_device *pdev)
 	} else if (num_cs > ZYNQ_QSPI_MAX_NUM_CS) {
 		ret = -EINVAL;
 		dev_err(&pdev->dev, "only 2 chip selects are available\n");
-		goto clk_dis_all;
+		goto remove_ctlr;
 	} else {
 		ctlr->num_chipselect = num_cs;
 	}
@@ -728,15 +705,11 @@ static int zynq_qspi_probe(struct platform_device *pdev)
 	ret = devm_spi_register_controller(&pdev->dev, ctlr);
 	if (ret) {
 		dev_err(&pdev->dev, "devm_spi_register_controller failed\n");
-		goto clk_dis_all;
+		goto remove_ctlr;
 	}
 
 	return ret;
 
-clk_dis_all:
-	clk_disable_unprepare(xqspi->refclk);
-clk_dis_pclk:
-	clk_disable_unprepare(xqspi->pclk);
 remove_ctlr:
 	spi_controller_put(ctlr);
 
@@ -758,9 +731,6 @@ static void zynq_qspi_remove(struct platform_device *pdev)
 	struct zynq_qspi *xqspi = platform_get_drvdata(pdev);
 
 	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
-
-	clk_disable_unprepare(xqspi->refclk);
-	clk_disable_unprepare(xqspi->pclk);
 }
 
 static const struct of_device_id zynq_qspi_of_match[] = {
-- 
2.25.1
Re: [PATCH V2] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
Posted by Mark Brown 2 months, 1 week ago 
On Tue, 07 Apr 2026 17:55:08 +0800, Pei Xiao wrote:
> spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.0

Thanks!

[1/1] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
      https://git.kernel.org/broonie/spi/c/1f8fd9490e31

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Re: [PATCH V2] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
Posted by Mark Brown 2 months, 1 week ago 
On Tue, Apr 07, 2026 at 05:55:08PM +0800, Pei Xiao wrote:
> Replace devm_clk_get() followed by clk_prepare_enable() with
> devm_clk_get_enabled() for both "pclk" and "ref_clk". This removes
> the need for explicit clock enable and disable calls, as the managed
> API automatically disables the clocks on device removal or probe
> failure.
> 
> Remove the now-unnecessary clk_disable_unprepare() calls from the
> probe error paths and the remove callback. Simplify error handling
> by jumping directly to the remove_ctlr label.

You've not mentioned the fact that the enables in _setup_op() were an
actual bug independently of the cleanup, causing us to leak the enables
since there were no corresponding disables.  No need to resend.
Re: [PATCH V2] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
Posted by Michal Simek 2 months, 1 week ago 

On 4/7/26 11:55, Pei Xiao wrote:
> Replace devm_clk_get() followed by clk_prepare_enable() with
> devm_clk_get_enabled() for both "pclk" and "ref_clk". This removes
> the need for explicit clock enable and disable calls, as the managed
> API automatically disables the clocks on device removal or probe
> failure.
> 
> Remove the now-unnecessary clk_disable_unprepare() calls from the
> probe error paths and the remove callback. Simplify error handling
> by jumping directly to the remove_ctlr label.
> 
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
> changlog in v2: remove clk enable in setup_op function
> ---
>   drivers/spi/spi-zynq-qspi.c | 42 ++++++-------------------------------
>   1 file changed, 6 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
> index 5232483c4a3a..af252500195c 100644
> --- a/drivers/spi/spi-zynq-qspi.c
> +++ b/drivers/spi/spi-zynq-qspi.c
> @@ -381,21 +381,10 @@ static int zynq_qspi_setup_op(struct spi_device *spi)
>   {
>   	struct spi_controller *ctlr = spi->controller;
>   	struct zynq_qspi *qspi = spi_controller_get_devdata(ctlr);
> -	int ret;
>   
>   	if (ctlr->busy)
>   		return -EBUSY;
>   
> -	ret = clk_enable(qspi->refclk);
> -	if (ret)
> -		return ret;
> -
> -	ret = clk_enable(qspi->pclk);
> -	if (ret) {
> -		clk_disable(qspi->refclk);
> -		return ret;
> -	}
> -
>   	zynq_qspi_write(qspi, ZYNQ_QSPI_ENABLE_OFFSET,
>   			ZYNQ_QSPI_ENABLE_ENABLE_MASK);
>   
> @@ -661,7 +650,7 @@ static int zynq_qspi_probe(struct platform_device *pdev)
>   		goto remove_ctlr;
>   	}
>   
> -	xqspi->pclk = devm_clk_get(&pdev->dev, "pclk");
> +	xqspi->pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
>   	if (IS_ERR(xqspi->pclk)) {
>   		dev_err(&pdev->dev, "pclk clock not found.\n");
>   		ret = PTR_ERR(xqspi->pclk);
> @@ -670,36 +659,24 @@ static int zynq_qspi_probe(struct platform_device *pdev)
>   
>   	init_completion(&xqspi->data_completion);
>   
> -	xqspi->refclk = devm_clk_get(&pdev->dev, "ref_clk");
> +	xqspi->refclk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
>   	if (IS_ERR(xqspi->refclk)) {
>   		dev_err(&pdev->dev, "ref_clk clock not found.\n");
>   		ret = PTR_ERR(xqspi->refclk);
>   		goto remove_ctlr;
>   	}
>   
> -	ret = clk_prepare_enable(xqspi->pclk);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Unable to enable APB clock.\n");
> -		goto remove_ctlr;
> -	}
> -
> -	ret = clk_prepare_enable(xqspi->refclk);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Unable to enable device clock.\n");
> -		goto clk_dis_pclk;
> -	}
> -
>   	xqspi->irq = platform_get_irq(pdev, 0);
>   	if (xqspi->irq < 0) {
>   		ret = xqspi->irq;
> -		goto clk_dis_all;
> +		goto remove_ctlr;
>   	}
>   	ret = devm_request_irq(&pdev->dev, xqspi->irq, zynq_qspi_irq,
>   			       0, pdev->name, xqspi);
>   	if (ret != 0) {
>   		ret = -ENXIO;
>   		dev_err(&pdev->dev, "request_irq failed\n");
> -		goto clk_dis_all;
> +		goto remove_ctlr;
>   	}
>   
>   	ret = of_property_read_u32(np, "num-cs",
> @@ -709,7 +686,7 @@ static int zynq_qspi_probe(struct platform_device *pdev)
>   	} else if (num_cs > ZYNQ_QSPI_MAX_NUM_CS) {
>   		ret = -EINVAL;
>   		dev_err(&pdev->dev, "only 2 chip selects are available\n");
> -		goto clk_dis_all;
> +		goto remove_ctlr;
>   	} else {
>   		ctlr->num_chipselect = num_cs;
>   	}
> @@ -728,15 +705,11 @@ static int zynq_qspi_probe(struct platform_device *pdev)
>   	ret = devm_spi_register_controller(&pdev->dev, ctlr);
>   	if (ret) {
>   		dev_err(&pdev->dev, "devm_spi_register_controller failed\n");
> -		goto clk_dis_all;
> +		goto remove_ctlr;
>   	}
>   
>   	return ret;
>   
> -clk_dis_all:
> -	clk_disable_unprepare(xqspi->refclk);
> -clk_dis_pclk:
> -	clk_disable_unprepare(xqspi->pclk);
>   remove_ctlr:
>   	spi_controller_put(ctlr);
>   
> @@ -758,9 +731,6 @@ static void zynq_qspi_remove(struct platform_device *pdev)
>   	struct zynq_qspi *xqspi = platform_get_drvdata(pdev);
>   
>   	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
> -
> -	clk_disable_unprepare(xqspi->refclk);
> -	clk_disable_unprepare(xqspi->pclk);
>   }
>   
>   static const struct of_device_id zynq_qspi_of_match[] = {

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.