All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pata_hpt37x: Updates from drivers/ide work
@ 2007-03-08 23:28 Alan Cox
  2007-03-09 14:17 ` Jeff Garzik
  2007-03-09 15:03 ` Sergei Shtylyov
  0 siblings, 2 replies; 4+ messages in thread
From: Alan Cox @ 2007-03-08 23:28 UTC (permalink / raw)
  To: akpm, linux-ide, jgarzik

Drag pata_hpt37x kicking and screaming in the direction of
drivers/ide/pci/hpt366.c and all the work that Sergei has been doing
there. Plenty left to be done but this is a good snapshot for folks to
work on and to review

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c linux-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c
--- linux.vanilla-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c	2007-03-08 16:01:10.000000000 +0000
+++ linux-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c	2007-03-08 17:08:53.000000000 +0000
@@ -8,6 +8,7 @@
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
  * Portions Copyright (C) 2003		Red Hat Inc
+ * Portions Copyright (C) 2005-2006	MontaVista Software, Inc.
  *
  * TODO
  *	PLL mode
@@ -25,7 +26,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"pata_hpt37x"
-#define DRV_VERSION	"0.6.0"
+#define DRV_VERSION	"0.6.4"
 
 struct hpt_clock {
 	u8	xfer_speed;
@@ -61,201 +62,75 @@
  * 31     FIFO enable.
  */
 
-/* from highpoint documentation. these are old values */
-static const struct hpt_clock hpt370_timings_33[] = {
-/*	{	XFER_UDMA_5,	0x1A85F442,	0x16454e31	}, */
-	{	XFER_UDMA_5,	0x16454e31	},
-	{	XFER_UDMA_4,	0x16454e31	},
-	{	XFER_UDMA_3,	0x166d4e31	},
-	{	XFER_UDMA_2,	0x16494e31	},
-	{	XFER_UDMA_1,	0x164d4e31	},
-	{	XFER_UDMA_0,	0x16514e31	},
-
-	{	XFER_MW_DMA_2,	0x26514e21	},
-	{	XFER_MW_DMA_1,	0x26514e33	},
-	{	XFER_MW_DMA_0,	0x26514e97	},
-
-	{	XFER_PIO_4,	0x06514e21	},
-	{	XFER_PIO_3,	0x06514e22	},
-	{	XFER_PIO_2,	0x06514e33	},
-	{	XFER_PIO_1,	0x06914e43	},
-	{	XFER_PIO_0,	0x06914e57	},
-	{	0,		0x06514e57	}
-};
-
-static const struct hpt_clock hpt370_timings_66[] = {
-	{	XFER_UDMA_5,	0x14846231	},
-	{	XFER_UDMA_4,	0x14886231	},
-	{	XFER_UDMA_3,	0x148c6231	},
-	{	XFER_UDMA_2,	0x148c6231	},
-	{	XFER_UDMA_1,	0x14906231	},
-	{	XFER_UDMA_0,	0x14986231	},
-
-	{	XFER_MW_DMA_2,	0x26514e21	},
-	{	XFER_MW_DMA_1,	0x26514e33	},
-	{	XFER_MW_DMA_0,	0x26514e97	},
-
-	{	XFER_PIO_4,	0x06514e21	},
-	{	XFER_PIO_3,	0x06514e22	},
-	{	XFER_PIO_2,	0x06514e33	},
-	{	XFER_PIO_1,	0x06914e43	},
-	{	XFER_PIO_0,	0x06914e57	},
-	{	0,		0x06514e57	}
-};
-
-/* these are the current (4 sep 2001) timings from highpoint */
-static const struct hpt_clock hpt370a_timings_33[] = {
-	{	XFER_UDMA_5,	0x12446231	},
-	{	XFER_UDMA_4,	0x12446231	},
-	{	XFER_UDMA_3,	0x126c6231	},
-	{	XFER_UDMA_2,	0x12486231	},
-	{	XFER_UDMA_1,	0x124c6233	},
-	{	XFER_UDMA_0,	0x12506297	},
-
-	{	XFER_MW_DMA_2,	0x22406c31	},
-	{	XFER_MW_DMA_1,	0x22406c33	},
-	{	XFER_MW_DMA_0,	0x22406c97	},
-
-	{	XFER_PIO_4,	0x06414e31	},
-	{	XFER_PIO_3,	0x06414e42	},
-	{	XFER_PIO_2,	0x06414e53	},
-	{	XFER_PIO_1,	0x06814e93	},
-	{	XFER_PIO_0,	0x06814ea7	},
-	{	0,		0x06814ea7	}
-};
-
-/* 2x 33MHz timings */
-static const struct hpt_clock hpt370a_timings_66[] = {
-	{	XFER_UDMA_5,	0x1488e673	},
-	{	XFER_UDMA_4,	0x1488e673	},
-	{	XFER_UDMA_3,	0x1498e673	},
-	{	XFER_UDMA_2,	0x1490e673	},
-	{	XFER_UDMA_1,	0x1498e677	},
-	{	XFER_UDMA_0,	0x14a0e73f	},
-
-	{	XFER_MW_DMA_2,	0x2480fa73	},
-	{	XFER_MW_DMA_1,	0x2480fa77	},
-	{	XFER_MW_DMA_0,	0x2480fb3f	},
-
-	{	XFER_PIO_4,	0x0c82be73	},
-	{	XFER_PIO_3,	0x0c82be95	},
-	{	XFER_PIO_2,	0x0c82beb7	},
-	{	XFER_PIO_1,	0x0d02bf37	},
-	{	XFER_PIO_0,	0x0d02bf5f	},
-	{	0,		0x0d02bf5f	}
-};
-
-static const struct hpt_clock hpt370a_timings_50[] = {
-	{	XFER_UDMA_5,	0x12848242	},
-	{	XFER_UDMA_4,	0x12ac8242	},
-	{	XFER_UDMA_3,	0x128c8242	},
-	{	XFER_UDMA_2,	0x120c8242	},
-	{	XFER_UDMA_1,	0x12148254	},
-	{	XFER_UDMA_0,	0x121882ea	},
-
-	{	XFER_MW_DMA_2,	0x22808242	},
-	{	XFER_MW_DMA_1,	0x22808254	},
-	{	XFER_MW_DMA_0,	0x228082ea	},
-
-	{	XFER_PIO_4,	0x0a81f442	},
-	{	XFER_PIO_3,	0x0a81f443	},
-	{	XFER_PIO_2,	0x0a81f454	},
-	{	XFER_PIO_1,	0x0ac1f465	},
-	{	XFER_PIO_0,	0x0ac1f48a	},
-	{	0,		0x0ac1f48a	}
-};
-
-static const struct hpt_clock hpt372_timings_33[] = {
-	{	XFER_UDMA_6,	0x1c81dc62	},
-	{	XFER_UDMA_5,	0x1c6ddc62	},
-	{	XFER_UDMA_4,	0x1c8ddc62	},
-	{	XFER_UDMA_3,	0x1c8edc62	},	/* checkme */
-	{	XFER_UDMA_2,	0x1c91dc62	},
-	{	XFER_UDMA_1,	0x1c9adc62	},	/* checkme */
-	{	XFER_UDMA_0,	0x1c82dc62	},	/* checkme */
-
-	{	XFER_MW_DMA_2,	0x2c829262	},
-	{	XFER_MW_DMA_1,	0x2c829266	},	/* checkme */
-	{	XFER_MW_DMA_0,	0x2c82922e	},	/* checkme */
-
-	{	XFER_PIO_4,	0x0c829c62	},
-	{	XFER_PIO_3,	0x0c829c84	},
-	{	XFER_PIO_2,	0x0c829ca6	},
-	{	XFER_PIO_1,	0x0d029d26	},
-	{	XFER_PIO_0,	0x0d029d5e	},
-	{	0,		0x0d029d5e	}
-};
-
-static const struct hpt_clock hpt372_timings_50[] = {
-	{	XFER_UDMA_5,	0x12848242	},
-	{	XFER_UDMA_4,	0x12ac8242	},
-	{	XFER_UDMA_3,	0x128c8242	},
-	{	XFER_UDMA_2,	0x120c8242	},
-	{	XFER_UDMA_1,	0x12148254	},
-	{	XFER_UDMA_0,	0x121882ea	},
-
-	{	XFER_MW_DMA_2,	0x22808242	},
-	{	XFER_MW_DMA_1,	0x22808254	},
-	{	XFER_MW_DMA_0,	0x228082ea	},
-
-	{	XFER_PIO_4,	0x0a81f442	},
-	{	XFER_PIO_3,	0x0a81f443	},
-	{	XFER_PIO_2,	0x0a81f454	},
-	{	XFER_PIO_1,	0x0ac1f465	},
-	{	XFER_PIO_0,	0x0ac1f48a	},
-	{	0,		0x0a81f443	}
-};
-
-static const struct hpt_clock hpt372_timings_66[] = {
-	{	XFER_UDMA_6,	0x1c869c62	},
-	{	XFER_UDMA_5,	0x1cae9c62	},
-	{	XFER_UDMA_4,	0x1c8a9c62	},
-	{	XFER_UDMA_3,	0x1c8e9c62	},
-	{	XFER_UDMA_2,	0x1c929c62	},
-	{	XFER_UDMA_1,	0x1c9a9c62	},
-	{	XFER_UDMA_0,	0x1c829c62	},
-
-	{	XFER_MW_DMA_2,	0x2c829c62	},
-	{	XFER_MW_DMA_1,	0x2c829c66	},
-	{	XFER_MW_DMA_0,	0x2c829d2e	},
-
-	{	XFER_PIO_4,	0x0c829c62	},
-	{	XFER_PIO_3,	0x0c829c84	},
-	{	XFER_PIO_2,	0x0c829ca6	},
-	{	XFER_PIO_1,	0x0d029d26	},
-	{	XFER_PIO_0,	0x0d029d5e	},
-	{	0,		0x0d029d26	}
-};
-
-static const struct hpt_clock hpt374_timings_33[] = {
-	{	XFER_UDMA_6,	0x12808242	},
-	{	XFER_UDMA_5,	0x12848242	},
-	{	XFER_UDMA_4,	0x12ac8242	},
-	{	XFER_UDMA_3,	0x128c8242	},
-	{	XFER_UDMA_2,	0x120c8242	},
-	{	XFER_UDMA_1,	0x12148254	},
-	{	XFER_UDMA_0,	0x121882ea	},
-
-	{	XFER_MW_DMA_2,	0x22808242	},
-	{	XFER_MW_DMA_1,	0x22808254	},
-	{	XFER_MW_DMA_0,	0x228082ea	},
-
-	{	XFER_PIO_4,	0x0a81f442	},
-	{	XFER_PIO_3,	0x0a81f443	},
-	{	XFER_PIO_2,	0x0a81f454	},
-	{	XFER_PIO_1,	0x0ac1f465	},
-	{	XFER_PIO_0,	0x0ac1f48a	},
-	{	0,		0x06814e93	}
+static struct hpt_clock hpt37x_timings_33[] = {
+	{ XFER_UDMA_6,		0x12446231 },	/* 0x12646231 ?? */
+	{ XFER_UDMA_5,		0x12446231 },
+	{ XFER_UDMA_4,		0x12446231 },
+	{ XFER_UDMA_3,		0x126c6231 },
+	{ XFER_UDMA_2,		0x12486231 },
+	{ XFER_UDMA_1,		0x124c6233 },
+	{ XFER_UDMA_0,		0x12506297 },
+
+	{ XFER_MW_DMA_2,	0x22406c31 },
+	{ XFER_MW_DMA_1,	0x22406c33 },
+	{ XFER_MW_DMA_0,	0x22406c97 },
+
+	{ XFER_PIO_4,		0x06414e31 },
+	{ XFER_PIO_3,		0x06414e42 },
+	{ XFER_PIO_2,		0x06414e53 },
+	{ XFER_PIO_1,		0x06814e93 },
+	{ XFER_PIO_0,		0x06814ea7 }
+};
+
+static struct hpt_clock hpt37x_timings_50[] = {
+	{ XFER_UDMA_6,		0x12848242 },
+	{ XFER_UDMA_5,		0x12848242 },
+	{ XFER_UDMA_4,		0x12ac8242 },
+	{ XFER_UDMA_3,		0x128c8242 },
+	{ XFER_UDMA_2,		0x120c8242 },
+	{ XFER_UDMA_1,		0x12148254 },
+	{ XFER_UDMA_0,		0x121882ea },
+
+	{ XFER_MW_DMA_2,	0x22808242 },
+	{ XFER_MW_DMA_1,	0x22808254 },
+	{ XFER_MW_DMA_0,	0x228082ea },
+
+	{ XFER_PIO_4,		0x0a81f442 },
+	{ XFER_PIO_3,		0x0a81f443 },
+	{ XFER_PIO_2,		0x0a81f454 },
+	{ XFER_PIO_1,		0x0ac1f465 },
+	{ XFER_PIO_0,		0x0ac1f48a }
+};
+
+static struct hpt_clock hpt37x_timings_66[] = {
+	{ XFER_UDMA_6,		0x1c869c62 },
+	{ XFER_UDMA_5,		0x1cae9c62 },	/* 0x1c8a9c62 */
+	{ XFER_UDMA_4,		0x1c8a9c62 },
+	{ XFER_UDMA_3,		0x1c8e9c62 },
+	{ XFER_UDMA_2,		0x1c929c62 },
+	{ XFER_UDMA_1,		0x1c9a9c62 },
+	{ XFER_UDMA_0,		0x1c829c62 },
+
+	{ XFER_MW_DMA_2,	0x2c829c62 },
+	{ XFER_MW_DMA_1,	0x2c829c66 },
+	{ XFER_MW_DMA_0,	0x2c829d2e },
+
+	{ XFER_PIO_4,		0x0c829c62 },
+	{ XFER_PIO_3,		0x0c829c84 },
+	{ XFER_PIO_2,		0x0c829ca6 },
+	{ XFER_PIO_1,		0x0d029d26 },
+	{ XFER_PIO_0,		0x0d029d5e }
 };
 
+
 static const struct hpt_chip hpt370 = {
 	"HPT370",
 	48,
 	{
-		hpt370_timings_33,
+		hpt37x_timings_33,
 		NULL,
 		NULL,
-		hpt370_timings_66
+		hpt37x_timings_66
 	}
 };
 
@@ -263,10 +138,10 @@
 	"HPT370A",
 	48,
 	{
-		hpt370a_timings_33,
+		hpt37x_timings_33,
 		NULL,
-		hpt370a_timings_50,
-		hpt370a_timings_66
+		hpt37x_timings_50,
+		hpt37x_timings_66
 	}
 };
 
@@ -274,10 +149,10 @@
 	"HPT372",
 	55,
 	{
-		hpt372_timings_33,
+		hpt37x_timings_33,
 		NULL,
-		hpt372_timings_50,
-		hpt372_timings_66
+		hpt37x_timings_50,
+		hpt37x_timings_66
 	}
 };
 
