AHCI - Hotplug. Increase timeout for BSY check, FIS adjusts for softreset, CAM
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 6 Jun 2009 22:45:14 +0000 (15:45 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 6 Jun 2009 22:45:14 +0000 (15:45 -0700)
* Wait up to 3 seconds for BSY to clear.  1 second was not enough when
  powering up an external 2.5" drive already connected to the port.

* It is unclear how other fields in the FIS should be initialized when
  performing a soft reset.  Zero the fields instead of inheriting whatever
  junk was in the FIS from prior commands.  This seems to fix random
  errors from unplugging and plugging in a "My Book".

* Adjust the CAM devq to ensure that one ata_xfer remains available for error
  processing.  Do not clean out the devq when reducing the number of tags to
  1 as we will not use NCQ in this case anyway and need the extra CCB for
  error processing.

sys/dev/disk/ahci/ahci.c
sys/dev/disk/ahci/ahci.h
sys/dev/disk/ahci/ahci_cam.c
sys/dev/disk/ahci/atascsi.h

index 7789ddf..d52235c 100644 (file)
@@ -88,10 +88,18 @@ void        ahci_empty_done(struct ahci_ccb *ccb);
 void   ahci_ata_cmd_done(struct ahci_ccb *ccb);
 
 /* Wait for all bits in _b to be cleared */
-#define ahci_pwait_clr(_ap, _r, _b) ahci_pwait_eq((_ap), (_r), (_b), 0)
+#define ahci_pwait_clr(_ap, _r, _b) \
+       ahci_pwait_eq((_ap), AHCI_PWAIT_TIMEOUT, (_r), (_b), 0)
+#define ahci_pwait_clr_to(_ap, _to,  _r, _b) \
+       ahci_pwait_eq((_ap), _to, (_r), (_b), 0)
 
 /* Wait for all bits in _b to be set */
-#define ahci_pwait_set(_ap, _r, _b) ahci_pwait_eq((_ap), (_r), (_b), (_b))
+#define ahci_pwait_set(_ap, _r, _b) \
+       ahci_pwait_eq((_ap), AHCI_PWAIT_TIMEOUT, (_r), (_b), (_b))
+#define ahci_pwait_set_to(_ap, _to, _r, _b) \
+       ahci_pwait_eq((_ap), _to, (_r), (_b), (_b))
+
+#define AHCI_PWAIT_TIMEOUT     1000
 
 /*
  * Initialize the global AHCI hardware.  This code does not set up any of
@@ -607,12 +615,18 @@ ahci_port_softreset(struct ahci_port *ap)
                goto err;
        }
 
-       /* Prep first D2H command with SRST feature & clear busy/reset flags */
+       /*
+        * Prep first D2H command with SRST feature & clear busy/reset flags
+        *
+        * It is unclear which other fields in the FIS are used.  Just zero
+        * everything.
+        */
        ccb = ahci_get_err_ccb(ap);
        cmd_slot = ccb->ccb_cmd_hdr;
        bzero(ccb->ccb_cmd_table, sizeof(struct ahci_cmd_table));
 
        fis = ccb->ccb_cmd_table->cfis;
+       bzero(fis, sizeof(ccb->ccb_cmd_table->cfis));
        fis[0] = 0x27;  /* Host to device */
        fis[15] = 0x04; /* SRST DEVCTL */
 
@@ -623,10 +637,24 @@ ahci_port_softreset(struct ahci_port *ap)
        cmd_slot->flags |= htole16(AHCI_CMD_LIST_FLAG_W); /* Write */
 
        ccb->ccb_xa.state = ATA_S_PENDING;
-       if (ahci_poll(ccb, hz, NULL) != 0)
+       if (ahci_poll(ccb, hz, NULL) != 0) {
+               kprintf("First FIS failed\n");
                goto err;
+       }
 
-       /* Prep second D2H command to read status and complete reset sequence */
+       /*
+        * Prep second D2H command to read status and complete reset sequence
+        * AHCI 10.4.1 and "Serial ATA Revision 2.6".  I can't find the ATA
+        * Rev 2.6 and it is unclear how the second FIS should be set up
+        * from the AHCI document.
+        *
+        * Give the device 1/10 of a second before sending the second
+        * FIS.
+        *
+        * It is unclear which other fields in the FIS are used.  Just zero
+        * everything.
+        */
+       bzero(fis, sizeof(ccb->ccb_cmd_table->cfis));
        fis[0] = 0x27;  /* Host to device */
        fis[15] = 0;
 
@@ -635,8 +663,10 @@ ahci_port_softreset(struct ahci_port *ap)
        cmd_slot->flags |= htole16(AHCI_CMD_LIST_FLAG_W);
 
        ccb->ccb_xa.state = ATA_S_PENDING;
-       if (ahci_poll(ccb, hz, NULL) != 0)
+       if (ahci_poll(ccb, hz, NULL) != 0) {
+               kprintf("Second FIS failed\n");
                goto err;
+       }
 
        if (ahci_pwait_clr(ap, AHCI_PREG_TFD, AHCI_PREG_TFD_STS_BSY |
            AHCI_PREG_TFD_STS_DRQ | AHCI_PREG_TFD_STS_ERR)) {
@@ -728,7 +758,8 @@ ahci_port_portreset(struct ahci_port *ap)
        DELAY(10000);
 
        /* Wait for device to be detected and communications established */
-       if (ahci_pwait_eq(ap, AHCI_PREG_SSTS, AHCI_PREG_SSTS_DET,
+       if (ahci_pwait_eq(ap, 1000,
+                         AHCI_PREG_SSTS, AHCI_PREG_SSTS_DET,
                          AHCI_PREG_SSTS_DET_DEV)) {
                rc = ENODEV;
                goto err;
@@ -737,9 +768,13 @@ ahci_port_portreset(struct ahci_port *ap)
        /* Clear SERR (incl X bit), so TFD can update */
        ahci_pwrite(ap, AHCI_PREG_SERR, ahci_pread(ap, AHCI_PREG_SERR));
 
-       /* Wait for device to become ready */
-       /* XXX maybe more than the default wait is appropriate here? */
-       if (ahci_pwait_clr(ap, AHCI_PREG_TFD, AHCI_PREG_TFD_STS_BSY |
+       /*
+        * Wait for device to become ready
+        *
+        * This can take more then a second, give it 3 seconds.
+        */
+       if (ahci_pwait_clr_to(ap, 3000,
+                              AHCI_PREG_TFD, AHCI_PREG_TFD_STS_BSY |
                               AHCI_PREG_TFD_STS_DRQ | AHCI_PREG_TFD_STS_ERR)) {
                rc = EBUSY;
                kprintf("%s: Device will not come ready 0x%b\n",
@@ -1219,10 +1254,11 @@ ahci_port_intr(struct ahci_port *ap, u_int32_t ci_mask)
        if (is & (AHCI_PREG_IS_PCS | AHCI_PREG_IS_PRCS)) {
                ahci_pwrite(ap, AHCI_PREG_SERR,
                        (AHCI_PREG_SERR_DIAG_N | AHCI_PREG_SERR_DIAG_X) << 16);
+
                switch (ahci_pread(ap, AHCI_PREG_SSTS) & AHCI_PREG_SSTS_DET) {
                case AHCI_PREG_SSTS_DET_DEV:
                        if (ap->ap_ata.ap_type == ATA_PORT_T_NONE) {
-                               kprintf("%s: HOTPLUG - Device added\n",
+                               kprintf("%s: HOTPLUG - Device inserted\n",
                                        PORTNAME(ap));
                                if (ahci_port_init(ap) == 0)
                                        ahci_cam_changed(ap, 1);
@@ -1660,12 +1696,12 @@ ahci_pwrite(struct ahci_port *ap, bus_size_t r, u_int32_t v)
 }
 
 int
-ahci_pwait_eq(struct ahci_port *ap, bus_size_t r, u_int32_t mask,
-    u_int32_t target)
+ahci_pwait_eq(struct ahci_port *ap, int timeout,
+             bus_size_t r, u_int32_t mask, u_int32_t target)
 {
        int                             i;
 
-       for (i = 0; i < 1000; i++) {
+       for (i = 0; i < timeout; i++) {
                if ((ahci_pread(ap, r) & mask) == target)
                        return (0);
                DELAY(1000);
index 0e0d088..8bd6556 100644 (file)
@@ -455,7 +455,8 @@ void        ahci_write(struct ahci_softc *, bus_size_t, u_int32_t);
 int    ahci_wait_ne(struct ahci_softc *, bus_size_t, u_int32_t, u_int32_t);
 u_int32_t ahci_pread(struct ahci_port *, bus_size_t);
 void   ahci_pwrite(struct ahci_port *, bus_size_t, u_int32_t);
-int    ahci_pwait_eq(struct ahci_port *, bus_size_t, u_int32_t, u_int32_t);
+int    ahci_pwait_eq(struct ahci_port *, int, bus_size_t,
+                       u_int32_t, u_int32_t);
 void   ahci_intr(void *);
 
 int    ahci_cam_attach(struct ahci_port *ap);
index afbbe21..0053590 100644 (file)
@@ -122,8 +122,15 @@ ahci_cam_attach(struct ahci_port *ap)
        int error;
        int unit;
 
+       /*
+        * We want at least one ccb to be available for error processing
+        * so don't let CAM use more then ncmds - 1.
+        */
        unit = device_get_unit(ap->ap_sc->sc_dev);
-       devq = cam_simq_alloc(ap->ap_sc->sc_ncmds);
+       if (ap->ap_sc->sc_ncmds > 1)
+               devq = cam_simq_alloc(ap->ap_sc->sc_ncmds - 1);
+       else
+               devq = cam_simq_alloc(ap->ap_sc->sc_ncmds);
        if (devq == NULL) {
                return (ENOMEM);
        }
@@ -303,6 +310,9 @@ ahci_cam_probe(struct ahci_port *ap)
         * NCQ is not used if ap_ncqdepth is 1 or the host controller does
         * not support it, and in that case the driver can handle extra
         * ccb's.
+        *
+        * Remember at least one extra CCB needs to be reserved for the
+        * error ccb.
         */
        if ((ap->ap_sc->sc_cap & AHCI_REG_CAP_SNCQ) &&
            (le16toh(ap->ap_ata.ap_identify.satacap) & (1 << 8))) {
@@ -310,17 +320,18 @@ ahci_cam_probe(struct ahci_port *ap)
                devncqdepth = ap->ap_ata.ap_ncqdepth;
                if (ap->ap_ata.ap_ncqdepth > ap->ap_sc->sc_ncmds)
                        ap->ap_ata.ap_ncqdepth = ap->ap_sc->sc_ncmds;
-               for (i = 0; i < ap->ap_sc->sc_ncmds; ++i) {
-                       xa = ahci_ata_get_xfer(ap);
-                       if (xa->tag < ap->ap_ata.ap_ncqdepth) {
-                               xa->state = ATA_S_COMPLETE;
-                               ahci_ata_put_xfer(xa);
+               if (ap->ap_ata.ap_ncqdepth > 1) {
+                       for (i = 0; i < ap->ap_sc->sc_ncmds; ++i) {
+                               xa = ahci_ata_get_xfer(ap);
+                               if (xa->tag < ap->ap_ata.ap_ncqdepth) {
+                                       xa->state = ATA_S_COMPLETE;
+                                       ahci_ata_put_xfer(xa);
+                               }
+                       }
+                       if (ap->ap_ata.ap_ncqdepth >= ap->ap_sc->sc_ncmds) {
+                               cam_devq_resize(ap->ap_sim->devq,
+                                               ap->ap_ata.ap_ncqdepth - 1);
                        }
-               }
-               if (ap->ap_ata.ap_ncqdepth > 1 &&
-                   ap->ap_ata.ap_ncqdepth >= ap->ap_sc->sc_ncmds) {
-                       cam_devq_resize(ap->ap_sim->devq,
-                                       ap->ap_ata.ap_ncqdepth - 1);
                }
        } else {
                devncqdepth = 0;
index cee8742..5041b66 100644 (file)
@@ -174,6 +174,7 @@ struct ata_fis_h2d {
        u_int8_t                sector_count_exp;
        u_int8_t                reserved0;
        u_int8_t                control;
+#define ATA_FIS_CONTROL_SRST   0x04
 
        u_int8_t                reserved1;
        u_int8_t                reserved2;