Ticket #12357: ahci-rework-v2b.diff

src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h

@@ -97 +97 @@
 #define IPM_TRANSITIONS_TO_PARTIAL_DISABLED 0x1
 #define IPM_TRANSITIONS_TO_SLUMBER_DISABLED 0x2
 
+// Device signatures
+#define SATA_SIG_ATA            0x00000101 // SATA drive
+#define SATA_SIG_ATAPI          0xEB140101 // ATAPI drive
+#define SATA_SIG_SEMB           0xC33C0101 // Enclosure management bridge
+#define SATA_SIG_PM             0x96690101 // Port multiplier
 
 typedef struct {
     uint32      clb;            // Command List Base Address
@@ -130 +135 @@
     PORT_CMD_ATAPI  = (1 << 24),    // Device is ATAPI
     PORT_CMD_CR     = (1 << 15),    // Command List Running (DMA active)
     PORT_CMD_FR     = (1 << 14),    // FIS Receive Running
-    PORT_CMD_FER    = (1 << 4),     // FIS Receive Enable
+    PORT_CMD_FRE    = (1 << 4),     // FIS Receive Enable
     PORT_CMD_CLO    = (1 << 3),     // Command List Override
     PORT_CMD_POD    = (1 << 2),     // Power On Device
     PORT_CMD_SUD    = (1 << 1),     // Spin-up Device

src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp

@@ -120 +120 @@
     fRegs->is = fRegs->is;
 
     // clear error bits
-    fRegs->serr = fRegs->serr;
+    _ClearErrorRegister();
 
     // power up device
     fRegs->cmd |= PORT_CMD_POD;
@@ -132 +132 @@
     fRegs->cmd = (fRegs->cmd & ~PORT_CMD_ICC_MASK) | PORT_CMD_ICC_ACTIVE;
 
     // enable FIS receive
-    fRegs->cmd |= PORT_CMD_FER;
+    fRegs->cmd |= PORT_CMD_FRE;
 
     FlushPostedWrites();
 
@@ -144 +144 @@
 status_t
 AHCIPort::Init2()
 {
-    TRACE("AHCIPort::Init2 port %d\n", fIndex);
+    TRACE("%s: port %d\n", __func__, fIndex);
 
-    // start DMA engine
-    fRegs->cmd |= PORT_CMD_ST;
+    // enable port
+    Enable();
 
     // enable interrupts
     fRegs->ie = PORT_INT_MASK;
 
     FlushPostedWrites();
 
+    // reset port and probe info
     ResetPort(true);
 
     TRACE("ie   0x%08" B_PRIx32 "\n", fRegs->ie);
@@ -172 +173 @@
 
     fDevicePresent = (fRegs->ssts & 0xf) == 0x3;
 
+    TRACE("%s: port %d, device %s\n", __func__, fIndex,
+        fDevicePresent ? "present" : "absent");
+
     return B_OK;
 }
 
@@ -181 +185 @@
 {
     TRACE("AHCIPort::Uninit port %d\n", fIndex);
 
-    // disable FIS receive
-    fRegs->cmd &= ~PORT_CMD_FER;
-
-    // wait for receive completion, up to 500ms
-    if (wait_until_clear(&fRegs->cmd, PORT_CMD_FR, 500000) < B_OK) {
-        TRACE("AHCIPort::Uninit port %d error FIS rx still running\n", fIndex);
-    }
-
-    // stop DMA engine
-    fRegs->cmd &= ~PORT_CMD_ST;
+    // Clear FRE and wait for completion
+    fRegs->cmd &= ~PORT_CMD_FRE;
+    if (wait_until_clear(&fRegs->cmd, PORT_CMD_FR, 500000) < B_OK)
+        TRACE("%s: port %d error FIS rx still running\n", __func__, fIndex);
 
-    // wait for DMA completion
-    if (wait_until_clear(&fRegs->cmd, PORT_CMD_CR, 500000) < B_OK) {
-        TRACE("AHCIPort::Uninit port %d error DMA engine still running\n",
-            fIndex);
-    }
+    // shutdown the port
+    Disable();
 
     // disable interrupts
     fRegs->ie = 0;
@@ -218 +213 @@
 AHCIPort::ResetDevice()
 {
     // perform a hard reset
-    if (!_HardReset())
+    if (!_HardReset()) {
+        ERROR("%s: port %d unable to hard reset device\n", __func__, fIndex);
         return;
+    }
 
     if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK)
         TRACE("AHCIPort::ResetDevice port %d no device detected\n", fIndex);
@@ -228 +225 @@
 
     if (fRegs->ssts & 1) {
         if (wait_until_set(&fRegs->ssts, 0x3, 500000) < B_OK) {
-            TRACE("AHCIPort::ResetDevice port %d device present but no phy "
+            TRACE("AHCIPort::ResetDevice port %d device present but no phy " 
                 "communication\n", fIndex);
         }
     }
@@ -241 +238 @@
 AHCIPort::ResetPort(bool forceDeviceReset)
 {
     if (!fTestUnitReadyActive)
-        TRACE("AHCIPort::ResetPort port %d\n", fIndex);
+        TRACE("%s: port %d\n", __func__, fIndex);
 
-    // stop DMA engine
-    fRegs->cmd &= ~PORT_CMD_ST;
-    FlushPostedWrites();
-
-    if (wait_until_clear(&fRegs->cmd, PORT_CMD_CR, 500000) < B_OK) {
-        TRACE("AHCIPort::ResetPort port %d error DMA engine doesn't stop\n",
-            fIndex);
-    }
+    // stop port, flush transactions
+    Disable();
 
     bool deviceBusy = fRegs->tfd & (ATA_BSY | ATA_DRQ);
 
     if (!fTestUnitReadyActive) {
-        TRACE("AHCIPort::ResetPort port %d, deviceBusy %d, "
-            "forceDeviceReset %d\n", fIndex, deviceBusy, forceDeviceReset);
+        TRACE("AHCIPort::ResetPort port %d, deviceBusy %d, " 
+            "forceDeviceReset %d\n", fIndex, deviceBusy, forceDeviceReset); 
     }
 
     if (deviceBusy || forceDeviceReset)
         ResetDevice();
 
-    // start DMA engine
-    fRegs->cmd |= PORT_CMD_ST;
-    FlushPostedWrites();
+    // start port
+    Enable();
 
     return PostReset();
 }
@@ -274 +264 @@
 AHCIPort::PostReset()
 {
     if (!fTestUnitReadyActive)
-        TRACE("AHCIPort::PostReset port %d\n", fIndex);
+        TRACE("AHCIPort::PostReset port %d\n", fIndex); 
 
     if ((fRegs->ssts & 0xf) != 0x3 || (fRegs->tfd & 0xff) == 0x7f) {
         TRACE("AHCIPort::PostReset port %d: no device\n", fIndex);
@@ -292 +282 @@
 
     wait_until_clear(&fRegs->tfd, ATA_BSY, 31000000);
 
-    fIsATAPI = fRegs->sig == 0xeb140101;
-
+    fIsATAPI = fRegs->sig == SATA_SIG_ATAPI;
     if (fIsATAPI)
         fRegs->cmd |= PORT_CMD_ATAPI;
     else
@@ -302 +291 @@
 
     if (!fTestUnitReadyActive) {
         TRACE("device signature 0x%08" B_PRIx32 " (%s)\n", fRegs->sig,
-            fRegs->sig == 0xeb140101 ? "ATAPI" : fRegs->sig == 0x00000101
+            fRegs->sig == SATA_SIG_ATAPI ? "ATAPI" : fRegs->sig == SATA_SIG_ATA
                 ? "ATA" : "unknown");
     }
 
@@ -374 +363 @@
     }
 
     // read and clear SError
-    uint32 serr = fRegs->serr;
-    fRegs->serr = serr;
+    _ClearErrorRegister();
 
     if (is & PORT_INT_TFE) {
         if (!fTestUnitReadyActive)
@@ -417 +405 @@
     if (is & PORT_INT_PC) {
         TRACE("Port Connect Change\n");
         // Spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT
-
-        // perform a hard reset
-        if (!_HardReset())
-            return;
-
-        // clear error bits to clear PxSERR.DIAG.X
+        // Spec v1.3.1, §7.4 Interaction of command list and port change status
+        ResetPort(true);
         _ClearErrorRegister();
     }
     if (is & PORT_INT_UF) {
@@ -452 +436 @@
     status_t status = get_memory_map_etc(B_CURRENT_TEAM, data, dataSize,
         entries, &entriesUsed);
     if (status != B_OK) {
-        TRACE("AHCIPort::FillPrdTable get_memory_map() failed: %s\n",
-            strerror(status));
+        TRACE("%s: get_memory_map() failed: %s\n", __func__, strerror(status));
         return B_ERROR;
     }
 
@@ -474 +457 @@
         FLOW("FillPrdTable: sg-entry addr %#" B_PRIxPHYSADDR ", size %lu\n",
             address, size);
         if (address & 1) {
-            TRACE("AHCIPort::FillPrdTable: data alignment error\n");
+            TRACE("%s: data alignment error\n", __func__);
             return B_ERROR;
         }
         dataSize -= size;
@@ -500 +483 @@
         sgCount--;
     }
     if (*prdCount == 0) {
-        TRACE("AHCIPort::FillPrdTable: count is 0\n");
+        TRACE("%s: count is 0\n", __func__);
         return B_ERROR;
     }
     if (dataSize > 0) {
-        TRACE("AHCIPort::FillPrdTable: sg table %ld bytes too small\n",
-            dataSize);
+        TRACE("%s: sg table %ld bytes too small\n", __func__, dataSize);
         return B_ERROR;
     }
     return B_OK;
@@ -1322 +1304 @@
 
 
 bool
+AHCIPort::Enable()
+{
+    TRACE("%s: port %d\n", __func__, fIndex);
+    if ((fRegs->cmd & PORT_CMD_ST) != 0) {
+        TRACE("%s: Starting port already running!\n", __func__);
+        return false;
+    }
+    // Start port
+    fRegs->cmd |= PORT_CMD_ST;
+    FlushPostedWrites();
+    return true;
+}
+
+
+bool
+AHCIPort::Disable()
+{
+    TRACE("%s: port %d\n", __func__, fIndex);
+
+    if ((fRegs->cmd & PORT_CMD_ST) == 0) {
+        TRACE("%s: port %d attempting to disable stopped port.\n",
+            __func__, fIndex);
+    }
+
+    // Disable port
+    fRegs->cmd &= ~PORT_CMD_ST;
+    FlushPostedWrites();
+
+    // Clear DMA engine and wait for completion
+    if (wait_until_clear(&fRegs->cmd, PORT_CMD_CR, 500000) < B_OK) {
+        TRACE("%s: port %d error DMA engine still running\n", __func__,
+            fIndex);
+        return false;
+    }
+
+    return true;
+}
+
+
+bool
 AHCIPort::_HardReset()
 {
     if ((fRegs->cmd & PORT_CMD_ST) != 0) {
         // We shouldn't perform a reset, but at least document it
-        TRACE("AHCIPort::_HardReset() PORT_CMD_ST set, behaviour undefined\n");
+        TRACE("%s: port %d warning: ST set, bypassing hard reset\n", __func__,
+            fIndex);
         return false;
     }
 

src/add-ons/kernel/busses/scsi/ahci/ahci_port.h

@@ -50 +50 @@
     void        ResetDevice();
     status_t    ResetPort(bool forceDeviceReset = false);
     status_t    PostReset();
+    bool        Enable();
+    bool        Disable();
+
     void        FlushPostedWrites();
     void        DumpD2HFis();