The controller may take much longer time to recover in this case, and leads to wrong HW state after stop_engine() inside ahci_hardreset() and cause device type checking failure due to unfinished HW state change and missing D2H FIS after start_engine() again inside ahci_hardreset(). I guess this is the reason why AHCI spec try to emphasize. Yes, without this change, Broadcom controller will fail due to above reason. Thanks, Jian -----Original Message----- From: Tejun Heo [mailto:tj@xxxxxxxxxx] Sent: Wednesday, December 08, 2010 11:55 AM To: Jian Peng Cc: Robert Hancock; linux-kernel@xxxxxxxxxxxxxxx; jgarzik@xxxxxxxxx; ide Subject: Re: questions regarding possible violation of AHCI spec in AHCI driver Hello, On 12/08/2010 08:49 PM, Jian Peng wrote: > I agree. I have AHCI based PCI card using HBA from Marvell, Via and > Silicon Image, and am going to test my patch. Before this patch can > be applied universally, I like to use it for specific PCI_VENDOR_ID > first. Here is my new patch to limit it to Broadcom's AHCI core Hmmm... is the change actually necessary for broadcom controllers? As I wrote before, any ahci controller should just work without the above checks because, > + /* avoid race condition per spec (end of section 10.1.2) */ > + if (pdev->vendor == PCI_VENDOR_ID_BROADCOM) { > + u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF; > + > + if (status & (ATA_BUSY | ATA_DRQ) || > + ahci_scr_read(&ap->link, SCR_STATUS, &tmp) || > + (tmp & 0x0f) != 0x03) > + return; PHY event can occur here which causes the device to send D2H Reg FIS w/ BSY set. 1. So, the controller _MUST NOT_ fail in irrecoverable way even if the driver sets ST while BSY is set. 2. The driver guarantees the final ST setting before entering normal operation is done when the prerequisites are met. If you combine 1 and 2, the current behavior is perfectly fine. Do broadcom controllers actually fail without the above change? Thanks. -- tejun -- 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
