Ticket #12295: ahci.patch

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

@@ -76 +76 @@
     INT_DHR         = (1 << 0),     // Device to Host Register FIS Interrupt/Enable
 };
 
+/*
+ * From Spec v1.3, §3.3.11
+ */
+typedef struct {
+    uint16 reserved:12; // Reserved
+    uint8 pmp:4; // Port Multiplier Port (PMP) : This field is not used by AHCI.
+    uint8 spm:4; // Select Power Management (SPM) : This field is not used by AHCI
+    uint8 ipm:4; // Interface Power Management Trnasitions Allowed (IPM)
+    uint8 spd:4; // Speed Allowed (SPD)
+    uint8 det:4; //Device Detection Initialization (DET)
+} _PACKED scontrol;
+
+#define TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED 3
+#define NO_INITIALIZATION 0
+#define INITIALIZATION 1
+
 
 typedef struct {
     uint32      clb;            // Command List Base Address (alignment 1024 byte)
@@ -89 +105 @@
     uint32      tfd;            // Task File Data
     uint32      sig;            // Signature
     uint32      ssts;           // Serial ATA Status (SCR0: SStatus)
-    uint32      sctl;           // Serial ATA Control (SCR2: SControl)
+    scontrol    sctl;       // Serial ATA Control (SCR2: SControl)
     uint32      serr;           // Serial ATA Error (SCR1: SError) **RWC**
     uint32      sact;           // Serial ATA Active (SCR3: SActive) **RW1**
     uint32      ci;             // Command Issue **RW1**

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

@@ -110 +110 @@
     // prdt follows after command table
 
     // disable transitions to partial or slumber state
-    fRegs->sctl |= 0x300;
+    fRegs->sctl.ipm |= TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED; /*TODO Why "|= and not "=" ??*/
 
     // clear IRQ status bits
     fRegs->is = fRegs->is;
@@ -156 +156 @@
     TRACE("is   0x%08" B_PRIx32 "\n", fRegs->is);
     TRACE("cmd  0x%08" B_PRIx32 "\n", fRegs->cmd);
     TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts);
-    TRACE("sctl 0x%08" B_PRIx32 "\n", fRegs->sctl);
+    TRACE("sctl.reserved 0x%04" B_PRIx16 "\n", fRegs->sctl.reserved);
+    TRACE("sctl.pmp 0x%02" B_PRIx8 "\n", fRegs->sctl.pmp);
+    TRACE("sctl.spm 0x%02" B_PRIx8 "\n", fRegs->sctl.spm);
+    TRACE("sctl.ipm 0x%02" B_PRIx8 "\n", fRegs->sctl.ipm);
+    TRACE("sctl.spd 0x%02" B_PRIx8 "\n", fRegs->sctl.spd);
+    TRACE("sctl.det 0x%02" B_PRIx8 "\n", fRegs->sctl.det);
     TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr);
     TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact);
     TRACE("tfd  0x%08" B_PRIx32 "\n", fRegs->tfd);
@@ -212 +217 @@
         TRACE("AHCIPort::ResetDevice PORT_CMD_ST set, behaviour undefined\n");
 
     // perform a hard reset
-    fRegs->sctl = (fRegs->sctl & ~0xf) | 1;
+    fRegs->sctl.det |= INITIALIZATION; //TODO Why "|=" instead of "=" ?
     FlushPostedWrites();
     spin(1100);
-    fRegs->sctl &= ~0xf;
+    fRegs->sctl.det = NO_INITIALIZATION;
     FlushPostedWrites();
 
     if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK) {
@@ -364 +369 @@
         TRACE("AHCIPort::InterruptErrorHandler port %d, fCommandsActive 0x%08"
             B_PRIx32 ", is 0x%08" B_PRIx32 ", ci 0x%08" B_PRIx32 "\n", fIndex,
             fCommandsActive, is, ci);
-
-        TRACE("ssts 0x%08" B_PRIx32 ", sctl 0x%08" B_PRIx32 ", serr 0x%08"
-            B_PRIx32 ", sact 0x%08" B_PRIx32 "\n",
-            fRegs->ssts, fRegs->sctl, fRegs->serr, fRegs->sact);
+        TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts);
+        TRACE("sctl.reserved 0x%04" B_PRIx16 "\n", fRegs->sctl.reserved);
+        TRACE("sctl.pmp 0x%02" B_PRIx8 "\n", fRegs->sctl.pmp);
+        TRACE("sctl.spm 0x%02" B_PRIx8 "\n", fRegs->sctl.spm);
+        TRACE("sctl.ipm 0x%02" B_PRIx8 "\n", fRegs->sctl.ipm);
+        TRACE("sctl.spd 0x%02" B_PRIx8 "\n", fRegs->sctl.spd);
+        TRACE("sctl.det 0x%02" B_PRIx8 "\n", fRegs->sctl.det);
+        TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr);
+        TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact);
     }
 
     // read and clear SError
@@ -411 +421 @@
         TRACE("PhyReady Change\n");
 //      fResetPort = true;
     }
+    //As we are in the InterruptErrorHandler, the §6.2.2.3 of the SATA specification v1.3 applies
     if (is & PORT_INT_PC) {
         TRACE("Port Connect Change\n");
+        /* 6.2.2.3 Recovery of Unsolicited COMINIT
+        An unsolicited COMINIT is a COMINIT that is not received as a consequence of issuing a COMRESET to
+        the device (refer to Serial ATA revision 2.6). If the HBA receives an unsolicited COMINIT during normal
+        operation, the HBA shall perform the following actions;
+         - Respond to the device with a COMRESET
+         - Halt execution until PxIS.PCS is cleared to '0' by software
+
+         To detect this condition, software should check whether PxIS.PCS is set to '1' on an interrupt. The HBA
+         cannot guarantee that a device received a COMRESET because a COMINIT may appear to be solicited
+         to the HBA if it happens to occur closely to an issued COMRESET. Therefore, when software detects
+         that PxIS.PCS is set, software should first issue a COMRESET to ensure that the device receives a
+         COMRESET. Then software should perform the appropriate actions to clear PxIC.PCS to '0'. To recover,
+         software should perform error recovery actions for a fatal error condition (including restarting the
+         controller). Then software should perform a re-enumeration to check whether a new device has been inserted.
+        */
 //      fResetPort = true;
     }
     if (is & PORT_INT_UF) {