From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: [RFC v3 04/13] ahci-platform: Undo pdata->resume on resume failure Date: Sun, 19 Jan 2014 06:27:37 -0500 Message-ID: <20140119112737.GC11123@htj.dyndns.org> References: <1390088935-7193-1-git-send-email-hdegoede@redhat.com> <1390088935-7193-5-git-send-email-hdegoede@redhat.com> Reply-To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Return-path: Sender: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org In-Reply-To: <1390088935-7193-5-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> List-Post: , List-Help: , List-Archive: List-Subscribe: , List-Unsubscribe: , Content-Disposition: inline To: Hans de Goede Cc: Oliver Schinagl , Maxime Ripard , Richard Zhu , linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree , linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org List-Id: devicetree@vger.kernel.org Hello, On Sun, Jan 19, 2014 at 12:48:46AM +0100, Hans de Goede wrote: > When the ahci_resume fails the error handling code tries to undo all changes > made, but it was not undoing the results of pdata->resume. > > Signed-off-by: Hans de Goede > --- > drivers/ata/ahci_platform.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c > index dc1ef73..41720cb 100644 > --- a/drivers/ata/ahci_platform.c > +++ b/drivers/ata/ahci_platform.c > @@ -307,7 +307,7 @@ static int ahci_resume(struct device *dev) > if (dev->power.power_state.event == PM_EVENT_SUSPEND) { > rc = ahci_reset_controller(host); > if (rc) > - goto disable_unprepare_clk; > + goto pdata_suspend; > > ahci_init_controller(host); > } > @@ -316,6 +316,9 @@ static int ahci_resume(struct device *dev) > > return 0; > > +pdata_suspend: > + if (pdata && pdata->suspend) > + pdata->suspend(dev); > disable_unprepare_clk: > if (!IS_ERR(hpriv->clk)) > clk_disable_unprepare(hpriv->clk); Hmmmm... resume isn't an operation you can revert without side-effect when the whole system is waking up from sleep. e.g. think about what should happen the driver is removed and loaded again - it should be able to reinitialized the device, which is unlikely to work if the device is suspended at the platform level. If resume fails, the right state to be in is "failed with as much as resumed" instead of "suspended". Thanks. -- tejun