Re: [PATCH] phy: ti: twl4030-usb: free the IRQ before canceling work

[Manivannan Sadhasivam]

On 2026-10-02 23:51 -0400, Myeonghun Pak wrote:
> The threaded IRQ handler can schedule id_workaround_work while the PHY
> is runtime active. The remove callback cancels that work, but leaves
> the managed IRQ registered until devres cleanup after remove returns.
> An interrupt after the cancellation can therefore queue work that
> accesses the freed twl4030_usb allocation.
> 
> The same work can be queued after IRQ registration if phy_create_lookup()
> fails during probe. Failed probe cleanup releases the managed IRQ and
> allocation without canceling the delayed work, and does not call remove.
> 
> Free the managed IRQ before canceling the delayed work on remove and on
> the post-IRQ probe failure path. This waits for the threaded IRQ producer
> to finish before canceling any work it queued. The synchronous work
> cancellation also handles the worker's own requeueing. Generic PHY
> consumer callbacks have separate lifetime requirements.
> 
> This issue was identified during our ongoing static-analysis research
> while reviewing kernel code.
> 
> Fixes: 85601b8d81e2 ("usb: phy: twl4030-usb: Fix lost interrupts after ID pin goes down")

This doesn't look like the correct tag,

> Cc: stable@xxxxxxxxxxxxxxx

Is this patch worth backporting?

> Assisted-by: LLM

Not needed anymore.

> Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> ---
>  drivers/phy/ti/phy-twl4030-usb.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/ti/phy-twl4030-usb.c b/drivers/phy/ti/phy-twl4030-usb.c
> index a26aec3ab29e..37c326cd1d85 100644
> --- a/drivers/phy/ti/phy-twl4030-usb.c
> +++ b/drivers/phy/ti/phy-twl4030-usb.c
> @@ -779,12 +779,17 @@ static int twl4030_usb_probe(struct platform_device *pdev)
>  	if (pdata)
>  		err = phy_create_lookup(phy, "usb", "musb-hdrc.0");
>  	if (err)
> -		return err;
> +		goto err_free_irq;
>  
>  	pm_runtime_mark_last_busy(&pdev->dev);
>  	pm_runtime_put_autosuspend(twl->dev);
>  
>  	return 0;
> +
> +err_free_irq:
> +	devm_free_irq(twl->dev, twl->irq, twl);
> +	cancel_delayed_work_sync(&twl->id_workaround_work);

This err path is not unwinding all resources taken during probe.

- Mani