State of resume for AHCI?

State of resume for AHCI?

[Jeremy Fitzhardinge]

Hi Jeff,

I'm trying to get a Thinkpad X60 working properly.  My biggest remaining 
problem is the failure of AHCI to resume properly after suspend-to-ram.  
I know the problem is known about and there are patches floating about 
to address it, but I don't really understand the larger picture.  There 
were some discussions about it a couple of months ago, but I haven't 
seen anything much since.

Is there a git tree which should work properly on this hardware, or a 
current patch to do it?  Does somebody need to bring the existing patch 
up to date?  Does fixing this require more infrastructure changes which 
are still brewing?

I'm running 2.6.17-rc2-mm1 (soon mm2).  I've tried porting Hannes 
Reinecke's patch to this tree, but there have been some code changes 
which cause significant conflicts, and I don't really know enough about 
the driver/hardware to be confident in my changes.  That said, I have 
got something compiling, but it crashes somewhere in the kobject stuff 
when it encounters the first port with no drive on it; this looks like a 
more general scsi/libata bug; I'll track it down if its interesting.

Anyway, I'd appreciate it if you could give me some hints about how to 
get a fix with some current kernel.

Thanks,
    J

Re: State of resume for AHCI?

[Mark Lord]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

The one-line "resume fix" (attached) *might* be all that you need.
This is in current Linus 2.6.17-rc*-git*

Cheers

