Hi, On Tuesday, May 27, 2014 04:20:20 PM Shawn Guo wrote: > On Mon, May 26, 2014 at 04:29:37PM -0300, Fabio Estevam wrote: > > The following warning is seen after suspend/resume sequence: > > > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 661 at drivers/ata/libahci.c:224 ahci_enable_ahci+0x74/0x8) > > Modules linked in: > > CPU: 0 PID: 661 Comm: sh Tainted: G W 3.15.0-rc5-next-20140521-000027 > > Backtrace: > > [<80011c90>] (dump_backtrace) from [<80011e2c>] (show_stack+0x18/0x1c) > > r6:803a22f4 r5:00000000 r4:00000000 r3:00000000 > > [<80011e14>] (show_stack) from [<80661e60>] (dump_stack+0x88/0xa4) > > [<80661dd8>] (dump_stack) from [<80028fdc>] (warn_slowpath_common+0x70/0x94) > > r5:00000009 r4:00000000 > > [<80028f6c>] (warn_slowpath_common) from [<80029024>] (warn_slowpath_null+0x24/) > > r8:808f68c4 r7:00000000 r6:00000000 r5:00000000 r4:e0810004 > > [<80029000>] (warn_slowpath_null) from [<803a22f4>] (ahci_enable_ahci+0x74/0x80) > > [<803a2280>] (ahci_enable_ahci) from [<803a2324>] (ahci_reset_controller+0x24/0) > > r8:ddcd9410 r7:80351178 r6:ddcd9444 r5:dde8b850 r4:e0810000 r3:ddf35e90 > > [<803a2300>] (ahci_reset_controller) from [<803a2c68>] (ahci_platform_resume_ho) > > r7:80351178 r6:ddcd9444 r5:dde8b850 r4:ddcd9410 > > [<803a2c30>] (ahci_platform_resume_host) from [<803a38f0>] (imx_ahci_resume+0x2) > > r5:00000000 r4:ddcd9410 > > [<803a38c4>] (imx_ahci_resume) from [<803511ac>] (platform_pm_resume+0x34/0x54) > > .... > > > > Add the missing ahci_platform_resume_host() call in order to fix the resume > > of the driver. > > As Bartlomiej already pointed out, you must mean > ahci_platform_enable_resources() here. Even with it, I'm not sure this > is correct. Calling ahci_platform_enable_resources() in .resume hook > only makes sense in case that ahci_platform_disable_resources() is being > called in .suspend hook. Indeed. > All ahci_platform_enable[disable]_resources() do is handling resources > phy, clock and regulator, which should have been managed by > imx_sata_enable[disable]() in imx case. It is related to imxpriv->no_device handling (or rather lack of it) in ->suspend and ->resume. > > Tested on imx6q-sabresd and imx53-qsb boards. > > I'm still wondering why I cannot reproduce the issue on my imx6q-sabresd > board. Have you tried booting with no devices attached (like suggested by Fabio) and then doing suspend/resume cycle? ahci_imx_error_handler() calls imx_sata_disable(hpriv) and then sets imxpriv->no_device to true. Later during ->resume imx_sata_enable() returns early (due to ->no_device flag being set) but still gives zero as a return value. Then ahci_platform_resume_host() tries to enable AHCI host (please note that the device was previously disabled by imx_sata_disable()) and fails on attempt to access the hardware (this is what triggers WARN_ON in ahci_enable_ahci()). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > Shawn > > > > > Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx> > > --- > > drivers/ata/ahci_imx.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c > > index 8befeb6..1941896 100644 > > --- a/drivers/ata/ahci_imx.c > > +++ b/drivers/ata/ahci_imx.c > > @@ -468,11 +468,25 @@ static int imx_ahci_resume(struct device *dev) > > struct ahci_host_priv *hpriv = host->private_data; > > int ret; > > > > - ret = imx_sata_enable(hpriv); > > + ret = ahci_platform_enable_resources(hpriv); > > if (ret) > > return ret; > > > > - return ahci_platform_resume_host(dev); > > + ret = imx_sata_enable(hpriv); > > + if (ret) > > + goto disable_resources; > > + > > + ret = ahci_platform_resume_host(dev); > > + if (ret) > > + goto disable_sata; > > + > > + return 0; > > + > > +disable_sata: > > + imx_sata_disable(hpriv); > > +disable_resources: > > + ahci_platform_disable_resources(hpriv); > > + return ret; > > } > > #endif -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
