Subject: Re: UltraDMA problems under 1.6F (probably hardware)
To: Matthias Drochner <M.Drochner@fz-juelich.de>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: current-users
Date: 08/24/2002 23:14:01
--OXfL5xGRrasGEqWY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Aug 14, 2002 at 03:48:34PM +0200, Matthias Drochner wrote:
> 
> I think I found the problem.
> The way the 8233A IDE controller is dealy with is wrong, it needs
> an own timing table.
> (the driver ties timing tables to UDMA capabilities, which is
> suboptimal)
> 
> The appended patch helps for me; it is a proof-of concept however
> which breaks all the UDMA100-only controllers.
> (The timing table is borrowed from FreeBSD.)
> The driver needs some cleanup to fix it correctly.
> (The UDMA66 stuff looks strange too... but I won't argue without
> a manual.)

Hi,
can you try the attached patch ? The timing values comme from FreeBSD,
confirmed from linux-2.4.19. This will also add support for Ultra/133
in the generic wdc part (trivial).
It's not usefull to set the 4 upper bits in the timing table, they're
masked by the APO_UDMA_TIME() macro. Some of these bits are set by other
macros.
BTW, this allowed me to find a bug in the UDMA 100 settings too (CLK66
shouldn't be set), but the 66 timmings looks fine :)

I'll commit this as soon as the cvs server is reachable again.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
--

--OXfL5xGRrasGEqWY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: ic/wdc.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/wdc.c,v
retrieving revision 1.116
diff -u -r1.116 wdc.c
--- wdc.c	2002/07/26 14:10:22	1.116
+++ wdc.c	2002/08/24 21:12:17
@@ -1148,6 +1148,8 @@
 						printf(" (Ultra/66)");
 					else if (i == 5)
 						printf(" (Ultra/100)");
+					else if (i == 6)
+						printf(" (Ultra/133)");
 					sep = ",";
 					printed = 1;
 				}
Index: pci/pciide.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/pciide.c,v
retrieving revision 1.166
diff -u -r1.166 pciide.c
--- pciide.c	2002/08/23 16:24:54	1.166
+++ pciide.c	2002/08/24 21:12:29
@@ -2216,8 +2216,7 @@
 		break;
 	case PCI_PRODUCT_VIATECH_VT8233A:
 		printf("VT8233A ATA133 controller\n");
-		/* XXX use ATA100 untill ATA133 is supported */
-		sc->sc_wdcdev.UDMA_cap = 5;
+		sc->sc_wdcdev.UDMA_cap = 6;
 		break;
 	default:
 		printf("unknown ATA controller\n");
@@ -2318,9 +2317,12 @@
 			drvp->drive_flags &= ~DRIVE_DMA;
 			udmatim_reg |= APO_UDMA_EN(chp->channel, drive) |
 			    APO_UDMA_EN_MTH(chp->channel, drive);
-			if (sc->sc_wdcdev.UDMA_cap == 5) {
+			if (sc->sc_wdcdev.UDMA_cap == 6) {
+				/* 8233a */
+				udmatim_reg |= APO_UDMA_TIME(chp->channel,
+				    drive, apollo_udma133_tim[drvp->UDMA_mode]);
+			} else if (sc->sc_wdcdev.UDMA_cap == 5) {
 				/* 686b */
-				udmatim_reg |= APO_UDMA_CLK66(chp->channel);
 				udmatim_reg |= APO_UDMA_TIME(chp->channel,
 				    drive, apollo_udma100_tim[drvp->UDMA_mode]);
 			} else if (sc->sc_wdcdev.UDMA_cap == 4) {
Index: pci/pciide_apollo_reg.h
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/pciide_apollo_reg.h,v
retrieving revision 1.11
diff -u -r1.11 pciide_apollo_reg.h
--- pciide_apollo_reg.h	2002/04/23 20:41:17	1.11
+++ pciide_apollo_reg.h	2002/08/24 21:12:30
@@ -89,9 +89,10 @@
 	(((1 - (channel)) << 4) + ((1 - (drive)) << 3)))
 #define APO_UDMA_CLK66(channel) (0x08 << ((1 - (channel)) << 4))
 
+static const int8_t apollo_udma133_tim[] __attribute__((__unused__)) =
+    {0x07, 0x07, 0x06, 0x04, 0x02, 0x01, 0x00};
 static const int8_t apollo_udma100_tim[] __attribute__((__unused__)) =
-    /* XXX Check modes other than 2, 4, 5 */
-    {0x0f, 0x07, 0x04, 0x02, 0x01, 0x00};
+    {0x07, 0x07, 0x04, 0x02, 0x01, 0x00};
 static const int8_t apollo_udma66_tim[] __attribute__((__unused__)) =
     {0x03, 0x03, 0x02, 0x01, 0x00};
 static const int8_t apollo_udma33_tim[] __attribute__((__unused__)) =

--OXfL5xGRrasGEqWY--