On Sat, Aug 24, 2002 at 11:14:01PM +0200, Manuel Bouyer wrote:
  | 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 applied this patch (after backing out the hacky one I had), and it
seems to work ok.  The drive only supports Ultra/100, but now it
doesn't complain and fall back to UDMA/33 :)

Luke.


  | 
  | I'll commit this as soon as the cvs server is reachable again.
  | 
  | -- 
  | Manuel Bouyer <bouyer@antioche.eu.org>
  | --

  | 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__)) =


-- 
Luke Mewburn  <lukem@wasabisystems.com>  http://www.wasabisystems.com
Luke Mewburn     <lukem@netbsd.org>      http://www.netbsd.org
Wasabi Systems - NetBSD hackers for hire
NetBSD - the world's most portable UNIX-like operating system