[PATCH v2 0/2] two cleanups in atcspi200

[PATCH v2 0/2] two cleanups in atcspi200

[Pei Xiao]

1.Use helper function devm_clk_get_enabled() to simple code.
2.fix mutex initialization order
---
changlog in v2: fix return code error and leak the controller
---
Pei Xiao (2):
  spi: atcspi200: Use helper function devm_clk_get_enabled()
  spi: atcspi200: fix mutex initialization order

 drivers/spi/spi-atcspi200.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] spi: atcspi200: Use helper function devm_clk_get_enabled()

[Pei Xiao]

Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared
and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be
replaced by devm_clk_get_enabled(). Moreover, it is no longer necessary to
unprepare and disable the clocks explicitly.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/spi/spi-atcspi200.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/spi-atcspi200.c b/drivers/spi/spi-atcspi200.c
index dd19fe9f9ee7..c0607193be8d 100644
--- a/drivers/spi/spi-atcspi200.c
+++ b/drivers/spi/spi-atcspi200.c
@@ -486,11 +486,6 @@ static int atcspi_init_resources(struct platform_device *pdev,
 		return dev_err_probe(spi->dev, PTR_ERR(spi->regmap),
 				     "Failed to init regmap\n");
 
-	spi->clk = devm_clk_get(spi->dev, NULL);
-	if (IS_ERR(spi->clk))
-		return dev_err_probe(spi->dev, PTR_ERR(spi->clk),
-				     "Failed to get SPI clock\n");
-
 	spi->sclk_rate = ATCSPI_MAX_SPEED_HZ;
 	return 0;
 }
@@ -512,13 +507,10 @@ static int atcspi_configure_dma(struct atcspi_dev *spi)
 
 static int atcspi_enable_clk(struct atcspi_dev *spi)
 {
-	int ret;
-
-	ret = clk_prepare_enable(spi->clk);
-	if (ret)
-		return dev_err_probe(spi->dev, ret,
-				     "Failed to enable clock\n");
-
+	spi->clk = devm_clk_get_enabled(spi->dev, NULL);
+	if (IS_ERR(spi->clk))
+		return dev_err_probe(spi->dev, PTR_ERR(spi->clk),
+				     "Failed to get SPI clock\n");
 	spi->clk_rate = clk_get_rate(spi->clk);
 	if (!spi->clk_rate)
 		return dev_err_probe(spi->dev, -EINVAL,
@@ -571,15 +563,14 @@ static int atcspi_probe(struct platform_device *pdev)
 
 	ret = atcspi_setup(spi);
 	if (ret)
-		goto disable_clk;
+		goto free_controller;
 
 	ret = devm_spi_register_controller(&pdev->dev, host);
 	if (ret) {
 		dev_err_probe(spi->dev, ret,
 			      "Failed to register SPI controller\n");
-		goto disable_clk;
+		goto free_controller;
 	}
-
 	spi->use_dma = false;
 	if (ATCSPI_DMA_SUPPORT) {
 		ret = atcspi_configure_dma(spi);
@@ -593,9 +584,6 @@ static int atcspi_probe(struct platform_device *pdev)
 
 	return 0;
 
-disable_clk:
-	clk_disable_unprepare(spi->clk);
-
 free_controller:
 	spi_controller_put(host);
 	return ret;
-- 
2.25.1

[PATCH v2 2/2] spi: atcspi200: fix mutex initialization order

[Pei Xiao]

The atcspi_exec_mem_op() function may call mutex_lock() on the
driver's mutex before it is properly initialized if a SPI memory
operation is initiated immediately after devm_spi_register_controller()
is called. The mutex initialization currently occurs after the
controller registration, which leaves a window where the mutex could
be used uninitialized.

Move the mutex initialization to the beginning of the probe function,
before any registration or resource allocation. Also replace the
plain mutex_init() with devm_mutex_init() to benefit from automatic
cleanup on driver unbind, preventing potential resource leaks.

Fixes: 34e3815ea459 ("spi: atcspi200: Add ATCSPI200 SPI controller driver")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/spi/spi-atcspi200.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-atcspi200.c b/drivers/spi/spi-atcspi200.c
index c0607193be8d..dd1292c5bb98 100644
--- a/drivers/spi/spi-atcspi200.c
+++ b/drivers/spi/spi-atcspi200.c
@@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev)
 	spi->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, host);
 
+	ret = devm_mutex_init(spi->dev, &spi->mutex_lock);
+	if (ret)
+		goto free_controller;
+
 	ret = atcspi_init_resources(pdev, spi, &mem_res);
 	if (ret)
 		goto free_controller;
@@ -580,7 +584,6 @@ static int atcspi_probe(struct platform_device *pdev)
 		else
 			spi->use_dma = true;
 	}
