On Tuesday, October 15, 2013 04:58:28 PM Lan Tianyu wrote:
> On 2013年10月11日 19:19, Rafael J. Wysocki wrote:
> > On Friday, October 11, 2013 04:16:25 PM tianyu.lan@xxxxxxxxx wrote:
> >> From: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> >>
> >> Currently, when one power resource is turned on, devices owning it
> >> will be requested to resume regardless of their runtime pm status.
> >> ACPI power resource maybe turn on in some devices' runtime pm
> >> resume callback(E.G, usb port) while turning on the power resource
> >> will trigger one new resume request of the device. It causes
> >> infinite loop between resume and suspend. This has happened on
> >> clearing usb port's PM Qos NO_POWER_OFF flag twice. This patch is
> >> to add check of physical device's runtime pm status and request resume
> >> if the device is suspended.
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> >> ---
> >> drivers/acpi/power.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> >> index 0dbe5cd..228c138 100644
> >> --- a/drivers/acpi/power.c
> >> +++ b/drivers/acpi/power.c
> >> @@ -250,8 +250,10 @@ static void acpi_power_resume_dependent(struct work_struct *work)
> >>
> >> mutex_lock(&adev->physical_node_lock);
> >>
> >> - list_for_each_entry(pn, &adev->physical_node_list, node)
> >> - pm_request_resume(pn->dev);
> >> + list_for_each_entry(pn, &adev->physical_node_list, node) {
> >> + if (pm_runtime_suspended(pn->dev))
> >> + pm_request_resume(pn->dev);
> >> + }
> >
> > This is racy, because the status may change right after you check it and before
> > you call pm_request_resume().
>
> Yes, the runtime status may be changed just after the check.
>
> >
> > Besides, pm_request_resume() checks the status of the device and won't
> > try to resume it if it is not suspended.
> >
>
> For this issue, usb port is in the RPM_SUSPENDING state when
> pm_request_resume() is called.
Why exactly does that happen?
> The deferred_resume will be set to true
> during this procedure and trigger resume after finishing suspend. USB
> port runtime resume callback will turn on its power resource again and
> the work of acpi_power_resume_dependent() is scheduled. Because the usb
> port's usage count remains zero, it's to be suspended soon. When
> pm_request_resume() of acpi_power_resume_dependent() is called, the usb
> port is always in the PRM_SUSPENDING. Fall in the loop of suspend and
> resume.
>
> How about running acpi_power_dependent when turning on power resource
> rather than scheduling a work to run it?
Is this actually going to help? Even if acpi_power_resume_dependent() is
run synchronously from within the resume callback, it will still see
RPM_SUSPENDING as the device's status, won't it?
> After this, pm_request_resume() can check device's right status just after
> turning on power resource.
The status doesn't change until the .runtime_suspend() callback returns and
running pm_request_resume() syncrhonously from that callback for the device
being suspended just plain doesn't make sense.
> Furthermore, pm_request_resume() is async resume and
> this change will not consume much time.
acpi_power_resume_dependent() is run from a work item to avoid locking
problems.
Can you please explain to me how this is possible that the USB port's power
resource is turned "on" while the port is RPM_SUSPENDING?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
