| Submitter | Tejun Heo |
|---|---|
| Date | July 22, 2011, 9:41 a.m. |
| Message ID | <20110722094126.GI2622@htj.dyndns.org> |
| Download | mbox | patch |
| Permalink | /patch/106248/ |
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Comments
On Fri, Jul 22, 2011 at 2:41 AM, Tejun Heo <tj@kernel.org> wrote: > This is another attempt at fixing the same problem that 270dac35c2 > (libata: ahci_start_engine compliant to AHCI spec) tried to solve. > Unfortunately, 270dac35c2 created regressions for a lot more common > controllers and got reverted. I've tested this on my problem controller with success so far. Also tried on Dell Latitude E6410 (with various boot, suspend, resume, unplug power, etc. cycles) with no problem. Thanks Tejun! In case it's useful I'll give a tested-by: Tested-by: Brian Norris <computersforpeace@gmail.com> -- 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
On 07/22/2011 05:41 AM, Tejun Heo wrote: > This is another attempt at fixing the same problem that 270dac35c2 > (libata: ahci_start_engine compliant to AHCI spec) tried to solve. > Unfortunately, 270dac35c2 created regressions for a lot more common > controllers and got reverted. > > This specific AHCI IP block becomes a brick if the DMA engine is > started while DRQ is set. It is not possible to avoid the condition > completely but the most common occurrence is caused by spurious use of > ahci_start_engine() from ahci_start_port() during init sequence. > > DMA engine is started after both soft and hard resets and > ahci_start_port() is always followed by resets, so there is no reason > to start DMA engine from ahci_start_port(). > > This patch removes ahci_start_engine() invocation from > ahci_start_port(). This change makes failure path of > ahci_port_suspend() leave engine stopped without following resets. > This is resolved by replacing ahci_start_port() call with > ata_port_freeze() which forces resets afterwards, which is the better > behavior anyway. > > Signed-off-by: Tejun Heo<tj@kernel.org> > Reported-by: Brian Norris<computersforpeace@gmail.com> > Reported-by: Jian Peng<jipeng2005@gmail.com> > --- > Jeff, this needs to be tested for a while in linux-next before sending > it mainline. Please apply it for 3.1 merge window and let's see > whether anything explodes. Stuffed this into branch #upstream-next, which appears periodically for situations like this. #upstream-next will become #upstream after the merge window closes. -- 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
Patch
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 41223c7..d57f4cf 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -745,9 +745,6 @@ static void ahci_start_port(struct ata_port *ap) /* enable FIS reception */ ahci_start_fis_rx(ap); - /* enable DMA */ - ahci_start_engine(ap); - /* turn on LEDs */ if (ap->flags & ATA_FLAG_EM) { ata_for_each_link(link, ap, EDGE) { @@ -1976,7 +1973,7 @@ static int ahci_port_suspend(struct ata_port *ap, pm_message_t mesg) ahci_power_down(ap); else { ata_port_printk(ap, KERN_ERR, "%s (%d)\n", emsg, rc); - ahci_start_port(ap); + ata_port_freeze(ap); } return rc;
This is another attempt at fixing the same problem that 270dac35c2 (libata: ahci_start_engine compliant to AHCI spec) tried to solve. Unfortunately, 270dac35c2 created regressions for a lot more common controllers and got reverted. This specific AHCI IP block becomes a brick if the DMA engine is started while DRQ is set. It is not possible to avoid the condition completely but the most common occurrence is caused by spurious use of ahci_start_engine() from ahci_start_port() during init sequence. DMA engine is started after both soft and hard resets and ahci_start_port() is always followed by resets, so there is no reason to start DMA engine from ahci_start_port(). This patch removes ahci_start_engine() invocation from ahci_start_port(). This change makes failure path of ahci_port_suspend() leave engine stopped without following resets. This is resolved by replacing ahci_start_port() call with ata_port_freeze() which forces resets afterwards, which is the better behavior anyway. Signed-off-by: Tejun Heo <tj@kernel.org> Reported-by: Brian Norris <computersforpeace@gmail.com> Reported-by: Jian Peng <jipeng2005@gmail.com> --- Jeff, this needs to be tested for a while in linux-next before sending it mainline. Please apply it for 3.1 merge window and let's see whether anything explodes. Thanks. drivers/ata/libahci.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) -- 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