Re: [PATCH v2] gpu: host1x: fix runtime PM reference leak on remove

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pm: host1x leaves hardware active and leaks resources on driver remove
--

commit 91e37111242ff2cea238fc242091a8f04f7f935f
Author: Guangshuo Li <lgs201920130244@gmail.com>
gpu: host1x: fix runtime PM reference leak on remove

This commit attempts to fix a runtime PM reference leak in host1x_remove() by
replacing pm_runtime_force_suspend() with pm_runtime_put_sync_suspend() and
pm_runtime_disable(). It drops the usage reference acquired in the probe
path before disabling runtime PM.

> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index 7b1d091f3c090..df483ef6419d0 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -677,7 +677,8 @@ static void host1x_remove(struct platform_device *pdev)
>  	host1x_unregister(host);
>  	host1x_debug_deinit(host);
>  
> -	pm_runtime_force_suspend(&pdev->dev);
> +	pm_runtime_put_sync_suspend(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);

[Severity: High]
Will this conditional suspend leave the hardware active while the driver is
torn down?

Unlike pm_runtime_force_suspend(), pm_runtime_put_sync_suspend() can silently
abort if the PM usage counter is greater than zero or if child devices are
still active. This might happen if userspace sets the device's power control
to "on", or if child devices populated by devm_of_platform_populate() are
still active, since devres cleans up children after host1x_remove() finishes.

If the suspend aborts, the device remains active with clocks enabled, reset
lines unasserted, and hardware interrupts enabled. Since host1x_remove()
continues and subsequently calls host1x_intr_deinit() and host1x_iommu_exit(),
could any active hardware DMA or interrupts result in IOMMU faults, memory
corruption, or an unhandled IRQ storm?

To ensure the hardware is forcefully quiesced while still fixing the usage
counter leak, should this instead drop the reference without triggering a
conditional idle, and then force the suspend? For example:

pm_runtime_put_noidle(&pdev->dev);
pm_runtime_force_suspend(&pdev->dev);

>  
>  	host1x_intr_deinit(host);
>  	host1x_syncpt_deinit(host);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921082206.234421-1-lgs201920130244@gmail.com?part=1