[-- Attachment #2: resume_fix.patch --]
[-- Type: text/x-patch, Size: 2731 bytes --]

Return-Path: <linux-ide-owner@vger.kernel.org>
X-Original-To: liml@rtr.ca
Delivered-To: liml@rtr.ca
Received: from vger.kernel.org (vger.kernel.org [209.132.176.167])
	by mail.rtr.ca (Postfix) with ESMTP id 2DCD7830162
	for <liml@rtr.ca>; Sun, 28 May 2006 11:28:07 -0400 (EDT)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1750768AbWE1P2E (ORCPT <rfc822;liml@rtr.ca>);
	Sun, 28 May 2006 11:28:04 -0400
Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750772AbWE1P2D
	(ORCPT <rfc822;linux-ide-outgoing>); Sun, 28 May 2006 11:28:03 -0400
Received: from rtr.ca ([64.26.128.89]:19922 "EHLO mail.rtr.ca")
	by vger.kernel.org with ESMTP id S1750768AbWE1P2C (ORCPT
	<rfc822;linux-ide@vger.kernel.org>); Sun, 28 May 2006 11:28:02 -0400
Received: from silvy.localnet (silvy.localnet [10.0.0.14])
	by mail.rtr.ca (Postfix) with ESMTP id D0C538300A1;
	Sun, 28 May 2006 11:28:00 -0400 (EDT)
From: Mark Lord <liml@rtr.ca>
Organization: Real-Time Remedies Inc.
To: Jeff Garzik <jeff@garzik.org>
Subject: [PATCH] 2.6.17-rc5: the latest consensus libata resume fix
Date:	Sun, 28 May 2006 11:28:00 -0400
User-Agent: KMail/1.9.1
Cc: Linus Torvalds <torvalds@osdl.org>, Jens Axboe <axboe@suse.de>,
	"zhao, forrest" <forrest.zhao@intel.com>,
	Tejun Heo <htejun@gmail.com>, linux-ide@vger.kernel.org
References: <200605272245.30108.axboe@suse.de> <Pine.LNX.4.64.0605271716030.5623@g5.osdl.org> <4478F681.8050607@garzik.org>
In-Reply-To: <4478F681.8050607@garzik.org>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200605281128.00532.liml@rtr.ca>
Sender: linux-ide-owner@vger.kernel.org
Precedence: bulk
X-Mailing-List:	linux-ide@vger.kernel.org

Okay, just to sum things up.

The patch below is the current "release candidate" for
improving the libata/ata_piix resume functionality in 2.6.17-rc*.

It forces libata to wait for up to 2 seconds for BUSY|DRQ to clear
on resume before continuing.  This is only for 2.6.17 at present.

We are waiting on Jens to test and report back for this specific version.

Signed-off-by:  Mark Lord <liml@rtr.ca>
---
--- linux-2.6.17-rc5/drivers/scsi/libata-core.c
+++ linux/drivers/scsi/libata-core.c
@@ -4296,6 +4296,7 @@ static int ata_start_drive(struct ata_po
  */
 int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
 {
 	if (ap->flags & ATA_FLAG_SUSPENDED) {
+		ata_busy_wait(ap, ATA_BUSY | ATA_DRQ, 200000);
 		ap->flags &= ~ATA_FLAG_SUSPENDED;
 		ata_set_mode(ap);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: State of resume for AHCI?

[Jens Axboe]

On Thu, Jun 01 2006, Mark Lord wrote:
> The one-line "resume fix" (attached) *might* be all that you need.
> This is in current Linus 2.6.17-rc*-git*

It's a lot more complicated than that, I'm afraid. ahci doesn't even
have the resume/suspend methods defined, plus it needs more work than
piix on resume.

libata should define a default suspend hook for devices that haven't
added their own, to prevent accidents with people trying to suspend
devices that haven't been fixed up. It's a data integrity issue!

-- 
Jens Axboe

Re: State of resume for AHCI?

[Mark Lord]

Mark Lord wrote:
> The one-line "resume fix" (attached) *might* be all that you need.
> This is in current Linus 2.6.17-rc*-git*

Oh.. yes, you'll have to switch to the ata_piix driver for this to work.
So long as your AHCI is an Intel one, that will probably work fine.

Cheers

Re: State of resume for AHCI?

[Jeremy Fitzhardinge]

Jens Axboe wrote:
> It's a lot more complicated than that, I'm afraid. ahci doesn't even
> have the resume/suspend methods defined, plus it needs more work than
> piix on resume.
>   
Hannes Reinecke's patch implements those functions, basically by 
factoring out the shutdown and init code and calling them at 
suspend/resume time as well.

Is that correct/sufficient?  Or should something else be happening?

    J

Re: State of resume for AHCI?

[zhao, forrest]

On Thu, 2006-06-01 at 13:19 -0700, Jeremy Fitzhardinge wrote:
> Jens Axboe wrote:
> > It's a lot more complicated than that, I'm afraid. ahci doesn't even
> > have the resume/suspend methods defined, plus it needs more work than
> > piix on resume.
> >   
> Hannes Reinecke's patch implements those functions, basically by 
> factoring out the shutdown and init code and calling them at 
> suspend/resume time as well.
> 
> Is that correct/sufficient?  Or should something else be happening?

According to our test of Hannes's patch, it's not sufficient to support
AHCI suspend/resume.

Now I'm writing a patch to try to provide complete support for AHCI
suspend/resume and will send out patch soon, hopefully by the end of
today.

Thanks,
Forrest

Re: State of resume for AHCI?

[Jeremy Fitzhardinge]

zhao, forrest wrote:
> According to our test of Hannes's patch, it's not sufficient to support
> AHCI suspend/resume.
>   
Yes.  I merged it into mm2, and it didn't make any difference.
> Now I'm writing a patch to try to provide complete support for AHCI
> suspend/resume and will send out patch soon, hopefully by the end of
> today.
Great, looking forward to it.

    J

Re: State of resume for AHCI?

[Jens Axboe]

On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> Jens Axboe wrote:
> >It's a lot more complicated than that, I'm afraid. ahci doesn't even
> >have the resume/suspend methods defined, plus it needs more work than
> >piix on resume.
> >  
> Hannes Reinecke's patch implements those functions, basically by 
> factoring out the shutdown and init code and calling them at 
> suspend/resume time as well.
> 
> Is that correct/sufficient?  Or should something else be happening?

No that's it, I know for a fact that suspend/resume works perfectly with
the 10.1 suse kernel. You can give that a shot, if you want.

-- 
Jens Axboe

Re: State of resume for AHCI?

[zhao, forrest]

On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
> On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> > Jens Axboe wrote:
> > >It's a lot more complicated than that, I'm afraid. ahci doesn't even
> > >have the resume/suspend methods defined, plus it needs more work than
> > >piix on resume.
> > >  
> > Hannes Reinecke's patch implements those functions, basically by 
> > factoring out the shutdown and init code and calling them at 
> > suspend/resume time as well.
> > 
> > Is that correct/sufficient?  Or should something else be happening?
> 
> No that's it, I know for a fact that suspend/resume works perfectly with
> the 10.1 suse kernel. You can give that a shot, if you want.

You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
posted in open source community(or in linux-ide mailing list) didn't
work.

Forrest

Re: State of resume for AHCI?

[Hannes Reinecke]

zhao, forrest wrote:
> On Thu, 2006-06-01 at 13:19 -0700, Jeremy Fitzhardinge wrote:
>> Jens Axboe wrote:
>>> It's a lot more complicated than that, I'm afraid. ahci doesn't even
>>> have the resume/suspend methods defined, plus it needs more work than
>>> piix on resume.
>>>   
>> Hannes Reinecke's patch implements those functions, basically by 
>> factoring out the shutdown and init code and calling them at 
>> suspend/resume time as well.
>>
>> Is that correct/sufficient?  Or should something else be happening?
> 
> According to our test of Hannes's patch, it's not sufficient to support
> AHCI suspend/resume.
> 
> Now I'm writing a patch to try to provide complete support for AHCI
> suspend/resume and will send out patch soon, hopefully by the end of
> today.
> 
Can I have details, please? What doesn't work?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

Re: State of resume for AHCI?

[Jens Axboe]

On Fri, Jun 02 2006, zhao, forrest wrote:
> On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
> > On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> > > Jens Axboe wrote:
> > > >It's a lot more complicated than that, I'm afraid. ahci doesn't even
> > > >have the resume/suspend methods defined, plus it needs more work than
> > > >piix on resume.
> > > >  
> > > Hannes Reinecke's patch implements those functions, basically by 
> > > factoring out the shutdown and init code and calling them at 
> > > suspend/resume time as well.
> > > 
> > > Is that correct/sufficient?  Or should something else be happening?
> > 
> > No that's it, I know for a fact that suspend/resume works perfectly with
> > the 10.1 suse kernel. You can give that a shot, if you want.
> 
> You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
> posted in open source community(or in linux-ide mailing list) didn't
> work.

I didn't say Hannes patch, I said I know that 10.1 works. And that is
probably in large due to the patch that Hannes did, which implents
resume/suspend and takes care of reinitializing the resources.

-- 
Jens Axboe

Re: State of resume for AHCI?

[Hannes Reinecke]

Jens Axboe wrote:
> On Fri, Jun 02 2006, zhao, forrest wrote:
>> On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
>>> On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
>>>> Jens Axboe wrote:
>>>>> It's a lot more complicated than that, I'm afraid. ahci doesn't even
>>>>> have the resume/suspend methods defined, plus it needs more work than
>>>>> piix on resume.
>>>>>  
>>>> Hannes Reinecke's patch implements those functions, basically by 
>>>> factoring out the shutdown and init code and calling them at 
>>>> suspend/resume time as well.
>>>>
>>>> Is that correct/sufficient?  Or should something else be happening?
>>> No that's it, I know for a fact that suspend/resume works perfectly with
>>> the 10.1 suse kernel. You can give that a shot, if you want.
>> You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
>> posted in open source community(or in linux-ide mailing list) didn't
>> work.
> 
> I didn't say Hannes patch, I said I know that 10.1 works. And that is
> probably in large due to the patch that Hannes did, which implents
> resume/suspend and takes care of reinitializing the resources.
> 
Indeed. I didn't post the latest set of patches to the open-source
community as Jeff indicated he would only accept patches against
libata-dev. And as I didn't have time to port them yet I didn't feel the
need to do so.

Forrest, please drop me a mail if I can be of further assistance.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

Re: State of resume for AHCI?

[zhao, forrest]

On Fri, 2006-06-02 at 08:43 +0200, Hannes Reinecke wrote:
> Jens Axboe wrote:
> > On Fri, Jun 02 2006, zhao, forrest wrote:
> >> On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
> >>> On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> >>>> Jens Axboe wrote:
> >>>>> It's a lot more complicated than that, I'm afraid. ahci doesn't even
> >>>>> have the resume/suspend methods defined, plus it needs more work than
> >>>>> piix on resume.
> >>>>>  
> >>>> Hannes Reinecke's patch implements those functions, basically by 
> >>>> factoring out the shutdown and init code and calling them at 
> >>>> suspend/resume time as well.
> >>>>
> >>>> Is that correct/sufficient?  Or should something else be happening?
> >>> No that's it, I know for a fact that suspend/resume works perfectly with
> >>> the 10.1 suse kernel. You can give that a shot, if you want.
> >> You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
> >> posted in open source community(or in linux-ide mailing list) didn't
> >> work.
> > 
> > I didn't say Hannes patch, I said I know that 10.1 works. And that is
> > probably in large due to the patch that Hannes did, which implents
> > resume/suspend and takes care of reinitializing the resources.
> > 
> Indeed. I didn't post the latest set of patches to the open-source
> community as Jeff indicated he would only accept patches against
> libata-dev. And as I didn't have time to port them yet I didn't feel the
> need to do so.
> 
> Forrest, please drop me a mail if I can be of further assistance.

I almost finished porting a patch from OpenSUSE, will send it out in an
hour, please help review it :)

Thanks,
Forrest

Re: State of resume for AHCI?

[Jens Axboe]

On Fri, Jun 02 2006, Hannes Reinecke wrote:
> Jens Axboe wrote:
> > On Fri, Jun 02 2006, zhao, forrest wrote:
> >> On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
> >>> On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> >>>> Jens Axboe wrote:
> >>>>> It's a lot more complicated than that, I'm afraid. ahci doesn't even
> >>>>> have the resume/suspend methods defined, plus it needs more work than
> >>>>> piix on resume.
> >>>>>  
> >>>> Hannes Reinecke's patch implements those functions, basically by 
> >>>> factoring out the shutdown and init code and calling them at 
> >>>> suspend/resume time as well.
> >>>>
> >>>> Is that correct/sufficient?  Or should something else be happening?
> >>> No that's it, I know for a fact that suspend/resume works perfectly with
> >>> the 10.1 suse kernel. You can give that a shot, if you want.
> >> You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
> >> posted in open source community(or in linux-ide mailing list) didn't
> >> work.
> > 
> > I didn't say Hannes patch, I said I know that 10.1 works. And that is
> > probably in large due to the patch that Hannes did, which implents
> > resume/suspend and takes care of reinitializing the resources.
> > 
> Indeed. I didn't post the latest set of patches to the open-source
> community as Jeff indicated he would only accept patches against
> libata-dev. And as I didn't have time to port them yet I didn't feel the
> need to do so.

Understandable, I hope you will have some time to push it for 2.6.18
though.

-- 
Jens Axboe

Re: State of resume for AHCI?

[Jens Axboe]

On Fri, Jun 02 2006, zhao, forrest wrote:
> On Fri, 2006-06-02 at 08:43 +0200, Hannes Reinecke wrote:
> > Jens Axboe wrote:
> > > On Fri, Jun 02 2006, zhao, forrest wrote:
> > >> On Fri, 2006-06-02 at 08:03 +0200, Jens Axboe wrote:
> > >>> On Thu, Jun 01 2006, Jeremy Fitzhardinge wrote:
> > >>>> Jens Axboe wrote:
> > >>>>> It's a lot more complicated than that, I'm afraid. ahci doesn't even
> > >>>>> have the resume/suspend methods defined, plus it needs more work than
> > >>>>> piix on resume.
> > >>>>>  
> > >>>> Hannes Reinecke's patch implements those functions, basically by 
> > >>>> factoring out the shutdown and init code and calling them at 
> > >>>> suspend/resume time as well.
> > >>>>
> > >>>> Is that correct/sufficient?  Or should something else be happening?
> > >>> No that's it, I know for a fact that suspend/resume works perfectly with
> > >>> the 10.1 suse kernel. You can give that a shot, if you want.
> > >> You may mean the Hannes's patch for 10.1 SUSE kernel. Hannes's patch
> > >> posted in open source community(or in linux-ide mailing list) didn't
> > >> work.
> > > 
> > > I didn't say Hannes patch, I said I know that 10.1 works. And that is
> > > probably in large due to the patch that Hannes did, which implents
> > > resume/suspend and takes care of reinitializing the resources.
> > > 
> > Indeed. I didn't post the latest set of patches to the open-source
> > community as Jeff indicated he would only accept patches against
> > libata-dev. And as I didn't have time to port them yet I didn't feel the
> > need to do so.
> > 
> > Forrest, please drop me a mail if I can be of further assistance.
> 
> I almost finished porting a patch from OpenSUSE, will send it out in an
> hour, please help review it :)

Wonderful! Please do test it as well :-)

-- 
Jens Axboe

Re: State of resume for AHCI?

[Tejun Heo]

Mark Lord wrote:
> Mark Lord wrote:
>> The one-line "resume fix" (attached) *might* be all that you need.
>> This is in current Linus 2.6.17-rc*-git*
> 
> Oh.. yes, you'll have to switch to the ata_piix driver for this to work.
> So long as your AHCI is an Intel one, that will probably work fine.

Many notebooks don't allow changing between piix and ahci modes.  My 
notebook detects one piix interface and four ahci ports.  The piix 
interface is connected to the harddisk while one of the ahci ports is 
the hotswap bay.  So, my harddisk suspends and resumes fine but I can't 
use my dvd rom after suspend/resume.

-- 
tejun