[PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[Mario Peter]

Switch to autosuspend mechanism in ci_otg_work() to prevent race
conditions during rapid device plug/unplug cycles.

The current implementation uses pm_runtime_put_sync() which triggers
immediate runtime suspend. This creates a race condition when a USB
device is unplugged and quickly replugged. The controller may suspend
before the new device detection completes, causing detection to fail.

Replace pm_runtime_put_sync() with pm_runtime_put_sync_autosuspend()
as recommended by Section 9 of Documentation/power/runtime_pm.rst.
This updates the device's last_busy timestamp and delays the suspend
until after the autosuspend_delay period, allowing pending device
detection to complete. As documented, this approach prevents
"bouncing too rapidly between low-power and full-power states".

The synchronous variant pm_runtime_put_sync_autosuspend() is used
(rather than the asynchronous __pm_runtime_put_autosuspend()) to match
the semantics of the original pm_runtime_put_sync() call.

Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
---

Testing was performed using a scripted sequence with a relay to simulate
USB plug/unplug operations at various timing intervals, verifying that
devices are consistently detected after replugging.

v1: submitted (https://lore.kernel.org/linux-usb/zuzsjr6isq6i5izw3rkyo45opyikiqjmy5xk7flpmlgmqb6mgh@rpbdvi3s4u54/)
v2: dropped redundant pm_runtime_mark_last_busy() call

 drivers/usb/chipidea/otg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 647e98f4e351..d19c27f44424 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -230,7 +230,7 @@ static void ci_otg_work(struct work_struct *work)
 		ci_handle_vbus_change(ci);
 	}

-	pm_runtime_put_sync(ci->dev);
+	pm_runtime_put_sync_autosuspend(ci->dev);

 	enable_irq(ci->irq);
 }

base-commit: f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1
--
2.43.0

Re: [PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[Alan Stern]

On Thu, Jan 08, 2026 at 03:34:58PM +0000, Mario Peter wrote:
> Switch to autosuspend mechanism in ci_otg_work() to prevent race
> conditions during rapid device plug/unplug cycles.
> 
> The current implementation uses pm_runtime_put_sync() which triggers
> immediate runtime suspend. This creates a race condition when a USB
> device is unplugged and quickly replugged. The controller may suspend
> before the new device detection completes, causing detection to fail.

You should be aware that this change alone won't prevent race 
conditions.  They might not occur during rapid plug/unplug cycles, but 
they can still occur.

Consider what would happen if the device is unplugged and then plugged 
back in a few seconds later, exactly when the autosuspend mechanism 
kicks in and starts suspending the controller.

The real way to fix this race is by ensuring that device detection will 
occur in all cases, whether the controller is at full power, suspended, 
or in the process of switching between the two.

Alan Stern

> Replace pm_runtime_put_sync() with pm_runtime_put_sync_autosuspend()
> as recommended by Section 9 of Documentation/power/runtime_pm.rst.
> This updates the device's last_busy timestamp and delays the suspend
> until after the autosuspend_delay period, allowing pending device
> detection to complete. As documented, this approach prevents
> "bouncing too rapidly between low-power and full-power states".
> 
> The synchronous variant pm_runtime_put_sync_autosuspend() is used
> (rather than the asynchronous __pm_runtime_put_autosuspend()) to match
> the semantics of the original pm_runtime_put_sync() call.
> 
> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
> ---
> 
> Testing was performed using a scripted sequence with a relay to simulate
> USB plug/unplug operations at various timing intervals, verifying that
> devices are consistently detected after replugging.
> 
> v1: submitted (https://lore.kernel.org/linux-usb/zuzsjr6isq6i5izw3rkyo45opyikiqjmy5xk7flpmlgmqb6mgh@rpbdvi3s4u54/)
> v2: dropped redundant pm_runtime_mark_last_busy() call
> 
>  drivers/usb/chipidea/otg.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 647e98f4e351..d19c27f44424 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -230,7 +230,7 @@ static void ci_otg_work(struct work_struct *work)
>  		ci_handle_vbus_change(ci);
>  	}
> 
> -	pm_runtime_put_sync(ci->dev);
> +	pm_runtime_put_sync_autosuspend(ci->dev);
> 
>  	enable_irq(ci->irq);
>  }
> 
> base-commit: f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1
> --
> 2.43.0
> 
> 

