* [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
@ 2026-04-07 7:32 Pei Xiao
2026-04-07 8:01 ` Michal Simek
0 siblings, 1 reply; 4+ messages in thread
From: Pei Xiao @ 2026-04-07 7:32 UTC (permalink / raw)
To: michal.simek, linux-spi, linux-arm-kernel, linux-kernel, broonie; +Cc: Pei Xiao
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>
---
drivers/spi/spi-zynq-qspi.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index 5232483c4a3a..8c3975030d0a 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -661,7 +661,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 +670,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 +697,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 +716,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 +742,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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
2026-04-07 7:32 [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled() Pei Xiao
@ 2026-04-07 8:01 ` Michal Simek
2026-04-07 8:14 ` Pei Xiao
0 siblings, 1 reply; 4+ messages in thread
From: Michal Simek @ 2026-04-07 8:01 UTC (permalink / raw)
To: Pei Xiao, linux-spi, linux-arm-kernel, linux-kernel, broonie
On 4/7/26 09:32, Pei Xiao wrote:
> [You don't often get email from xiaopei01@kylinos.cn. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> 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>
> ---
> drivers/spi/spi-zynq-qspi.c | 31 ++++++-------------------------
> 1 file changed, 6 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
> index 5232483c4a3a..8c3975030d0a 100644
> --- a/drivers/spi/spi-zynq-qspi.c
> +++ b/drivers/spi/spi-zynq-qspi.c
> @@ -661,7 +661,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 +670,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 +697,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 +716,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 +742,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
>
There is also clock manipulation in zynq_qspi_setup_op() which needs to be handled.
Thanks,
Michal
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
2026-04-07 8:01 ` Michal Simek
@ 2026-04-07 8:14 ` Pei Xiao
2026-04-07 8:30 ` Michal Simek
0 siblings, 1 reply; 4+ messages in thread
From: Pei Xiao @ 2026-04-07 8:14 UTC (permalink / raw)
To: Michal Simek, linux-spi, linux-arm-kernel, linux-kernel, broonie
在 2026/4/7 16:01, Michal Simek 写道:
>
>
> On 4/7/26 09:32, Pei Xiao wrote:
>> [You don't often get email from xiaopei01@kylinos.cn. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> 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>
>> ---
>> drivers/spi/spi-zynq-qspi.c | 31 ++++++-------------------------
>> 1 file changed, 6 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
>> index 5232483c4a3a..8c3975030d0a 100644
>> --- a/drivers/spi/spi-zynq-qspi.c
>> +++ b/drivers/spi/spi-zynq-qspi.c
>> @@ -661,7 +661,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 +670,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 +697,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 +716,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 +742,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
>>
>
> There is also clock manipulation in zynq_qspi_setup_op() which needs to be handled.
>
Can I remove this code? If not, please ignore this patch, and I apologize for the noise.
In zynq_qspi_setup_op:
~~~~remove this clk enable code
ret = clk_enable(qspi->refclk);
if (ret)
return ret;
ret = clk_enable(qspi->pclk);
if (ret) {
clk_disable(qspi->refclk);
return ret;
}
~~~
Thanks!
Pei.
> Thanks,
> Michal
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()
2026-04-07 8:14 ` Pei Xiao
@ 2026-04-07 8:30 ` Michal Simek
0 siblings, 0 replies; 4+ messages in thread
From: Michal Simek @ 2026-04-07 8:30 UTC (permalink / raw)
To: Pei Xiao, linux-spi, linux-arm-kernel, linux-kernel, broonie
On 4/7/26 10:14, Pei Xiao wrote:
> 在 2026/4/7 16:01, Michal Simek 写道:
>>
>>
>> On 4/7/26 09:32, 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>
>>> ---
>>> drivers/spi/spi-zynq-qspi.c | 31 ++++++-------------------------
>>> 1 file changed, 6 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
>>> index 5232483c4a3a..8c3975030d0a 100644
>>> --- a/drivers/spi/spi-zynq-qspi.c
>>> +++ b/drivers/spi/spi-zynq-qspi.c
>>> @@ -661,7 +661,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 +670,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 +697,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 +716,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 +742,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
>>>
>>
>> There is also clock manipulation in zynq_qspi_setup_op() which needs to be handled.
>>
> Can I remove this code? If not, please ignore this patch, and I apologize for the noise.
>
> In zynq_qspi_setup_op:
> ~~~~remove this clk enable code
> ret = clk_enable(qspi->refclk);
> if (ret)
> return ret;
>
> ret = clk_enable(qspi->pclk);
> if (ret) {
> clk_disable(qspi->refclk);
> return ret;
> }
> ~~~
Yes, remove it completely.
M
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-07 8:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 7:32 [PATCH] spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled() Pei Xiao
2026-04-07 8:01 ` Michal Simek
2026-04-07 8:14 ` Pei Xiao
2026-04-07 8:30 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox