Subject: Testers needed for revised Promise SATA IDE driver (patch included)
To: None <port-i386@netbsd.org, current-users@netbsd.org>
From: Brian Buhrow <buhrow@lothlorien.nfbcal.org>
List: current-users
Date: 05/02/2006 10:30:55
[...My appologies if this got to you twice.  I realized I'd sent from the
wrong mailbox, and I'm not sure if it got posted to the list.]

	Hello.  I've just finished up a revision of the Promise SATA driver
which fixes a number of problems:

O  Interrupt handling now works properly.  Interrupts were being lost
regularly, and once lost, the driver could not recover.

O  If a drive goes to sleep, either because of an atactl request, or
because the drive errors in some way, the driver can now recover.

O  Consolidated interrupt routines for all the supported cards.  

O  Installed a reset routine for all supported cards.

	I've been using this patched version with a Promise PDC40718 with 4
500GB disks attached.  I've got 2 cards in one machine with 4 drives each,
for a total of 8 drives running at once.  I'm using raidframe to hook 7 of
them together in a raid 5 configuration.  Before this patch, I could crash
the thing in 10 minutes with 2 concurrent writing processes.  Now, I've
gotten the interrupt rate up to 1200 interrupts/sec without a problem.

	A quick way to see if the driver you're using exhibits the old
problems is to do the following:

1.  Boot the system.

2.  As root run:
atactl /dev/some-drive-on-card sleep
disklabel -r same-drive

I.E. If wd3 is attached to your promise SATA card:
atactl /dev/rwd3d sleep
disklabel -r wd3

	I'm pretty sure the existing driver won't recover at all, and you'll
have to reboot to get the drive back.  The patched driver should complain
about some lost interrupts, and some drive timeouts, and then should get
things going again.

	I'm particularly interested in knowing if these changes work well with
the Promise 203XX SATA150 cards.  I think they should, as my changes are
based on what the Linux 2.6.16 driver does with the SATA150 cards, and I'm
using the SATA300 version.

	this diff is against the NetBSD-3.0 version of the pdcsata.c file, but
should be fairly easy to patch with the -current version as well.
If it works for people, I'll send-pr this diff and ask it to be pulled up
into the 3.x and 4.x branches.

	Thanks for any feedback in advance.
-Brian


Index: pdcsata.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pdcsata.c,v
retrieving revision 1.3.2.2
diff -u -r1.3.2.2 pdcsata.c
--- pdcsata.c	5 Feb 2006 17:13:57 -0000	1.3.2.2
+++ pdcsata.c	2 May 2006 16:47:22 -0000
@@ -48,17 +48,19 @@
 
 #define PDC203xx_BAR_IDEREGS 0x1c /* BAR where the IDE registers are mapped */
 
+#define PDC_CHANNELBASE(ch) 0x200 + ((ch) * 0x80)
+#define PDC_ERRMASK 0x00780700
+
 static void pdcsata_chip_map(struct pciide_softc *, struct pci_attach_args *);
 static void pdc203xx_setup_channel(struct ata_channel *);
-static int  pdc203xx_pci_intr(void *);
 static void pdc203xx_irqack(struct ata_channel *);
 static int  pdc203xx_dma_init(void *, int, int, void *, size_t, int);
 static void pdc203xx_dma_start(void *,int ,int);
 static int  pdc203xx_dma_finish(void *, int, int, int);
+static int  pdcsata_pci_intr(void *);
+static void pdcsata_do_reset(struct ata_channel *, int);
 
 /* PDC205xx, PDC405xx and PDC407xx. but tested only pdc40718 */
-static int  pdc205xx_pci_intr(void *);
-static void pdc205xx_do_reset(struct ata_channel *, int);
 static void pdc205xx_drv_probe(struct ata_channel *);
 
 static int  pdcsata_match(struct device *, struct cfdata *, void *);
@@ -183,30 +185,8 @@
 		return;
 	}
 	intrstr = pci_intr_string(pa->pa_pc, intrhandle);
-
-	switch (sc->sc_pp->ide_product) {
-	case PCI_PRODUCT_PROMISE_PDC20318:
-	case PCI_PRODUCT_PROMISE_PDC20319:
-	case PCI_PRODUCT_PROMISE_PDC20371:
-	case PCI_PRODUCT_PROMISE_PDC20375:
-	case PCI_PRODUCT_PROMISE_PDC20376:
-	case PCI_PRODUCT_PROMISE_PDC20377:
-	case PCI_PRODUCT_PROMISE_PDC20378:
-	case PCI_PRODUCT_PROMISE_PDC20379:
-	default:
-		sc->sc_pci_ih = pci_intr_establish(pa->pa_pc,
-		    intrhandle, IPL_BIO, pdc203xx_pci_intr, sc);
-		break;
-
-	case PCI_PRODUCT_PROMISE_PDC40718:
-	case PCI_PRODUCT_PROMISE_PDC40719:
-	case PCI_PRODUCT_PROMISE_PDC20571:
-	case PCI_PRODUCT_PROMISE_PDC20575:
-	case PCI_PRODUCT_PROMISE_PDC20579:
-		sc->sc_pci_ih = pci_intr_establish(pa->pa_pc,
-		    intrhandle, IPL_BIO, pdc205xx_pci_intr, sc);
-		break;
-	}
+	sc->sc_pci_ih = pci_intr_establish(pa->pa_pc,
+	    intrhandle, IPL_BIO, pdcsata_pci_intr, sc);
 
 	if (sc->sc_pci_ih == NULL) {
 		aprint_error("%s: couldn't establish native-PCI interrupt",
@@ -258,6 +238,8 @@
 	sc->sc_wdcdev.sc_atac.atac_set_modes = pdc203xx_setup_channel;
 	sc->sc_wdcdev.sc_atac.atac_channels = sc->wdc_chanarray;
 
+	sc->sc_wdcdev.reset = pdcsata_do_reset;
+
 	switch (sc->sc_pp->ide_product) {
 	case PCI_PRODUCT_PROMISE_PDC20318:
 	case PCI_PRODUCT_PROMISE_PDC20319:
@@ -281,7 +263,6 @@
 		bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x60, 0x00ff00ff);
 		sc->sc_wdcdev.sc_atac.atac_nchannels = PDC40718_NCHANNELS;
 
-		sc->sc_wdcdev.reset = pdc205xx_do_reset;
 		sc->sc_wdcdev.sc_atac.atac_probe = pdc205xx_drv_probe;
 
 		break;
@@ -290,7 +271,6 @@
 		bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x60, 0x00ff00ff);
 		sc->sc_wdcdev.sc_atac.atac_nchannels = PDC20575_NCHANNELS;
 
-		sc->sc_wdcdev.reset = pdc205xx_do_reset;
 		sc->sc_wdcdev.sc_atac.atac_probe = pdc205xx_drv_probe;
 
 		break;
@@ -403,53 +383,36 @@
 }
 
 static int
-pdc203xx_pci_intr(void *arg)
+pdcsata_pci_intr(void *arg)
 {
 	struct pciide_softc *sc = arg;
 	struct pciide_channel *cp;
 	struct ata_channel *wdc_cp;
 	int i, rv, crv;
-	u_int32_t scr;
-
-	rv = 0;
-	scr = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x00040);
-
-	for (i = 0; i < sc->sc_wdcdev.sc_atac.atac_nchannels; i++) {
-		cp = &sc->pciide_channels[i];
-		wdc_cp = &cp->ata_channel;
-		if (scr & (1 << (i + 1))) {
-			crv = wdcintr(wdc_cp);
-			if (crv == 0) {
-				printf("%s:%d: bogus intr (reg 0x%x)\n",
-				    sc->sc_wdcdev.sc_atac.atac_dev.dv_xname,
-				    i, scr);
-			} else
-				rv = 1;
-		}
-	}
-	return rv;
-}
-
-static int
-pdc205xx_pci_intr(void *arg)
-{
-	struct pciide_softc *sc = arg;
-	struct pciide_channel *cp;
-	struct ata_channel *wdc_cp;
-	int i, rv, crv;
-	u_int32_t scr, status;
+	u_int32_t scr, status, chanbase;
 
 	rv = 0;
 	scr = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x40);
+	if (scr == 0xffffffff) return(1);
 	bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x40, scr & 0x0000ffff);
-
-	status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x60);
-	bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, 0x60, status & 0x000000ff);
+	scr = scr & 0x0000ffff;
+	if (!scr) return(1);
 
 	for (i = 0; i < sc->sc_wdcdev.sc_atac.atac_nchannels; i++) {
 		cp = &sc->pciide_channels[i];
 		wdc_cp = &cp->ata_channel;
 		if (scr & (1 << (i + 1))) {
+			chanbase = PDC_CHANNELBASE(i) + 0x48;
+			status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase);
+			if (status & PDC_ERRMASK) {
+				chanbase = PDC_CHANNELBASE(i) + 0x60;
+				status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase);
+				status |= 0x800;
+				bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase, status);
+				status &= ~0x800;
+				bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase, status);
+				continue;
+			}
 			crv = wdcintr(wdc_cp);
 			if (crv == 0) {
 				printf("%s:%d: bogus intr (reg 0x%x)\n",
@@ -541,24 +504,29 @@
 
 
 static void
-pdc205xx_do_reset(struct ata_channel *chp, int poll)
+pdcsata_do_reset(struct ata_channel *chp, int poll)
 {
 	struct pciide_softc *sc = CHAN_TO_PCIIDE(chp);
-	u_int32_t scontrol;
-
-	wdc_do_reset(chp, poll);
+	int reset, status, i, chanbase;
 
 	/* reset SATA */
-	scontrol = SControl_DET_INIT | SControl_SPD_ANY | SControl_IPM_NONE;
-	SCONTROL_WRITE(sc, chp->ch_channel, scontrol);
-	delay(50*1000);
-
-	scontrol &= ~SControl_DET_INIT;
-	SCONTROL_WRITE(sc, chp->ch_channel, scontrol);
-	delay(50*1000);
-}
+	reset = (1 << 11);
+	chanbase = PDC_CHANNELBASE(chp->ch_channel) + 0x60;
+	for (i = 0; i < 11;i ++) {
+		status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase);
+		if (status & reset) break;
+		delay(100);
+		status |= reset;
+		bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase, status);
+	}
+	status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase);
+	status &= ~reset;
+	bus_space_write_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase, status);
+	status = bus_space_read_4(sc->sc_ba5_st, sc->sc_ba5_sh, chanbase);
 
+	wdc_do_reset(chp, poll);
 
+}
 
 static void
 pdc205xx_drv_probe(struct ata_channel *chp)