[PATCH] mfd: stm32-timers: depopulate child devices on populate failure

[PATCH] mfd: stm32-timers: depopulate child devices on populate failure

[Pengpeng Hou]

stm32_timers_probe() releases the timer DMA resources when
of_platform_populate() fails, but it does not depopulate any child
devices that were created before the failure.

The remove path explicitly depopulates child devices before releasing
DMA resources to avoid races with children using DMA.  Apply the same
ordering on the populate failure path.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/mfd/stm32-timers.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index b3dbc02aaf79..1f0aecae83a5 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -329,8 +329,10 @@ static int stm32_timers_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, ddata);
 
 	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
-	if (ret)
+	if (ret) {
+		of_platform_depopulate(&pdev->dev);
 		stm32_timers_dma_remove(dev, ddata);
+	}
 
 	return ret;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] mfd: stm32-timers: depopulate child devices on populate failure

[Lee Jones]

On Mon, 15 Jun 2026, Pengpeng Hou wrote:

> stm32_timers_probe() releases the timer DMA resources when
> of_platform_populate() fails, but it does not depopulate any child
> devices that were created before the failure.
> 
> The remove path explicitly depopulates child devices before releasing
> DMA resources to avoid races with children using DMA.  Apply the same
> ordering on the populate failure path.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/mfd/stm32-timers.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index b3dbc02aaf79..1f0aecae83a5 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -329,8 +329,10 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, ddata);
>  
>  	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> -	if (ret)
> +	if (ret) {
> +		of_platform_depopulate(&pdev->dev);

It's strange to see an un-winder inside the error handling of its
original call.  We're used to seeing them called only on success during
a subsequent step.

Wouldn't devm_of_platform_populate() be better here instead?

>  		stm32_timers_dma_remove(dev, ddata);
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.50.1 (Apple Git-155)
> 

-- 
Lee Jones

Re: [PATCH] mfd: stm32-timers: depopulate child devices on populate failure

[Pengpeng Hou]

Hi Lee,

Thanks for taking a look.

I checked devm_of_platform_populate(), but I don't think it covers this
particular failure path on its own.

devm_of_platform_populate() only installs the devres cleanup after
of_platform_populate() has returned success.  If of_platform_populate()
returns an error after creating some earlier children, the helper just
frees its devres record and those partial children are not depopulated.

For stm32-timers, I think we still need the explicit ordering used by
remove: depopulate children before stm32_timers_dma_remove().  The child
drivers get the parent drvdata, and the PWM child can call the parent
stm32_timers_dma_burst_read() helper, so releasing the parent DMA
channels while partially-created children remain would keep the same
ordering problem the remove path avoids.

I agree the inline unwind is not the clearest form.  I can send a v2
using a normal goto unwind label and a short comment explaining the
partial-populate case, unless you prefer a different shape.

Thanks,
Pengpeng

Re: [PATCH] mfd: stm32-timers: depopulate child devices on populate failure

[Lee Jones]

On Fri, 03 Jul 2026, Pengpeng Hou wrote:

> Hi Lee,
> 
> Thanks for taking a look.
> 
> I checked devm_of_platform_populate(), but I don't think it covers this
> particular failure path on its own.
> 
> devm_of_platform_populate() only installs the devres cleanup after
> of_platform_populate() has returned success.  If of_platform_populate()
> returns an error after creating some earlier children, the helper just
> frees its devres record and those partial children are not depopulated.
> 
> For stm32-timers, I think we still need the explicit ordering used by
> remove: depopulate children before stm32_timers_dma_remove().  The child
> drivers get the parent drvdata, and the PWM child can call the parent
> stm32_timers_dma_burst_read() helper, so releasing the parent DMA
> channels while partially-created children remain would keep the same
> ordering problem the remove path avoids.
> 
> I agree the inline unwind is not the clearest form.  I can send a v2
> using a normal goto unwind label and a short comment explaining the
> partial-populate case, unless you prefer a different shape.

I'm not concerned about where in the file the call sits.  Calls like
this that fail usually do so cleanly.  I think this deserves a wider
piece of work that ensures that of_platform_populate() doesn't leave
dangling resources when it fails.

Also, why doesn't every call to of_platform_populate() have this?

-- 
Lee Jones