Re: [PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[PETER Mario]

On 1/8/26 17:12, Alan Stern wrote:

> On Thu, Jan 08, 2026 at 03:34:58PM +0000, Mario Peter wrote:
>> Switch to autosuspend mechanism in ci_otg_work() to prevent race
>> conditions during rapid device plug/unplug cycles.
>>
>> The current implementation uses pm_runtime_put_sync() which triggers
>> immediate runtime suspend. This creates a race condition when a USB
>> device is unplugged and quickly replugged. The controller may suspend
>> before the new device detection completes, causing detection to fail.
> You should be aware that this change alone won't prevent race
> conditions.  They might not occur during rapid plug/unplug cycles, but
> they can still occur.
>
> Consider what would happen if the device is unplugged and then plugged
> back in a few seconds later, exactly when the autosuspend mechanism
> kicks in and starts suspending the controller.

I tested this scenario specifically with my automated test setup,
including plug/unplug cycles timed around the autosuspend delay of 2
seconds to provoke exactly the issue you describe. I was not able to
reproduce the detection failure, which does not mean the race condition
doesn't exist - only that it may be harder to trigger or dependent on
other timing factors.

> The real way to fix this race is by ensuring that device detection will
> occur in all cases, whether the controller is at full power, suspended,
> or in the process of switching between the two.

I agree this would be the proper fix. Do you have any proposals or ideas
on how this potential race could be addressed?

Thanks,
Mario

> Alan Stern
>
>> Replace pm_runtime_put_sync() with pm_runtime_put_sync_autosuspend()
>> as recommended by Section 9 of Documentation/power/runtime_pm.rst.
>> This updates the device's last_busy timestamp and delays the suspend
>> until after the autosuspend_delay period, allowing pending device
>> detection to complete. As documented, this approach prevents
>> "bouncing too rapidly between low-power and full-power states".
>>
>> The synchronous variant pm_runtime_put_sync_autosuspend() is used
>> (rather than the asynchronous __pm_runtime_put_autosuspend()) to match
>> the semantics of the original pm_runtime_put_sync() call.
>>
>> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
>> ---
>>
>> Testing was performed using a scripted sequence with a relay to simulate
>> USB plug/unplug operations at various timing intervals, verifying that
>> devices are consistently detected after replugging.
>>
>> v1: submitted (https://lore.kernel.org/linux-usb/zuzsjr6isq6i5izw3rkyo45opyikiqjmy5xk7flpmlgmqb6mgh@rpbdvi3s4u54/)
>> v2: dropped redundant pm_runtime_mark_last_busy() call
>>
>>   drivers/usb/chipidea/otg.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
>> index 647e98f4e351..d19c27f44424 100644
>> --- a/drivers/usb/chipidea/otg.c
>> +++ b/drivers/usb/chipidea/otg.c
>> @@ -230,7 +230,7 @@ static void ci_otg_work(struct work_struct *work)
>>                ci_handle_vbus_change(ci);
>>        }
>>
>> -     pm_runtime_put_sync(ci->dev);
>> +     pm_runtime_put_sync_autosuspend(ci->dev);
>>
>>        enable_irq(ci->irq);
>>   }
>>
>> base-commit: f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1
>> --
>> 2.43.0