@@ -285,10 +160,10 @@
 	"HPT302",
 	66,
 	{
-		hpt372_timings_33,
+		hpt37x_timings_33,
 		NULL,
-		hpt372_timings_50,
-		hpt372_timings_66
+		hpt37x_timings_50,
+		hpt37x_timings_66
 	}
 };
 
@@ -296,10 +171,10 @@
 	"HPT371",
 	66,
 	{
-		hpt372_timings_33,
+		hpt37x_timings_33,
 		NULL,
-		hpt372_timings_50,
-		hpt372_timings_66
+		hpt37x_timings_50,
+		hpt37x_timings_66
 	}
 };
 
@@ -307,10 +182,10 @@
 	"HPT372A",
 	66,
 	{
-		hpt372_timings_33,
+		hpt37x_timings_33,
 		NULL,
-		hpt372_timings_50,
-		hpt372_timings_66
+		hpt37x_timings_50,
+		hpt37x_timings_66
 	}
 };
 
@@ -318,7 +193,7 @@
 	"HPT374",
 	48,
 	{
-		hpt374_timings_33,
+		hpt37x_timings_33,
 		NULL,
 		NULL,
 		NULL
@@ -463,8 +336,7 @@
 		ap->cbl = ATA_CBL_PATA80;
 
 	/* Reset the state machine */
-	pci_write_config_byte(pdev, 0x50, 0x37);
-	pci_write_config_byte(pdev, 0x54, 0x37);
+	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
 	udelay(100);
 
 	return ata_std_prereset(ap, deadline);
@@ -514,8 +386,7 @@
 		ap->cbl = ATA_CBL_PATA80;
 
 	/* Reset the state machine */
-	pci_write_config_byte(pdev, 0x50, 0x37);
-	pci_write_config_byte(pdev, 0x54, 0x37);
+	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
 	udelay(100);
 
 	return ata_std_prereset(ap, deadline);
@@ -1033,6 +904,24 @@
 		.udma_mask = 0x3f,
 		.port_ops = &hpt370a_port_ops
 	};
