lkml.org 
[lkml]   [2011]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v5 5/6] ata: add ata port system PM callbacks
    From
    Date
    On Tue, 2011-12-13 at 23:47 +0800, Alan Stern wrote:
    > On Tue, 13 Dec 2011, Lin Ming wrote:
    >
    > > I just realized that the ata port runtime PM status need to be updated
    > > after system resume.
    > >
    > > I tried below patch.
    > > Unfortunately, it causes a problem that sd can't resume correctly.
    > >
    > > During system resume, ata port is resumed first and then sd resumed.
    > >
    > > When ata port resumed, device_resume(...) calls pm_runtime_put_sync(..),
    > > which causes ata port to be runtime suspended immediately.
    > >
    > > So sd resume fails because it requires ata port to be active to handle
    > > start device command.
    > >
    > > This seems a PM core's bug.
    > >
    > > device_resume(...) should not runtime suspend the parent device, because
    > > its children have not resumed yet.
    > >
    > > Alan,
    > >
    > > What do you think?
    >
    > This appears to be the first time this problem has come up. But it is
    > a real problem.
    >
    > If a child device was runtime-suspended when a system suspend began,
    > then there will be nothing to prevent its parent from
    > runtime-suspending as soon as it is woken up during the system resume.
    > Then when the time comes to resume the child, the resume will fail
    > because the parent is already back at low power.
    >
    > On the other hand, there are some devices which should remain at low
    > power across an entire suspend-resume cycle. The details depend on the
    > device and the platform.
    >
    > This suggests that the PM core is not the right place to solve the
    > problem. One possible solution is for the subsystem or device driver
    > to call pm_runtime_get_sync(dev->parent) at the start of the
    > system-resume procedure and pm_runtime_put_sync(dev->parent) at the
    > end.

    How about below?
    (Not tested yet)

    diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
    index a633076..5cf9a19 100644
    --- a/drivers/scsi/scsi_pm.c
    +++ b/drivers/scsi/scsi_pm.c
    @@ -71,9 +71,17 @@ static int scsi_bus_suspend_common(struct device *dev, pm_message_t msg)
    static int scsi_bus_resume_common(struct device *dev)
    {
    int err = 0;
    + bool put = false;

    - if (scsi_is_sdev_device(dev))
    + if (scsi_is_sdev_device(dev)) {
    + if (dev->parent && pm_runtime_suspended(dev->parent)) {
    + pm_runtime_get_sync(dev->parent);
    + put = true;
    + }
    err = scsi_dev_type_resume(dev);
    + if (put)
    + pm_runtime_put_sync(dev->parent);
    + }

    if (err == 0) {
    pm_runtime_disable(dev);


    >
    > Alan Stern
    >




    \
     
     \ /
      Last update: 2011-12-13 20:01    [W:0.025 / U:1.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site