Re: [PATCH net] net: ipa: balance runtime PM reference on remove error

On 8/14/26 8:40 AM, Ruoyu Wang wrote:
> ipa_remove() takes a runtime PM reference before accessing IPA hardware
> during teardown. If a concurrent modem start or stop keeps
> ipa_modem_stop() busy across both attempts, the callback intentionally
> returns without releasing the remaining resources because proceeding
> with teardown could crash. That return also skips the matching
> pm_runtime_put_noidle(), leaving the callback's usage-count reference
> held.

I concur that there should be a pm_runtime_put_noidle()
call before returning, to match the pm_runtime_get_sync()
earlier.

The exact code you're modifying here was added by commit
a92dbb9cdf046 ("net: ipa: Convert to platform remove
callback returning void"), but this particular error
was present before that.

The comment above this dev_err() call is rather confusing
and repetitive.  Could you try to clarify what it means,
or write your own simple explanation to replace it?

Otherwise, this looks like a good change to me.
Thank you very much.

Reviewed-by: Alex Elder <[email protected]>


> 
> Drop only this runtime PM reference before returning.
> pm_runtime_put_noidle() does not request an idle transition, so the
> hardware and resources retained on this exceptional path remain
> untouched while the usage count stays balanced.
> 
> This issue was found by a static analysis checker and confirmed by
> manual source review.
> 
> Fixes: 923a6b698447 ("net: ipa: get clock in ipa_probe()")
> Signed-off-by: Ruoyu Wang <[email protected]>
> ---
>   drivers/net/ipa/ipa_main.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index 788dd99af2a4d..aab38fea91c61 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -975,6 +975,7 @@ static void ipa_remove(struct platform_device *pdev)
>   			 */
>   			dev_err(dev, "Failed to stop modem (%pe), leaking resources\n",
>   				ERR_PTR(ret));
> +			pm_runtime_put_noidle(dev);
>   			return;
>   		}
>