Re: [PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[Alan Stern]

On Fri, Jan 09, 2026 at 11:57:17AM +0000, PETER Mario wrote:
> On 1/8/26 17:12, Alan Stern wrote:
> > The real way to fix this race is by ensuring that device detection will
> > occur in all cases, whether the controller is at full power, suspended,
> > or in the process of switching between the two.
> 
> I agree this would be the proper fix. Do you have any proposals or ideas
> on how this potential race could be addressed?

No, because I don't know how the hardware or the driver works.  Peter 
Chen would be a better person to ask.

Alan Stern

Re: [PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[Peter Chen (CIX)]

On 26-01-09 11:57:17, PETER Mario wrote:
> On 1/8/26 17:12, Alan Stern wrote:
> 
> > On Thu, Jan 08, 2026 at 03:34:58PM +0000, Mario Peter wrote:
> >> Switch to autosuspend mechanism in ci_otg_work() to prevent race
> >> conditions during rapid device plug/unplug cycles.
> >>
> >> The current implementation uses pm_runtime_put_sync() which triggers
> >> immediate runtime suspend. This creates a race condition when a USB
> >> device is unplugged and quickly replugged. The controller may suspend
> >> before the new device detection completes, causing detection to fail.
> > You should be aware that this change alone won't prevent race
> > conditions.  They might not occur during rapid plug/unplug cycles, but
> > they can still occur.
> >
> > Consider what would happen if the device is unplugged and then plugged
> > back in a few seconds later, exactly when the autosuspend mechanism
> > kicks in and starts suspending the controller.
> 
> I tested this scenario specifically with my automated test setup,
> including plug/unplug cycles timed around the autosuspend delay of 2
> seconds to provoke exactly the issue you describe. I was not able to
> reproduce the detection failure, which does not mean the race condition
> doesn't exist - only that it may be harder to trigger or dependent on
> other timing factors.
> 
> > The real way to fix this race is by ensuring that device detection will
> > occur in all cases, whether the controller is at full power, suspended,
> > or in the process of switching between the two.
> 
> I agree this would be the proper fix. Do you have any proposals or ideas
> on how this potential race could be addressed?
> 

Hi Mario,

Since you have enabled runtime PM for chipidea core, I assmue your SoC
has designed wakeup logic for USB controller or you have non-USB external
events (eg GPIO) for hot plug, which one for your platform?

- If your platform has VBUS wakeup logic at SoC, you may enable wakeup
at your glue layer, it is later than chipidea core's runtime suspend.
If VBUS change occurs before controller goes to suspend, you get normal
vbus change interrupt, else, you get an VBUS wakeup interrupt.

- If you use external event for hot plug, then this event will not lost
because it is not related to controller's suspend, unless this event
itself has debounce requirement.

Peter


> >
> >> Replace pm_runtime_put_sync() with pm_runtime_put_sync_autosuspend()
> >> as recommended by Section 9 of Documentation/power/runtime_pm.rst.
> >> This updates the device's last_busy timestamp and delays the suspend
> >> until after the autosuspend_delay period, allowing pending device
> >> detection to complete. As documented, this approach prevents
> >> "bouncing too rapidly between low-power and full-power states".
> >>
> >> The synchronous variant pm_runtime_put_sync_autosuspend() is used
> >> (rather than the asynchronous __pm_runtime_put_autosuspend()) to match
> >> the semantics of the original pm_runtime_put_sync() call.
> >>
> >> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
> >> ---
> >>
> >> Testing was performed using a scripted sequence with a relay to simulate
> >> USB plug/unplug operations at various timing intervals, verifying that
> >> devices are consistently detected after replugging.
> >>
> >> v1: submitted (https://lore.kernel.org/linux-usb/zuzsjr6isq6i5izw3rkyo45opyikiqjmy5xk7flpmlgmqb6mgh@rpbdvi3s4u54/)
> >> v2: dropped redundant pm_runtime_mark_last_busy() call
> >>
> >>   drivers/usb/chipidea/otg.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> >> index 647e98f4e351..d19c27f44424 100644
> >> --- a/drivers/usb/chipidea/otg.c
> >> +++ b/drivers/usb/chipidea/otg.c
> >> @@ -230,7 +230,7 @@ static void ci_otg_work(struct work_struct *work)
> >>                ci_handle_vbus_change(ci);
> >>        }
> >>
> >> -     pm_runtime_put_sync(ci->dev);
> >> +     pm_runtime_put_sync_autosuspend(ci->dev);
> >>
> >>        enable_irq(ci->irq);
> >>   }
> >>
> >> base-commit: f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1
> >> --
> >> 2.43.0
> 
> 

-- 

Best regards,
Peter

Re: [PATCH v2] usb: chipidea: otg: use autosuspend in ci_otg_work

[PETER Mario]

On 1/14/26 02:34, Peter Chen (CIX) wrote:
> On 26-01-09 11:57:17, PETER Mario wrote:
>> On 1/8/26 17:12, Alan Stern wrote:
>>
>>> On Thu, Jan 08, 2026 at 03:34:58PM +0000, Mario Peter wrote:
>>>> Switch to autosuspend mechanism in ci_otg_work() to prevent race
>>>> conditions during rapid device plug/unplug cycles.
>>>>
>>>> The current implementation uses pm_runtime_put_sync() which triggers
>>>> immediate runtime suspend. This creates a race condition when a USB
>>>> device is unplugged and quickly replugged. The controller may suspend
>>>> before the new device detection completes, causing detection to fail.
>>> You should be aware that this change alone won't prevent race
>>> conditions.  They might not occur during rapid plug/unplug cycles, but
>>> they can still occur.
>>>
>>> Consider what would happen if the device is unplugged and then plugged
>>> back in a few seconds later, exactly when the autosuspend mechanism
>>> kicks in and starts suspending the controller.
>>
>> I tested this scenario specifically with my automated test setup,
>> including plug/unplug cycles timed around the autosuspend delay of 2
>> seconds to provoke exactly the issue you describe. I was not able to
>> reproduce the detection failure, which does not mean the race condition
>> doesn't exist - only that it may be harder to trigger or dependent on
>> other timing factors.
>>
>>> The real way to fix this race is by ensuring that device detection will
>>> occur in all cases, whether the controller is at full power, suspended,
>>> or in the process of switching between the two.
>>
>> I agree this would be the proper fix. Do you have any proposals or ideas
>> on how this potential race could be addressed?
>>
> 
> Hi Mario,
> 
> Since you have enabled runtime PM for chipidea core, I assmue your SoC
> has designed wakeup logic for USB controller or you have non-USB external
> events (eg GPIO) for hot plug, which one for your platform?
> 
> - If your platform has VBUS wakeup logic at SoC, you may enable wakeup
> at your glue layer, it is later than chipidea core's runtime suspend.
> If VBUS change occurs before controller goes to suspend, you get normal
> vbus change interrupt, else, you get an VBUS wakeup interrupt.
> 
> - If you use external event for hot plug, then this event will not lost
> because it is not related to controller's suspend, unless this event
> itself has debounce requirement.
> 
> Peter
> 

Hi Peter, Alan,

Thank you for the review and your helpful comments.

You're absolutely right. This patch was just masking the real problem. Following 
your feedback, we ran more detailed traces and discovered the actual issue 
is in our firmware, which is generating invalid USB state transitions on 
the VBUS/ID pins. The autosuspend approach was simply hiding these bad 
states.

Please ignore this patch. The proper fix needs to be on our end rather 
than working around in the driver.

Thanks again for your feedback.

Best regards,
Mario

> 
>>>
>>>> Replace pm_runtime_put_sync() with pm_runtime_put_sync_autosuspend()
>>>> as recommended by Section 9 of Documentation/power/runtime_pm.rst.
>>>> This updates the device's last_busy timestamp and delays the suspend
>>>> until after the autosuspend_delay period, allowing pending device
>>>> detection to complete. As documented, this approach prevents
>>>> "bouncing too rapidly between low-power and full-power states".
>>>>
>>>> The synchronous variant pm_runtime_put_sync_autosuspend() is used
>>>> (rather than the asynchronous __pm_runtime_put_autosuspend()) to match
>>>> the semantics of the original pm_runtime_put_sync() call.
>>>>
>>>> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
>>>> ---
>>>>
>>>> Testing was performed using a scripted sequence with a relay to simulate
>>>> USB plug/unplug operations at various timing intervals, verifying that
>>>> devices are consistently detected after replugging.
>>>>
>>>> v1: submitted (https://lore.kernel.org/linux-usb/zuzsjr6isq6i5izw3rkyo45opyikiqjmy5xk7flpmlgmqb6mgh@rpbdvi3s4u54/)
>>>> v2: dropped redundant pm_runtime_mark_last_busy() call
>>>>
>>>>   drivers/usb/chipidea/otg.c | 3 +--
>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
>>>> index 647e98f4e351..d19c27f44424 100644
>>>> --- a/drivers/usb/chipidea/otg.c
>>>> +++ b/drivers/usb/chipidea/otg.c
>>>> @@ -230,7 +230,7 @@ static void ci_otg_work(struct work_struct *work)
>>>>                ci_handle_vbus_change(ci);
>>>>        }
>>>>
>>>> -     pm_runtime_put_sync(ci->dev);
>>>> +     pm_runtime_put_sync_autosuspend(ci->dev);
>>>>
>>>>        enable_irq(ci->irq);
>>>>   }
>>>>
>>>> base-commit: f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1
>>>> --
>>>> 2.43.0
>>
>>
> 
> --
> 
> Best regards,
> Peter