+	/* HPT370 - UDMA100 */
+	static struct ata_port_info info_hpt370_33 = {
+		.sht = &hpt37x_sht,
+		.flags = ATA_FLAG_SLAVE_POSS|ATA_FLAG_SRST,
+		.pio_mask = 0x1f,
+		.mwdma_mask = 0x07,
+		.udma_mask = 0x0f,
+		.port_ops = &hpt370_port_ops
+	};
+	/* HPT370A - UDMA100 */
+	static struct ata_port_info info_hpt370a_33 = {
+		.sht = &hpt37x_sht,
+		.flags = ATA_FLAG_SLAVE_POSS|ATA_FLAG_SRST,
+		.pio_mask = 0x1f,
+		.mwdma_mask = 0x07,
+		.udma_mask = 0x0f,
+		.port_ops = &hpt370a_port_ops
+	};
 	/* HPT371, 372 and friends - UDMA133 */
 	static struct ata_port_info info_hpt372 = {
 		.sht = &hpt37x_sht,
@@ -1068,7 +957,11 @@
 
 	u8 irqmask;
 	u32 class_rev;
+	u8 mcr1;
 	u32 freq;
+	int prefer_dpll = 1;
+	
+	unsigned long iobase = pci_resource_start(dev, 4);
 
 	const struct hpt_chip *chip_table;
 	int clock_slot;
@@ -1089,10 +982,12 @@
 			case 3:
 				port = &info_hpt370;
 				chip_table = &hpt370;
+				prefer_dpll = 0;
 				break;
 			case 4:
 				port = &info_hpt370a;
 				chip_table = &hpt370a;
+				prefer_dpll = 0;
 				break;
 			case 5:
 				port = &info_hpt372;
@@ -1120,8 +1015,16 @@
 				chip_table = &hpt302;
 				break;
 			case PCI_DEVICE_ID_TTI_HPT371:
+				if (class_rev > 1)
+					return -ENODEV;
 				port = &info_hpt372;
 				chip_table = &hpt371;
+				/* Single channel device, paster is not present
+				   but the NIOS (or us for non x86) must mark it
+				   absent */
+				pci_read_config_byte(dev, 0x50, &mcr1);
+				mcr1 &= ~0x04;
+				pci_write_config_byte(dev, 0x50, mcr1);
 				break;
 			case PCI_DEVICE_ID_TTI_HPT374:
 				chip_table = &hpt374;
@@ -1151,8 +1054,18 @@
 	 */
 
 	pci_write_config_byte(dev, 0x5b, 0x23);
+	
+	/*
+	 * HighPoint does this for HPT372A.
+	 * NOTE: This register is only writeable via I/O space.
+	 */
+	if (chip_table == &hpt372a)
+		outb(0x0e, iobase + 0x9c);
 
-	pci_read_config_dword(dev, 0x70, &freq);
+	/* Some devices do not let this value be accessed via PCI space
+	   according to the old driver */
+
+	freq = inl(iobase + 0x90);
 	if ((freq >> 12) != 0xABCDE) {
 		int i;
 		u8 sr;
@@ -1163,7 +1076,7 @@
 		/* This is the process the HPT371 BIOS is reported to use */
 		for(i = 0; i < 128; i++) {
 			pci_read_config_byte(dev, 0x78, &sr);
-			total += sr;
+			total += sr & 0x1FF;
 			udelay(15);
 		}
 		freq = total / 128;
@@ -1174,15 +1087,27 @@
 	 *	Turn the frequency check into a band and then find a timing
 	 *	table to match it.
 	 */
-
+	 
 	clock_slot = hpt37x_clock_slot(freq, chip_table->base);
-	if (chip_table->clocks[clock_slot] == NULL) {
+	if (chip_table->clocks[clock_slot] == NULL || prefer_dpll) {
 		/*
 		 *	We need to try PLL mode instead
+		 *
+		 *	For non UDMA133 capable devices we should
+		 *	use a 50MHz DPLL by choice
 		 */
-		unsigned int f_low = (MHz[clock_slot] * chip_table->base) / 192;
-		unsigned int f_high = f_low + 2;
+		unsigned int f_low, f_high;
 		int adjust;
+		
+		clock_slot = 2;
+		if (port->udma_mask & 0xE0)
+			clock_slot = 3;
+		
+		f_low = (MHz[clock_slot] * chip_table->base) / 192;
+		f_high = f_low + 2;
+
+		/* Select the DPLL clock. */
+		pci_write_config_byte(dev, 0x5b, 0x21);
 
 		for(adjust = 0; adjust < 8; adjust++) {
 			if (hpt37x_calibrate_dpll(dev))
@@ -1198,15 +1123,17 @@
 			printk(KERN_WARNING "hpt37x: DPLL did not stabilize.\n");
 			return -ENODEV;
 		}
-		/* Check if this works for all cases */
-		port->private_data = (void *)hpt370_timings_66;
+		if (clock_slot == 3)
+			port->private_data = (void *)hpt37x_timings_66;
+		else
+			port->private_data = (void *)hpt37x_timings_50;
 
 		printk(KERN_INFO "hpt37x: Bus clock %dMHz, using DPLL.\n", MHz[clock_slot]);
 	} else {
 		port->private_data = (void *)chip_table->clocks[clock_slot];
 		/*
 		 *	Perform a final fixup. The 371 and 372 clock determines
-		 *	if UDMA133 is available.
+		 *	if UDMA133 is available. (FIXME: should we use DPLL then ?)
 		 */
 
 		if (clock_slot == 2 && chip_table == &hpt372) {	/* 50Mhz */
@@ -1215,8 +1142,13 @@
 				port = &info_hpt372_50;
 			else BUG();
 		}
+		if (clock_slot < 2 && port == &info_hpt370)
+			port = &info_hpt370_33;
+		if (clock_slot < 2 && port == &info_hpt370a)
+			port = &info_hpt370a_33;
 		printk(KERN_INFO "hpt37x: %s: Bus clock %dMHz.\n", chip_table->name, MHz[clock_slot]);
-	}
+	}
+
 	port_info[0] = port_info[1] = port;
 	/* Now kick off ATA set up */
 	return ata_pci_init_one(dev, port_info, 2);



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pata_hpt37x: Updates from drivers/ide work
  2007-03-08 23:28 [PATCH] pata_hpt37x: Updates from drivers/ide work Alan Cox
@ 2007-03-09 14:17 ` Jeff Garzik
  2007-03-09 15:03 ` Sergei Shtylyov
  1 sibling, 0 replies; 4+ messages in thread
From: Jeff Garzik @ 2007-03-09 14:17 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, linux-ide

Alan Cox wrote:
> Drag pata_hpt37x kicking and screaming in the direction of
> drivers/ide/pci/hpt366.c and all the work that Sergei has been doing
> there. Plenty left to be done but this is a good snapshot for folks to
> work on and to review
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

applied



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pata_hpt37x: Updates from drivers/ide work
  2007-03-08 23:28 [PATCH] pata_hpt37x: Updates from drivers/ide work Alan Cox
  2007-03-09 14:17 ` Jeff Garzik
@ 2007-03-09 15:03 ` Sergei Shtylyov
  2007-03-09 16:53   ` Alan Cox
  1 sibling, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2007-03-09 15:03 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, linux-ide, jgarzik

Alan Cox wrote:
> Drag pata_hpt37x kicking and screaming in the direction of
> drivers/ide/pci/hpt366.c and all the work that Sergei has been doing
> there. Plenty left to be done but this is a good snapshot for folks to
> work on and to review

> Signed-off-by: Alan Cox <alan@redhat.com>

> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c linux-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c
> --- linux.vanilla-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c	2007-03-08 16:01:10.000000000 +0000
> +++ linux-2.6.21-rc3-mm2/drivers/ata/pata_hpt37x.c	2007-03-08 17:08:53.000000000 +0000
> @@ -8,6 +8,7 @@
>   * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
>   * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
>   * Portions Copyright (C) 2003		Red Hat Inc
> + * Portions Copyright (C) 2005-2006	MontaVista Software, Inc.
>   *
>   * TODO
>   *	PLL mode
> @@ -25,7 +26,7 @@
>  #include <linux/libata.h>
>  
>  #define DRV_NAME	"pata_hpt37x"
> -#define DRV_VERSION	"0.6.0"
> +#define DRV_VERSION	"0.6.4"
>  
>  struct hpt_clock {
>  	u8	xfer_speed;
> @@ -61,201 +62,75 @@
>   * 31     FIFO enable.
>   */
>  
> -/* from highpoint documentation. these are old values */
> -static const struct hpt_clock hpt370_timings_33[] = {
> -/*	{	XFER_UDMA_5,	0x1A85F442,	0x16454e31	}, */
> -	{	XFER_UDMA_5,	0x16454e31	},
> -	{	XFER_UDMA_4,	0x16454e31	},
> -	{	XFER_UDMA_3,	0x166d4e31	},
> -	{	XFER_UDMA_2,	0x16494e31	},
> -	{	XFER_UDMA_1,	0x164d4e31	},
> -	{	XFER_UDMA_0,	0x16514e31	},
> -
> -	{	XFER_MW_DMA_2,	0x26514e21	},
> -	{	XFER_MW_DMA_1,	0x26514e33	},
> -	{	XFER_MW_DMA_0,	0x26514e97	},
> -
> -	{	XFER_PIO_4,	0x06514e21	},
> -	{	XFER_PIO_3,	0x06514e22	},
> -	{	XFER_PIO_2,	0x06514e33	},
> -	{	XFER_PIO_1,	0x06914e43	},
> -	{	XFER_PIO_0,	0x06914e57	},
> -	{	0,		0x06514e57	}
> -};
> -
> -static const struct hpt_clock hpt370_timings_66[] = {
> -	{	XFER_UDMA_5,	0x14846231	},
> -	{	XFER_UDMA_4,	0x14886231	},
> -	{	XFER_UDMA_3,	0x148c6231	},
> -	{	XFER_UDMA_2,	0x148c6231	},
> -	{	XFER_UDMA_1,	0x14906231	},
> -	{	XFER_UDMA_0,	0x14986231	},
> -
> -	{	XFER_MW_DMA_2,	0x26514e21	},
> -	{	XFER_MW_DMA_1,	0x26514e33	},
> -	{	XFER_MW_DMA_0,	0x26514e97	},
> -
> -	{	XFER_PIO_4,	0x06514e21	},
> -	{	XFER_PIO_3,	0x06514e22	},
> -	{	XFER_PIO_2,	0x06514e33	},
> -	{	XFER_PIO_1,	0x06914e43	},
> -	{	XFER_PIO_0,	0x06914e57	},
> -	{	0,		0x06514e57	}
> -};
> -
> -/* these are the current (4 sep 2001) timings from highpoint */
> -static const struct hpt_clock hpt370a_timings_33[] = {
> -	{	XFER_UDMA_5,	0x12446231	},
> -	{	XFER_UDMA_4,	0x12446231	},
> -	{	XFER_UDMA_3,	0x126c6231	},
> -	{	XFER_UDMA_2,	0x12486231	},
> -	{	XFER_UDMA_1,	0x124c6233	},
> -	{	XFER_UDMA_0,	0x12506297	},
> -
> -	{	XFER_MW_DMA_2,	0x22406c31	},
> -	{	XFER_MW_DMA_1,	0x22406c33	},
> -	{	XFER_MW_DMA_0,	0x22406c97	},
> -
> -	{	XFER_PIO_4,	0x06414e31	},
> -	{	XFER_PIO_3,	0x06414e42	},
> -	{	XFER_PIO_2,	0x06414e53	},
> -	{	XFER_PIO_1,	0x06814e93	},
> -	{	XFER_PIO_0,	0x06814ea7	},
> -	{	0,		0x06814ea7	}
> -};
> -
> -/* 2x 33MHz timings */
> -static const struct hpt_clock hpt370a_timings_66[] = {
> -	{	XFER_UDMA_5,	0x1488e673	},
> -	{	XFER_UDMA_4,	0x1488e673	},
> -	{	XFER_UDMA_3,	0x1498e673	},
> -	{	XFER_UDMA_2,	0x1490e673	},
> -	{	XFER_UDMA_1,	0x1498e677	},
> -	{	XFER_UDMA_0,	0x14a0e73f	},
> -
> -	{	XFER_MW_DMA_2,	0x2480fa73	},
> -	{	XFER_MW_DMA_1,	0x2480fa77	},
> -	{	XFER_MW_DMA_0,	0x2480fb3f	},
> -
> -	{	XFER_PIO_4,	0x0c82be73	},
> -	{	XFER_PIO_3,	0x0c82be95	},
> -	{	XFER_PIO_2,	0x0c82beb7	},
> -	{	XFER_PIO_1,	0x0d02bf37	},
> -	{	XFER_PIO_0,	0x0d02bf5f	},
> -	{	0,		0x0d02bf5f	}
> -};
> -
> -static const struct hpt_clock hpt370a_timings_50[] = {
> -	{	XFER_UDMA_5,	0x12848242	},
> -	{	XFER_UDMA_4,	0x12ac8242	},
> -	{	XFER_UDMA_3,	0x128c8242	},
> -	{	XFER_UDMA_2,	0x120c8242	},
> -	{	XFER_UDMA_1,	0x12148254	},
> -	{	XFER_UDMA_0,	0x121882ea	},
> -
> -	{	XFER_MW_DMA_2,	0x22808242	},
> -	{	XFER_MW_DMA_1,	0x22808254	},
> -	{	XFER_MW_DMA_0,	0x228082ea	},
> -
> -	{	XFER_PIO_4,	0x0a81f442	},
> -	{	XFER_PIO_3,	0x0a81f443	},
> -	{	XFER_PIO_2,	0x0a81f454	},
> -	{	XFER_PIO_1,	0x0ac1f465	},
> -	{	XFER_PIO_0,	0x0ac1f48a	},
> -	{	0,		0x0ac1f48a	}
> -};
> -
> -static const struct hpt_clock hpt372_timings_33[] = {
> -	{	XFER_UDMA_6,	0x1c81dc62	},
> -	{	XFER_UDMA_5,	0x1c6ddc62	},
> -	{	XFER_UDMA_4,	0x1c8ddc62	},
> -	{	XFER_UDMA_3,	0x1c8edc62	},	/* checkme */
> -	{	XFER_UDMA_2,	0x1c91dc62	},
> -	{	XFER_UDMA_1,	0x1c9adc62	},	/* checkme */
> -	{	XFER_UDMA_0,	0x1c82dc62	},	/* checkme */
> -
> -	{	XFER_MW_DMA_2,	0x2c829262	},
> -	{	XFER_MW_DMA_1,	0x2c829266	},	/* checkme */
> -	{	XFER_MW_DMA_0,	0x2c82922e	},	/* checkme */
> -
> -	{	XFER_PIO_4,	0x0c829c62	},
> -	{	XFER_PIO_3,	0x0c829c84	},
> -	{	XFER_PIO_2,	0x0c829ca6	},
> -	{	XFER_PIO_1,	0x0d029d26	},
> -	{	XFER_PIO_0,	0x0d029d5e	},
> -	{	0,		0x0d029d5e	}
> -};
> -
> -static const struct hpt_clock hpt372_timings_50[] = {
> -	{	XFER_UDMA_5,	0x12848242	},
> -	{	XFER_UDMA_4,	0x12ac8242	},
> -	{	XFER_UDMA_3,	0x128c8242	},
> -	{	XFER_UDMA_2,	0x120c8242	},
> -	{	XFER_UDMA_1,	0x12148254	},
> -	{	XFER_UDMA_0,	0x121882ea	},
> -
> -	{	XFER_MW_DMA_2,	0x22808242	},
> -	{	XFER_MW_DMA_1,	0x22808254	},
> -	{	XFER_MW_DMA_0,	0x228082ea	},
> -
> -	{	XFER_PIO_4,	0x0a81f442	},
> -	{	XFER_PIO_3,	0x0a81f443	},
> -	{	XFER_PIO_2,	0x0a81f454	},
> -	{	XFER_PIO_1,	0x0ac1f465	},
> -	{	XFER_PIO_0,	0x0ac1f48a	},
> -	{	0,		0x0a81f443	}
> -};
> -
> -static const struct hpt_clock hpt372_timings_66[] = {
> -	{	XFER_UDMA_6,	0x1c869c62	},
> -	{	XFER_UDMA_5,	0x1cae9c62	},
> -	{	XFER_UDMA_4,	0x1c8a9c62	},
> -	{	XFER_UDMA_3,	0x1c8e9c62	},
> -	{	XFER_UDMA_2,	0x1c929c62	},
> -	{	XFER_UDMA_1,	0x1c9a9c62	},
> -	{	XFER_UDMA_0,	0x1c829c62	},
> -
> -	{	XFER_MW_DMA_2,	0x2c829c62	},
> -	{	XFER_MW_DMA_1,	0x2c829c66	},
> -	{	XFER_MW_DMA_0,	0x2c829d2e	},
> -
> -	{	XFER_PIO_4,	0x0c829c62	},
> -	{	XFER_PIO_3,	0x0c829c84	},
> -	{	XFER_PIO_2,	0x0c829ca6	},
> -	{	XFER_PIO_1,	0x0d029d26	},
> -	{	XFER_PIO_0,	0x0d029d5e	},
> -	{	0,		0x0d029d26	}
> -};
> -
> -static const struct hpt_clock hpt374_timings_33[] = {
> -	{	XFER_UDMA_6,	0x12808242	},
> -	{	XFER_UDMA_5,	0x12848242	},
> -	{	XFER_UDMA_4,	0x12ac8242	},
> -	{	XFER_UDMA_3,	0x128c8242	},
> -	{	XFER_UDMA_2,	0x120c8242	},
> -	{	XFER_UDMA_1,	0x12148254	},
> -	{	XFER_UDMA_0,	0x121882ea	},
> -
> -	{	XFER_MW_DMA_2,	0x22808242	},
> -	{	XFER_MW_DMA_1,	0x22808254	},
> -	{	XFER_MW_DMA_0,	0x228082ea	},
> -
> -	{	XFER_PIO_4,	0x0a81f442	},
> -	{	XFER_PIO_3,	0x0a81f443	},
> -	{	XFER_PIO_2,	0x0a81f454	},
> -	{	XFER_PIO_1,	0x0ac1f465	},
> -	{	XFER_PIO_0,	0x0ac1f48a	},
> -	{	0,		0x06814e93	}
> +static struct hpt_clock hpt37x_timings_33[] = {
> +	{ XFER_UDMA_6,		0x12446231 },	/* 0x12646231 ?? */
> +	{ XFER_UDMA_5,		0x12446231 },
> +	{ XFER_UDMA_4,		0x12446231 },
> +	{ XFER_UDMA_3,		0x126c6231 },
> +	{ XFER_UDMA_2,		0x12486231 },
> +	{ XFER_UDMA_1,		0x124c6233 },
> +	{ XFER_UDMA_0,		0x12506297 },
> +
> +	{ XFER_MW_DMA_2,	0x22406c31 },
> +	{ XFER_MW_DMA_1,	0x22406c33 },
> +	{ XFER_MW_DMA_0,	0x22406c97 },
> +
> +	{ XFER_PIO_4,		0x06414e31 },
> +	{ XFER_PIO_3,		0x06414e42 },
> +	{ XFER_PIO_2,		0x06414e53 },
> +	{ XFER_PIO_1,		0x06814e93 },
> +	{ XFER_PIO_0,		0x06814ea7 }
> +};
> +
> +static struct hpt_clock hpt37x_timings_50[] = {
> +	{ XFER_UDMA_6,		0x12848242 },
> +	{ XFER_UDMA_5,		0x12848242 },
> +	{ XFER_UDMA_4,		0x12ac8242 },
> +	{ XFER_UDMA_3,		0x128c8242 },
> +	{ XFER_UDMA_2,		0x120c8242 },
> +	{ XFER_UDMA_1,		0x12148254 },
> +	{ XFER_UDMA_0,		0x121882ea },
> +
> +	{ XFER_MW_DMA_2,	0x22808242 },
> +	{ XFER_MW_DMA_1,	0x22808254 },
> +	{ XFER_MW_DMA_0,	0x228082ea },
> +
> +	{ XFER_PIO_4,		0x0a81f442 },
> +	{ XFER_PIO_3,		0x0a81f443 },
> +	{ XFER_PIO_2,		0x0a81f454 },
> +	{ XFER_PIO_1,		0x0ac1f465 },
> +	{ XFER_PIO_0,		0x0ac1f48a }
> +};
> +
> +static struct hpt_clock hpt37x_timings_66[] = {
> +	{ XFER_UDMA_6,		0x1c869c62 },
> +	{ XFER_UDMA_5,		0x1cae9c62 },	/* 0x1c8a9c62 */
> +	{ XFER_UDMA_4,		0x1c8a9c62 },
> +	{ XFER_UDMA_3,		0x1c8e9c62 },
> +	{ XFER_UDMA_2,		0x1c929c62 },
> +	{ XFER_UDMA_1,		0x1c9a9c62 },
> +	{ XFER_UDMA_0,		0x1c829c62 },
> +
> +	{ XFER_MW_DMA_2,	0x2c829c62 },
> +	{ XFER_MW_DMA_1,	0x2c829c66 },
> +	{ XFER_MW_DMA_0,	0x2c829d2e },
> +
> +	{ XFER_PIO_4,		0x0c829c62 },
> +	{ XFER_PIO_3,		0x0c829c84 },
> +	{ XFER_PIO_2,		0x0c829ca6 },
> +	{ XFER_PIO_1,		0x0d029d26 },
> +	{ XFER_PIO_0,		0x0d029d5e }
>  };

    Yeah, I was going to cook a patch to remove broken tables for starters...

> +
>  static const struct hpt_chip hpt370 = {
>  	"HPT370",
>  	48,
>  	{
> -		hpt370_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
>  		NULL,
> -		hpt370_timings_66
> +		hpt37x_timings_66
>  	}
>  };
>  
> @@ -263,10 +138,10 @@
>  	"HPT370A",
>  	48,
>  	{
> -		hpt370a_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
> -		hpt370a_timings_50,
> -		hpt370a_timings_66
> +		hpt37x_timings_50,
> +		hpt37x_timings_66
>  	}
>  };

    HPT370 chips aren't clockable to 66MHz -- they neither support Ultra133 
nor are PCI66 capable.

> @@ -274,10 +149,10 @@
>  	"HPT372",
>  	55,
>  	{
> -		hpt372_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
> -		hpt372_timings_50,
> -		hpt372_timings_66
> +		hpt37x_timings_50,
> +		hpt37x_timings_66
>  	}
>  };
>  
> @@ -285,10 +160,10 @@
>  	"HPT302",
>  	66,
>  	{
> -		hpt372_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
> -		hpt372_timings_50,
> -		hpt372_timings_66
> +		hpt37x_timings_50,
> +		hpt37x_timings_66
>  	}
>  };
>  
> @@ -296,10 +171,10 @@
>  	"HPT371",
>  	66,
>  	{
> -		hpt372_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
> -		hpt372_timings_50,
> -		hpt372_timings_66
> +		hpt37x_timings_50,
> +		hpt37x_timings_66
>  	}
>  };
>  
> @@ -307,10 +182,10 @@
>  	"HPT372A",
>  	66,
>  	{
> -		hpt372_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
> -		hpt372_timings_50,
> -		hpt372_timings_66
> +		hpt37x_timings_50,
> +		hpt37x_timings_66
>  	}
>  };
>  
> @@ -318,7 +193,7 @@
>  	"HPT374",
>  	48,
>  	{
> -		hpt374_timings_33,
> +		hpt37x_timings_33,
>  		NULL,
>  		NULL,
>  		NULL

    But HPT374 is DPLL-clockable to 50 MHz!
    That effectivy means that we don't need the separate clock tables for each 
chip anymore -- they should be all the same, with only 66 MHz capability 
disabled for earlier chips (by other means)...

> @@ -463,8 +336,7 @@
>  		ap->cbl = ATA_CBL_PATA80;

    Erm, no cable_detect() here?

>  	/* Reset the state machine */
> -	pci_write_config_byte(pdev, 0x50, 0x37);
> -	pci_write_config_byte(pdev, 0x54, 0x37);
> +	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
>  	udelay(100);
>  
>  	return ata_std_prereset(ap, deadline);
> @@ -514,8 +386,7 @@
>  		ap->cbl = ATA_CBL_PATA80;
>  
>  	/* Reset the state machine */
> -	pci_write_config_byte(pdev, 0x50, 0x37);
> -	pci_write_config_byte(pdev, 0x54, 0x37);
> +	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
>  	udelay(100);
>  
>  	return ata_std_prereset(ap, deadline);
> @@ -1033,6 +904,24 @@
>  		.udma_mask = 0x3f,
>  		.port_ops = &hpt370a_port_ops
>  	};
> +	/* HPT370 - UDMA100 */
> +	static struct ata_port_info info_hpt370_33 = {
> +		.sht = &hpt37x_sht,
> +		.flags = ATA_FLAG_SLAVE_POSS|ATA_FLAG_SRST,
> +		.pio_mask = 0x1f,
> +		.mwdma_mask = 0x07,
> +		.udma_mask = 0x0f,
> +		.port_ops = &hpt370_port_ops
> +	};
> +	/* HPT370A - UDMA100 */
> +	static struct ata_port_info info_hpt370a_33 = {
> +		.sht = &hpt37x_sht,
> +		.flags = ATA_FLAG_SLAVE_POSS|ATA_FLAG_SRST,
> +		.pio_mask = 0x1f,
> +		.mwdma_mask = 0x07,
> +		.udma_mask = 0x0f,
> +		.port_ops = &hpt370a_port_ops
> +	};

    Well, these are clockable to Ultra/100 but my experiments have shown that 
it's slower than Ultra/66 (!).  And they needed a clock trunaround at this 
speed, according to 2.4.18 driver...
    Wait, wait, wait... why limit them to Ultra/44 on 33 MHz?! I certainly 
didn't do (or intend) anything alike... :-O

> @@ -1068,7 +957,11 @@
>  
>  	u8 irqmask;
>  	u32 class_rev;
> +	u8 mcr1;
>  	u32 freq;
> +	int prefer_dpll = 1;
> +	
> +	unsigned long iobase = pci_resource_start(dev, 4);
>  
>  	const struct hpt_chip *chip_table;
>  	int clock_slot;
> @@ -1089,10 +982,12 @@
>  			case 3:
>  				port = &info_hpt370;
>  				chip_table = &hpt370;
> +				prefer_dpll = 0;
>  				break;
>  			case 4:
>  				port = &info_hpt370a;
>  				chip_table = &hpt370a;
> +				prefer_dpll = 0;
>  				break;
>  			case 5:
>  				port = &info_hpt372;
> @@ -1120,8 +1015,16 @@
>  				chip_table = &hpt302;
>  				break;
>  			case PCI_DEVICE_ID_TTI_HPT371:
> +				if (class_rev > 1)
> +					return -ENODEV;
>  				port = &info_hpt372;
>  				chip_table = &hpt371;
> +				/* Single channel device, paster is not present
> +				   but the NIOS (or us for non x86) must mark it

    Earlier NIOSes (sic? :-) didn't do that either...

> +				   absent */
> +				pci_read_config_byte(dev, 0x50, &mcr1);
> +				mcr1 &= ~0x04;
> +				pci_write_config_byte(dev, 0x50, mcr1);
>  				break;
>  			case PCI_DEVICE_ID_TTI_HPT374:
>  				chip_table = &hpt374;
> @@ -1151,8 +1054,18 @@
>  	 */
>  
>  	pci_write_config_byte(dev, 0x5b, 0x23);
> +	
> +	/*
> +	 * HighPoint does this for HPT372A.
> +	 * NOTE: This register is only writeable via I/O space.
> +	 */
> +	if (chip_table == &hpt372a)
> +		outb(0x0e, iobase + 0x9c);
>  
> -	pci_read_config_dword(dev, 0x70, &freq);
> +	/* Some devices do not let this value be accessed via PCI space
> +	   according to the old driver */
> +
> +	freq = inl(iobase + 0x90);

    I wonder if any chips do let it... :-)

> @@ -1174,15 +1087,27 @@
>  	 *	Turn the frequency check into a band and then find a timing
>  	 *	table to match it.
>  	 */
> -
> +	 
>  	clock_slot = hpt37x_clock_slot(freq, chip_table->base);
> -	if (chip_table->clocks[clock_slot] == NULL) {
> +	if (chip_table->clocks[clock_slot] == NULL || prefer_dpll) {
>  		/*
>  		 *	We need to try PLL mode instead
> +		 *
> +		 *	For non UDMA133 capable devices we should
> +		 *	use a 50MHz DPLL by choice
>  		 */
> -		unsigned int f_low = (MHz[clock_slot] * chip_table->base) / 192;
> -		unsigned int f_high = f_low + 2;
> +		unsigned int f_low, f_high;
>  		int adjust;
> +		
> +		clock_slot = 2;
> +		if (port->udma_mask & 0xE0)
> +			clock_slot = 3;
> +		
> +		f_low = (MHz[clock_slot] * chip_table->base) / 192;
> +		f_high = f_low + 2;
> +
> +		/* Select the DPLL clock. */
> +		pci_write_config_byte(dev, 0x5b, 0x21);
>  
>  		for(adjust = 0; adjust < 8; adjust++) {
>  			if (hpt37x_calibrate_dpll(dev))
> @@ -1198,15 +1123,17 @@
>  			printk(KERN_WARNING "hpt37x: DPLL did not stabilize.\n");
>  			return -ENODEV;
>  		}
> -		/* Check if this works for all cases */
> -		port->private_data = (void *)hpt370_timings_66;
> +		if (clock_slot == 3)
> +			port->private_data = (void *)hpt37x_timings_66;
> +		else
> +			port->private_data = (void *)hpt37x_timings_50;
>  
>  		printk(KERN_INFO "hpt37x: Bus clock %dMHz, using DPLL.\n", MHz[clock_slot]);
>  	} else {
>  		port->private_data = (void *)chip_table->clocks[clock_slot];
>  		/*
>  		 *	Perform a final fixup. The 371 and 372 clock determines
> -		 *	if UDMA133 is available.
> +		 *	if UDMA133 is available. (FIXME: should we use DPLL then ?)
>  		 */
>  
>  		if (clock_slot == 2 && chip_table == &hpt372) {	/* 50Mhz */

    This else block won't be reachable with prefer_dpll == 1 anyway...

> @@ -1215,8 +1142,13 @@
>  				port = &info_hpt372_50;
>  			else BUG();
>  		}
> +		if (clock_slot < 2 && port == &info_hpt370)
> +			port = &info_hpt370_33;
> +		if (clock_slot < 2 && port == &info_hpt370a)
> +			port = &info_hpt370a_33;

    I didn't get this change at all... Ultra/66 is perfectly reachable at 33 
MHz PCI.

MBR, Sergei

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pata_hpt37x: Updates from drivers/ide work
  2007-03-09 15:03 ` Sergei Shtylyov
@ 2007-03-09 16:53   ` Alan Cox
  0 siblings, 0 replies; 4+ messages in thread
From: Alan Cox @ 2007-03-09 16:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: akpm, linux-ide, jgarzik

>     HPT370 chips aren't clockable to 66MHz -- they neither support Ultra133 
> nor are PCI66 capable.

Ok. Easily fixed thanks (Unfortunately HPT docs are one thing I don't
have just their various at times contradictory and weird drivers).


> >  	"HPT374",
> >  	48,
> >  	{
> > -		hpt374_timings_33,
> > +		hpt37x_timings_33,
> >  		NULL,
> >  		NULL,
> >  		NULL
> 
>     But HPT374 is DPLL-clockable to 50 MHz!
>     That effectivy means that we don't need the separate clock tables for each 
> chip anymore -- they should be all the same, with only 66 MHz capability 
> disabled for earlier chips (by other means)...

That should be fine as we cantry and use the DPLL in that case and will
go for a 50MHz or 66MHz DPLL timing not the timing array.

> 
> > @@ -463,8 +336,7 @@
> >  		ap->cbl = ATA_CBL_PATA80;
> 
>     Erm, no cable_detect() here?

Not yet. 

> >  	} else {
> >  		port->private_data = (void *)chip_table->clocks[clock_slot];
> >  		/*
> >  		 *	Perform a final fixup. The 371 and 372 clock determines
> > -		 *	if UDMA133 is available.
> > +		 *	if UDMA133 is available. (FIXME: should we use DPLL then ?)
> >  		 */
> >  
> >  		if (clock_slot == 2 && chip_table == &hpt372) {	/* 50Mhz */
> 
>     This else block won't be reachable with prefer_dpll == 1 anyway...

Good point

> 
> > @@ -1215,8 +1142,13 @@
> >  				port = &info_hpt372_50;
> >  			else BUG();
> >  		}
> > +		if (clock_slot < 2 && port == &info_hpt370)
> > +			port = &info_hpt370_33;
> > +		if (clock_slot < 2 && port == &info_hpt370a)
> > +			port = &info_hpt370a_33;
> 
>     I didn't get this change at all... Ultra/66 is perfectly reachable at 33 
> MHz PCI.

I'll investigate that further with your driver and the HPT "code".

Alan

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-03-09 15:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-08 23:28 [PATCH] pata_hpt37x: Updates from drivers/ide work Alan Cox
2007-03-09 14:17 ` Jeff Garzik
2007-03-09 15:03 ` Sergei Shtylyov
2007-03-09 16:53   ` Alan Cox

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.