-	mutex_init(&spi->mutex_lock);
 
 	return 0;
 
-- 
2.25.1

Re: [PATCH v2 2/2] spi: atcspi200: fix mutex initialization order

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote:

> @@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev)
>  	spi->dev = &pdev->dev;
>  	dev_set_drvdata(&pdev->dev, host);
>  
> +	ret = devm_mutex_init(spi->dev, &spi->mutex_lock);
> +	if (ret)
> +		goto free_controller;
> +

This means we're now using devm to free the mutex that is in spi, but
when we pass spi into devm_spi_register_controller() we'll cause spi to
be freed before we free the mutex.  This pattern gets a bit clunky with
devm...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: atcspi200: fix mutex initialization order

[Pei Xiao]

在 2026/3/11 20:29, Mark Brown 写道:
> On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote:
>
>> @@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev)
>>  	spi->dev = &pdev->dev;
>>  	dev_set_drvdata(&pdev->dev, host);
>>  
>> +	ret = devm_mutex_init(spi->dev, &spi->mutex_lock);
>> +	if (ret)
>> +		goto free_controller;
>> +
> This means we're now using devm to free the mutex that is in spi, but
> when we pass spi into devm_spi_register_controller() we'll cause spi to
> be freed before we free the mutex.  This pattern gets a bit clunky with
> devm...
Thanks for review! I will use mutex_init() for V3 version.

Re: [PATCH v2 2/2] spi: atcspi200: fix mutex initialization order

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On Thu, Mar 12, 2026 at 10:25:11AM +0800, Pei Xiao wrote:
> 在 2026/3/11 20:29, Mark Brown 写道:
> > On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote:

> >> +	ret = devm_mutex_init(spi->dev, &spi->mutex_lock);
> >> +	if (ret)
> >> +		goto free_controller;
> >> +

> > This means we're now using devm to free the mutex that is in spi, but
> > when we pass spi into devm_spi_register_controller() we'll cause spi to
> > be freed before we free the mutex.  This pattern gets a bit clunky with
> > devm...

> Thanks for review! I will use mutex_init() for V3 version.

That's still going to have trouble because if you do the free in the
removal function the lock will go away before the controller which is
it's own issue (this is what I mean about clunky).  It's especially
annoying as it's only an issue for debugging builds.  I can't actually
see a good way of dealing with this, perhaps just a plain mutex_init()
without a free is the least bad option.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH v2 0/2] two cleanups in atcspi200

[Mark Brown]

On Wed, 11 Mar 2026 09:55:12 +0800, Pei Xiao wrote:
> 1.Use helper function devm_clk_get_enabled() to simple code.
> 2.fix mutex initialization order

Applied to

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

Thanks!

[1/2] spi: atcspi200: Use helper function devm_clk_get_enabled()
      commit: 97545e37234fdbe457f5104a09f55033550b3d84

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: (subset) [PATCH v2 0/2] two cleanups in atcspi200

[Mark Brown]

On Wed, 11 Mar 2026 09:55:12 +0800, Pei Xiao wrote:
> two cleanups in atcspi200
> 
> 1.Use helper function devm_clk_get_enabled() to simple code.
> 2.fix mutex initialization order

Applied to

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

Thanks!

[1/2] spi: atcspi200: Use helper function devm_clk_get_enabled()
      https://git.kernel.org/broonie/misc/c/97545e37234f

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