linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][RFT] HPT3xxN clocking fixes
@ 2006-04-23  8:33 Sergei Shtylyov
  2006-04-24 22:09 ` [PATCH][RFT] HPT3xxN clocking fixes (take 2) Sergei Shtylyov
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-04-23  8:33 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, linux-mips, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

Hello.

    Fix serious problems with the HPT372N clock turnaround code:
  - wrong I/O port writes in case of the secondary channel;
  - serialize access to the channels;
  - disable turnaround for 66 MHz PCI;
  - read the current clock mode directly from register (which is shared by
    both channels, so caching its value per-channel was sub-optimal).

    Additionally, avoid calibrating PLL twice (for each channel) as the
second try results in a wrong PCI frequency, and thus in the wrong timings.
    Make the driver deal with HPT302N and HPT371N correctly -- the clocking is
the same as for HPT372N, with HPT302N needing the clock turnaround (according
to the HighPoint driver) and HPT371N not needing it.
    Also, while I'm at it, disable UltraATA/133 for HPT372 by default -- 50
MHz DPLL clock don't allow for this speed anyway. And remove the traces of the
former bad patch that wasn't even applicable to this version of driver.
    Have been tested on HPT370/371N, unfortunately I don't have access to
other chips...

MBR, Sergei

PS: The driver has many more issues which I'm dealing with in the rewrite --
the large patch is to follow soon, on top of this quick and dirty one...

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xxN-clocking-fix.patch --]
[-- Type: text/plain, Size: 10539 bytes --]

diff --git a/drivers/ide/pci/hpt366.c b/drivers/ide/pci/hpt366.c
index 940bdd4..ce30eb9 100644
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -4,6 +4,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.
  *
  * Thanks to HighPoint Technologies for their assistance, and hardware.
  * Special Thanks to Jon Burchmore in SanDiego for the deep pockets, his
@@ -11,9 +12,11 @@
  * development and support.
  *
  *
- * Highpoint have their own driver (source except for the raid part)
- * available from http://www.highpoint-tech.com/hpt3xx-opensource-v131.tgz
- * This may be useful to anyone wanting to work on the mainstream hpt IDE.
+ * HighPoint has its own drivers (open source except for the RAID part)
+ * available from http://www.highpoint-tech.com/BIOS%20+%20Driver/.
+ * This  may be useful to anyone wanting to work on this driver, however do not
+ * trust them  too much since the code tends to become less and less meaningful
+ * as the time passes... :-/
  *
  * Note that final HPT370 support was done by force extraction of GPL.
  *
@@ -52,6 +55,18 @@
  * keeping me sane. 
  *		Alan Cox <alan@redhat.com>
  *
+ * - fix  wrong I/O port writes in case of the secondary channel in the clock
+ *   turnaround code, make this code read the current clock mode directly from
+ *   register (which is shared by both channels), serialize channels, and
+ *   disable turnaround for 66 MHz PCI
+ * - avoid calibrating PLL twice (for each channel) as the second try results
+ *   in a wrong PCI frequency, thus in the wrong timings
+ * - add  support for HPT302N and HPT371N clocking (the same as for HPT372N,
+ *   with HPT302N needing the clock turnaround and HPT371N not needing it)
+ * - disable UltraATA/133 for HPT372 by default (50 MHz DPLL clock don't
+ *   allow for this speed anyway)
+ *		<source@mvista.com>
+ *
  */
 
 
@@ -77,8 +92,8 @@
 
 /* various tuning parameters */
 #define HPT_RESET_STATE_ENGINE
-#undef HPT_DELAY_INTERRUPT
-#undef HPT_SERIALIZE_IO
+#undef	HPT_DELAY_INTERRUPT
+#define HPT_SERIALIZE_IO	0
 
 static const char *quirk_drives[] = {
 	"QUANTUM FIREBALLlct08 08",
@@ -440,7 +455,7 @@ static struct chipset_bus_clock_list_ent
 #define HPT374_ALLOW_ATA133_6		0
 #define HPT371_ALLOW_ATA133_6		0
 #define HPT302_ALLOW_ATA133_6		0
-#define HPT372_ALLOW_ATA133_6		1
+#define HPT372_ALLOW_ATA133_6		0
 #define HPT370_ALLOW_ATA100_5		1
 #define HPT366_ALLOW_ATA66_4		1
 #define HPT366_ALLOW_ATA66_3		1
@@ -462,7 +477,9 @@ struct hpt_info
 	int revision;		/* Chipset revision */
 	int flags;		/* Chipset properties */
 #define PLL_MODE	1
-#define IS_372N		2
+#define IS_3x2N 	2
+#define IS_371N 	4
+#define PCI_66MHZ	8
 				/* Speed table */
 	struct chipset_bus_clock_list_entry *speed;
 };
@@ -957,59 +974,56 @@ static int hpt374_ide_dma_end (ide_drive
 }
 
 /**
- *	hpt372n_set_clock	-	perform clock switching dance
- *	@drive: Drive to switch
- *	@mode: Switching mode (0x21 for write, 0x23 otherwise)
+ *	hpt3x2n_set_clock	-	perform clock switching dance
+ *	@hwif: hwif to switch
+ *	@mode: clocking mode (0x21 for write, 0x23 otherwise)
  *
- *	Switch the DPLL clock on the HPT372N devices. This is a
+ *	Switch the DPLL clock on the HPT3x2N devices. This is a
  *	right mess.
  */
  
-static void hpt372n_set_clock(ide_drive_t *drive, int mode)
+static inline void hpt3x2n_set_clock(ide_hwif_t *hwif, u8 mode)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	
-	/* FIXME: should we check for DMA active and BUG() */
+	u8 scr2 = hwif->INB(hwif->dma_master + 0x7b);
+
+	if ((scr2 & 0x7f) == mode)
+		return;
+
 	/* Tristate the bus */
-	outb(0x80, hwif->dma_base+0x73);
-	outb(0x80, hwif->dma_base+0x77);
-	
+	hwif->OUTB(0x80, hwif->dma_master + 0x73);
+	hwif->OUTB(0x80, hwif->dma_master + 0x77);
+
 	/* Switch clock and reset channels */
-	outb(mode, hwif->dma_base+0x7B);
-	outb(0xC0, hwif->dma_base+0x79);
-	
+	hwif->OUTB(mode, hwif->dma_master + 0x7b);
+	hwif->OUTB(0xc0, hwif->dma_master + 0x79);
+
 	/* Reset state machines */
-	outb(0x37, hwif->dma_base+0x70);
-	outb(0x37, hwif->dma_base+0x74);
-	
+	hwif->OUTB(0x37, hwif->dma_master + 0x70);
+	hwif->OUTB(0x37, hwif->dma_master + 0x74);
+
 	/* Complete reset */
-	outb(0x00, hwif->dma_base+0x79);
-	
+	hwif->OUTB(0x00, hwif->dma_master + 0x79);
+
 	/* Reconnect channels to bus */
-	outb(0x00, hwif->dma_base+0x73);
-	outb(0x00, hwif->dma_base+0x77);
+	hwif->OUTB(0x00, hwif->dma_master + 0x73);
+	hwif->OUTB(0x00, hwif->dma_master + 0x77);
 }
 
 /**
- *	hpt372n_rw_disk		-	prepare for I/O
+ *	hpt3x2n_rw_disk		-	prepare for I/O
  *	@drive: drive for command
  *	@rq: block request structure
  *
- *	This is called when a disk I/O is issued to the 372N.
+ *	This is called when a disk I/O is issued to HPT302N/372N.
  *	We need it because of the clock switching.
  */
 
-static void hpt372n_rw_disk(ide_drive_t *drive, struct request *rq)
+static void hpt3x2n_rw_disk(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif = drive->hwif;
-	int wantclock;
-
-	wantclock = rq_data_dir(rq) ? 0x23 : 0x21;
+	ide_hwif_t *hwif	= HWIF(drive);
+	u8 wantclock		= rq_data_dir(rq) ? 0x23 : 0x21;
 
-	if (hwif->config_data != wantclock) {
-		hpt372n_set_clock(drive, wantclock);
-		hwif->config_data = wantclock;
-	}
+	hpt3x2n_set_clock(hwif, wantclock);
 }
 
 /*
@@ -1162,17 +1176,11 @@ static void __devinit hpt37x_clocking(id
 	freq &= 0x1FF;
 	
 	/*
-	 * The 372N uses different PCI clock information and has
-	 * some other complications
-	 *	On PCI33 timing we must clock switch
-	 *	On PCI66 timing we must NOT use the PCI clock
-	 *
-	 * Currently we always set up the PLL for the 372N
+	 * HPT3xxN chips use different PCI clock information.
+	 * Currently we always set up the PLL for them.
 	 */
-	 
-	if(info->flags & IS_372N)
-	{
-		printk(KERN_INFO "hpt: HPT372N detected, using 372N timing.\n");
+
+	if (info->flags & (IS_3x2N | IS_371N)) {
 		if(freq < 0x55)
 			pll = F_LOW_PCI_33;
 		else if(freq < 0x70)
@@ -1181,10 +1189,8 @@ static void __devinit hpt37x_clocking(id
 			pll = F_LOW_PCI_50;
 		else
 			pll = F_LOW_PCI_66;
-			
-		printk(KERN_INFO "FREQ: %d PLL: %d\n", freq, pll);
-			
-		/* We always use the pll not the PCI clock on 372N */
+
+		printk(KERN_INFO "HPT3xxN detected, FREQ: %d, PLL: %d\n", freq, pll);
 	}
 	else
 	{
@@ -1232,7 +1238,10 @@ static void __devinit hpt37x_clocking(id
 			printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
 		}
 	}
-	
+
+	if (pll == F_LOW_PCI_66)
+		info->flags |= PCI_66MHZ;
+
 	/*
 	 * only try the pll if we don't have a table for the clock
 	 * speed that we're running at. NOTE: the internal PLL will
@@ -1288,10 +1297,6 @@ static void __devinit hpt37x_clocking(id
 				goto init_hpt37X_done;
 			}
 		}
-		if (!pci_get_drvdata(dev)) {
-			printk("No Clock Stabilization!!!\n");
-			return;
-		}
 pll_recal:
 		if (adjust & 1)
 			pll -= (adjust >> 1);
@@ -1301,8 +1306,8 @@ pll_recal:
 
 init_hpt37X_done:
 	if (!info->speed)
-		printk(KERN_ERR "HPT37X%s: unknown bus timing [%d %d].\n",
-			(info->flags & IS_372N)?"N":"", pll, freq);
+		printk(KERN_ERR "HPT37x%s: unknown bus timing [%d %d].\n",
+		       (info->flags & (IS_3x2N | IS_371N)) ? "N" : "", pll, freq);
 	/* reset state engine */
 	pci_write_config_byte(dev, 0x50, 0x37); 
 	pci_write_config_byte(dev, 0x54, 0x37); 
@@ -1368,6 +1373,7 @@ static void __devinit init_hwif_hpt366(i
 	struct pci_dev *dev		= hwif->pci_dev;
 	struct hpt_info *info		= ide_get_hwifdata(hwif);
 	u8 ata66 = 0, regmask		= (hwif->channel) ? 0x01 : 0x02;
+	int serialize			= HPT_SERIALIZE_IO;
 	
 	hwif->tuneproc			= &hpt3xx_tune_drive;
 	hwif->speedproc			= &hpt3xx_tune_chipset;
@@ -1375,8 +1381,20 @@ static void __devinit init_hwif_hpt366(i
 	hwif->intrproc			= &hpt3xx_intrproc;
 	hwif->maskproc			= &hpt3xx_maskproc;
 	
-	if(info->flags & IS_372N)
-		hwif->rw_disk = &hpt372n_rw_disk;
+	/*
+	 * HPT372N/302N chips have some complications:
+	 *
+	 * - on 33 MHz PCI we must clock switch
+	 * - on 66 MHz PCI we must NOT use the PCI clock
+	 */
+	if ((info->flags & (IS_3x2N | PCI_66MHZ)) == IS_3x2N) {
+		/*
+		 * Clock is shared between the channels,
+		 * so we'll have to serialize them... :-(
+		 */
+		serialize = 1;
+		hwif->rw_disk = &hpt3x2n_rw_disk;
+	}
 
 	/*
 	 * The HPT37x uses the CBLID pins as outputs for MA15/MA16
@@ -1419,11 +1437,9 @@ static void __devinit init_hwif_hpt366(i
 		PCI_FUNC(hwif->pci_dev->devfn));
 #endif /* DEBUG */
 
-#ifdef HPT_SERIALIZE_IO
-	/* serialize access to this device */
-	if (hwif->mate)
+	/* Serialize access to this device */
+	if (serialize && hwif->mate)
 		hwif->serialized = hwif->mate->serialized = 1;
-#endif
 
 	if (info->revision >= 3) {
 		u8 reg5ah = 0;
@@ -1491,7 +1507,7 @@ static void __devinit init_dma_hpt366(id
 		return;
 		
 	if(info->speed == NULL) {
-		printk(KERN_WARNING "hpt: no known IDE timings, disabling DMA.\n");
+		printk(KERN_WARNING "hpt366: no known IDE timings, disabling DMA.\n");
 		return;
 	}
 
@@ -1520,9 +1536,10 @@ static void __devinit init_dma_hpt366(id
 
 static void __devinit init_iops_hpt366(ide_hwif_t *hwif)
 {
-	struct hpt_info *info = kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
-	unsigned long dmabase = pci_resource_start(hwif->pci_dev, 4);
-	u8 did, rid;
+	struct hpt_info *info	= kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	u16 did			= dev->device;
+	u8  rid			= 0;
 
 	if(info == NULL) {
 		printk(KERN_WARNING "hpt366: out of memory.\n");
@@ -1530,15 +1547,23 @@ static void __devinit init_iops_hpt366(i
 	}
 	ide_set_hwifdata(hwif, info);
 
-	if(dmabase) {
-		did = inb(dmabase + 0x22);
-		rid = inb(dmabase + 0x28);
-
-		if((did == 4 && rid == 6) || (did == 5 && rid > 1))
-			info->flags |= IS_372N;
+	/* Avoid doing the same thing twice. */
+	if (hwif->channel && hwif->mate) {
+		memcpy(info, ide_get_hwifdata(hwif->mate), sizeof(struct hpt_info));
+		return;
 	}
 
-	info->revision = hpt_revision(hwif->pci_dev);
+	pci_read_config_byte(dev, PCI_CLASS_REVISION, &rid);
+
+	if (( did == PCI_DEVICE_ID_TTI_HPT366  && rid == 6) ||
+	    ((did == PCI_DEVICE_ID_TTI_HPT372  ||
+	      did == PCI_DEVICE_ID_TTI_HPT302) && rid >  1) ||
+	      did == PCI_DEVICE_ID_TTI_HPT372N)
+		info->flags |= IS_3x2N;
+	else if (did == PCI_DEVICE_ID_TTI_HPT371 && rid > 1)
+		info->flags |= IS_371N;
+
+	info->revision = hpt_revision(dev);
 
 	if (info->revision >= 3)
 		hpt37x_clocking(hwif);





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

* [PATCH][RFT] HPT3xxN clocking fixes (take 2)
  2006-04-23  8:33 [PATCH][RFT] HPT3xxN clocking fixes Sergei Shtylyov
@ 2006-04-24 22:09 ` Sergei Shtylyov
  2006-05-02 21:55 ` [PATCH][RFT] Fix HPT37x timing tables Sergei Shtylyov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-04-24 22:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, linux-mips, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

Hello.

    Fix serious problems with the HPT372N clock turnaround code:
  - the wrong ports were written to when called for the secondary channel;
  - it didn't serialize access to the channels;
  - turnaround shou;dn't be done on 66 MHz PCI;
  - caching the clock mode per-channel caused it to get out of sync with the
actual register value.

    Additionally, avoid calibrating PLL twice (for each channel) as the
second try results in a wrong PCI frequency and thus in the wrong timings.
    Make the driver deal with HPT302N and HPT371N correctly -- the clocking
and (seemingly) a need for clock tunaround is the same as for HPT372N. 
HPT371/N chips have only one, secondary channel, so avoid touching their "pure 
virtual" primary channel, and disable it if the BIOS haven't done this already.
    Also, while at it, disable UltraATA/133 for HPT372 by default -- 50 MHz
DPLL clock don't allow for this speed anyway. And remove the traces of the
former bad patch that wasn't even applicable to this version of driver.
    Have been tested on HPT370/371N, unfortunately I don't have an instant
access to the other chips...

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xxN-clocking-fixes.patch --]
[-- Type: text/plain, Size: 12600 bytes --]

diff --git a/drivers/ide/pci/hpt366.c b/drivers/ide/pci/hpt366.c
index 940bdd4..b6d329a 100644
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -4,6 +4,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.
  *
  * Thanks to HighPoint Technologies for their assistance, and hardware.
  * Special Thanks to Jon Burchmore in SanDiego for the deep pockets, his
@@ -11,9 +12,11 @@
  * development and support.
  *
  *
- * Highpoint have their own driver (source except for the raid part)
- * available from http://www.highpoint-tech.com/hpt3xx-opensource-v131.tgz
- * This may be useful to anyone wanting to work on the mainstream hpt IDE.
+ * HighPoint has its own drivers (open source except for the RAID part)
+ * available from http://www.highpoint-tech.com/BIOS%20+%20Driver/.
+ * This may be useful to anyone wanting to work on this driver, however  do not
+ * trust  them too much since the code tends to become less and less meaningful
+ * as the time passes... :-/
  *
  * Note that final HPT370 support was done by force extraction of GPL.
  *
@@ -52,6 +55,20 @@
  * keeping me sane. 
  *		Alan Cox <alan@redhat.com>
  *
+ * - fix the clock turnaround code: it was writing to the wrong ports when
+ *   called for the secondary channel, caching the current clock mode per-
+ *   channel caused the cached register value to get out of sync with the
+ *   actual one, the channels weren't serialized, the turnaround shouldn't
+ *   be done on 66 MHz PCI bus
+ * - avoid calibrating PLL twice as the second time results in a wrong PCI
+ *   frequency and thus in the wrong timings for the secondary channel
+ * - disable UltraATA/133 for HPT372 by default (50 MHz DPLL clock do not
+ *   allow for this speed anyway)
+ * - add support for HPT302N and HPT371N clocking (the same as for HPT372N)
+ * - HPT371/N are single channel chips, so avoid touching the primary channel
+ *   which exists only virtually (there's no pins for it)
+ *		<source@mvista.com>
+ *
  */
 
 
@@ -77,8 +94,8 @@
 
 /* various tuning parameters */
 #define HPT_RESET_STATE_ENGINE
-#undef HPT_DELAY_INTERRUPT
-#undef HPT_SERIALIZE_IO
+#undef	HPT_DELAY_INTERRUPT
+#define HPT_SERIALIZE_IO	0
 
 static const char *quirk_drives[] = {
 	"QUANTUM FIREBALLlct08 08",
@@ -440,7 +457,7 @@ static struct chipset_bus_clock_list_ent
 #define HPT374_ALLOW_ATA133_6		0
 #define HPT371_ALLOW_ATA133_6		0
 #define HPT302_ALLOW_ATA133_6		0
-#define HPT372_ALLOW_ATA133_6		1
+#define HPT372_ALLOW_ATA133_6		0
 #define HPT370_ALLOW_ATA100_5		1
 #define HPT366_ALLOW_ATA66_4		1
 #define HPT366_ALLOW_ATA66_3		1
@@ -462,7 +479,8 @@ struct hpt_info
 	int revision;		/* Chipset revision */
 	int flags;		/* Chipset properties */
 #define PLL_MODE	1
-#define IS_372N		2
+#define IS_3xxN 	2
+#define PCI_66MHZ	4
 				/* Speed table */
 	struct chipset_bus_clock_list_entry *speed;
 };
@@ -957,59 +975,63 @@ static int hpt374_ide_dma_end (ide_drive
 }
 
 /**
- *	hpt372n_set_clock	-	perform clock switching dance
- *	@drive: Drive to switch
- *	@mode: Switching mode (0x21 for write, 0x23 otherwise)
+ *	hpt3xxn_set_clock	-	perform clock switching dance
+ *	@hwif: hwif to switch
+ *	@mode: clocking mode (0x21 for write, 0x23 otherwise)
  *
- *	Switch the DPLL clock on the HPT372N devices. This is a
- *	right mess.
+ *	Switch the DPLL clock on the HPT3xxN devices. This is a	right mess.
+ *	NOTE: avoid touching the disabled primary channel on HPT371N -- it
+ *	doesn't physically exist anyway...
  */
- 
-static void hpt372n_set_clock(ide_drive_t *drive, int mode)
+
+static void hpt3xxn_set_clock(ide_hwif_t *hwif, u8 mode)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	
-	/* FIXME: should we check for DMA active and BUG() */
+	u8 mcr1, scr2 = hwif->INB(hwif->dma_master + 0x7b);
+
+	if ((scr2 & 0x7f) == mode)
+		return;
+
+	/* MISC. control register 1 has the channel enable bit... */
+	mcr1 = hwif->INB(hwif->dma_master + 0x70);
+
 	/* Tristate the bus */
-	outb(0x80, hwif->dma_base+0x73);
-	outb(0x80, hwif->dma_base+0x77);
-	
+	if (mcr1 & 0x04)
+		hwif->OUTB(0x80, hwif->dma_master + 0x73);
+	hwif->OUTB(0x80, hwif->dma_master + 0x77);
+
 	/* Switch clock and reset channels */
-	outb(mode, hwif->dma_base+0x7B);
-	outb(0xC0, hwif->dma_base+0x79);
-	
+	hwif->OUTB(mode, hwif->dma_master + 0x7b);
+	hwif->OUTB(0xc0, hwif->dma_master + 0x79);
+
 	/* Reset state machines */
-	outb(0x37, hwif->dma_base+0x70);
-	outb(0x37, hwif->dma_base+0x74);
-	
+	if (mcr1 & 0x04)
+		hwif->OUTB(0x37, hwif->dma_master + 0x70);
+	hwif->OUTB(0x37, hwif->dma_master + 0x74);
+
 	/* Complete reset */
-	outb(0x00, hwif->dma_base+0x79);
-	
+	hwif->OUTB(0x00, hwif->dma_master + 0x79);
+
 	/* Reconnect channels to bus */
-	outb(0x00, hwif->dma_base+0x73);
-	outb(0x00, hwif->dma_base+0x77);
+	if (mcr1 & 0x04)
+		hwif->OUTB(0x00, hwif->dma_master + 0x73);
+	hwif->OUTB(0x00, hwif->dma_master + 0x77);
 }
 
 /**
- *	hpt372n_rw_disk		-	prepare for I/O
+ *	hpt3xxn_rw_disk		-	prepare for I/O
  *	@drive: drive for command
  *	@rq: block request structure
  *
- *	This is called when a disk I/O is issued to the 372N.
+ *	This is called when a disk I/O is issued to HPT3xxN.
  *	We need it because of the clock switching.
  */
 
-static void hpt372n_rw_disk(ide_drive_t *drive, struct request *rq)
+static void hpt3xxn_rw_disk(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif = drive->hwif;
-	int wantclock;
-
-	wantclock = rq_data_dir(rq) ? 0x23 : 0x21;
+	ide_hwif_t *hwif	= HWIF(drive);
+	u8 wantclock		= rq_data_dir(rq) ? 0x23 : 0x21;
 
-	if (hwif->config_data != wantclock) {
-		hpt372n_set_clock(drive, wantclock);
-		hwif->config_data = wantclock;
-	}
+	hpt3xxn_set_clock(hwif, wantclock);
 }
 
 /*
@@ -1139,7 +1161,7 @@ static void __devinit hpt37x_clocking(id
 	int adjust, i;
 	u16 freq;
 	u32 pll;
-	u8 reg5bh;
+	u8 reg5bh = 0, mcr1 = 0;
 	
 	/*
 	 * default to pci clock. make sure MA15/16 are set to output
@@ -1162,17 +1184,11 @@ static void __devinit hpt37x_clocking(id
 	freq &= 0x1FF;
 	
 	/*
-	 * The 372N uses different PCI clock information and has
-	 * some other complications
-	 *	On PCI33 timing we must clock switch
-	 *	On PCI66 timing we must NOT use the PCI clock
-	 *
-	 * Currently we always set up the PLL for the 372N
+	 * HPT3xxN chips use different PCI clock information.
+	 * Currently we always set up the PLL for them.
 	 */
-	 
-	if(info->flags & IS_372N)
-	{
-		printk(KERN_INFO "hpt: HPT372N detected, using 372N timing.\n");
+
+	if (info->flags & IS_3xxN) {
 		if(freq < 0x55)
 			pll = F_LOW_PCI_33;
 		else if(freq < 0x70)
@@ -1181,10 +1197,8 @@ static void __devinit hpt37x_clocking(id
 			pll = F_LOW_PCI_50;
 		else
 			pll = F_LOW_PCI_66;
-			
-		printk(KERN_INFO "FREQ: %d PLL: %d\n", freq, pll);
-			
-		/* We always use the pll not the PCI clock on 372N */
+
+		printk(KERN_INFO "HPT3xxN detected, FREQ: %d, PLL: %d\n", freq, pll);
 	}
 	else
 	{
@@ -1232,7 +1246,10 @@ static void __devinit hpt37x_clocking(id
 			printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
 		}
 	}
-	
+
+	if (pll == F_LOW_PCI_66)
+		info->flags |= PCI_66MHZ;
+
 	/*
 	 * only try the pll if we don't have a table for the clock
 	 * speed that we're running at. NOTE: the internal PLL will
@@ -1288,10 +1305,6 @@ static void __devinit hpt37x_clocking(id
 				goto init_hpt37X_done;
 			}
 		}
-		if (!pci_get_drvdata(dev)) {
-			printk("No Clock Stabilization!!!\n");
-			return;
-		}
 pll_recal:
 		if (adjust & 1)
 			pll -= (adjust >> 1);
@@ -1301,11 +1314,16 @@ pll_recal:
 
 init_hpt37X_done:
 	if (!info->speed)
-		printk(KERN_ERR "HPT37X%s: unknown bus timing [%d %d].\n",
-			(info->flags & IS_372N)?"N":"", pll, freq);
-	/* reset state engine */
-	pci_write_config_byte(dev, 0x50, 0x37); 
-	pci_write_config_byte(dev, 0x54, 0x37); 
+		printk(KERN_ERR "HPT37x%s: unknown bus timing [%d %d].\n",
+		       (info->flags & IS_3xxN) ? "N" : "", pll, freq);
+	/*
+	 * Reset the state engines.
+	 * NOTE: avoid accidentally enabling the primary channel on HPT371N.
+	 */
+	pci_read_config_byte(dev, 0x50, &mcr1);
+	if (mcr1 & 0x04)
+		pci_write_config_byte(dev, 0x50, 0x37);
+	pci_write_config_byte(dev, 0x54, 0x37);
 	udelay(100);
 }
 
@@ -1368,6 +1386,7 @@ static void __devinit init_hwif_hpt366(i
 	struct pci_dev *dev		= hwif->pci_dev;
 	struct hpt_info *info		= ide_get_hwifdata(hwif);
 	u8 ata66 = 0, regmask		= (hwif->channel) ? 0x01 : 0x02;
+	int serialize			= HPT_SERIALIZE_IO;
 	
 	hwif->tuneproc			= &hpt3xx_tune_drive;
 	hwif->speedproc			= &hpt3xx_tune_chipset;
@@ -1375,8 +1394,20 @@ static void __devinit init_hwif_hpt366(i
 	hwif->intrproc			= &hpt3xx_intrproc;
 	hwif->maskproc			= &hpt3xx_maskproc;
 	
-	if(info->flags & IS_372N)
-		hwif->rw_disk = &hpt372n_rw_disk;
+	/*
+	 * HPT3xxN chips have some complications:
+	 *
+	 * - on 33 MHz PCI we must clock switch
+	 * - on 66 MHz PCI we must NOT use the PCI clock
+	 */
+	if ((info->flags & (IS_3xxN | PCI_66MHZ)) == IS_3xxN) {
+		/*
+		 * Clock is shared between the channels,
+		 * so we'll have to serialize them... :-(
+		 */
+		serialize = 1;
+		hwif->rw_disk = &hpt3xxn_rw_disk;
+	}
 
 	/*
 	 * The HPT37x uses the CBLID pins as outputs for MA15/MA16
@@ -1419,11 +1450,9 @@ static void __devinit init_hwif_hpt366(i
 		PCI_FUNC(hwif->pci_dev->devfn));
 #endif /* DEBUG */
 
-#ifdef HPT_SERIALIZE_IO
-	/* serialize access to this device */
-	if (hwif->mate)
+	/* Serialize access to this device */
+	if (serialize && hwif->mate)
 		hwif->serialized = hwif->mate->serialized = 1;
-#endif
 
 	if (info->revision >= 3) {
 		u8 reg5ah = 0;
@@ -1491,7 +1520,7 @@ static void __devinit init_dma_hpt366(id
 		return;
 		
 	if(info->speed == NULL) {
-		printk(KERN_WARNING "hpt: no known IDE timings, disabling DMA.\n");
+		printk(KERN_WARNING "hpt366: no known IDE timings, disabling DMA.\n");
 		return;
 	}
 
@@ -1520,9 +1549,10 @@ static void __devinit init_dma_hpt366(id
 
 static void __devinit init_iops_hpt366(ide_hwif_t *hwif)
 {
-	struct hpt_info *info = kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
-	unsigned long dmabase = pci_resource_start(hwif->pci_dev, 4);
-	u8 did, rid;
+	struct hpt_info *info	= kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	u16 did			= dev->device;
+	u8  rid			= 0;
 
 	if(info == NULL) {
 		printk(KERN_WARNING "hpt366: out of memory.\n");
@@ -1530,15 +1560,22 @@ static void __devinit init_iops_hpt366(i
 	}
 	ide_set_hwifdata(hwif, info);
 
-	if(dmabase) {
-		did = inb(dmabase + 0x22);
-		rid = inb(dmabase + 0x28);
-
-		if((did == 4 && rid == 6) || (did == 5 && rid > 1))
-			info->flags |= IS_372N;
+	/* Avoid doing the same thing twice. */
+	if (hwif->channel && hwif->mate) {
+		memcpy(info, ide_get_hwifdata(hwif->mate), sizeof(struct hpt_info));
+		return;
 	}
 
-	info->revision = hpt_revision(hwif->pci_dev);
+	pci_read_config_byte(dev, PCI_CLASS_REVISION, &rid);
+
+	if (( did == PCI_DEVICE_ID_TTI_HPT366  && rid == 6) ||
+	    ((did == PCI_DEVICE_ID_TTI_HPT372  ||
+	      did == PCI_DEVICE_ID_TTI_HPT302  ||
+	      did == PCI_DEVICE_ID_TTI_HPT371) && rid > 1) ||
+	      did == PCI_DEVICE_ID_TTI_HPT372N)
+		info->flags |= IS_3xxN;
+
+	info->revision = hpt_revision(dev);
 
 	if (info->revision >= 3)
 		hpt37x_clocking(hwif);
@@ -1575,6 +1612,23 @@ static int __devinit init_setup_hpt37x(s
 	return ide_setup_pci_device(dev, d);
 }
 
+static int __devinit init_setup_hpt371(struct pci_dev *dev, ide_pci_device_t *d)
+{
+	u8 mcr1 = 0;
+
+	/*
+	 * HPT371 chips physically have only one channel, the secondary one,
+	 * but the primary channel registers do exist!  Go figure...
+	 * So,  we manually disable the non-existing channel here
+	 * (if the BIOS hasn't done this already).
+	 */
+	pci_read_config_byte(dev, 0x50, &mcr1);
+	if (mcr1 & 0x04)
+		pci_write_config_byte(dev, 0x50, (mcr1 & ~0x04));
+
+	return ide_setup_pci_device(dev, d);
+}
+
 static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
 {
 	struct pci_dev *findev = NULL;
@@ -1662,13 +1716,14 @@ static ide_pci_device_t hpt366_chipsets[
 		.bootable	= OFF_BOARD,
 	},{	/* 3 */
 		.name		= "HPT371",
-		.init_setup	= init_setup_hpt37x,
+		.init_setup	= init_setup_hpt371,
 		.init_chipset	= init_chipset_hpt366,
 		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 	},{	/* 4 */
 		.name		= "HPT374",



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

* [PATCH][RFT] Fix HPT37x timing tables
  2006-04-23  8:33 [PATCH][RFT] HPT3xxN clocking fixes Sergei Shtylyov
  2006-04-24 22:09 ` [PATCH][RFT] HPT3xxN clocking fixes (take 2) Sergei Shtylyov
@ 2006-05-02 21:55 ` Sergei Shtylyov
  2006-05-06 20:08   ` [PATCH] Optimize " Sergei Shtylyov
  2006-05-02 22:14 ` [PATCH][RFT] Fix HPT3xx hotswap support Sergei Shtylyov
  2006-05-02 22:26 ` [PATCH] Fix the case of multiple HPT3xx chips present Sergei Shtylyov
  3 siblings, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-02 21:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

Hello.

    Fix/remove bad/unused timing tables: HPT370/A 66 MHz tables weren't really
needed (the chips are not UltraATA/133 capable and shouldn't support 66 MHz
PCI) and had many modes over- and underclocked, HPT372 33 MHz table was in
fact for 66 MHz and 50 MHz table missed UltraDMA mode 6, HPT374 33 MHz table
was really for 50 MHz... (Actually, HPT370/A 33 MHz tables also have issues.
e.g. HPT370 has PIO modes 0/1 overlocked.)
    There's also no need in the separate HPT374 tables because HPT372 timings
should be the same (and those tables has UltraDMA mode 6 which HPT374 supports
depending on HPT374_ALLOW_ATA133_6 #define)...
    Have been tested and works fine on HPT370 and 371N...

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>




[-- Attachment #2: HPT37x-fix-timing-tables.patch --]
[-- Type: text/plain, Size: 7184 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -67,6 +67,10 @@
  * - add support for HPT302N and HPT371N clocking (the same as for HPT372N)
  * - HPT371/N are single channel chips, so avoid touching the primary channel
  *   which exists only virtually (there's no pins for it)
+ * - fix/remove bad/unused timing tables: HPT370/A  66 MHz tables weren't really
+ *   needed and had many modes over- and  underclocked,  HPT372 33 MHz table was
+ *   for 66 MHz and 50 MHz table missed UltraDMA mode 6, HPT374 33 MHz table was
+ *   really for 50 MHz; switch to using HPT372 tables for HPT374...
  *		<source@mvista.com>
  *
  */
@@ -265,26 +269,6 @@ static struct chipset_bus_clock_list_ent
 	{	0,		0x06514e57	}
 };
 
-static struct chipset_bus_clock_list_entry sixty_six_base_hpt370[] = {
-	{	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 struct chipset_bus_clock_list_entry thirty_three_base_hpt370a[] = {
 	{	XFER_UDMA_5,	0x12446231	},
@@ -306,27 +290,6 @@ static struct chipset_bus_clock_list_ent
 	{	0,		0x06814ea7	}
 };
 
-/* 2x 33MHz timings */
-static struct chipset_bus_clock_list_entry sixty_six_base_hpt370a[] = {
-	{	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 struct chipset_bus_clock_list_entry fifty_base_hpt370a[] = {
 	{	XFER_UDMA_5,	0x12848242	},
 	{	XFER_UDMA_4,	0x12ac8242	},
@@ -348,27 +311,28 @@ static struct chipset_bus_clock_list_ent
 };
 
 static struct chipset_bus_clock_list_entry thirty_three_base_hpt372[] = {
-	{	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_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_PIO_4,	0x0c829c62	},
-	{	XFER_PIO_3,	0x0c829c84	},
-	{	XFER_PIO_2,	0x0c829ca6	},
-	{	XFER_PIO_1,	0x0d029d26	},
-	{	XFER_PIO_0,	0x0d029d5e	},
-	{	0,		0x0d029d5e	}
+	{	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	}
 };
 
 static struct chipset_bus_clock_list_entry fifty_base_hpt372[] = {
+	{	XFER_UDMA_6,	0x12848242	},
 	{	XFER_UDMA_5,	0x12848242	},
 	{	XFER_UDMA_4,	0x12ac8242	},
 	{	XFER_UDMA_3,	0x128c8242	},
@@ -390,7 +354,7 @@ static struct chipset_bus_clock_list_ent
 
 static struct chipset_bus_clock_list_entry sixty_six_base_hpt372[] = {
 	{	XFER_UDMA_6,	0x1c869c62	},
-	{	XFER_UDMA_5,	0x1cae9c62	},
+	{	XFER_UDMA_5,	0x1cae9c62	},	/* 0x1c8a9c62 */
 	{	XFER_UDMA_4,	0x1c8a9c62	},
 	{	XFER_UDMA_3,	0x1c8e9c62	},
 	{	XFER_UDMA_2,	0x1c929c62	},
@@ -409,50 +373,6 @@ static struct chipset_bus_clock_list_ent
 	{	0,		0x0d029d26	}
 };
 
-static struct chipset_bus_clock_list_entry thirty_three_base_hpt374[] = {
-	{	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	}
-};
-
-/* FIXME: 50MHz timings for HPT374 */
-
-#if 0
-static struct chipset_bus_clock_list_entry sixty_six_base_hpt374[] = {
-	{	XFER_UDMA_6,	0x12406231	},	/* checkme */
-	{	XFER_UDMA_5,	0x12446231	},	/* 0x14846231 */
-	{	XFER_UDMA_4,	0x16814ea7	},	/* 0x14886231 */
-	{	XFER_UDMA_3,	0x16814ea7	},	/* 0x148c6231 */
-	{	XFER_UDMA_2,	0x16814ea7	},	/* 0x148c6231 */
-	{	XFER_UDMA_1,	0x16814ea7	},	/* 0x14906231 */
-	{	XFER_UDMA_0,	0x16814ea7	},	/* 0x14986231 */
-	{	XFER_MW_DMA_2,	0x16814ea7	},	/* 0x26514e21 */
-	{	XFER_MW_DMA_1,	0x16814ea7	},	/* 0x26514e97 */
-	{	XFER_MW_DMA_0,	0x16814ea7	},	/* 0x26514e97 */
-	{	XFER_PIO_4,	0x06814ea7	},	/* 0x06514e21 */
-	{	XFER_PIO_3,	0x06814ea7	},	/* 0x06514e22 */
-	{	XFER_PIO_2,	0x06814ea7	},	/* 0x06514e33 */
-	{	XFER_PIO_1,	0x06814ea7	},	/* 0x06914e43 */
-	{	XFER_PIO_0,	0x06814ea7	},	/* 0x06914e57 */
-	{	0,		0x06814ea7	}
-};
-#endif
-
 #define HPT366_DEBUG_DRIVE_INFO		0
 #define HPT374_ALLOW_ATA133_6		0
 #define HPT371_ALLOW_ATA133_6		0
@@ -1212,9 +1132,7 @@ static void __devinit hpt37x_clocking(id
 			pll = F_LOW_PCI_66;
 	
 		if (pll == F_LOW_PCI_33) {
-			if (info->revision >= 8)
-				info->speed = thirty_three_base_hpt374;
-			else if (info->revision >= 5)
+			if (info->revision >= 5)
 				info->speed = thirty_three_base_hpt372;
 			else if (info->revision >= 4)
 				info->speed = thirty_three_base_hpt370a;
@@ -1224,26 +1142,17 @@ static void __devinit hpt37x_clocking(id
 		} else if (pll == F_LOW_PCI_40) {
 			/* Unsupported */
 		} else if (pll == F_LOW_PCI_50) {
-			if (info->revision >= 8)
-				info->speed = fifty_base_hpt370a;
-			else if (info->revision >= 5)
+			if (info->revision >= 5)
 				info->speed = fifty_base_hpt372;
-			else if (info->revision >= 4)
-				info->speed = fifty_base_hpt370a;
 			else
 				info->speed = fifty_base_hpt370a;
 			printk(KERN_DEBUG "HPT37X: using 50MHz PCI clock\n");
 		} else {
-			if (info->revision >= 8) {
-				printk(KERN_ERR "HPT37x: 66MHz timings are not supported.\n");
-			}
-			else if (info->revision >= 5)
+			if (info->revision >= 5) {
 				info->speed = sixty_six_base_hpt372;
-			else if (info->revision >= 4)
-				info->speed = sixty_six_base_hpt370a;
-			else
-				info->speed = sixty_six_base_hpt370;
-			printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
+				printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
+			} else
+				printk(KERN_ERR "HPT37x: 66MHz timings not supported.\n");
 		}
 	}
 




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

* [PATCH][RFT] Fix HPT3xx hotswap support
  2006-04-23  8:33 [PATCH][RFT] HPT3xxN clocking fixes Sergei Shtylyov
  2006-04-24 22:09 ` [PATCH][RFT] HPT3xxN clocking fixes (take 2) Sergei Shtylyov
  2006-05-02 21:55 ` [PATCH][RFT] Fix HPT37x timing tables Sergei Shtylyov
@ 2006-05-02 22:14 ` Sergei Shtylyov
  2006-05-02 22:26 ` [PATCH] Fix the case of multiple HPT3xx chips present Sergei Shtylyov
  3 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-02 22:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

Hello.

    Fix the broken hotswap code: on HPT37x it caused RESET- to glitch  when
tristating the bus (the MISC control 3/6 and soft control 2 need to be
written to in the certain order), and for HPT36x the obsolete
HDIO_TRISTATE_HWIF ioctl() handler was called instead which treated the state
argument wrong. Also, get rid of the soft control reg. 1 wtite to enable IDE
interrupt -- this is done in init_hpt37x() already...
    Have been tested on HPT370 and 371N. Should apply on top of the HPT37x
timing table fix.

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-fix-hotswap.patch --]
[-- Type: text/plain, Size: 5394 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -71,6 +71,8 @@
  *   needed and had many modes over- and  underclocked,  HPT372 33 MHz table was
  *   for 66 MHz and 50 MHz table missed UltraDMA mode 6, HPT374 33 MHz table was
  *   really for 50 MHz; switch to using HPT372 tables for HPT374...
+ * - fix the hotswap code:  it caused RESET- to glitch when tristating the bus,
+ *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
  *		<source@mvista.com>
  *
  */
@@ -954,101 +956,68 @@ static void hpt3xxn_rw_disk(ide_drive_t 
 	hpt3xxn_set_clock(hwif, wantclock);
 }
 
-/*
- * Since SUN Cobalt is attempting to do this operation, I should disclose
- * this has been a long time ago Thu Jul 27 16:40:57 2000 was the patch date
- * HOTSWAP ATA Infrastructure.
- */
-
-static void hpt3xx_reset (ide_drive_t *drive)
-{
-}
-
-static int hpt3xx_tristate (ide_drive_t * drive, int state)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	struct pci_dev *dev	= hwif->pci_dev;
-	u8 reg59h = 0, reset	= (hwif->channel) ? 0x80 : 0x40;
-	u8 regXXh = 0, state_reg= (hwif->channel) ? 0x57 : 0x53;
-
-	pci_read_config_byte(dev, 0x59, &reg59h);
-	pci_read_config_byte(dev, state_reg, &regXXh);
-
-	if (state) {
-		(void) ide_do_reset(drive);
-		pci_write_config_byte(dev, state_reg, regXXh|0x80);
-		pci_write_config_byte(dev, 0x59, reg59h|reset);
-	} else {
-		pci_write_config_byte(dev, 0x59, reg59h & ~(reset));
-		pci_write_config_byte(dev, state_reg, regXXh & ~(0x80));
-		(void) ide_do_reset(drive);
-	}
-	return 0;
-}
-
 /* 
- * set/get power state for a drive.
- * turning the power off does the following things:
- *   1) soft-reset the drive
- *   2) tri-states the ide bus
+ * Set/get power state for a drive.
  *
- * when we turn things back on, we need to re-initialize things.
+ * When we turn the power back on, we need to re-initialize things.
  */
 #define TRISTATE_BIT  0x8000
-static int hpt370_busproc(ide_drive_t * drive, int state)
+
+static int hpt3xx_busproc(ide_drive_t *drive, int state)
 {
 	ide_hwif_t *hwif	= drive->hwif;
 	struct pci_dev *dev	= hwif->pci_dev;
-	u8 tristate = 0, resetmask = 0, bus_reg = 0;
-	u16 tri_reg;
+	u8  tristate, resetmask, bus_reg = 0;
+	u16 tri_reg = 0;
 
 	hwif->bus_state = state;
 
 	if (hwif->channel) { 
 		/* secondary channel */
-		tristate = 0x56;
-		resetmask = 0x80; 
+		tristate  = 0x56;
+		resetmask = 0x80;
 	} else { 
 		/* primary channel */
-		tristate = 0x52;
+		tristate  = 0x52;
 		resetmask = 0x40;
 	}
 
-	/* grab status */
+	/* Grab the status. */
 	pci_read_config_word(dev, tristate, &tri_reg);
 	pci_read_config_byte(dev, 0x59, &bus_reg);
 
-	/* set the state. we don't set it if we don't need to do so.
-	 * make sure that the drive knows that it has failed if it's off */
+	/*
+	 * Set the state. We don't set it if we don't need to do so.
+	 * Make sure that the drive knows that it has failed if it's off.
+	 */
 	switch (state) {
 	case BUSSTATE_ON:
-		hwif->drives[0].failures = 0;
-		hwif->drives[1].failures = 0;
-		if ((bus_reg & resetmask) == 0)
+		if (!(bus_reg & resetmask))
 			return 0;
-		tri_reg &= ~TRISTATE_BIT;
-		bus_reg &= ~resetmask;
-		break;
+		hwif->drives[0].failures = hwif->drives[1].failures = 0;
+
+		pci_write_config_byte(dev, 0x59, bus_reg & ~resetmask);
+		pci_write_config_word(dev, tristate, tri_reg & ~TRISTATE_BIT);
+		return 0;
 	case BUSSTATE_OFF:
-		hwif->drives[0].failures = hwif->drives[0].max_failures + 1;
-		hwif->drives[1].failures = hwif->drives[1].max_failures + 1;
-		if ((tri_reg & TRISTATE_BIT) == 0 && (bus_reg & resetmask))
+		if ((bus_reg & resetmask) && !(tri_reg & TRISTATE_BIT))
 			return 0;
 		tri_reg &= ~TRISTATE_BIT;
-		bus_reg |= resetmask;
 		break;
 	case BUSSTATE_TRISTATE:
-		hwif->drives[0].failures = hwif->drives[0].max_failures + 1;
-		hwif->drives[1].failures = hwif->drives[1].max_failures + 1;
-		if ((tri_reg & TRISTATE_BIT) && (bus_reg & resetmask))
+		if ((bus_reg & resetmask) &&  (tri_reg & TRISTATE_BIT))
 			return 0;
 		tri_reg |= TRISTATE_BIT;
-		bus_reg |= resetmask;
 		break;
+	default:
+		return -EINVAL;
 	}
-	pci_write_config_byte(dev, 0x59, bus_reg);
-	pci_write_config_word(dev, tristate, tri_reg);
 
+	hwif->drives[0].failures = hwif->drives[0].max_failures + 1;
+	hwif->drives[1].failures = hwif->drives[1].max_failures + 1;
+
+	pci_write_config_word(dev, tristate, tri_reg);
+	pci_write_config_byte(dev, 0x59, bus_reg | resetmask);
 	return 0;
 }
 
@@ -1363,23 +1332,11 @@ static void __devinit init_hwif_hpt366(i
 	if (serialize && hwif->mate)
 		hwif->serialized = hwif->mate->serialized = 1;
 
-	if (info->revision >= 3) {
-		u8 reg5ah = 0;
-			pci_write_config_byte(dev, 0x5a, reg5ah & ~0x10);
-		/*
-		 * set up ioctl for power status.
-		 * note: power affects both
-		 * drives on each channel
-		 */
-		hwif->resetproc	= &hpt3xx_reset;
-		hwif->busproc	= &hpt370_busproc;
-	} else if (info->revision >= 2) {
-		hwif->resetproc	= &hpt3xx_reset;
-		hwif->busproc	= &hpt3xx_tristate;
-	} else {
-		hwif->resetproc = &hpt3xx_reset;
-		hwif->busproc   = &hpt3xx_tristate;
-	}
+	/*
+	 * Set up ioctl for power status.
+	 * NOTE:  power affects both drives on each channel.
+	 */
+	hwif->busproc = &hpt3xx_busproc;
 
 	if (!hwif->dma_base) {
 		hwif->drives[0].autotune = 1;




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

* [PATCH] Fix the case of multiple HPT3xx chips present
  2006-04-23  8:33 [PATCH][RFT] HPT3xxN clocking fixes Sergei Shtylyov
                   ` (2 preceding siblings ...)
  2006-05-02 22:14 ` [PATCH][RFT] Fix HPT3xx hotswap support Sergei Shtylyov
@ 2006-05-02 22:26 ` Sergei Shtylyov
  2006-05-04 19:46   ` [PATCH] HPT3xx: fix PCI clock detection Sergei Shtylyov
  3 siblings, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-02 22:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

Hello.

    init_chipset_hpt366() modifies some fields of the ide_pci_device_t 
structure depending on the chip's revision, so pass it a copy of the structure 
to avoid issues when multiple different chips are present.
    Should apply on top of the hotswap fix.

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-fix-case-of-multiple-chips.patch --]
[-- Type: text/plain, Size: 1291 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -73,6 +73,8 @@
  *   really for 50 MHz; switch to using HPT372 tables for HPT374...
  * - fix the hotswap code:  it caused RESET- to glitch when tristating the bus,
  *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
+ * - pass to init_chipset() handlers a copy of the IDE PCI device structure as
+ *   they tamper with its fields
  *		<source@mvista.com>
  *
  */
@@ -1621,13 +1623,16 @@ static ide_pci_device_t hpt366_chipsets[
  *
  *	Called when the PCI registration layer (or the IDE initialization)
  *	finds a device matching our IDE device tables.
+ *
+ *	NOTE: since we'll have to modify some fields of the ide_pci_device_t
+ *	structure depending on the chip's revision, we'd better pass a local
+ *	copy down the call chain...
  */
- 
 static int __devinit hpt366_init_one(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	ide_pci_device_t *d = &hpt366_chipsets[id->driver_data];
+	ide_pci_device_t d = hpt366_chipsets[id->driver_data];
 
-	return d->init_setup(dev, d);
+	return d.init_setup(dev, &d);
 }
 
 static struct pci_device_id hpt366_pci_tbl[] = {



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

* Re: [PATCH] HPT3xx: fix PCI clock detection
  2006-05-02 22:26 ` [PATCH] Fix the case of multiple HPT3xx chips present Sergei Shtylyov
@ 2006-05-04 19:46   ` Sergei Shtylyov
  2006-05-16 22:44     ` [PATCH] HPT3xx: rework rate filtering Sergei Shtylyov
  2006-05-20  8:41     ` [PATCH] HPT37x: read f_CNT saved by BIOS from port Sergei Shtylyov
  0 siblings, 2 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-04 19:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

    Use the f_CNT value saved by the HighPoint BIOS if available as reading it 
directly would give us a wrong PCI frequency after DPLL has already been 
calibrated by BIOS.

    Should apply on top of my recent patches.

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-use-f_CNT-saved-by-BIOS.patch --]
[-- Type: text/plain, Size: 2922 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -71,6 +71,8 @@
  *   needed and had many modes over- and  underclocked,  HPT372 33 MHz table was
  *   for 66 MHz and 50 MHz table missed UltraDMA mode 6, HPT374 33 MHz table was
  *   really for 50 MHz; switch to using HPT372 tables for HPT374...
+ * - use f_CNT value saved by  the HighPoint BIOS as reading it directly gives
+ *   the wrong PCI frequency since DPLL has already been calibrated by BIOS
  * - fix the hotswap code:  it caused RESET- to glitch when tristating the bus,
  *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
  * - pass to init_chipset() handlers a copy of the IDE PCI device structure as
@@ -1050,8 +1052,8 @@ static void __devinit hpt37x_clocking(id
 	struct hpt_info *info = ide_get_hwifdata(hwif);
 	struct pci_dev *dev = hwif->pci_dev;
 	int adjust, i;
-	u16 freq;
-	u32 pll;
+	u16 freq = 0;
+	u32 pll, temp = 0;
 	u8 reg5bh = 0, mcr1 = 0;
 	
 	/*
@@ -1065,15 +1067,34 @@ static void __devinit hpt37x_clocking(id
 	pci_write_config_byte(dev, 0x5b, 0x23);
 
 	/*
-	 * set up the PLL. we need to adjust it so that it's stable. 
-	 * freq = Tpll * 192 / Tpci
+	 * We'll have to read f_CNT value in order to determine
+	 * the PCI clock frequency according to the following ratio:
 	 *
-	 * Todo. For non x86 should probably check the dword is
-	 * set to 0xABCDExxx indicating the BIOS saved f_CNT
+	 * f_CNT = Fpci * 192 / Fdpll
+	 *
+	 * First try reading the register in which the HighPoint BIOS
+	 * saves f_CNT value before  reprogramming the DPLL from its
+	 * default setting (which differs for the various chips).
+	 * In case the signature check fails, we'll have to resort to
+	 * reading the f_CNT register itself in hopes that nobody has
+	 * touched the DPLL yet...
 	 */
-	pci_read_config_word(dev, 0x78, &freq);
-	freq &= 0x1FF;
-	
+	pci_read_config_dword(dev, 0x70, &temp);
+	if ((temp & 0xFFFFF000) != 0xABCDE000) {
+		int i;
+
+		printk(KERN_WARNING "HPT37X: no clock data saved by BIOS\n");
+
+		/* Calculate the average value of f_CNT */
+		for (temp = i = 0; i < 128; i++) {
+			pci_read_config_word(dev, 0x78, &freq);
+			temp += freq & 0x1ff;
+			mdelay(1);
+		}
+		freq = temp / 128;
+	} else
+		freq = temp & 0x1ff;
+
 	/*
 	 * HPT3xxN chips use different PCI clock information.
 	 * Currently we always set up the PLL for them.
@@ -1146,11 +1167,8 @@ static void __devinit hpt37x_clocking(id
 	info->flags |= PLL_MODE;
 	
 	/*
-	 * FIXME: make this work correctly, esp with 372N as per
-	 * reference driver code.
-	 *
-	 * adjust PLL based upon PCI clock, enable it, and wait for
-	 * stabilization.
+	 * Adjust the PLL based upon the PCI clock, enable it, and
+	 * wait for stabilization...
 	 */
 	adjust = 0;
 	freq = (pll < F_LOW_PCI_50) ? 2 : 4;


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

* [PATCH] Optimize HPT37x timing tables
  2006-05-02 21:55 ` [PATCH][RFT] Fix HPT37x timing tables Sergei Shtylyov
@ 2006-05-06 20:08   ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-06 20:08 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

Hello.

    Save some space on the timing tables by introducing the separate transfer
mode table in which the mode lookup is done to get the index into the timing
table itself. Get rid of the rest of the obsolete/duplicate tables and use one
set of tables for the whole HPT37x chip family like the HighPoint open-source
drivers do. Documnent the different timing register layout for the HPT36x chip 
family (this is my guesswork based on the timing values).
    Have been tested and works fine on HPT370/302/371N. Should apply on top of
the previously sent patches...

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>



[-- Attachment #2: HPT3xx-optimize-timing-tables.patch --]
[-- Type: text/plain, Size: 14911 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -67,10 +67,9 @@
  * - add support for HPT302N and HPT371N clocking (the same as for HPT372N)
  * - HPT371/N are single channel chips, so avoid touching the primary channel
  *   which exists only virtually (there's no pins for it)
- * - fix/remove bad/unused timing tables: HPT370/A  66 MHz tables weren't really
- *   needed and had many modes over- and  underclocked,  HPT372 33 MHz table was
- *   for 66 MHz and 50 MHz table missed UltraDMA mode 6, HPT374 33 MHz table was
- *   really for 50 MHz; switch to using HPT372 tables for HPT374...
+ * - fix/remove bad/unused timing tables and use one set of tables for the whole
+ *   HPT37x chip family; save space by introducing the separate transfer mode
+ *   table in which the mode lookup is done
  * - use f_CNT value saved by  the HighPoint BIOS as reading it directly gives
  *   the wrong PCI frequency since DPLL has already been calibrated by BIOS
  * - fix the hotswap code:  it caused RESET- to glitch when tristating the bus,
@@ -169,214 +168,168 @@ static const char *bad_ata33[] = {
 	NULL
 };
 
-struct chipset_bus_clock_list_entry {
-	u8		xfer_speed;
-	unsigned int	chipset_settings;
+static u8 xfer_speeds[] = {
+	XFER_UDMA_6,
+	XFER_UDMA_5,
+	XFER_UDMA_4,
+	XFER_UDMA_3,
+	XFER_UDMA_2,
+	XFER_UDMA_1,
+	XFER_UDMA_0,
+
+	XFER_MW_DMA_2,
+	XFER_MW_DMA_1,
+	XFER_MW_DMA_0,
+
+	XFER_PIO_4,
+	XFER_PIO_3,
+	XFER_PIO_2,
+	XFER_PIO_1,
+	XFER_PIO_0
 };
 
-/* key for bus clock timings
- * bit
- * 0:3    data_high_time. inactive time of DIOW_/DIOR_ for PIO and MW
- *        DMA. cycles = value + 1
- * 4:8    data_low_time. active time of DIOW_/DIOR_ for PIO and MW
- *        DMA. cycles = value + 1
- * 9:12   cmd_high_time. inactive time of DIOW_/DIOR_ during task file
- *        register access.
- * 13:17  cmd_low_time. active time of DIOW_/DIOR_ during task file
- *        register access.
- * 18:21  udma_cycle_time. clock freq and clock cycles for UDMA xfer.
- *        during task file register access.
- * 22:24  pre_high_time. time to initialize 1st cycle for PIO and MW DMA
- *        xfer.
- * 25:27  cmd_pre_high_time. time to initialize 1st PIO cycle for task
- *        register access.
- * 28     UDMA enable
- * 29     DMA enable
- * 30     PIO_MST enable. if set, the chip is in bus master mode during
- *        PIO.
- * 31     FIFO enable.
+/* Key for bus clock timings
+ * 36x   37x
+ * bits  bits
+ * 0:3	 0:3	data_high_time. Inactive time of DIOW_/DIOR_ for PIO and MW DMA.
+ *		cycles = value + 1
+ * 4:7	 4:8	data_low_time. Active time of DIOW_/DIOR_ for PIO and MW DMA.
+ *		cycles = value + 1
+ * 8:11  9:12	cmd_high_time. Inactive time of DIOW_/DIOR_ during task file
+ *		register access.
+ * 12:15 13:17	cmd_low_time. Active time of DIOW_/DIOR_ during task file
+ *		register access.
+ * 16:18 18:20	udma_cycle_time. Clock cycles for UDMA xfer.
+ * -	 21	CLK frequency: 0=ATA clock, 1=dual ATA clock.
+ * 19:21 22:24	pre_high_time. Time to initialize the 1st cycle for PIO and
+ *		MW DMA xfer.
+ * 22:24 25:27	cmd_pre_high_time. Time to initialize the 1st PIO cycle for
+ *		task file register access.
+ * 28	 28	UDMA enable.
+ * 29	 29	DMA  enable.
+ * 30	 30	PIO MST enable. If set, the chip is in bus master mode during
+ *		PIO xfer.
+ * 31	 31	FIFO enable.
  */
-static struct chipset_bus_clock_list_entry forty_base_hpt366[] = {
-	{	XFER_UDMA_4,	0x900fd943	},
-	{	XFER_UDMA_3,	0x900ad943	},
-	{	XFER_UDMA_2,	0x900bd943	},
-	{	XFER_UDMA_1,	0x9008d943	},
-	{	XFER_UDMA_0,	0x9008d943	},
-
-	{	XFER_MW_DMA_2,	0xa008d943	},
-	{	XFER_MW_DMA_1,	0xa010d955	},
-	{	XFER_MW_DMA_0,	0xa010d9fc	},
-
-	{	XFER_PIO_4,	0xc008d963	},
-	{	XFER_PIO_3,	0xc010d974	},
-	{	XFER_PIO_2,	0xc010d997	},
-	{	XFER_PIO_1,	0xc010d9c7	},
-	{	XFER_PIO_0,	0xc018d9d9	},
-	{	0,		0x0120d9d9	}
-};
-
-static struct chipset_bus_clock_list_entry thirty_three_base_hpt366[] = {
-	{	XFER_UDMA_4,	0x90c9a731	},
-	{	XFER_UDMA_3,	0x90cfa731	},
-	{	XFER_UDMA_2,	0x90caa731	},
-	{	XFER_UDMA_1,	0x90cba731	},
-	{	XFER_UDMA_0,	0x90c8a731	},
-
-	{	XFER_MW_DMA_2,	0xa0c8a731	},
-	{	XFER_MW_DMA_1,	0xa0c8a732	},	/* 0xa0c8a733 */
-	{	XFER_MW_DMA_0,	0xa0c8a797	},
-
-	{	XFER_PIO_4,	0xc0c8a731	},
-	{	XFER_PIO_3,	0xc0c8a742	},
-	{	XFER_PIO_2,	0xc0d0a753	},
-	{	XFER_PIO_1,	0xc0d0a7a3	},	/* 0xc0d0a793 */
-	{	XFER_PIO_0,	0xc0d0a7aa	},	/* 0xc0d0a7a7 */
-	{	0,		0x0120a7a7	}
-};
 
-static struct chipset_bus_clock_list_entry twenty_five_base_hpt366[] = {
-	{	XFER_UDMA_4,	0x90c98521	},
-	{	XFER_UDMA_3,	0x90cf8521	},
-	{	XFER_UDMA_2,	0x90cf8521	},
-	{	XFER_UDMA_1,	0x90cb8521	},
-	{	XFER_UDMA_0,	0x90cb8521	},
-
-	{	XFER_MW_DMA_2,	0xa0ca8521	},
-	{	XFER_MW_DMA_1,	0xa0ca8532	},
-	{	XFER_MW_DMA_0,	0xa0ca8575	},
-
-	{	XFER_PIO_4,	0xc0ca8521	},
-	{	XFER_PIO_3,	0xc0ca8532	},
-	{	XFER_PIO_2,	0xc0ca8542	},
-	{	XFER_PIO_1,	0xc0d08572	},
-	{	XFER_PIO_0,	0xc0d08585	},
-	{	0,		0x01208585	}
+static u32 forty_base_hpt36x[] = {
+	/* XFER_UDMA_6 */	0x900fd943,
+	/* XFER_UDMA_5 */	0x900fd943,
+	/* XFER_UDMA_4 */	0x900fd943,
+	/* XFER_UDMA_3 */	0x900ad943,
+	/* XFER_UDMA_2 */	0x900bd943,
+	/* XFER_UDMA_1 */	0x9008d943,
+	/* XFER_UDMA_0 */	0x9008d943,
+
+	/* XFER_MW_DMA_2 */	0xa008d943,
+	/* XFER_MW_DMA_1 */	0xa010d955,
+	/* XFER_MW_DMA_0 */	0xa010d9fc,
+
+	/* XFER_PIO_4 */	0xc008d963,
+	/* XFER_PIO_3 */	0xc010d974,
+	/* XFER_PIO_2 */	0xc010d997,
+	/* XFER_PIO_1 */	0xc010d9c7,
+	/* XFER_PIO_0 */	0xc018d9d9
 };
 
-/* from highpoint documentation. these are old values */
-static struct chipset_bus_clock_list_entry thirty_three_base_hpt370[] = {
-/*	{	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 u32 thirty_three_base_hpt36x[] = {
+	/* XFER_UDMA_6 */	0x90c9a731,
+	/* XFER_UDMA_5 */	0x90c9a731,
+	/* XFER_UDMA_4 */	0x90c9a731,
+	/* XFER_UDMA_3 */	0x90cfa731,
+	/* XFER_UDMA_2 */	0x90caa731,
+	/* XFER_UDMA_1 */	0x90cba731,
+	/* XFER_UDMA_0 */	0x90c8a731,
+
+	/* XFER_MW_DMA_2 */	0xa0c8a731,
+	/* XFER_MW_DMA_1 */	0xa0c8a732,	/* 0xa0c8a733 */
+	/* XFER_MW_DMA_0 */	0xa0c8a797,
+
+	/* XFER_PIO_4 */	0xc0c8a731,
+	/* XFER_PIO_3 */	0xc0c8a742,
+	/* XFER_PIO_2 */	0xc0d0a753,
+	/* XFER_PIO_1 */	0xc0d0a7a3,	/* 0xc0d0a793 */
+	/* XFER_PIO_0 */	0xc0d0a7aa	/* 0xc0d0a7a7 */
 };
 
-/* these are the current (4 sep 2001) timings from highpoint */
-static struct chipset_bus_clock_list_entry thirty_three_base_hpt370a[] = {
-	{	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	}
+static u32 twenty_five_base_hpt36x[] = {
+	/* XFER_UDMA_6 */	0x90c98521,
+	/* XFER_UDMA_5 */	0x90c98521,
+	/* XFER_UDMA_4 */	0x90c98521,
+	/* XFER_UDMA_3 */	0x90cf8521,
+	/* XFER_UDMA_2 */	0x90cf8521,
+	/* XFER_UDMA_1 */	0x90cb8521,
+	/* XFER_UDMA_0 */	0x90cb8521,
+
+	/* XFER_MW_DMA_2 */	0xa0ca8521,
+	/* XFER_MW_DMA_1 */	0xa0ca8532,
+	/* XFER_MW_DMA_0 */	0xa0ca8575,
+
+	/* XFER_PIO_4 */	0xc0ca8521,
+	/* XFER_PIO_3 */	0xc0ca8532,
+	/* XFER_PIO_2 */	0xc0ca8542,
+	/* XFER_PIO_1 */	0xc0d08572,
+	/* XFER_PIO_0 */	0xc0d08585
 };
 
-static struct chipset_bus_clock_list_entry fifty_base_hpt370a[] = {
-	{	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 u32 thirty_three_base_hpt37x[] = {
+	/* 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 chipset_bus_clock_list_entry thirty_three_base_hpt372[] = {
-	{	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	},
-	{	0,		0x06814ea7	}
+static u32 fifty_base_hpt37x[] = {
+	/* 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 chipset_bus_clock_list_entry fifty_base_hpt372[] = {
-	{	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	},
-	{	0,		0x0a81f443	}
-};
-
-static struct chipset_bus_clock_list_entry sixty_six_base_hpt372[] = {
-	{	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	},
-	{	0,		0x0d029d26	}
+static u32 sixty_six_base_hpt37x[] = {
+	/* 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
 };
 
 #define HPT366_DEBUG_DRIVE_INFO		0
@@ -408,7 +361,7 @@ struct hpt_info
 #define IS_3xxN 	2
 #define PCI_66MHZ	4
 				/* Speed table */
-	struct chipset_bus_clock_list_entry *speed;
+	u32 *speed;
 };
 
 /*
@@ -545,12 +498,20 @@ static int check_in_drive_lists (ide_dri
 	return 0;
 }
 
-static unsigned int pci_bus_clock_list (u8 speed, struct chipset_bus_clock_list_entry * chipset_table)
+static u32 pci_bus_clock_list(u8 speed, u32 *chipset_table)
 {
-	for ( ; chipset_table->xfer_speed ; chipset_table++)
-		if (chipset_table->xfer_speed == speed)
-			return chipset_table->chipset_settings;
-	return chipset_table->chipset_settings;
+	int i;
+
+	/*
+	 * Lookup the transfer mode table to get the index into
+	 * the timing table.
+	 *
+	 * NOTE: For XFER_PIO_SLOW, PIO mode 0 timings will be used.
+	 */
+	for (i = 0; i < ARRAY_SIZE(xfer_speeds) - 1; i++)
+		if (xfer_speeds[i] == speed)
+			break;
+	return chipset_table[i];
 }
 
 static int hpt36x_tune_chipset(ide_drive_t *drive, u8 xferspeed)
@@ -1035,14 +996,14 @@ static void __devinit hpt366_clocking(id
 	/* detect bus speed by looking at control reg timing: */
 	switch((reg1 >> 8) & 7) {
 		case 5:
-			info->speed = forty_base_hpt366;
+			info->speed = forty_base_hpt36x;
 			break;
 		case 9:
-			info->speed = twenty_five_base_hpt366;
+			info->speed = twenty_five_base_hpt36x;
 			break;
 		case 7:
 		default:
-			info->speed = thirty_three_base_hpt366;
+			info->speed = thirty_three_base_hpt36x;
 			break;
 	}
 }
@@ -1124,27 +1085,16 @@ static void __devinit hpt37x_clocking(id
 			pll = F_LOW_PCI_66;
 	
 		if (pll == F_LOW_PCI_33) {
-			if (info->revision >= 5)
-				info->speed = thirty_three_base_hpt372;
-			else if (info->revision >= 4)
-				info->speed = thirty_three_base_hpt370a;
-			else
-				info->speed = thirty_three_base_hpt370;
+			info->speed = thirty_three_base_hpt37x;
 			printk(KERN_DEBUG "HPT37X: using 33MHz PCI clock\n");
 		} else if (pll == F_LOW_PCI_40) {
 			/* Unsupported */
 		} else if (pll == F_LOW_PCI_50) {
-			if (info->revision >= 5)
-				info->speed = fifty_base_hpt372;
-			else
-				info->speed = fifty_base_hpt370a;
+			info->speed = fifty_base_hpt37x;
 			printk(KERN_DEBUG "HPT37X: using 50MHz PCI clock\n");
 		} else {
-			if (info->revision >= 5) {
-				info->speed = sixty_six_base_hpt372;
-				printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
-			} else
-				printk(KERN_ERR "HPT37x: 66MHz timings not supported.\n");
+			info->speed = sixty_six_base_hpt37x;
+			printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
 		}
 	}
 
@@ -1191,14 +1141,8 @@ static void __devinit hpt37x_clocking(id
 				pci_write_config_dword(dev, 0x5c, 
 						       pll & ~0x100);
 				pci_write_config_byte(dev, 0x5b, 0x21);
-				if (info->revision >= 8)
-					info->speed = fifty_base_hpt370a;
-				else if (info->revision >= 5)
-					info->speed = fifty_base_hpt372;
-				else if (info->revision >= 4)
-					info->speed = fifty_base_hpt370a;
-				else
-					info->speed = fifty_base_hpt370a;
+
+				info->speed = fifty_base_hpt37x;
 				printk("HPT37X: using 50MHz internal PLL\n");
 				goto init_hpt37X_done;
 			}



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

* [PATCH] HPT3xx: rework rate filtering
  2006-05-04 19:46   ` [PATCH] HPT3xx: fix PCI clock detection Sergei Shtylyov
@ 2006-05-16 22:44     ` Sergei Shtylyov
  2006-05-20  8:51       ` [PATCH] HPT3xx: print the real chip name at startup Sergei Shtylyov
  2006-05-20  8:41     ` [PATCH] HPT37x: read f_CNT saved by BIOS from port Sergei Shtylyov
  1 sibling, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-16 22:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 835 bytes --]

Hello.

    Rework hpt3xx_ratemask() and hpt3xx_ratefilter() so that the former
returns the max. mode computed at the load time and doesn't have to do bad
Ultra33 drive list lookups anymore; remove the duplicate code from the latter
function. Move the quirky drive list lookup into hpt3xx_quirkproc() where it
should have been from the start...
    Disable UltraATA/100 for HPT370 by default as the 33 MHz ATA clock being 
used does not allow for it, and this *greatly* increases the transfer speed.
    Save some space by using byte-wide fields in struct hpt_info; switch to
reading the 8-bit PCI revision ID reg. only, not the whole 32-bit reg.
    Start incrementing the driver version number with each patch (should have
been done from the first one posted).

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-rework-rate-filtering.patch --]
[-- Type: text/plain, Size: 10296 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.36	April 25, 2003
+ * linux/drivers/ide/pci/hpt366.c		Version 0.43	May 17, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -62,8 +62,8 @@
  *   be done on 66 MHz PCI bus
  * - avoid calibrating PLL twice as the second time results in a wrong PCI
  *   frequency and thus in the wrong timings for the secondary channel
- * - disable UltraATA/133 for HPT372 by default (50 MHz DPLL clock do not
- *   allow for this speed anyway)
+ * - disable UltraATA/133 for HPT372 and UltraATA/100 for HPT370 by default
+ *   as the ATA clock being used does not allow for this speed anyway
  * - add support for HPT302N and HPT371N clocking (the same as for HPT372N)
  * - HPT371/N are single channel chips, so avoid touching the primary channel
  *   which exists only virtually (there's no pins for it)
@@ -76,6 +76,7 @@
  *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
  * - pass to init_chipset() handlers a copy of the IDE PCI device structure as
  *   they tamper with its fields
+ * - optimize the rate masking/filtering and the drive list lookup code
  *		<source@mvista.com>
  *
  */
@@ -337,7 +338,7 @@ static u32 sixty_six_base_hpt37x[] = {
 #define HPT371_ALLOW_ATA133_6		0
 #define HPT302_ALLOW_ATA133_6		0
 #define HPT372_ALLOW_ATA133_6		0
-#define HPT370_ALLOW_ATA100_5		1
+#define HPT370_ALLOW_ATA100_5		0
 #define HPT366_ALLOW_ATA66_4		1
 #define HPT366_ALLOW_ATA66_3		1
 #define HPT366_MAX_DEVS			8
@@ -355,8 +356,8 @@ static u32 sixty_six_base_hpt37x[] = {
 struct hpt_info
 {
 	u8 max_mode;		/* Speeds allowed */
-	int revision;		/* Chipset revision */
-	int flags;		/* Chipset properties */
+	u8 revision;		/* Chipset revision */
+	u8 flags;		/* Chipset properties */
 #define PLL_MODE	1
 #define IS_3xxN 	2
 #define PCI_66MHZ	4
@@ -365,61 +366,50 @@ struct hpt_info
 };
 
 /*
- *	This wants fixing so that we do everything not by classrev
+ *	This wants fixing so that we do everything not by revision
  *	(which breaks on the newest chips) but by creating an
  *	enumeration of chip variants and using that
  */
 
-static __devinit u32 hpt_revision (struct pci_dev *dev)
+static __devinit u8 hpt_revision(struct pci_dev *dev)
 {
-	u32 class_rev;
-	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
-	class_rev &= 0xff;
+	u8 rev = 0;
+
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
 	switch(dev->device) {
 		/* Remap new 372N onto 372 */
 		case PCI_DEVICE_ID_TTI_HPT372N:
-			class_rev = PCI_DEVICE_ID_TTI_HPT372; break;
+			rev = PCI_DEVICE_ID_TTI_HPT372; break;
 		case PCI_DEVICE_ID_TTI_HPT374:
-			class_rev = PCI_DEVICE_ID_TTI_HPT374; break;
+			rev = PCI_DEVICE_ID_TTI_HPT374; break;
 		case PCI_DEVICE_ID_TTI_HPT371:
-			class_rev = PCI_DEVICE_ID_TTI_HPT371; break;
+			rev = PCI_DEVICE_ID_TTI_HPT371; break;
 		case PCI_DEVICE_ID_TTI_HPT302:
-			class_rev = PCI_DEVICE_ID_TTI_HPT302; break;
+			rev = PCI_DEVICE_ID_TTI_HPT302; break;
 		case PCI_DEVICE_ID_TTI_HPT372:
-			class_rev = PCI_DEVICE_ID_TTI_HPT372; break;
+			rev = PCI_DEVICE_ID_TTI_HPT372; break;
 		default:
 			break;
 	}
-	return class_rev;
+	return rev;
 }
 
-static int check_in_drive_lists(ide_drive_t *drive, const char **list);
-
-static u8 hpt3xx_ratemask (ide_drive_t *drive)
+static int check_in_drive_list(ide_drive_t *drive, const char **list)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	struct hpt_info *info	= ide_get_hwifdata(hwif);
-	u8 mode			= 0;
+	struct hd_driveid *id = drive->id;
 
-	/* FIXME: TODO - move this to set info->mode once at boot */
+	while (*list)
+		if (!strcmp(*list++,id->model))
+			return 1;
+	return 0;
+}
+
+static u8 hpt3xx_ratemask(ide_drive_t *drive)
+{
+	struct hpt_info *info	= ide_get_hwifdata(HWIF(drive));
+	u8 mode			= info->max_mode;
 
-	if (info->revision >= 8) {		/* HPT374 */
-		mode = (HPT374_ALLOW_ATA133_6) ? 4 : 3;
-	} else if (info->revision >= 7) {	/* HPT371 */
-		mode = (HPT371_ALLOW_ATA133_6) ? 4 : 3;
-	} else if (info->revision >= 6) {	/* HPT302 */
-		mode = (HPT302_ALLOW_ATA133_6) ? 4 : 3;
-	} else if (info->revision >= 5) {	/* HPT372 */
-		mode = (HPT372_ALLOW_ATA133_6) ? 4 : 3;
-	} else if (info->revision >= 4) {	/* HPT370A */
-		mode = (HPT370_ALLOW_ATA100_5) ? 3 : 2;
-	} else if (info->revision >= 3) {	/* HPT370 */
-		mode = (HPT370_ALLOW_ATA100_5) ? 3 : 2;
-		mode = (check_in_drive_lists(drive, bad_ata33)) ? 0 : mode;
-	} else {				/* HPT366 and HPT368 */
-		mode = (check_in_drive_lists(drive, bad_ata33)) ? 0 : 2;
-	}
 	if (!eighty_ninty_three(drive) && mode)
 		mode = min(mode, (u8)1);
 	return mode;
@@ -430,16 +420,15 @@ static u8 hpt3xx_ratemask (ide_drive_t *
  *	either PIO or UDMA modes 0,4,5
  */
  
-static u8 hpt3xx_ratefilter (ide_drive_t *drive, u8 speed)
+static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	struct hpt_info *info	= ide_get_hwifdata(hwif);
+	struct hpt_info *info	= ide_get_hwifdata(HWIF(drive));
 	u8 mode			= hpt3xx_ratemask(drive);
 
 	if (drive->media != ide_disk)
 		return min(speed, (u8)XFER_PIO_4);
 
-	switch(mode) {
+	switch (mode) {
 		case 0x04:
 			speed = min(speed, (u8)XFER_UDMA_6);
 			break;
@@ -447,33 +436,34 @@ static u8 hpt3xx_ratefilter (ide_drive_t
 			speed = min(speed, (u8)XFER_UDMA_5);
 			if (info->revision >= 5)
 				break;
-			if (check_in_drive_lists(drive, bad_ata100_5))
-				speed = min(speed, (u8)XFER_UDMA_4);
-			break;
+			if (!check_in_drive_list(drive, bad_ata100_5))
+				goto check_bad_ata33;
+			/* fall thru */
 		case 0x02:
 			speed = min(speed, (u8)XFER_UDMA_4);
 	/*
 	 * CHECK ME, Does this need to be set to 5 ??
 	 */
 			if (info->revision >= 3)
-				break;
-			if ((check_in_drive_lists(drive, bad_ata66_4)) ||
-			    (!(HPT366_ALLOW_ATA66_4)))
-				speed = min(speed, (u8)XFER_UDMA_3);
-			if ((check_in_drive_lists(drive, bad_ata66_3)) ||
-			    (!(HPT366_ALLOW_ATA66_3)))
-				speed = min(speed, (u8)XFER_UDMA_2);
-			break;
+				goto check_bad_ata33;
+			if (HPT366_ALLOW_ATA66_4 &&
+			    !check_in_drive_list(drive, bad_ata66_4))
+				goto check_bad_ata33;
+
+			speed = min(speed, (u8)XFER_UDMA_3);
+			if (HPT366_ALLOW_ATA66_3 &&
+			    !check_in_drive_list(drive, bad_ata66_3))
+				goto check_bad_ata33;
+			/* fall thru */
 		case 0x01:
 			speed = min(speed, (u8)XFER_UDMA_2);
-	/*
-	 * CHECK ME, Does this need to be set to 5 ??
-	 */
-			if (info->revision >= 3)
+
+		check_bad_ata33:
+ 			if (info->revision >= 4)
 				break;
-			if (check_in_drive_lists(drive, bad_ata33))
-				speed = min(speed, (u8)XFER_MW_DMA_2);
-			break;
+			if (!check_in_drive_list(drive, bad_ata33))
+				break;
+			/* fall thru */
 		case 0x00:
 		default:
 			speed = min(speed, (u8)XFER_MW_DMA_2);
@@ -482,22 +472,6 @@ static u8 hpt3xx_ratefilter (ide_drive_t
 	return speed;
 }
 
-static int check_in_drive_lists (ide_drive_t *drive, const char **list)
-{
-	struct hd_driveid *id = drive->id;
-
-	if (quirk_drives == list) {
-		while (*list)
-			if (strstr(id->model, *list++))
-				return 1;
-	} else {
-		while (*list)
-			if (!strcmp(*list++,id->model))
-				return 1;
-	}
-	return 0;
-}
-
 static u32 pci_bus_clock_list(u8 speed, u32 *chipset_table)
 {
 	int i;
@@ -672,9 +646,15 @@ static int config_chipset_for_dma (ide_d
 	return ide_dma_enable(drive);
 }
 
-static int hpt3xx_quirkproc (ide_drive_t *drive)
+static int hpt3xx_quirkproc(ide_drive_t *drive)
 {
-	return ((int) check_in_drive_lists(drive, quirk_drives));
+	struct hd_driveid *id	= drive->id;
+	const  char **list	= quirk_drives;
+
+	while (*list)
+		if (strstr(id->model, *list++))
+			return 1;
+	return 0;
 }
 
 static void hpt3xx_intrproc (ide_drive_t *drive)
@@ -1382,7 +1362,7 @@ static void __devinit init_iops_hpt366(i
 	struct hpt_info *info	= kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
 	struct pci_dev  *dev	= hwif->pci_dev;
 	u16 did			= dev->device;
-	u8  rid			= 0;
+	u8 mode, rid		= 0;
 
 	if(info == NULL) {
 		printk(KERN_WARNING "hpt366: out of memory.\n");
@@ -1396,7 +1376,7 @@ static void __devinit init_iops_hpt366(i
 		return;
 	}
 
-	pci_read_config_byte(dev, PCI_CLASS_REVISION, &rid);
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rid);
 
 	if (( did == PCI_DEVICE_ID_TTI_HPT366  && rid == 6) ||
 	    ((did == PCI_DEVICE_ID_TTI_HPT372  ||
@@ -1405,9 +1385,22 @@ static void __devinit init_iops_hpt366(i
 	      did == PCI_DEVICE_ID_TTI_HPT372N)
 		info->flags |= IS_3xxN;
 
-	info->revision = hpt_revision(dev);
+	rid = info->revision = hpt_revision(dev);
+	if (rid >= 8)			/* HPT374 */
+		mode = HPT374_ALLOW_ATA133_6 ? 4 : 3;
+	else if (rid >= 7)		/* HPT371 and HPT371N */
+		mode = HPT371_ALLOW_ATA133_6 ? 4 : 3;
+	else if (rid >= 6)		/* HPT302 and HPT302N */
+		mode = HPT302_ALLOW_ATA133_6 ? 4 : 3;
+	else if (rid >= 5)		/* HPT372, HPT372A, and HPT372N */
+		mode = HPT372_ALLOW_ATA133_6 ? 4 : 3;
+	else if (rid >= 3)		/* HPT370 and HPT370A */
+		mode = HPT370_ALLOW_ATA100_5 ? 3 : 2;
+	else				/* HPT366 and HPT368 */
+		mode = (HPT366_ALLOW_ATA66_4 || HPT366_ALLOW_ATA66_3) ? 2 : 1;
+	info->max_mode = mode;
 
-	if (info->revision >= 3)
+	if (rid >= 3)
 		hpt37x_clocking(hwif);
 	else
 		hpt366_clocking(hwif);
@@ -1462,8 +1455,7 @@ static int __devinit init_setup_hpt371(s
 static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
 {
 	struct pci_dev *findev = NULL;
-	u8 pin1 = 0, pin2 = 0;
-	unsigned int class_rev;
+	u8 rev = 0, pin1 = 0, pin2 = 0;
 	char *chipset_names[] = {"HPT366", "HPT366",  "HPT368",
 				 "HPT370", "HPT370A", "HPT372",
 				 "HPT372N" };
@@ -1471,16 +1463,15 @@ static int __devinit init_setup_hpt366(s
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
 
-	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
-	class_rev &= 0xff;
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
 	if(dev->device == PCI_DEVICE_ID_TTI_HPT372N)
-		class_rev = 6;
+		rev = 6;
 		
-	if(class_rev <= 6)
-		d->name = chipset_names[class_rev];
+	if(rev <= 6)
+		d->name = chipset_names[rev];
 
-	switch(class_rev) {
+	switch(rev) {
 		case 6:
 		case 5:
 		case 4:



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

* Re: [PATCH] HPT37x: read f_CNT saved by BIOS from port
  2006-05-04 19:46   ` [PATCH] HPT3xx: fix PCI clock detection Sergei Shtylyov
  2006-05-16 22:44     ` [PATCH] HPT3xx: rework rate filtering Sergei Shtylyov
@ 2006-05-20  8:41     ` Sergei Shtylyov
  1 sibling, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-20  8:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

    The undocumented register BIOS uses for saving f_CNT seems to only be
mapped to I/O space while all the other HPT3xx regs are dual-mapped. Looks
like another HighPoint's dirty trick.
    With this patch, the deadly kernel oops on the cards having the modern
HighPoint BIOSes is now at last gone!

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT37x-read-f_CNT-from-port.patch --]
[-- Type: text/plain, Size: 922 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1016,14 +1016,14 @@ static void __devinit hpt37x_clocking(id
 	 * First try reading the register in which the HighPoint BIOS
 	 * saves f_CNT value before  reprogramming the DPLL from its
 	 * default setting (which differs for the various chips).
+	 * NOTE: This register is only accessible via I/O space.
+	 *
 	 * In case the signature check fails, we'll have to resort to
 	 * reading the f_CNT register itself in hopes that nobody has
 	 * touched the DPLL yet...
 	 */
-	pci_read_config_dword(dev, 0x70, &temp);
+	temp = inl(pci_resource_start(dev, 4) + 0x90);
 	if ((temp & 0xFFFFF000) != 0xABCDE000) {
-		int i;
-
 		printk(KERN_WARNING "HPT37X: no clock data saved by BIOS\n");
 
 		/* Calculate the average value of f_CNT */



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

* [PATCH] HPT3xx: print the real chip name at startup
  2006-05-16 22:44     ` [PATCH] HPT3xx: rework rate filtering Sergei Shtylyov
@ 2006-05-20  8:51       ` Sergei Shtylyov
  2006-05-27 22:05         ` [PATCH] HPT3xx: switch to using pci_find_slot() Sergei Shtylyov
  0 siblings, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-20  8:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

Hello.

- Rework the driver setup code so that it prefixes the driver startup messages
     with the real chip name.

- Print the measured f_CNT value and the DPLL setting for non-HPT3xx chips
     as well.

- Claim the extra 240 bytes of I/O space for all chips, not only for those
     having PCI device ID of 0x0004.

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-print-real-chip-name.patch --]
[-- Type: text/plain, Size: 7890 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.43	May 17, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.44	May 20, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -76,6 +76,8 @@
  *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
  * - pass to init_chipset() handlers a copy of the IDE PCI device structure as
  *   they tamper with its fields
+ * - prefix the driver startup messages with the real chip name
+ * - claim the extra 240 bytes of I/O space for all chips
  * - optimize the rate masking/filtering and the drive list lookup code
  *		<source@mvista.com>
  *
@@ -990,8 +992,9 @@ static void __devinit hpt366_clocking(id
 
 static void __devinit hpt37x_clocking(ide_hwif_t *hwif)
 {
-	struct hpt_info *info = ide_get_hwifdata(hwif);
-	struct pci_dev *dev = hwif->pci_dev;
+	struct hpt_info *info	= ide_get_hwifdata(hwif);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	char *name		= hwif->cds->name;
 	int adjust, i;
 	u16 freq = 0;
 	u32 pll, temp = 0;
@@ -1024,7 +1027,7 @@ static void __devinit hpt37x_clocking(id
 	 */
 	temp = inl(pci_resource_start(dev, 4) + 0x90);
 	if ((temp & 0xFFFFF000) != 0xABCDE000) {
-		printk(KERN_WARNING "HPT37X: no clock data saved by BIOS\n");
+		printk(KERN_WARNING "%s: no clock data saved by BIOS\n", name);
 
 		/* Calculate the average value of f_CNT */
 		for (temp = i = 0; i < 128; i++) {
@@ -1051,10 +1054,7 @@ static void __devinit hpt37x_clocking(id
 		else
 			pll = F_LOW_PCI_66;
 
-		printk(KERN_INFO "HPT3xxN detected, FREQ: %d, PLL: %d\n", freq, pll);
-	}
-	else
-	{
+	} else {
 		if(freq < 0x9C)
 			pll = F_LOW_PCI_33;
 		else if(freq < 0xb0)
@@ -1063,18 +1063,21 @@ static void __devinit hpt37x_clocking(id
 			pll = F_LOW_PCI_50;
 		else
 			pll = F_LOW_PCI_66;
+	}
+	printk(KERN_INFO "%s: FREQ: %d, PLL: %d\n", name, freq, pll);
 	
+	if (!(info->flags & IS_3xxN)) {
 		if (pll == F_LOW_PCI_33) {
 			info->speed = thirty_three_base_hpt37x;
-			printk(KERN_DEBUG "HPT37X: using 33MHz PCI clock\n");
+			printk(KERN_DEBUG "%s: using 33MHz PCI clock\n", name);
 		} else if (pll == F_LOW_PCI_40) {
 			/* Unsupported */
 		} else if (pll == F_LOW_PCI_50) {
 			info->speed = fifty_base_hpt37x;
-			printk(KERN_DEBUG "HPT37X: using 50MHz PCI clock\n");
+			printk(KERN_DEBUG "%s: using 50MHz PCI clock\n", name);
 		} else {
 			info->speed = sixty_six_base_hpt37x;
-			printk(KERN_DEBUG "HPT37X: using 66MHz PCI clock\n");
+			printk(KERN_DEBUG "%s: using 66MHz PCI clock\n", name);
 		}
 	}
 
@@ -1123,7 +1126,7 @@ static void __devinit hpt37x_clocking(id
 				pci_write_config_byte(dev, 0x5b, 0x21);
 
 				info->speed = fifty_base_hpt37x;
-				printk("HPT37X: using 50MHz internal PLL\n");
+				printk("%s: using 50MHz internal PLL\n", name);
 				goto init_hpt37X_done;
 			}
 		}
@@ -1136,8 +1139,8 @@ pll_recal:
 
 init_hpt37X_done:
 	if (!info->speed)
-		printk(KERN_ERR "HPT37x%s: unknown bus timing [%d %d].\n",
-		       (info->flags & IS_3xxN) ? "N" : "", pll, freq);
+		printk(KERN_ERR "%s: unknown bus timing [%d %d].\n",
+		       name, pll, freq);
 	/*
 	 * Reset the state engines.
 	 * NOTE: avoid accidentally enabling the primary channel on HPT371N.
@@ -1330,7 +1333,8 @@ static void __devinit init_dma_hpt366(id
 		return;
 		
 	if(info->speed == NULL) {
-		printk(KERN_WARNING "hpt366: no known IDE timings, disabling DMA.\n");
+		printk(KERN_WARNING "%s: no known IDE timings, disabling DMA.\n",
+		       hwif->cds->name);
 		return;
 	}
 
@@ -1365,7 +1369,7 @@ static void __devinit init_iops_hpt366(i
 	u8 mode, rid		= 0;
 
 	if(info == NULL) {
-		printk(KERN_WARNING "hpt366: out of memory.\n");
+		printk(KERN_WARNING "%s: out of memory.\n", hwif->cds->name);
 		return;
 	}
 	ide_set_hwifdata(hwif, info);
@@ -1430,14 +1434,19 @@ static int __devinit init_setup_hpt374(s
 	return ide_setup_pci_device(dev, d);
 }
 
-static int __devinit init_setup_hpt37x(struct pci_dev *dev, ide_pci_device_t *d)
+static int __devinit init_setup_hpt372n(struct pci_dev *dev, ide_pci_device_t *d)
 {
 	return ide_setup_pci_device(dev, d);
 }
 
 static int __devinit init_setup_hpt371(struct pci_dev *dev, ide_pci_device_t *d)
 {
-	u8 mcr1 = 0;
+	u8 rev = 0, mcr1 = 0;
+
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
+
+	if (rev > 1)
+		d->name = "HPT371N";
 
 	/*
 	 * HPT371 chips physically have only one channel, the secondary one,
@@ -1447,7 +1456,31 @@ static int __devinit init_setup_hpt371(s
 	 */
 	pci_read_config_byte(dev, 0x50, &mcr1);
 	if (mcr1 & 0x04)
-		pci_write_config_byte(dev, 0x50, (mcr1 & ~0x04));
+		pci_write_config_byte(dev, 0x50, mcr1 & ~0x04);
+
+	return ide_setup_pci_device(dev, d);
+}
+
+static int __devinit init_setup_hpt372a(struct pci_dev *dev, ide_pci_device_t *d)
+{
+	u8 rev = 0;
+
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
+
+	if (rev > 1)
+		d->name = "HPT372N";
+
+	return ide_setup_pci_device(dev, d);
+}
+
+static int __devinit init_setup_hpt302(struct pci_dev *dev, ide_pci_device_t *d)
+{
+	u8 rev = 0;
+
+	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
+
+	if (rev > 1)
+		d->name = "HPT302N";
 
 	return ide_setup_pci_device(dev, d);
 }
@@ -1456,30 +1489,22 @@ static int __devinit init_setup_hpt366(s
 {
 	struct pci_dev *findev = NULL;
 	u8 rev = 0, pin1 = 0, pin2 = 0;
-	char *chipset_names[] = {"HPT366", "HPT366",  "HPT368",
-				 "HPT370", "HPT370A", "HPT372",
-				 "HPT372N" };
+	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
+					   "HPT370", "HPT370A", "HPT372",
+					   "HPT372N" };
 
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
 
 	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
-	if(dev->device == PCI_DEVICE_ID_TTI_HPT372N)
+	if (rev > 6)
 		rev = 6;
 		
-	if(rev <= 6)
-		d->name = chipset_names[rev];
+	d->name = chipset_names[rev];
 
-	switch(rev) {
-		case 6:
-		case 5:
-		case 4:
-		case 3:
-			goto init_single;
-		default:
-			break;
-	}
+	if (rev > 2)
+		goto init_single;
 
 	d->channels = 1;
 
@@ -1517,7 +1542,7 @@ static ide_pci_device_t hpt366_chipsets[
 		.extra		= 240
 	},{	/* 1 */
 		.name		= "HPT372A",
-		.init_setup	= init_setup_hpt37x,
+		.init_setup	= init_setup_hpt372a,
 		.init_chipset	= init_chipset_hpt366,
 		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
@@ -1525,9 +1550,10 @@ static ide_pci_device_t hpt366_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.extra		= 240
 	},{	/* 2 */
 		.name		= "HPT302",
-		.init_setup	= init_setup_hpt37x,
+		.init_setup	= init_setup_hpt302,
 		.init_chipset	= init_chipset_hpt366,
 		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
@@ -1535,6 +1561,7 @@ static ide_pci_device_t hpt366_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.extra		= 240
 	},{	/* 3 */
 		.name		= "HPT371",
 		.init_setup	= init_setup_hpt371,
@@ -1546,6 +1573,7 @@ static ide_pci_device_t hpt366_chipsets[
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.extra		= 240
 	},{	/* 4 */
 		.name		= "HPT374",
 		.init_setup	= init_setup_hpt374,
@@ -1556,9 +1584,10 @@ static ide_pci_device_t hpt366_chipsets[
 		.channels	= 2,	/* 4 */
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.extra		= 240
 	},{	/* 5 */
 		.name		= "HPT372N",
-		.init_setup	= init_setup_hpt37x,
+		.init_setup	= init_setup_hpt372n,
 		.init_chipset	= init_chipset_hpt366,
 		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
@@ -1566,6 +1595,7 @@ static ide_pci_device_t hpt366_chipsets[
 		.channels	= 2,	/* 4 */
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.extra		= 240
 	}
 };
 



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

* [PATCH] HPT3xx: switch to using pci_find_slot()
  2006-05-20  8:51       ` [PATCH] HPT3xx: print the real chip name at startup Sergei Shtylyov
@ 2006-05-27 22:05         ` Sergei Shtylyov
  2006-05-27 22:54           ` Jiri Slaby
  2006-05-27 23:30           ` [PATCH] HPT3xx: switch to using pci_get_slot() Sergei Shtylyov
  0 siblings, 2 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-27 22:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 284 bytes --]

    Switch to using pci_find_slot() to get to the function 1 of HPT36x/374
chips -- there's no need for the driver itself to walk the list of the PCI 
devices, and it also forgets to check the bus number of the device found.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-use-pci_find_slot.patch --]
[-- Type: text/plain, Size: 3510 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.44	May 20, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.45	May 21, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -79,6 +79,7 @@
  * - prefix the driver startup messages with the real chip name
  * - claim the extra 240 bytes of I/O space for all chips
  * - optimize the rate masking/filtering and the drive list lookup code
+ * - use pci_find_slot() to get to the function 1 of HPT36x/374
  *		<source@mvista.com>
  *
  */
@@ -1412,24 +1413,20 @@ static void __devinit init_iops_hpt366(i
 
 static int __devinit init_setup_hpt374(struct pci_dev *dev, ide_pci_device_t *d)
 {
-	struct pci_dev *findev = NULL;
+	struct pci_dev *dev2;
 
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
 
-	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
-		if ((findev->vendor == dev->vendor) &&
-		    (findev->device == dev->device) &&
-		    ((findev->devfn - dev->devfn) == 1) &&
-		    (PCI_FUNC(findev->devfn) & 1)) {
-			if (findev->irq != dev->irq) {
-				/* FIXME: we need a core pci_set_interrupt() */
-				findev->irq = dev->irq;
-				printk(KERN_WARNING "%s: pci-config space interrupt "
-					"fixed.\n", d->name);
-			}
-			return ide_setup_pci_devices(dev, findev, d);
+	dev2 = pci_find_slot(dev->bus->number, dev->devfn + 1);
+	if (dev2 != NULL) {
+		if (dev2->irq != dev->irq) {
+			/* FIXME: we need a core pci_set_interrupt() */
+			dev2->irq = dev->irq;
+			printk(KERN_WARNING "%s: pci-config space interrupt "
+			       "fixed.\n", d->name);
 		}
+		return ide_setup_pci_devices(dev, dev2, d);
 	}
 	return ide_setup_pci_device(dev, d);
 }
@@ -1487,8 +1484,8 @@ static int __devinit init_setup_hpt302(s
 
 static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
 {
-	struct pci_dev *findev = NULL;
-	u8 rev = 0, pin1 = 0, pin2 = 0;
+	struct pci_dev *dev2;
+	u8 rev = 0;
 	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
 					   "HPT370", "HPT370A", "HPT372",
 					   "HPT372N" };
@@ -1508,21 +1505,18 @@ static int __devinit init_setup_hpt366(s
 
 	d->channels = 1;
 
-	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin1);
-	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
-		if ((findev->vendor == dev->vendor) &&
-		    (findev->device == dev->device) &&
-		    ((findev->devfn - dev->devfn) == 1) &&
-		    (PCI_FUNC(findev->devfn) & 1)) {
-			pci_read_config_byte(findev, PCI_INTERRUPT_PIN, &pin2);
-			if ((pin1 != pin2) && (dev->irq == findev->irq)) {
-				d->bootable = ON_BOARD;
-				printk("%s: onboard version of chipset, "
-					"pin1=%d pin2=%d\n", d->name,
-					pin1, pin2);
-			}
-			return ide_setup_pci_devices(dev, findev, d);
+	dev2 = pci_find_slot(dev->bus->number, dev->devfn + 1);
+	if (dev2 != NULL) {
+	  	u8 pin1 = 0, pin2 = 0;
+
+		pci_read_config_byte(dev,  PCI_INTERRUPT_PIN, &pin1);
+		pci_read_config_byte(dev2, PCI_INTERRUPT_PIN, &pin2);
+		if (pin1 != pin2 && dev->irq == dev2->irq) {
+			d->bootable = ON_BOARD;
+			printk("%s: onboard version of chipset, pin1=%d pin2=%d\n",
+			       d->name, pin1, pin2);
 		}
+		return ide_setup_pci_devices(dev, dev2, d);
 	}
 init_single:
 	return ide_setup_pci_device(dev, d);



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

* Re: [PATCH] HPT3xx: switch to using pci_find_slot()
  2006-05-27 22:05         ` [PATCH] HPT3xx: switch to using pci_find_slot() Sergei Shtylyov
@ 2006-05-27 22:54           ` Jiri Slaby
  2006-05-27 23:01             ` Sergei Shtylyov
  2006-05-27 23:30           ` [PATCH] HPT3xx: switch to using pci_get_slot() Sergei Shtylyov
  1 sibling, 1 reply; 20+ messages in thread
From: Jiri Slaby @ 2006-05-27 22:54 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, linux-ide,
	linux-kernel, Alan Cox

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sergei Shtylyov napsal(a):
>    Switch to using pci_find_slot() to get to the function 1 of HPT36x/374
Better to use pci_get_slot()+pci_dev_put(), i. e. refcounting.
> chips -- there's no need for the driver itself to walk the list of the
> PCI devices, and it also forgets to check the bus number of the device
> found.
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> 
> ------------------------------------------------------------------------
> 
> Index: linus/drivers/ide/pci/hpt366.c
> ===================================================================
> --- linus.orig/drivers/ide/pci/hpt366.c
> +++ linus/drivers/ide/pci/hpt366.c
> @@ -1,5 +1,5 @@
>  /*
> - * linux/drivers/ide/pci/hpt366.c		Version 0.44	May 20, 2006
> + * linux/drivers/ide/pci/hpt366.c		Version 0.45	May 21, 2006
>   *
>   * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
>   * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
> @@ -79,6 +79,7 @@
>   * - prefix the driver startup messages with the real chip name
>   * - claim the extra 240 bytes of I/O space for all chips
>   * - optimize the rate masking/filtering and the drive list lookup code
> + * - use pci_find_slot() to get to the function 1 of HPT36x/374
We have git repos for logging changes, don't we?
>   *		<source@mvista.com>
>   *
>   */
> @@ -1412,24 +1413,20 @@ static void __devinit init_iops_hpt366(i
>  
>  static int __devinit init_setup_hpt374(struct pci_dev *dev, ide_pci_device_t *d)
>  {
> -	struct pci_dev *findev = NULL;
> +	struct pci_dev *dev2;
>  
>  	if (PCI_FUNC(dev->devfn) & 1)
>  		return -ENODEV;
>  
> -	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
> -		if ((findev->vendor == dev->vendor) &&
> -		    (findev->device == dev->device) &&
> -		    ((findev->devfn - dev->devfn) == 1) &&
> -		    (PCI_FUNC(findev->devfn) & 1)) {
> -			if (findev->irq != dev->irq) {
> -				/* FIXME: we need a core pci_set_interrupt() */
> -				findev->irq = dev->irq;
> -				printk(KERN_WARNING "%s: pci-config space interrupt "
> -					"fixed.\n", d->name);
> -			}
> -			return ide_setup_pci_devices(dev, findev, d);
> +	dev2 = pci_find_slot(dev->bus->number, dev->devfn + 1);
> +	if (dev2 != NULL) {
> +		if (dev2->irq != dev->irq) {
> +			/* FIXME: we need a core pci_set_interrupt() */
> +			dev2->irq = dev->irq;
> +			printk(KERN_WARNING "%s: pci-config space interrupt "
> +			       "fixed.\n", d->name);
>  		}
> +		return ide_setup_pci_devices(dev, dev2, d);
>  	}
>  	return ide_setup_pci_device(dev, d);
>  }
> @@ -1487,8 +1484,8 @@ static int __devinit init_setup_hpt302(s
>  
>  static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
>  {
> -	struct pci_dev *findev = NULL;
> -	u8 rev = 0, pin1 = 0, pin2 = 0;
> +	struct pci_dev *dev2;
> +	u8 rev = 0;
>  	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
>  					   "HPT370", "HPT370A", "HPT372",
>  					   "HPT372N" };
> @@ -1508,21 +1505,18 @@ static int __devinit init_setup_hpt366(s
>  
>  	d->channels = 1;
>  
> -	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin1);
> -	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
> -		if ((findev->vendor == dev->vendor) &&
> -		    (findev->device == dev->device) &&
> -		    ((findev->devfn - dev->devfn) == 1) &&
> -		    (PCI_FUNC(findev->devfn) & 1)) {
> -			pci_read_config_byte(findev, PCI_INTERRUPT_PIN, &pin2);
> -			if ((pin1 != pin2) && (dev->irq == findev->irq)) {
> -				d->bootable = ON_BOARD;
> -				printk("%s: onboard version of chipset, "
> -					"pin1=%d pin2=%d\n", d->name,
> -					pin1, pin2);
> -			}
> -			return ide_setup_pci_devices(dev, findev, d);
> +	dev2 = pci_find_slot(dev->bus->number, dev->devfn + 1);
> +	if (dev2 != NULL) {
> +	  	u8 pin1 = 0, pin2 = 0;
> +
> +		pci_read_config_byte(dev,  PCI_INTERRUPT_PIN, &pin1);
> +		pci_read_config_byte(dev2, PCI_INTERRUPT_PIN, &pin2);
> +		if (pin1 != pin2 && dev->irq == dev2->irq) {
> +			d->bootable = ON_BOARD;
> +			printk("%s: onboard version of chipset, pin1=%d pin2=%d\n",
> +			       d->name, pin1, pin2);
>  		}
> +		return ide_setup_pci_devices(dev, dev2, d);
>  	}
>  init_single:
>  	return ide_setup_pci_device(dev, d);
> 
> 

regards,
- --
Jiri Slaby         www.fi.muni.cz/~xslaby
\_.-^-._   jirislaby@gmail.com   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEeNiJMsxVwznUen4RAvL7AKCLth5crJcvUmMHSLEtX02VztoJUwCfZtNo
uOKYd32ClussVe5FyRtZ2TM=
=W479
-----END PGP SIGNATURE-----

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

* Re: [PATCH] HPT3xx: switch to using pci_find_slot()
  2006-05-27 22:54           ` Jiri Slaby
@ 2006-05-27 23:01             ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-27 23:01 UTC (permalink / raw)
  To: Jiri Slaby, Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

Hello.

Jiri Slaby wrote:
> Sergei Shtylyov napsal(a):

>>   Switch to using pci_find_slot() to get to the function 1 of HPT36x/374

> Better to use pci_get_slot()+pci_dev_put(), i. e. refcounting.

    Indeed. I'll recast.

WBR, Sergei

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

* [PATCH] HPT3xx: switch to using pci_get_slot()
  2006-05-27 22:05         ` [PATCH] HPT3xx: switch to using pci_find_slot() Sergei Shtylyov
  2006-05-27 22:54           ` Jiri Slaby
@ 2006-05-27 23:30           ` Sergei Shtylyov
  2006-05-28  2:55             ` [PATCH] HPT3xx: cache channel's MCR address Sergei Shtylyov
  2006-05-28 13:51             ` [PATCH] HPT3xx: switch to using pci_get_slot() Jiri Slaby
  1 sibling, 2 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-27 23:30 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, linux-ide,
	linux-kernel, Alan Cox, jirislaby

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

    Switch to using pci_get_slot() to get to the function 1 of HPT36x/374
chips -- there's no need for the driver itself to walk the list of the
PCI devices, and it also forgets to check the bus number of the device
found.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-use-pci_get_slot.patch --]
[-- Type: text/plain, Size: 3611 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.44	May 20, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.45	May 27, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -79,6 +79,7 @@
  * - prefix the driver startup messages with the real chip name
  * - claim the extra 240 bytes of I/O space for all chips
  * - optimize the rate masking/filtering and the drive list lookup code
+ * - use pci_get_slot() to get to the function 1 of HPT36x/374
  *		<source@mvista.com>
  *
  */
@@ -1412,24 +1413,24 @@ static void __devinit init_iops_hpt366(i
 
 static int __devinit init_setup_hpt374(struct pci_dev *dev, ide_pci_device_t *d)
 {
-	struct pci_dev *findev = NULL;
+	struct pci_dev *dev2;
 
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
 
-	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
-		if ((findev->vendor == dev->vendor) &&
-		    (findev->device == dev->device) &&
-		    ((findev->devfn - dev->devfn) == 1) &&
-		    (PCI_FUNC(findev->devfn) & 1)) {
-			if (findev->irq != dev->irq) {
-				/* FIXME: we need a core pci_set_interrupt() */
-				findev->irq = dev->irq;
-				printk(KERN_WARNING "%s: pci-config space interrupt "
-					"fixed.\n", d->name);
-			}
-			return ide_setup_pci_devices(dev, findev, d);
+	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
+		int ret;
+
+		if (dev2->irq != dev->irq) {
+			/* FIXME: we need a core pci_set_interrupt() */
+			dev2->irq = dev->irq;
+			printk(KERN_WARNING "%s: PCI config space interrupt "
+			       "fixed.\n", d->name);
 		}
+		ret = ide_setup_pci_devices(dev, dev2, d);
+		if (ret < 0)
+			pci_dev_put(dev2);
+		return ret;
 	}
 	return ide_setup_pci_device(dev, d);
 }
@@ -1487,8 +1488,8 @@ static int __devinit init_setup_hpt302(s
 
 static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
 {
-	struct pci_dev *findev = NULL;
-	u8 rev = 0, pin1 = 0, pin2 = 0;
+	struct pci_dev *dev2;
+	u8 rev = 0;
 	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
 					   "HPT370", "HPT370A", "HPT372",
 					   "HPT372N" };
@@ -1508,21 +1509,21 @@ static int __devinit init_setup_hpt366(s
 
 	d->channels = 1;
 
-	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin1);
-	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
-		if ((findev->vendor == dev->vendor) &&
-		    (findev->device == dev->device) &&
-		    ((findev->devfn - dev->devfn) == 1) &&
-		    (PCI_FUNC(findev->devfn) & 1)) {
-			pci_read_config_byte(findev, PCI_INTERRUPT_PIN, &pin2);
-			if ((pin1 != pin2) && (dev->irq == findev->irq)) {
-				d->bootable = ON_BOARD;
-				printk("%s: onboard version of chipset, "
-					"pin1=%d pin2=%d\n", d->name,
-					pin1, pin2);
-			}
-			return ide_setup_pci_devices(dev, findev, d);
+	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
+	  	u8  pin1 = 0, pin2 = 0;
+		int ret;
+
+		pci_read_config_byte(dev,  PCI_INTERRUPT_PIN, &pin1);
+		pci_read_config_byte(dev2, PCI_INTERRUPT_PIN, &pin2);
+		if (pin1 != pin2 && dev->irq == dev2->irq) {
+			d->bootable = ON_BOARD;
+			printk("%s: onboard version of chipset, pin1=%d pin2=%d\n",
+			       d->name, pin1, pin2);
 		}
+		ret = ide_setup_pci_devices(dev, dev2, d);
+		if (ret < 0)
+			pci_dev_put(dev2);
+		return ret;
 	}
 init_single:
 	return ide_setup_pci_device(dev, d);


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

* Re: [PATCH] HPT3xx: cache channel's MCR address
  2006-05-27 23:30           ` [PATCH] HPT3xx: switch to using pci_get_slot() Sergei Shtylyov
@ 2006-05-28  2:55             ` Sergei Shtylyov
  2006-06-04 22:24               ` [PATCH] HPT3x7: merge speedproc handlers Sergei Shtylyov
  2006-05-28 13:51             ` [PATCH] HPT3xx: switch to using pci_get_slot() Jiri Slaby
  1 sibling, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-28  2:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

Begin the real driver redesign. For the starters:

- cache the offset of the IDE channel's MISC. control registers which are used
   throughout the driver in hwif->select_data;

- only touch the relevant MCR when detecting the cable type on HPT374's
   function 1;

- make HPT36x's speedproc handler look the same way as HPT37x ones; fix the
   PIO timing register mask for HPT37x.

- rename all the HPT3xx register related variables consistently; clean up the
   whitespace.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-cache-MCR-address.patch --]
[-- Type: text/plain, Size: 21388 bytes --]

Index: linus/drivers/ide/pci/hpt366.c
===================================================================
--- linus.orig/drivers/ide/pci/hpt366.c
+++ linus/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.45	May 27, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.50	May 28, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -80,6 +80,11 @@
  * - claim the extra 240 bytes of I/O space for all chips
  * - optimize the rate masking/filtering and the drive list lookup code
  * - use pci_get_slot() to get to the function 1 of HPT36x/374
+ * - cache the channel's MCRs' offset; only touch the relevant MCR when detecting
+ *   the cable type on HPT374's function 1
+ * - rename all the register related variables consistently
+ * - make HPT36x's speedproc handler look the same way as HPT37x ones; fix the
+ *   PIO timing register mask for HPT37x
  *		<source@mvista.com>
  *
  */
@@ -493,109 +498,106 @@ static u32 pci_bus_clock_list(u8 speed, 
 
 static int hpt36x_tune_chipset(ide_drive_t *drive, u8 xferspeed)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	struct pci_dev *dev	= hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
-	u8 speed		= hpt3xx_ratefilter(drive, xferspeed);
-	u8 regtime		= (drive->select.b.unit & 0x01) ? 0x44 : 0x40;
-	u8 regfast		= (hwif->channel) ? 0x55 : 0x51;
-	u8 drive_fast		= 0;
-	u32 reg1 = 0, reg2	= 0;
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	struct hpt_info	*info	= ide_get_hwifdata (hwif);
+	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
+	u8  itr_addr		= drive->dn ? 0x44 : 0x40;
+	u8  mcr_addr		= hwif->select_data + 1;
+	u8  mcr			= 0;
+	u32 new_itr, old_itr	= 0;
+	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x30070000 : 0xc0000000;
 
 	/*
 	 * Disable the "fast interrupt" prediction.
 	 */
-	pci_read_config_byte(dev, regfast, &drive_fast);
-	if (drive_fast & 0x80)
-		pci_write_config_byte(dev, regfast, drive_fast & ~0x80);
+	pci_read_config_byte(dev, mcr_addr, &mcr);
+	if (mcr & 0x80)
+		pci_write_config_byte(dev, mcr_addr, mcr & ~0x80);
 
-	reg2 = pci_bus_clock_list(speed, info->speed);
+	new_itr = pci_bus_clock_list(speed, info->speed);
 
 	/*
-	 * Disable on-chip PIO FIFO/buffer
-	 *  (to avoid problems handling I/O errors later)
+	 * Disable on-chip PIO FIFO/buffer (and PIO MST mode as well)
+	 * to avoid problems handling I/O errors later
 	 */
-	pci_read_config_dword(dev, regtime, &reg1);
-	if (speed >= XFER_MW_DMA_0) {
-		reg2 = (reg2 & ~0xc0000000) | (reg1 & 0xc0000000);
-	} else {
-		reg2 = (reg2 & ~0x30070000) | (reg1 & 0x30070000);
-	}	
-	reg2 &= ~0x80000000;
+	pci_read_config_dword(dev, itr_addr, &old_itr);
+	new_itr  = (new_itr & ~itr_mask) | (old_itr & itr_mask);
+	new_itr &= ~0xc0000000;
 
-	pci_write_config_dword(dev, regtime, reg2);
+	pci_write_config_dword(dev, itr_addr, new_itr);
 
 	return ide_config_drive_speed(drive, speed);
 }
 
 static int hpt370_tune_chipset(ide_drive_t *drive, u8 xferspeed)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	struct pci_dev *dev = hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
-	u8 speed	= hpt3xx_ratefilter(drive, xferspeed);
-	u8 regfast	= (drive->hwif->channel) ? 0x55 : 0x51;
-	u8 drive_pci	= 0x40 + (drive->dn * 4);
-	u8 new_fast	= 0, drive_fast = 0;
-	u32 list_conf	= 0, drive_conf = 0;
-	u32 conf_mask	= (speed >= XFER_MW_DMA_0) ? 0xc0000000 : 0x30070000;
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	struct hpt_info	*info	= ide_get_hwifdata (hwif);
+	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
+	u8  mcr_addr		= hwif->select_data + 1;
+	u8  itr_addr		= 0x40 + (drive->dn * 4);
+	u8  new_mcr		= 0, old_mcr = 0;
+	u32 new_itr, old_itr	= 0;
+	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x303c0000 : 0xc0000000;
 
 	/*
 	 * Disable the "fast interrupt" prediction.
 	 * don't holdoff on interrupts. (== 0x01 despite what the docs say) 
 	 */
-	pci_read_config_byte(dev, regfast, &drive_fast);
-	new_fast = drive_fast;
-	if (new_fast & 0x02)
-		new_fast &= ~0x02;
+	pci_read_config_byte(dev, mcr_addr, &old_mcr);
+	new_mcr = old_mcr;
+	if (new_mcr & 0x02)
+		new_mcr &= ~0x02;
 
 #ifdef HPT_DELAY_INTERRUPT
-	if (new_fast & 0x01)
-		new_fast &= ~0x01;
+	if (new_mcr & 0x01)
+		new_mcr &= ~0x01;
 #else
-	if ((new_fast & 0x01) == 0)
-		new_fast |= 0x01;
+	if ((new_mcr & 0x01) == 0)
+		new_mcr |= 0x01;
 #endif
-	if (new_fast != drive_fast)
-		pci_write_config_byte(dev, regfast, new_fast);
+	if (new_mcr != old_mcr)
+		pci_write_config_byte(dev, mcr_addr, new_mcr);
 
-	list_conf = pci_bus_clock_list(speed, info->speed);
+	new_itr = pci_bus_clock_list(speed, info->speed);
 
-	pci_read_config_dword(dev, drive_pci, &drive_conf);
-	list_conf = (list_conf & ~conf_mask) | (drive_conf & conf_mask);
+	pci_read_config_dword(dev, itr_addr, &old_itr);
+	new_itr = (new_itr & ~itr_mask) | (old_itr & itr_mask);
 	
 	if (speed < XFER_MW_DMA_0)
-		list_conf &= ~0x80000000; /* Disable on-chip PIO FIFO/buffer */
-	pci_write_config_dword(dev, drive_pci, list_conf);
+		new_itr &= ~0x80000000; /* Disable on-chip PIO FIFO/buffer */
+	pci_write_config_dword(dev, itr_addr, new_itr);
 
 	return ide_config_drive_speed(drive, speed);
 }
 
 static int hpt372_tune_chipset(ide_drive_t *drive, u8 xferspeed)
 {
-	ide_hwif_t *hwif	= drive->hwif;
-	struct pci_dev *dev	= hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
-	u8 speed	= hpt3xx_ratefilter(drive, xferspeed);
-	u8 regfast	= (drive->hwif->channel) ? 0x55 : 0x51;
-	u8 drive_fast	= 0, drive_pci = 0x40 + (drive->dn * 4);
-	u32 list_conf	= 0, drive_conf = 0;
-	u32 conf_mask	= (speed >= XFER_MW_DMA_0) ? 0xc0000000 : 0x30070000;
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev  *dev	= hwif->pci_dev;
+	struct hpt_info	*info	= ide_get_hwifdata (hwif);
+	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
+	u8  mcr_addr		= hwif->select_data + 1;
+	u8  itr_addr		= 0x40 + (drive->dn * 4);
+	u8  mcr			= 0;
+	u32 new_itr, old_itr	= 0;
+	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x303c0000 : 0xc0000000;
 
 	/*
 	 * Disable the "fast interrupt" prediction.
 	 * don't holdoff on interrupts. (== 0x01 despite what the docs say)
 	 */
-	pci_read_config_byte(dev, regfast, &drive_fast);
-	drive_fast &= ~0x07;
-	pci_write_config_byte(dev, regfast, drive_fast);
-
-	list_conf = pci_bus_clock_list(speed, info->speed);
-	pci_read_config_dword(dev, drive_pci, &drive_conf);
-	list_conf = (list_conf & ~conf_mask) | (drive_conf & conf_mask);
+	pci_read_config_byte (dev, mcr_addr, &mcr);
+	pci_write_config_byte(dev, mcr_addr, (mcr & ~0x07));
+
+	new_itr = pci_bus_clock_list(speed, info->speed);
+	pci_read_config_dword(dev, itr_addr, &old_itr);
+	new_itr = (new_itr & ~itr_mask) | (old_itr & itr_mask);
 	if (speed < XFER_MW_DMA_0)
-		list_conf &= ~0x80000000; /* Disable on-chip PIO FIFO/buffer */
-	pci_write_config_dword(dev, drive_pci, list_conf);
+		new_itr &= ~0x80000000; /* Disable on-chip PIO FIFO/buffer */
+	pci_write_config_dword(dev, itr_addr, new_itr);
 
 	return ide_config_drive_speed(drive, speed);
 }
@@ -662,39 +664,41 @@ static int hpt3xx_quirkproc(ide_drive_t 
 
 static void hpt3xx_intrproc (ide_drive_t *drive)
 {
-	ide_hwif_t *hwif = drive->hwif;
+	ide_hwif_t *hwif = HWIF(drive);
 
 	if (drive->quirk_list)
 		return;
 	/* drives in the quirk_list may not like intr setups/cleanups */
-	hwif->OUTB(drive->ctl|2, IDE_CONTROL_REG);
+	hwif->OUTB(drive->ctl | 2, IDE_CONTROL_REG);
 }
 
 static void hpt3xx_maskproc (ide_drive_t *drive, int mask)
 {
-	ide_hwif_t *hwif = drive->hwif;
-	struct hpt_info *info = ide_get_hwifdata(hwif);
-	struct pci_dev *dev = hwif->pci_dev;
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev	*dev	= hwif->pci_dev;
+	struct hpt_info *info	= ide_get_hwifdata(hwif);
 
 	if (drive->quirk_list) {
 		if (info->revision >= 3) {
-			u8 reg5a = 0;
-			pci_read_config_byte(dev, 0x5a, &reg5a);
-			if (((reg5a & 0x10) >> 4) != mask)
-				pci_write_config_byte(dev, 0x5a, mask ? (reg5a | 0x10) : (reg5a & ~0x10));
+			u8 scr1 = 0;
+
+			pci_read_config_byte(dev, 0x5a, &scr1);
+			if (((scr1 & 0x10) >> 4) != mask) {
+				if (mask)
+					scr1 |=  0x10;
+				else
+					scr1 &= ~0x10;
+				pci_write_config_byte(dev, 0x5a, scr1);
+			}
 		} else {
-			if (mask) {
+			if (mask)
 				disable_irq(hwif->irq);
-			} else {
-				enable_irq(hwif->irq);
-			}
+			else
+				enable_irq (hwif->irq);
 		}
-	} else {
-		if (IDE_CONTROL_REG)
-			hwif->OUTB(mask ? (drive->ctl | 2) :
-						 (drive->ctl & ~2),
-						 IDE_CONTROL_REG);
-	}
+	} else
+		hwif->OUTB(mask ? (drive->ctl | 2) : (drive->ctl & ~2),
+			   IDE_CONTROL_REG);
 }
 
 static int hpt366_config_drive_xfer_rate (ide_drive_t *drive)
@@ -723,28 +727,29 @@ fast_ata_pio:
 }
 
 /*
- * This is specific to the HPT366 UDMA bios chipset
+ * This is specific to the HPT366 UDMA chipset
  * by HighPoint|Triones Technologies, Inc.
  */
-static int hpt366_ide_dma_lostirq (ide_drive_t *drive)
+static int hpt366_ide_dma_lostirq(ide_drive_t *drive)
 {
-	struct pci_dev *dev	= HWIF(drive)->pci_dev;
-	u8 reg50h = 0, reg52h = 0, reg5ah = 0;
+	struct pci_dev *dev = HWIF(drive)->pci_dev;
+	u8 mcr1 = 0, mcr3 = 0, scr1 = 0;
 
-	pci_read_config_byte(dev, 0x50, &reg50h);
-	pci_read_config_byte(dev, 0x52, &reg52h);
-	pci_read_config_byte(dev, 0x5a, &reg5ah);
-	printk("%s: (%s)  reg50h=0x%02x, reg52h=0x%02x, reg5ah=0x%02x\n",
-		drive->name, __FUNCTION__, reg50h, reg52h, reg5ah);
-	if (reg5ah & 0x10)
-		pci_write_config_byte(dev, 0x5a, reg5ah & ~0x10);
+	pci_read_config_byte(dev, 0x50, &mcr1);
+	pci_read_config_byte(dev, 0x52, &mcr3);
+	pci_read_config_byte(dev, 0x5a, &scr1);
+	printk("%s: (%s)  mcr1=0x%02x, mcr3=0x%02x, scr1=0x%02x\n",
+		drive->name, __FUNCTION__, mcr1, mcr3, scr1);
+	if (scr1 & 0x10)
+		pci_write_config_byte(dev, 0x5a, scr1 & ~0x10);
 	return __ide_dma_lostirq(drive);
 }
 
 static void hpt370_clear_engine (ide_drive_t *drive)
 {
-	u8 regstate = HWIF(drive)->channel ? 0x54 : 0x50;
-	pci_write_config_byte(HWIF(drive)->pci_dev, regstate, 0x37);
+	ide_hwif_t *hwif = HWIF(drive);
+
+	pci_write_config_byte(hwif->pci_dev, hwif->select_data, 0x37);
 	udelay(10);
 }
 
@@ -776,10 +781,10 @@ static int hpt370_ide_dma_end (ide_drive
 static void hpt370_lostirq_timeout (ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8 bfifo = 0, reginfo	= hwif->channel ? 0x56 : 0x52;
+	u8 bfifo = 0;
 	u8 dma_stat = 0, dma_cmd = 0;
 
-	pci_read_config_byte(HWIF(drive)->pci_dev, reginfo, &bfifo);
+	pci_read_config_byte(HWIF(drive)->pci_dev, hwif->select_data + 2, &bfifo);
 	printk(KERN_DEBUG "%s: %d bytes in FIFO\n", drive->name, bfifo);
 	hpt370_clear_engine(drive);
 	/* get dma command mode */
@@ -810,10 +815,9 @@ static int hpt374_ide_dma_test_irq(ide_d
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	u16 bfifo		= 0;
-	u8 reginfo		= hwif->channel ? 0x56 : 0x52;
-	u8 dma_stat;
+	u8  dma_stat;
 
-	pci_read_config_word(hwif->pci_dev, reginfo, &bfifo);
+	pci_read_config_word(hwif->pci_dev, hwif->select_data + 2, &bfifo);
 	if (bfifo & 0x1FF) {
 //		printk("%s: %d bytes in FIFO\n", drive->name, bfifo);
 		return 0;
@@ -821,7 +825,7 @@ static int hpt374_ide_dma_test_irq(ide_d
 
 	dma_stat = hwif->INB(hwif->dma_status);
 	/* return 1 if INTR asserted */
-	if ((dma_stat & 4) == 4)
+	if (dma_stat & 4)
 		return 1;
 
 	if (!drive->waiting_for_dma)
@@ -830,17 +834,17 @@ static int hpt374_ide_dma_test_irq(ide_d
 	return 0;
 }
 
-static int hpt374_ide_dma_end (ide_drive_t *drive)
+static int hpt374_ide_dma_end(ide_drive_t *drive)
 {
-	struct pci_dev *dev	= HWIF(drive)->pci_dev;
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8 msc_stat = 0, mscreg	= hwif->channel ? 0x54 : 0x50;
-	u8 bwsr_stat = 0, bwsr_mask = hwif->channel ? 0x02 : 0x01;
-
-	pci_read_config_byte(dev, 0x6a, &bwsr_stat);
-	pci_read_config_byte(dev, mscreg, &msc_stat);
-	if ((bwsr_stat & bwsr_mask) == bwsr_mask)
-		pci_write_config_byte(dev, mscreg, msc_stat|0x30);
+	struct pci_dev	*dev	= hwif->pci_dev;
+	u8 mcr	= 0, mcr_addr	= hwif->select_data;
+	u8 bwsr = 0, mask	= hwif->channel ? 0x02 : 0x01;
+
+	pci_read_config_byte(dev, 0x6a, &bwsr);
+	pci_read_config_byte(dev, mcr_addr, &mcr);
+	if (bwsr & mask)
+		pci_write_config_byte(dev, mcr_addr, mcr | 0x30);
 	return __ide_dma_end(drive);
 }
 
@@ -906,6 +910,7 @@ static void hpt3xxn_rw_disk(ide_drive_t 
 
 /* 
  * Set/get power state for a drive.
+ * NOTE: affects both drives on each channel.
  *
  * When we turn the power back on, we need to re-initialize things.
  */
@@ -913,26 +918,18 @@ static void hpt3xxn_rw_disk(ide_drive_t 
 
 static int hpt3xx_busproc(ide_drive_t *drive, int state)
 {
-	ide_hwif_t *hwif	= drive->hwif;
+	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= hwif->pci_dev;
-	u8  tristate, resetmask, bus_reg = 0;
-	u16 tri_reg = 0;
+	u8  mcr_addr		= hwif->select_data + 2;
+	u8  resetmask		= hwif->channel ? 0x80 : 0x40;
+	u8  bsr2		= 0;
+	u16 mcr			= 0;
 
 	hwif->bus_state = state;
 
-	if (hwif->channel) { 
-		/* secondary channel */
-		tristate  = 0x56;
-		resetmask = 0x80;
-	} else { 
-		/* primary channel */
-		tristate  = 0x52;
-		resetmask = 0x40;
-	}
-
 	/* Grab the status. */
-	pci_read_config_word(dev, tristate, &tri_reg);
-	pci_read_config_byte(dev, 0x59, &bus_reg);
+	pci_read_config_word(dev, mcr_addr, &mcr);
+	pci_read_config_byte(dev, 0x59, &bsr2);
 
 	/*
 	 * Set the state. We don't set it if we don't need to do so.
@@ -940,22 +937,22 @@ static int hpt3xx_busproc(ide_drive_t *d
 	 */
 	switch (state) {
 	case BUSSTATE_ON:
-		if (!(bus_reg & resetmask))
+		if (!(bsr2 & resetmask))
 			return 0;
 		hwif->drives[0].failures = hwif->drives[1].failures = 0;
 
-		pci_write_config_byte(dev, 0x59, bus_reg & ~resetmask);
-		pci_write_config_word(dev, tristate, tri_reg & ~TRISTATE_BIT);
+		pci_write_config_byte(dev, 0x59, bsr2 & ~resetmask);
+		pci_write_config_word(dev, mcr_addr, mcr & ~TRISTATE_BIT);
 		return 0;
 	case BUSSTATE_OFF:
-		if ((bus_reg & resetmask) && !(tri_reg & TRISTATE_BIT))
+		if ((bsr2 & resetmask) && !(mcr & TRISTATE_BIT))
 			return 0;
-		tri_reg &= ~TRISTATE_BIT;
+		mcr &= ~TRISTATE_BIT;
 		break;
 	case BUSSTATE_TRISTATE:
-		if ((bus_reg & resetmask) &&  (tri_reg & TRISTATE_BIT))
+		if ((bsr2 & resetmask) &&  (mcr & TRISTATE_BIT))
 			return 0;
-		tri_reg |= TRISTATE_BIT;
+		mcr |= TRISTATE_BIT;
 		break;
 	default:
 		return -EINVAL;
@@ -964,20 +961,20 @@ static int hpt3xx_busproc(ide_drive_t *d
 	hwif->drives[0].failures = hwif->drives[0].max_failures + 1;
 	hwif->drives[1].failures = hwif->drives[1].max_failures + 1;
 
-	pci_write_config_word(dev, tristate, tri_reg);
-	pci_write_config_byte(dev, 0x59, bus_reg | resetmask);
+	pci_write_config_word(dev, mcr_addr, mcr);
+	pci_write_config_byte(dev, 0x59, bsr2 | resetmask);
 	return 0;
 }
 
 static void __devinit hpt366_clocking(ide_hwif_t *hwif)
 {
-	u32 reg1	= 0;
+	u32 itr1	= 0;
 	struct hpt_info *info = ide_get_hwifdata(hwif);
 
-	pci_read_config_dword(hwif->pci_dev, 0x40, &reg1);
+	pci_read_config_dword(hwif->pci_dev, 0x40, &itr1);
 
 	/* detect bus speed by looking at control reg timing: */
-	switch((reg1 >> 8) & 7) {
+	switch((itr1 >> 8) & 7) {
 		case 5:
 			info->speed = forty_base_hpt36x;
 			break;
@@ -999,7 +996,7 @@ static void __devinit hpt37x_clocking(id
 	int adjust, i;
 	u16 freq = 0;
 	u32 pll, temp = 0;
-	u8 reg5bh = 0, mcr1 = 0;
+	u8  scr2 = 0, mcr1 = 0;
 	
 	/*
 	 * default to pci clock. make sure MA15/16 are set to output
@@ -1112,13 +1109,13 @@ static void __devinit hpt37x_clocking(id
 
 		/* wait for clock stabilization */
 		for (i = 0; i < 0x50000; i++) {
-			pci_read_config_byte(dev, 0x5b, &reg5bh);
-			if (reg5bh & 0x80) {
+			pci_read_config_byte(dev, 0x5b, &scr2);
+			if (scr2 & 0x80) {
 				/* spin looking for the clock to destabilize */
 				for (i = 0; i < 0x1000; ++i) {
 					pci_read_config_byte(dev, 0x5b, 
-							     &reg5bh);
-					if ((reg5bh & 0x80) == 0)
+							     &scr2);
+					if ((scr2 & 0x80) == 0)
 						goto pll_recal;
 				}
 				pci_read_config_dword(dev, 0x5c, &pll);
@@ -1155,26 +1152,24 @@ init_hpt37X_done:
 
 static int __devinit init_hpt37x(struct pci_dev *dev)
 {
-	u8 reg5ah;
+	u8 scr1;
 
-	pci_read_config_byte(dev, 0x5a, &reg5ah);
+	pci_read_config_byte (dev, 0x5a, &scr1);
 	/* interrupt force enable */
-	pci_write_config_byte(dev, 0x5a, (reg5ah & ~0x10));
+	pci_write_config_byte(dev, 0x5a, (scr1 & ~0x10));
 	return 0;
 }
 
 static int __devinit init_hpt366(struct pci_dev *dev)
 {
-	u32 reg1	= 0;
-	u8 drive_fast	= 0;
+	u8 mcr	= 0;
 
 	/*
 	 * Disable the "fast interrupt" prediction.
 	 */
-	pci_read_config_byte(dev, 0x51, &drive_fast);
-	if (drive_fast & 0x80)
-		pci_write_config_byte(dev, 0x51, drive_fast & ~0x80);
-	pci_read_config_dword(dev, 0x40, &reg1);
+	pci_read_config_byte(dev, 0x51, &mcr);
+	if (mcr & 0x80)
+		pci_write_config_byte(dev, 0x51, mcr & ~0x80);
 									
 	return 0;
 }
@@ -1209,17 +1204,21 @@ static unsigned int __devinit init_chips
 
 static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
 {
-	struct pci_dev *dev		= hwif->pci_dev;
+	struct pci_dev  *dev		= hwif->pci_dev;
 	struct hpt_info *info		= ide_get_hwifdata(hwif);
-	u8 ata66 = 0, regmask		= (hwif->channel) ? 0x01 : 0x02;
 	int serialize			= HPT_SERIALIZE_IO;
-	
+	u8  scr1 = 0, ata66		= (hwif->channel) ? 0x01 : 0x02;
+
+	/* Cache the channel's MISC. control registers' offset */
+	hwif->select_data		= hwif->channel ? 0x54 : 0x50;
+
 	hwif->tuneproc			= &hpt3xx_tune_drive;
 	hwif->speedproc			= &hpt3xx_tune_chipset;
 	hwif->quirkproc			= &hpt3xx_quirkproc;
 	hwif->intrproc			= &hpt3xx_intrproc;
 	hwif->maskproc			= &hpt3xx_maskproc;
-	
+	hwif->busproc			= &hpt3xx_busproc;
+
 	/*
 	 * HPT3xxN chips have some complications:
 	 *
@@ -1237,7 +1236,7 @@ static void __devinit init_hwif_hpt366(i
 
 	/*
 	 * The HPT37x uses the CBLID pins as outputs for MA15/MA16
-	 * address lines to access an external eeprom.  To read valid
+	 * address lines to access an external EEPROM.  To read valid
 	 * cable detect state the pins must be enabled as inputs.
 	 */
 	if (info->revision >= 8 && (PCI_FUNC(dev->devfn) & 1)) {
@@ -1246,35 +1245,28 @@ static void __devinit init_hwif_hpt366(i
 		 * - set bit 15 of reg 0x52 to enable TCBLID as input
 		 * - set bit 15 of reg 0x56 to enable FCBLID as input
 		 */
-		u16 mcr3, mcr6;
-		pci_read_config_word(dev, 0x52, &mcr3);
-		pci_read_config_word(dev, 0x56, &mcr6);
-		pci_write_config_word(dev, 0x52, mcr3 | 0x8000);
-		pci_write_config_word(dev, 0x56, mcr6 | 0x8000);
+		u8  mcr_addr = hwif->select_data + 2;
+		u16 mcr;
+
+		pci_read_config_word (dev, mcr_addr, &mcr);
+		pci_write_config_word(dev, mcr_addr, (mcr | 0x8000));
 		/* now read cable id register */
-		pci_read_config_byte(dev, 0x5a, &ata66);
-		pci_write_config_word(dev, 0x52, mcr3);
-		pci_write_config_word(dev, 0x56, mcr6);
+		pci_read_config_byte (dev, 0x5a, &scr1);
+		pci_write_config_word(dev, mcr_addr, mcr);
 	} else if (info->revision >= 3) {
 		/*
 		 * HPT370/372 and 374 pcifn 0
-		 * - clear bit 0 of 0x5b to enable P/SCBLID as inputs
+		 * - clear bit 0 of reg 0x5b to enable P/SCBLID as inputs
 		 */
-		u8 scr2;
-		pci_read_config_byte(dev, 0x5b, &scr2);
-		pci_write_config_byte(dev, 0x5b, scr2 & ~1);
-		/* now read cable id register */
-		pci_read_config_byte(dev, 0x5a, &ata66);
-		pci_write_config_byte(dev, 0x5b, scr2);
-	} else {
-		pci_read_config_byte(dev, 0x5a, &ata66);
-	}
+		u8 scr2 = 0;
 
-#ifdef DEBUG
-	printk("HPT366: reg5ah=0x%02x ATA-%s Cable Port%d\n",
-		ata66, (ata66 & regmask) ? "33" : "66",
-		PCI_FUNC(hwif->pci_dev->devfn));
-#endif /* DEBUG */
+		pci_read_config_byte (dev, 0x5b, &scr2);
+		pci_write_config_byte(dev, 0x5b, (scr2 & ~1));
+		/* now read cable id register */
+		pci_read_config_byte (dev, 0x5a, &scr1);
+		pci_write_config_byte(dev, 0x5b,  scr2);
+	} else
+		pci_read_config_byte (dev, 0x5a, &scr1);
 
 	/* Serialize access to this device */
 	if (serialize && hwif->mate)
@@ -1296,7 +1288,7 @@ static void __devinit init_hwif_hpt366(i
 	hwif->mwdma_mask = 0x07;
 
 	if (!(hwif->udma_four))
-		hwif->udma_four = ((ata66 & regmask) ? 0 : 1);
+		hwif->udma_four = ((scr1 & ata66) ? 0 : 1);
 	hwif->ide_dma_check = &hpt366_config_drive_xfer_rate;
 
 	if (info->revision >= 8) {
@@ -1323,11 +1315,10 @@ static void __devinit init_hwif_hpt366(i
 
 static void __devinit init_dma_hpt366(ide_hwif_t *hwif, unsigned long dmabase)
 {
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
-	u8 masterdma	= 0, slavedma = 0;
-	u8 dma_new	= 0, dma_old = 0;
-	u8 primary	= hwif->channel ? 0x4b : 0x43;
-	u8 secondary	= hwif->channel ? 0x4f : 0x47;
+	struct pci_dev  *dev		= hwif->pci_dev;
+	struct hpt_info	*info		= ide_get_hwifdata(hwif);
+	u8 masterdma	= 0, slavedma	= 0;
+	u8 dma_new	= 0, dma_old	= 0;
 	unsigned long flags;
 
 	if (!dmabase)
@@ -1344,13 +1335,13 @@ static void __devinit init_dma_hpt366(id
 	local_irq_save(flags);
 
 	dma_new = dma_old;
-	pci_read_config_byte(hwif->pci_dev, primary, &masterdma);
-	pci_read_config_byte(hwif->pci_dev, secondary, &slavedma);
+	pci_read_config_byte(dev, hwif->channel ? 0x4b : 0x43, &masterdma);
+	pci_read_config_byte(dev, hwif->channel ? 0x4f : 0x47,  &slavedma);
 
 	if (masterdma & 0x30)	dma_new |= 0x20;
-	if (slavedma & 0x30)	dma_new |= 0x40;
+	if ( slavedma & 0x30)	dma_new |= 0x40;
 	if (dma_new != dma_old)
-		hwif->OUTB(dma_new, dmabase+2);
+		hwif->OUTB(dma_new, dmabase + 2);
 
 	local_irq_restore(flags);
 



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

* Re: [PATCH] HPT3xx: switch to using pci_get_slot()
  2006-05-27 23:30           ` [PATCH] HPT3xx: switch to using pci_get_slot() Sergei Shtylyov
  2006-05-28  2:55             ` [PATCH] HPT3xx: cache channel's MCR address Sergei Shtylyov
@ 2006-05-28 13:51             ` Jiri Slaby
  2006-05-29 14:26               ` Sergei Shtylyov
  1 sibling, 1 reply; 20+ messages in thread
From: Jiri Slaby @ 2006-05-28 13:51 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, linux-ide,
	linux-kernel, Alan Cox

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sergei Shtylyov napsal(a):
>    Switch to using pci_get_slot() to get to the function 1 of HPT36x/374
> chips -- there's no need for the driver itself to walk the list of the
> PCI devices, and it also forgets to check the bus number of the device
> found.
It's better, but you missed to call pci_dev_put() in some __exit function.
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 
> 
> ------------------------------------------------------------------------
> 
> Index: linus/drivers/ide/pci/hpt366.c
> ===================================================================
> --- linus.orig/drivers/ide/pci/hpt366.c
> +++ linus/drivers/ide/pci/hpt366.c
> @@ -1,5 +1,5 @@
>  /*
> - * linux/drivers/ide/pci/hpt366.c		Version 0.44	May 20, 2006
> + * linux/drivers/ide/pci/hpt366.c		Version 0.45	May 27, 2006
>   *
>   * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
>   * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
> @@ -79,6 +79,7 @@
>   * - prefix the driver startup messages with the real chip name
>   * - claim the extra 240 bytes of I/O space for all chips
>   * - optimize the rate masking/filtering and the drive list lookup code
> + * - use pci_get_slot() to get to the function 1 of HPT36x/374
>   *		<source@mvista.com>
>   *
>   */
> @@ -1412,24 +1413,24 @@ static void __devinit init_iops_hpt366(i
>  
>  static int __devinit init_setup_hpt374(struct pci_dev *dev, ide_pci_device_t *d)
>  {
> -	struct pci_dev *findev = NULL;
> +	struct pci_dev *dev2;
>  
>  	if (PCI_FUNC(dev->devfn) & 1)
>  		return -ENODEV;
>  
> -	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
> -		if ((findev->vendor == dev->vendor) &&
> -		    (findev->device == dev->device) &&
> -		    ((findev->devfn - dev->devfn) == 1) &&
> -		    (PCI_FUNC(findev->devfn) & 1)) {
> -			if (findev->irq != dev->irq) {
> -				/* FIXME: we need a core pci_set_interrupt() */
> -				findev->irq = dev->irq;
> -				printk(KERN_WARNING "%s: pci-config space interrupt "
> -					"fixed.\n", d->name);
> -			}
> -			return ide_setup_pci_devices(dev, findev, d);
> +	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
> +		int ret;
> +
> +		if (dev2->irq != dev->irq) {
> +			/* FIXME: we need a core pci_set_interrupt() */
> +			dev2->irq = dev->irq;
> +			printk(KERN_WARNING "%s: PCI config space interrupt "
> +			       "fixed.\n", d->name);
>  		}
> +		ret = ide_setup_pci_devices(dev, dev2, d);
> +		if (ret < 0)
> +			pci_dev_put(dev2);
> +		return ret;
>  	}
>  	return ide_setup_pci_device(dev, d);
>  }
> @@ -1487,8 +1488,8 @@ static int __devinit init_setup_hpt302(s
>  
>  static int __devinit init_setup_hpt366(struct pci_dev *dev, ide_pci_device_t *d)
>  {
> -	struct pci_dev *findev = NULL;
> -	u8 rev = 0, pin1 = 0, pin2 = 0;
> +	struct pci_dev *dev2;
> +	u8 rev = 0;
>  	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
>  					   "HPT370", "HPT370A", "HPT372",
>  					   "HPT372N" };
> @@ -1508,21 +1509,21 @@ static int __devinit init_setup_hpt366(s
>  
>  	d->channels = 1;
>  
> -	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin1);
> -	while ((findev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, findev)) != NULL) {
> -		if ((findev->vendor == dev->vendor) &&
> -		    (findev->device == dev->device) &&
> -		    ((findev->devfn - dev->devfn) == 1) &&
> -		    (PCI_FUNC(findev->devfn) & 1)) {
> -			pci_read_config_byte(findev, PCI_INTERRUPT_PIN, &pin2);
> -			if ((pin1 != pin2) && (dev->irq == findev->irq)) {
> -				d->bootable = ON_BOARD;
> -				printk("%s: onboard version of chipset, "
> -					"pin1=%d pin2=%d\n", d->name,
> -					pin1, pin2);
> -			}
> -			return ide_setup_pci_devices(dev, findev, d);
> +	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
> +	  	u8  pin1 = 0, pin2 = 0;
> +		int ret;
> +
> +		pci_read_config_byte(dev,  PCI_INTERRUPT_PIN, &pin1);
> +		pci_read_config_byte(dev2, PCI_INTERRUPT_PIN, &pin2);
> +		if (pin1 != pin2 && dev->irq == dev2->irq) {
> +			d->bootable = ON_BOARD;
> +			printk("%s: onboard version of chipset, pin1=%d pin2=%d\n",
> +			       d->name, pin1, pin2);
>  		}
> +		ret = ide_setup_pci_devices(dev, dev2, d);
> +		if (ret < 0)
> +			pci_dev_put(dev2);
> +		return ret;
>  	}
>  init_single:
>  	return ide_setup_pci_device(dev, d);
> 


- --
Jiri Slaby         www.fi.muni.cz/~xslaby
\_.-^-._   jirislaby@gmail.com   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEeaq4MsxVwznUen4RAgeRAKDFZZWKIOW1kEOOnsMPRmsNBGc0AQCfWM1f
v7Ub4sGcHL0nCGJ6dhoj4g0=
=e7BC
-----END PGP SIGNATURE-----

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

* Re: [PATCH] HPT3xx: switch to using pci_get_slot()
  2006-05-28 13:51             ` [PATCH] HPT3xx: switch to using pci_get_slot() Jiri Slaby
@ 2006-05-29 14:26               ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-05-29 14:26 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, linux-ide,
	linux-kernel, Alan Cox

Hello.

Jiri Slaby wrote:

>>   Switch to using pci_get_slot() to get to the function 1 of HPT36x/374
>>chips -- there's no need for the driver itself to walk the list of the
>>PCI devices, and it also forgets to check the bus number of the device
>>found.

> It's better, but you missed to call pci_dev_put() in some __exit function.

    If you knew the PCI IDE drivers better, you'd know there's simply no 
__exit functions in them. ;-)

WBR, Sergei

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

* Re: [PATCH] HPT3x7: merge speedproc handlers
  2006-05-28  2:55             ` [PATCH] HPT3xx: cache channel's MCR address Sergei Shtylyov
@ 2006-06-04 22:24               ` Sergei Shtylyov
  2006-06-11 21:18                 ` [PATCH] HPT370: clean up DMA timeout handling Sergei Shtylyov
  0 siblings, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-06-04 22:24 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

Continue with the driver rewrite:

- move the interrupt twiddling code from the speedproc handlers into the
    init_hwif_hpt366 which allows to merge the two HPT37x speedproc handlers
    into one;

- get rid of in init_hpt366 which solely consists of the duplicate code, then
    fold init_hpt37x() into init_chipset_hpt366();

- fix hpt3xx_tune_drive() to always set the PIO mode requested, not the best
    possible one, change hpt366_config_drive_xfer_rate() accordingly, simplify
    it a bit;

- group all the DMA related code together init_hwif_hpt366(), and generally
    clean up and beautify it.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT37x-merge-speedproc.patch --]
[-- Type: text/plain, Size: 13096 bytes --]

Index: linux-2.6/drivers/ide/pci/hpt366.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/hpt366.c
+++ linux-2.6/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.50	May 28, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.51	Jun 04, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -83,13 +83,17 @@
  * - cache the channel's MCRs' offset; only touch the relevant MCR when detecting
  *   the cable type on HPT374's function 1
  * - rename all the register related variables consistently
- * - make HPT36x's speedproc handler look the same way as HPT37x ones; fix the
- *   PIO timing register mask for HPT37x
+ * - move the interrupt twiddling code from the speedproc handlers into the
+ *   init_hwif handler, also grouping all the DMA related code together there;
+ *   simplify  the init_chipset handler
+ * - merge two HPT37x speedproc handlers and fix the PIO timing register mask
+ *   there; make HPT36x speedproc handler look the same way as the HPT37x one
+ * - fix  the tuneproc handler to always set the PIO mode requested,  not the
+ *   best possible one
  *		<source@mvista.com>
  *
  */
 
-
 #include <linux/config.h>
 #include <linux/types.h>
 #include <linux/module.h>
@@ -503,19 +507,9 @@ static int hpt36x_tune_chipset(ide_drive
 	struct hpt_info	*info	= ide_get_hwifdata (hwif);
 	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
 	u8  itr_addr		= drive->dn ? 0x44 : 0x40;
-	u8  mcr_addr		= hwif->select_data + 1;
-	u8  mcr			= 0;
-	u32 new_itr, old_itr	= 0;
 	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x30070000 : 0xc0000000;
-
-	/*
-	 * Disable the "fast interrupt" prediction.
-	 */
-	pci_read_config_byte(dev, mcr_addr, &mcr);
-	if (mcr & 0x80)
-		pci_write_config_byte(dev, mcr_addr, mcr & ~0x80);
-
-	new_itr = pci_bus_clock_list(speed, info->speed);
+	u32 new_itr		= pci_bus_clock_list(speed, info->speed);
+	u32 old_itr		= 0;
 
 	/*
 	 * Disable on-chip PIO FIFO/buffer (and PIO MST mode as well)
@@ -530,38 +524,16 @@ static int hpt36x_tune_chipset(ide_drive
 	return ide_config_drive_speed(drive, speed);
 }
 
-static int hpt370_tune_chipset(ide_drive_t *drive, u8 xferspeed)
+static int hpt37x_tune_chipset(ide_drive_t *drive, u8 xferspeed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev  *dev	= hwif->pci_dev;
 	struct hpt_info	*info	= ide_get_hwifdata (hwif);
 	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
-	u8  mcr_addr		= hwif->select_data + 1;
 	u8  itr_addr		= 0x40 + (drive->dn * 4);
-	u8  new_mcr		= 0, old_mcr = 0;
-	u32 new_itr, old_itr	= 0;
 	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x303c0000 : 0xc0000000;
-
-	/*
-	 * Disable the "fast interrupt" prediction.
-	 * don't holdoff on interrupts. (== 0x01 despite what the docs say) 
-	 */
-	pci_read_config_byte(dev, mcr_addr, &old_mcr);
-	new_mcr = old_mcr;
-	if (new_mcr & 0x02)
-		new_mcr &= ~0x02;
-
-#ifdef HPT_DELAY_INTERRUPT
-	if (new_mcr & 0x01)
-		new_mcr &= ~0x01;
-#else
-	if ((new_mcr & 0x01) == 0)
-		new_mcr |= 0x01;
-#endif
-	if (new_mcr != old_mcr)
-		pci_write_config_byte(dev, mcr_addr, new_mcr);
-
-	new_itr = pci_bus_clock_list(speed, info->speed);
+	u32 new_itr		= pci_bus_clock_list(speed, info->speed);
+	u32 old_itr		= 0;
 
 	pci_read_config_dword(dev, itr_addr, &old_itr);
 	new_itr = (new_itr & ~itr_mask) | (old_itr & itr_mask);
@@ -573,71 +545,34 @@ static int hpt370_tune_chipset(ide_drive
 	return ide_config_drive_speed(drive, speed);
 }
 
-static int hpt372_tune_chipset(ide_drive_t *drive, u8 xferspeed)
+static int hpt3xx_tune_chipset(ide_drive_t *drive, u8 speed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	struct pci_dev  *dev	= hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata (hwif);
-	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
-	u8  mcr_addr		= hwif->select_data + 1;
-	u8  itr_addr		= 0x40 + (drive->dn * 4);
-	u8  mcr			= 0;
-	u32 new_itr, old_itr	= 0;
-	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x303c0000 : 0xc0000000;
-
-	/*
-	 * Disable the "fast interrupt" prediction.
-	 * don't holdoff on interrupts. (== 0x01 despite what the docs say)
-	 */
-	pci_read_config_byte (dev, mcr_addr, &mcr);
-	pci_write_config_byte(dev, mcr_addr, (mcr & ~0x07));
-
-	new_itr = pci_bus_clock_list(speed, info->speed);
-	pci_read_config_dword(dev, itr_addr, &old_itr);
-	new_itr = (new_itr & ~itr_mask) | (old_itr & itr_mask);
-	if (speed < XFER_MW_DMA_0)
-		new_itr &= ~0x80000000; /* Disable on-chip PIO FIFO/buffer */
-	pci_write_config_dword(dev, itr_addr, new_itr);
-
-	return ide_config_drive_speed(drive, speed);
-}
-
-static int hpt3xx_tune_chipset (ide_drive_t *drive, u8 speed)
-{
-	ide_hwif_t *hwif	= drive->hwif;
 	struct hpt_info	*info	= ide_get_hwifdata(hwif);
 
-	if (info->revision >= 8)
-		return hpt372_tune_chipset(drive, speed); /* not a typo */
-	else if (info->revision >= 5)
-		return hpt372_tune_chipset(drive, speed);
-	else if (info->revision >= 3)
-		return hpt370_tune_chipset(drive, speed);
+	if (info->revision >= 3)
+		return hpt37x_tune_chipset(drive, speed);
 	else	/* hpt368: hpt_minimum_revision(dev, 2) */
 		return hpt36x_tune_chipset(drive, speed);
 }
 
-static void hpt3xx_tune_drive (ide_drive_t *drive, u8 pio)
+static void hpt3xx_tune_drive(ide_drive_t *drive, u8 pio)
 {
-	pio = ide_get_best_pio_mode(drive, 255, pio, NULL);
-	(void) hpt3xx_tune_chipset(drive, (XFER_PIO_0 + pio));
+	pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+	(void) hpt3xx_tune_chipset (drive, XFER_PIO_0 + pio);
 }
 
 /*
  * This allows the configuration of ide_pci chipset registers
  * for cards that learn about the drive's UDMA, DMA, PIO capabilities
- * after the drive is reported by the OS.  Initially for designed for
+ * after the drive is reported by the OS.  Initially designed for
  * HPT366 UDMA chipset by HighPoint|Triones Technologies, Inc.
  *
- * check_in_drive_lists(drive, bad_ata66_4)
- * check_in_drive_lists(drive, bad_ata66_3)
- * check_in_drive_lists(drive, bad_ata33)
- *
  */
-static int config_chipset_for_dma (ide_drive_t *drive)
+static int config_chipset_for_dma(ide_drive_t *drive)
 {
 	u8 speed = ide_dma_speed(drive, hpt3xx_ratemask(drive));
-	ide_hwif_t *hwif = drive->hwif;
+	ide_hwif_t *hwif	= HWIF(drive);
 	struct hpt_info	*info	= ide_get_hwifdata(hwif);
 
 	if (!speed)
@@ -662,7 +597,7 @@ static int hpt3xx_quirkproc(ide_drive_t 
 	return 0;
 }
 
-static void hpt3xx_intrproc (ide_drive_t *drive)
+static void hpt3xx_intrproc(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = HWIF(drive);
 
@@ -672,7 +607,7 @@ static void hpt3xx_intrproc (ide_drive_t
 	hwif->OUTB(drive->ctl | 2, IDE_CONTROL_REG);
 }
 
-static void hpt3xx_maskproc (ide_drive_t *drive, int mask)
+static void hpt3xx_maskproc(ide_drive_t *drive, int mask)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev	*dev	= hwif->pci_dev;
@@ -701,25 +636,22 @@ static void hpt3xx_maskproc (ide_drive_t
 			   IDE_CONTROL_REG);
 }
 
-static int hpt366_config_drive_xfer_rate (ide_drive_t *drive)
+static int hpt366_config_drive_xfer_rate(ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= drive->hwif;
+	ide_hwif_t *hwif	= HWIF(drive);
 	struct hd_driveid *id	= drive->id;
 
 	drive->init_speed = 0;
 
 	if ((id->capability & 1) && drive->autodma) {
-
-		if (ide_use_dma(drive)) {
-			if (config_chipset_for_dma(drive))
-				return hwif->ide_dma_on(drive);
-		}
+		if (ide_use_dma(drive) && config_chipset_for_dma(drive))
+			return hwif->ide_dma_on(drive);
 
 		goto fast_ata_pio;
 
 	} else if ((id->capability & 8) || (id->field_valid & 2)) {
 fast_ata_pio:
-		hpt3xx_tune_drive(drive, 5);
+		hpt3xx_tune_drive(drive, 255);
 		return hwif->ide_dma_off_quietly(drive);
 	}
 	/* IORDY not supported */
@@ -1150,34 +1082,8 @@ init_hpt37X_done:
 	udelay(100);
 }
 
-static int __devinit init_hpt37x(struct pci_dev *dev)
-{
-	u8 scr1;
-
-	pci_read_config_byte (dev, 0x5a, &scr1);
-	/* interrupt force enable */
-	pci_write_config_byte(dev, 0x5a, (scr1 & ~0x10));
-	return 0;
-}
-
-static int __devinit init_hpt366(struct pci_dev *dev)
-{
-	u8 mcr	= 0;
-
-	/*
-	 * Disable the "fast interrupt" prediction.
-	 */
-	pci_read_config_byte(dev, 0x51, &mcr);
-	if (mcr & 0x80)
-		pci_write_config_byte(dev, 0x51, mcr & ~0x80);
-									
-	return 0;
-}
-
 static unsigned int __devinit init_chipset_hpt366(struct pci_dev *dev, const char *name)
 {
-	int ret = 0;
-
 	/*
 	 * FIXME: Not portable. Also, why do we enable the ROM in the first place?
 	 * We don't seem to be using it.
@@ -1191,23 +1097,25 @@ static unsigned int __devinit init_chips
 	pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
 	pci_write_config_byte(dev, PCI_MAX_LAT, 0x08);
 
-	if (hpt_revision(dev) >= 3)
-		ret = init_hpt37x(dev);
-	else
-		ret = init_hpt366(dev);
+	if (hpt_revision(dev) >= 3) {
+		u8 scr1 = 0;
 
-	if (ret)
-		return ret;
+		/* Interrupt force enable. */
+		pci_read_config_byte(dev, 0x5a, &scr1);
+		if (scr1 & 0x10)
+			pci_write_config_byte(dev, 0x5a, scr1 & ~0x10);
+	}
 
 	return dev->irq;
 }
 
 static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
 {
-	struct pci_dev  *dev		= hwif->pci_dev;
+	struct pci_dev	*dev		= hwif->pci_dev;
 	struct hpt_info *info		= ide_get_hwifdata(hwif);
 	int serialize			= HPT_SERIALIZE_IO;
 	u8  scr1 = 0, ata66		= (hwif->channel) ? 0x01 : 0x02;
+	u8  new_mcr, old_mcr 		= 0;
 
 	/* Cache the channel's MISC. control registers' offset */
 	hwif->select_data		= hwif->channel ? 0x54 : 0x50;
@@ -1234,6 +1142,41 @@ static void __devinit init_hwif_hpt366(i
 		hwif->rw_disk = &hpt3xxn_rw_disk;
 	}
 
+	/* Serialize access to this device if needed */
+	if (serialize && hwif->mate)
+		hwif->serialized = hwif->mate->serialized = 1;
+
+	/*
+	 * Disable the "fast interrupt" prediction.  Don't hold off
+	 * on interrupts. (== 0x01 despite what the docs say)
+	 */
+	pci_read_config_byte(dev, hwif->select_data + 1, &old_mcr);
+
+	if (info->revision >= 5)		/* HPT372 and newer   */
+		new_mcr = old_mcr & ~0x07;
+	else if (info->revision >= 3) {		/* HPT370 and HPT370A */
+		new_mcr = old_mcr;
+		new_mcr &= ~0x02;
+
+#ifdef HPT_DELAY_INTERRUPT
+		new_mcr &= ~0x01;
+#else
+		new_mcr |=  0x01;
+#endif
+	} else					/* HPT366 and HPT368  */
+		new_mcr = old_mcr & ~0x80;
+
+	if (new_mcr != old_mcr)
+		pci_write_config_byte(dev, hwif->select_data + 1, new_mcr);
+
+	if (!hwif->dma_base) {
+		hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
+		return;
+	}
+
+	hwif->ultra_mask = 0x7f;
+	hwif->mwdma_mask = 0x07;
+
 	/*
 	 * The HPT37x uses the CBLID pins as outputs for MA15/MA16
 	 * address lines to access an external EEPROM.  To read valid
@@ -1268,54 +1211,30 @@ static void __devinit init_hwif_hpt366(i
 	} else
 		pci_read_config_byte (dev, 0x5a, &scr1);
 
-	/* Serialize access to this device */
-	if (serialize && hwif->mate)
-		hwif->serialized = hwif->mate->serialized = 1;
+	if (!hwif->udma_four)
+		hwif->udma_four = (scr1 & ata66) ? 0 : 1;
 
-	/*
-	 * Set up ioctl for power status.
-	 * NOTE:  power affects both drives on each channel.
-	 */
-	hwif->busproc = &hpt3xx_busproc;
-
-	if (!hwif->dma_base) {
-		hwif->drives[0].autotune = 1;
-		hwif->drives[1].autotune = 1;
-		return;
-	}
+	hwif->ide_dma_check		= &hpt366_config_drive_xfer_rate;
 
-	hwif->ultra_mask = 0x7f;
-	hwif->mwdma_mask = 0x07;
-
-	if (!(hwif->udma_four))
-		hwif->udma_four = ((scr1 & ata66) ? 0 : 1);
-	hwif->ide_dma_check = &hpt366_config_drive_xfer_rate;
-
-	if (info->revision >= 8) {
-		hwif->ide_dma_test_irq = &hpt374_ide_dma_test_irq;
-		hwif->ide_dma_end = &hpt374_ide_dma_end;
-	} else if (info->revision >= 5) {
-		hwif->ide_dma_test_irq = &hpt374_ide_dma_test_irq;
-		hwif->ide_dma_end = &hpt374_ide_dma_end;
+	if (info->revision >= 5) {
+		hwif->ide_dma_test_irq	= &hpt374_ide_dma_test_irq;
+		hwif->ide_dma_end	= &hpt374_ide_dma_end;
 	} else if (info->revision >= 3) {
-		hwif->dma_start = &hpt370_ide_dma_start;
-		hwif->ide_dma_end = &hpt370_ide_dma_end;
-		hwif->ide_dma_timeout = &hpt370_ide_dma_timeout;
-		hwif->ide_dma_lostirq = &hpt370_ide_dma_lostirq;
-	} else if (info->revision >= 2)
-		hwif->ide_dma_lostirq = &hpt366_ide_dma_lostirq;
-	else
-		hwif->ide_dma_lostirq = &hpt366_ide_dma_lostirq;
+		hwif->dma_start 	= &hpt370_ide_dma_start;
+		hwif->ide_dma_end	= &hpt370_ide_dma_end;
+		hwif->ide_dma_timeout	= &hpt370_ide_dma_timeout;
+		hwif->ide_dma_lostirq	= &hpt370_ide_dma_lostirq;
+	} else
+		hwif->ide_dma_lostirq	= &hpt366_ide_dma_lostirq;
 
 	if (!noautodma)
 		hwif->autodma = 1;
-	hwif->drives[0].autodma = hwif->autodma;
-	hwif->drives[1].autodma = hwif->autodma;
+	hwif->drives[0].autodma = hwif->drives[1].autodma = hwif->autodma;
 }
 
 static void __devinit init_dma_hpt366(ide_hwif_t *hwif, unsigned long dmabase)
 {
-	struct pci_dev  *dev		= hwif->pci_dev;
+	struct pci_dev	*dev		= hwif->pci_dev;
 	struct hpt_info	*info		= ide_get_hwifdata(hwif);
 	u8 masterdma	= 0, slavedma	= 0;
 	u8 dma_new	= 0, dma_old	= 0;
@@ -1330,7 +1249,7 @@ static void __devinit init_dma_hpt366(id
 		return;
 	}
 
-	dma_old = hwif->INB(dmabase+2);
+	dma_old = hwif->INB(dmabase + 2);
 
 	local_irq_save(flags);
 


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

* [PATCH] HPT370: clean up DMA timeout handling
  2006-06-04 22:24               ` [PATCH] HPT3x7: merge speedproc handlers Sergei Shtylyov
@ 2006-06-11 21:18                 ` Sergei Shtylyov
  2006-06-27 21:41                   ` [PATCH][RFT] HPT3xx: init code rewrite Sergei Shtylyov
  0 siblings, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2006-06-11 21:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Clean up DMA timeout handling for HPT370:

- hpt370_lostirq_timeout() cleared the DMA status which made __ide_dma_end()
   called afterwards return the incorrect result, and the DMA engine was reset
   both before and after stopping DMA while the HighPoint drivers only do it
   after (which seems logical) -- fix this and also rename the function;

- get rid of the needless mutual recursion in hpt370_ide_dma_end() and
   hpt370_ide_dma_timeout();

- get rid of hpt370_lostirq_timeout() since hwif->ide_dma_end() called from
   the driver's interrupt handler later does all its work.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT370-cleanup-DMA-timeout-handling.patch --]
[-- Type: text/plain, Size: 3665 bytes --]

Index: linux-2.6/drivers/ide/pci/hpt366.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/hpt366.c
+++ linux-2.6/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.51	Jun 04, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 0.52	Jun 07, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -90,6 +90,7 @@
  *   there; make HPT36x speedproc handler look the same way as the HPT37x one
  * - fix  the tuneproc handler to always set the PIO mode requested,  not the
  *   best possible one
+ * - clean up DMA timeout handling for HPT370
  *		<source@mvista.com>
  *
  */
@@ -677,7 +678,7 @@ static int hpt366_ide_dma_lostirq(ide_dr
 	return __ide_dma_lostirq(drive);
 }
 
-static void hpt370_clear_engine (ide_drive_t *drive)
+static void hpt370_clear_engine(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = HWIF(drive);
 
@@ -685,6 +686,22 @@ static void hpt370_clear_engine (ide_dri
 	udelay(10);
 }
 
+static void hpt370_irq_timeout(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif	= HWIF(drive);
+	u16 bfifo		= 0;
+	u8  dma_cmd;
+
+	pci_read_config_word(hwif->pci_dev, hwif->select_data + 2, &bfifo);
+	printk(KERN_DEBUG "%s: %d bytes in FIFO\n", drive->name, bfifo & 0x1ff);
+
+	/* get DMA command mode */
+	dma_cmd = hwif->INB(hwif->dma_command);
+	/* stop DMA */
+	hwif->OUTB(dma_cmd & ~0x1, hwif->dma_command);
+	hpt370_clear_engine(drive);
+}
+
 static void hpt370_ide_dma_start(ide_drive_t *drive)
 {
 #ifdef HPT_RESET_STATE_ENGINE
@@ -693,55 +710,28 @@ static void hpt370_ide_dma_start(ide_dri
 	ide_dma_start(drive);
 }
 
-static int hpt370_ide_dma_end (ide_drive_t *drive)
+static int hpt370_ide_dma_end(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	u8 dma_stat		= hwif->INB(hwif->dma_status);
+	u8  dma_stat		= hwif->INB(hwif->dma_status);
 
 	if (dma_stat & 0x01) {
 		/* wait a little */
 		udelay(20);
 		dma_stat = hwif->INB(hwif->dma_status);
+		if (dma_stat & 0x01)
+			/* fallthrough */
+			hpt370_irq_timeout(drive);
 	}
-	if ((dma_stat & 0x01) != 0) 
-		/* fallthrough */
-		(void) HWIF(drive)->ide_dma_timeout(drive);
-
 	return __ide_dma_end(drive);
 }
 
-static void hpt370_lostirq_timeout (ide_drive_t *drive)
+static int hpt370_ide_dma_timeout(ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 bfifo = 0;
-	u8 dma_stat = 0, dma_cmd = 0;
-
-	pci_read_config_byte(HWIF(drive)->pci_dev, hwif->select_data + 2, &bfifo);
-	printk(KERN_DEBUG "%s: %d bytes in FIFO\n", drive->name, bfifo);
-	hpt370_clear_engine(drive);
-	/* get dma command mode */
-	dma_cmd = hwif->INB(hwif->dma_command);
-	/* stop dma */
-	hwif->OUTB(dma_cmd & ~0x1, hwif->dma_command);
-	dma_stat = hwif->INB(hwif->dma_status);
-	/* clear errors */
-	hwif->OUTB(dma_stat | 0x6, hwif->dma_status);
-}
-
-static int hpt370_ide_dma_timeout (ide_drive_t *drive)
-{
-	hpt370_lostirq_timeout(drive);
-	hpt370_clear_engine(drive);
+	hpt370_irq_timeout(drive);
 	return __ide_dma_timeout(drive);
 }
 
-static int hpt370_ide_dma_lostirq (ide_drive_t *drive)
-{
-	hpt370_lostirq_timeout(drive);
-	hpt370_clear_engine(drive);
-	return __ide_dma_lostirq(drive);
-}
-
 /* returns 1 if DMA IRQ issued, 0 otherwise */
 static int hpt374_ide_dma_test_irq(ide_drive_t *drive)
 {
@@ -1223,7 +1213,6 @@ static void __devinit init_hwif_hpt366(i
 		hwif->dma_start 	= &hpt370_ide_dma_start;
 		hwif->ide_dma_end	= &hpt370_ide_dma_end;
 		hwif->ide_dma_timeout	= &hpt370_ide_dma_timeout;
-		hwif->ide_dma_lostirq	= &hpt370_ide_dma_lostirq;
 	} else
 		hwif->ide_dma_lostirq	= &hpt366_ide_dma_lostirq;
 

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

* [PATCH][RFT] HPT3xx: init code rewrite
  2006-06-11 21:18                 ` [PATCH] HPT370: clean up DMA timeout handling Sergei Shtylyov
@ 2006-06-27 21:41                   ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2006-06-27 21:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel, Alan Cox

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

    Finally, rework the driver init. code to correctly handle all the chip
variants HighPoint has created so far. This should cure the rest of the timing
issues in the driver (especially, on 66 MHz PCI) caused by the HighPoint's
habit of switching the base DPLL clock with every new revision of the chips...

  - switch to using the enumeration type to differ between the numerous chip
    variants, matching PCI device/revision ID with the chip type early, at the
    init_setup stage;

  - extend the hpt_info structure to hold the DPLL and PCI clock frequencies,
    stop duplicating it for each channel by storing the pointer in the pci_dev
    structure: first, at the init_setup stage, point it to a static "template"
    with only the chip type and its specific base DPLL frequency, the highest
    supported DMA mode, and the chip settings table pointer filled, then, at
    the init_chipset stage, allocate per-chip instance  and fill it with the
    rest of the necessary information;

  - get rid of the constant thresholds in the HPT37x PCI clock detection code,
    switch  to calculating  PCI clock frequency based on the chip's base DPLL
    frequency;

  - switch to using the DPLL clock and enable UltraATA/133 mode by default on
    anything newer than HPT370/A;

  - fold PCI clock detection and DPLL setup code into init_chipset_hpt366(),
    unify the HPT36x/37x setup code and the speedproc handlers by joining the
    register setting lists into the table indexed by the clock selected;

  - add enablebits for all the chips to avoid touching disabled channels
    (though the HighPoint BIOS seem to only disable the primary one on
    HPT371/N);

  - separate the UltraDMA and MWDMA masks there to avoid changing PIO timings
    when setting an UltraDMA mode in hpt37x_tune_chipset().

    This version has been tested on HPT370/302/371N.
    Thanks to Alan for the inspiration. Hopefully, his libata driver will also
benefit from the work done on this "obsolete" driver...

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: HPT3xx-init-code-rewrite.patch --]
[-- Type: text/plain, Size: 39968 bytes --]

Index: linux-2.6/drivers/ide/pci/hpt366.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/hpt366.c
+++ linux-2.6/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 0.52	Jun 07, 2006
+ * linux/drivers/ide/pci/hpt366.c		Version 1.00	Jun 25, 2006
  *
  * Copyright (C) 1999-2003		Andre Hedrick <andre@linux-ide.org>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -60,13 +60,10 @@
  *   channel caused the cached register value to get out of sync with the
  *   actual one, the channels weren't serialized, the turnaround shouldn't
  *   be done on 66 MHz PCI bus
- * - avoid calibrating PLL twice as the second time results in a wrong PCI
- *   frequency and thus in the wrong timings for the secondary channel
- * - disable UltraATA/133 for HPT372 and UltraATA/100 for HPT370 by default
- *   as the ATA clock being used does not allow for this speed anyway
- * - add support for HPT302N and HPT371N clocking (the same as for HPT372N)
- * - HPT371/N are single channel chips, so avoid touching the primary channel
- *   which exists only virtually (there's no pins for it)
+ * - disable UltraATA/100 for HPT370 by default as the 33 MHz clock being used
+ *   does not allow for this speed anyway
+ * - avoid touching disabled channels (e.g. HPT371/N are single channel chips,
+ *   their primary channel is kind of virtual, it isn't tied to any pins)
  * - fix/remove bad/unused timing tables and use one set of tables for the whole
  *   HPT37x chip family; save space by introducing the separate transfer mode
  *   table in which the mode lookup is done
@@ -74,24 +71,44 @@
  *   the wrong PCI frequency since DPLL has already been calibrated by BIOS
  * - fix the hotswap code:  it caused RESET- to glitch when tristating the bus,
  *   and for HPT36x the obsolete HDIO_TRISTATE_HWIF handler was called instead
- * - pass to init_chipset() handlers a copy of the IDE PCI device structure as
- *   they tamper with its fields
+ * - pass  to the init_setup handlers a copy of the ide_pci_device_t structure
+ *   since they may tamper with its fields
  * - prefix the driver startup messages with the real chip name
  * - claim the extra 240 bytes of I/O space for all chips
  * - optimize the rate masking/filtering and the drive list lookup code
  * - use pci_get_slot() to get to the function 1 of HPT36x/374
- * - cache the channel's MCRs' offset; only touch the relevant MCR when detecting
- *   the cable type on HPT374's function 1
+ * - cache offset of the channel's misc. control registers (MCRs) being used
+ *   throughout the driver
+ * - only touch the relevant MCR when detecting the cable type on HPT374's
+ *   function 1
  * - rename all the register related variables consistently
- * - move the interrupt twiddling code from the speedproc handlers into the
- *   init_hwif handler, also grouping all the DMA related code together there;
- *   simplify  the init_chipset handler
- * - merge two HPT37x speedproc handlers and fix the PIO timing register mask
- *   there; make HPT36x speedproc handler look the same way as the HPT37x one
- * - fix  the tuneproc handler to always set the PIO mode requested,  not the
- *   best possible one
+ * - move all the interrupt twiddling code from the speedproc handlers into
+ *   init_hwif_hpt366(), also grouping all the DMA related code together there
+ * - merge two HPT37x speedproc handlers, fix the PIO timing register mask and
+ *   separate the UltraDMA and MWDMA masks there to avoid changing PIO timings
+ *   when setting an UltraDMA mode
+ * - fix hpt3xx_tune_drive() to set the PIO mode requested, not always select
+ *   the best possible one
  * - clean up DMA timeout handling for HPT370
- *		<source@mvista.com>
+ * - switch to using the enumeration type to differ between the numerous chip
+ *   variants, matching PCI device/revision ID with the chip type early, at the
+ *   init_setup stage
+ * - extend the hpt_info structure to hold the DPLL and PCI clock frequencies,
+ *   stop duplicating it for each channel by storing the pointer in the pci_dev
+ *   structure: first, at the init_setup stage, point it to a static "template"
+ *   with only the chip type and its specific base DPLL frequency, the highest
+ *   supported DMA mode, and the chip settings table pointer filled, then, at
+ *   the init_chipset stage, allocate per-chip instance  and fill it with the
+ *   rest of the necessary information
+ * - get rid of the constant thresholds in the HPT37x PCI clock detection code,
+ *   switch  to calculating  PCI clock frequency based on the chip's base DPLL
+ *   frequency
+ * - switch to using the  DPLL clock and enable UltraATA/133 mode by default on
+ *   anything  newer than HPT370/A
+ * - fold PCI clock detection and DPLL setup code into init_chipset_hpt366();
+ *   unify HPT36x/37x timing setup code and the speedproc handlers by joining
+ *   the register setting lists into the table indexed by the clock selected
+ *	Sergei Shtylyov, <sshtylyov@ru.mvista.com> or <source@mvista.com>
  *
  */
 
@@ -347,71 +364,143 @@ static u32 sixty_six_base_hpt37x[] = {
 };
 
 #define HPT366_DEBUG_DRIVE_INFO		0
-#define HPT374_ALLOW_ATA133_6		0
-#define HPT371_ALLOW_ATA133_6		0
-#define HPT302_ALLOW_ATA133_6		0
-#define HPT372_ALLOW_ATA133_6		0
+#define HPT374_ALLOW_ATA133_6		1
+#define HPT371_ALLOW_ATA133_6		1
+#define HPT302_ALLOW_ATA133_6		1
+#define HPT372_ALLOW_ATA133_6		1
 #define HPT370_ALLOW_ATA100_5		0
 #define HPT366_ALLOW_ATA66_4		1
 #define HPT366_ALLOW_ATA66_3		1
 #define HPT366_MAX_DEVS			8
 
-#define F_LOW_PCI_33	0x23
-#define F_LOW_PCI_40	0x29
-#define F_LOW_PCI_50	0x2d
-#define F_LOW_PCI_66	0x42
+/* Supported ATA clock frequencies */
+enum ata_clock {
+	ATA_CLOCK_25MHZ,
+	ATA_CLOCK_33MHZ,
+	ATA_CLOCK_40MHZ,
+	ATA_CLOCK_50MHZ,
+	ATA_CLOCK_66MHZ,
+	NUM_ATA_CLOCKS
+};
 
 /*
- *	Hold all the highpoint quirks and revision information in one
- *	place.
+ *	Hold all the HighPoint chip information in one place.
  */
 
-struct hpt_info
-{
+struct hpt_info {
+	u8 chip_type;		/* Chip type */
 	u8 max_mode;		/* Speeds allowed */
-	u8 revision;		/* Chipset revision */
-	u8 flags;		/* Chipset properties */
-#define PLL_MODE	1
-#define IS_3xxN 	2
-#define PCI_66MHZ	4
-				/* Speed table */
-	u32 *speed;
+	u8 dpll_clk;		/* DPLL clock in MHz */
+	u8 pci_clk;		/* PCI  clock in MHz */
+	u32 **settings; 	/* Chipset settings table */
 };
 
-/*
- *	This wants fixing so that we do everything not by revision
- *	(which breaks on the newest chips) but by creating an
- *	enumeration of chip variants and using that
- */
+/* Supported HighPoint chips */
+enum {
+	HPT36x,
+	HPT370,
+	HPT370A,
+	HPT374,
+	HPT372,
+	HPT372A,
+	HPT302,
+	HPT371,
+	HPT372N,
+	HPT302N,
+	HPT371N
+};
 
-static __devinit u8 hpt_revision(struct pci_dev *dev)
-{
-	u8 rev = 0;
+static u32 *hpt36x_settings[NUM_ATA_CLOCKS] = {
+	twenty_five_base_hpt36x,
+	thirty_three_base_hpt36x,
+	forty_base_hpt36x,
+	NULL,
+	NULL
+};
 
-	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
+static u32 *hpt37x_settings[NUM_ATA_CLOCKS] = {
+	NULL,
+	thirty_three_base_hpt37x,
+	NULL,
+	fifty_base_hpt37x,
+	sixty_six_base_hpt37x
+};
 
-	switch(dev->device) {
-		/* Remap new 372N onto 372 */
-		case PCI_DEVICE_ID_TTI_HPT372N:
-			rev = PCI_DEVICE_ID_TTI_HPT372;
-			break;
-		case PCI_DEVICE_ID_TTI_HPT374:
-			rev = PCI_DEVICE_ID_TTI_HPT374;
-			break;
-		case PCI_DEVICE_ID_TTI_HPT371:
-			rev = PCI_DEVICE_ID_TTI_HPT371;
-			break;
-		case PCI_DEVICE_ID_TTI_HPT302:
-			rev = PCI_DEVICE_ID_TTI_HPT302;
-			break;
-		case PCI_DEVICE_ID_TTI_HPT372:
-			rev = PCI_DEVICE_ID_TTI_HPT372;
-			break;
-		default:
-			break;
-	}
-	return rev;
-}
+static struct hpt_info hpt36x __devinitdata = {
+	.chip_type	= HPT36x,
+	.max_mode	= (HPT366_ALLOW_ATA66_4 || HPT366_ALLOW_ATA66_3) ? 2 : 1,
+	.dpll_clk	= 0,	/* no DPLL */
+	.settings	= hpt36x_settings
+};
+
+static struct hpt_info hpt370 __devinitdata = {
+	.chip_type	= HPT370,
+	.max_mode	= HPT370_ALLOW_ATA100_5 ? 3 : 2,
+	.dpll_clk	= 48,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt370a __devinitdata = {
+	.chip_type	= HPT370A,
+	.max_mode	= HPT370_ALLOW_ATA100_5 ? 3 : 2,
+	.dpll_clk	= 48,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt374 __devinitdata = {
+	.chip_type	= HPT374,
+	.max_mode	= HPT374_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 48,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt372 __devinitdata = {
+	.chip_type	= HPT372,
+	.max_mode	= HPT372_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 55,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt372a __devinitdata = {
+	.chip_type	= HPT372A,
+	.max_mode	= HPT372_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 66,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt302 __devinitdata = {
+	.chip_type	= HPT302,
+	.max_mode	= HPT302_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 66,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt371 __devinitdata = {
+	.chip_type	= HPT371,
+	.max_mode	= HPT371_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 66,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt372n __devinitdata = {
+	.chip_type	= HPT372N,
+	.max_mode	= HPT372_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 77,
+	.settings	= hpt37x_settings
+};
+
+static struct hpt_info hpt302n __devinitdata = {
+	.chip_type	= HPT302N,
+	.max_mode	= HPT302_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 77,
+};
+
+static struct hpt_info hpt371n __devinitdata = {
+	.chip_type	= HPT371N,
+	.max_mode	= HPT371_ALLOW_ATA133_6 ? 4 : 3,
+	.dpll_clk	= 77,
+	.settings	= hpt37x_settings
+};
 
 static int check_in_drive_list(ide_drive_t *drive, const char **list)
 {
@@ -425,7 +514,7 @@ static int check_in_drive_list(ide_drive
 
 static u8 hpt3xx_ratemask(ide_drive_t *drive)
 {
-	struct hpt_info *info	= ide_get_hwifdata(HWIF(drive));
+	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
 	u8 mode			= info->max_mode;
 
 	if (!eighty_ninty_three(drive) && mode)
@@ -440,7 +529,8 @@ static u8 hpt3xx_ratemask(ide_drive_t *d
  
 static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
 {
-	struct hpt_info *info	= ide_get_hwifdata(HWIF(drive));
+	struct hpt_info *info	= pci_get_drvdata(HWIF(drive)->pci_dev);
+	u8 chip_type		= info->chip_type;
 	u8 mode			= hpt3xx_ratemask(drive);
 
 	if (drive->media != ide_disk)
@@ -448,21 +538,22 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
 
 	switch (mode) {
 		case 0x04:
-			speed = min(speed, (u8)XFER_UDMA_6);
+			speed = min_t(u8, speed, XFER_UDMA_6);
 			break;
 		case 0x03:
-			speed = min(speed, (u8)XFER_UDMA_5);
-			if (info->revision >= 5)
+			speed = min_t(u8, speed, XFER_UDMA_5);
+			if (chip_type >= HPT374)
 				break;
 			if (!check_in_drive_list(drive, bad_ata100_5))
 				goto check_bad_ata33;
 			/* fall thru */
 		case 0x02:
 			speed = min_t(u8, speed, XFER_UDMA_4);
-	/*
-	 * CHECK ME, Does this need to be set to 5 ??
-	 */
-			if (info->revision >= 3)
+
+			/*
+			 * CHECK ME, Does this need to be changed to HPT374 ??
+			 */
+			if (chip_type >= HPT370)
 				goto check_bad_ata33;
 			if (HPT366_ALLOW_ATA66_4 &&
 			    !check_in_drive_list(drive, bad_ata66_4))
@@ -477,7 +568,7 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
 			speed = min_t(u8, speed, XFER_UDMA_2);
 
 		check_bad_ata33:
- 			if (info->revision >= 4)
+			if (chip_type >= HPT370A)
 				break;
 			if (!check_in_drive_list(drive, bad_ata33))
 				break;
@@ -490,7 +581,7 @@ static u8 hpt3xx_ratefilter(ide_drive_t 
 	return speed;
 }
 
-static u32 pci_bus_clock_list(u8 speed, u32 *chipset_table)
+static u32 get_speed_setting(u8 speed, struct hpt_info *info)
 {
 	int i;
 
@@ -503,18 +594,23 @@ static u32 pci_bus_clock_list(u8 speed, 
 	for (i = 0; i < ARRAY_SIZE(xfer_speeds) - 1; i++)
 		if (xfer_speeds[i] == speed)
 			break;
-	return chipset_table[i];
+	/*
+	 * NOTE: info->settings only points to the pointer
+	 * to the list of the actual register values
+	 */
+	return (*info->settings)[i];
 }
 
 static int hpt36x_tune_chipset(ide_drive_t *drive, u8 xferspeed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev  *dev	= hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata (hwif);
+	struct hpt_info	*info	= pci_get_drvdata(dev);
 	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
 	u8  itr_addr		= drive->dn ? 0x44 : 0x40;
-	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x30070000 : 0xc0000000;
-	u32 new_itr		= pci_bus_clock_list(speed, info->speed);
+	u32 itr_mask		= speed < XFER_MW_DMA_0 ? 0x30070000 :
+				 (speed < XFER_UDMA_0   ? 0xc0070000 : 0xc03800ff);
+	u32 new_itr		= get_speed_setting(speed, info);
 	u32 old_itr		= 0;
 
 	/*
@@ -534,11 +630,12 @@ static int hpt37x_tune_chipset(ide_drive
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev  *dev	= hwif->pci_dev;
-	struct hpt_info	*info	= ide_get_hwifdata (hwif);
+	struct hpt_info	*info	= pci_get_drvdata(dev);
 	u8  speed		= hpt3xx_ratefilter(drive, xferspeed);
 	u8  itr_addr		= 0x40 + (drive->dn * 4);
-	u32 itr_mask		= (speed < XFER_MW_DMA_0) ? 0x303c0000 : 0xc0000000;
-	u32 new_itr		= pci_bus_clock_list(speed, info->speed);
+	u32 itr_mask		= speed < XFER_MW_DMA_0 ? 0x303c0000 :
+				 (speed < XFER_UDMA_0   ? 0xc03c0000 : 0xc1c001ff);
+	u32 new_itr		= get_speed_setting(speed, info);
 	u32 old_itr		= 0;
 
 	pci_read_config_dword(dev, itr_addr, &old_itr);
@@ -554,9 +651,9 @@ static int hpt37x_tune_chipset(ide_drive
 static int hpt3xx_tune_chipset(ide_drive_t *drive, u8 speed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
+	struct hpt_info	*info	= pci_get_drvdata(hwif->pci_dev);
 
-	if (info->revision >= 3)
+	if (info->chip_type >= HPT370)
 		return hpt37x_tune_chipset(drive, speed);
 	else	/* hpt368: hpt_minimum_revision(dev, 2) */
 		return hpt36x_tune_chipset(drive, speed);
@@ -578,16 +675,10 @@ static void hpt3xx_tune_drive(ide_drive_
 static int config_chipset_for_dma(ide_drive_t *drive)
 {
 	u8 speed = ide_dma_speed(drive, hpt3xx_ratemask(drive));
-	ide_hwif_t *hwif	= HWIF(drive);
-	struct hpt_info	*info	= ide_get_hwifdata(hwif);
 
 	if (!speed)
 		return 0;
 
-	/* If we don't have any timings we can't do a lot */
-	if (info->speed == NULL)
-		return 0;
-
 	(void) hpt3xx_tune_chipset(drive, speed);
 	return ide_dma_enable(drive);
 }
@@ -617,10 +708,10 @@ static void hpt3xx_maskproc(ide_drive_t 
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev	*dev	= hwif->pci_dev;
-	struct hpt_info *info	= ide_get_hwifdata(hwif);
+	struct hpt_info *info	= pci_get_drvdata(dev);
 
 	if (drive->quirk_list) {
-		if (info->revision >= 3) {
+		if (info->chip_type >= HPT370) {
 			u8 scr1 = 0;
 
 			pci_read_config_byte(dev, 0x5a, &scr1);
@@ -780,40 +871,37 @@ static int hpt374_ide_dma_end(ide_drive_
  *	@mode: clocking mode (0x21 for write, 0x23 otherwise)
  *
  *	Switch the DPLL clock on the HPT3xxN devices. This is a	right mess.
- *	NOTE: avoid touching the disabled primary channel on HPT371N -- it
- *	doesn't physically exist anyway...
  */
 
 static void hpt3xxn_set_clock(ide_hwif_t *hwif, u8 mode)
 {
-	u8 mcr1, scr2 = hwif->INB(hwif->dma_master + 0x7b);
+	u8 scr2 = hwif->INB(hwif->dma_master + 0x7b);
 
 	if ((scr2 & 0x7f) == mode)
 		return;
 
-	/* MISC. control register 1 has the channel enable bit... */
-	mcr1 = hwif->INB(hwif->dma_master + 0x70);
-
 	/* Tristate the bus */
-	if (mcr1 & 0x04)
-		hwif->OUTB(0x80, hwif->dma_master + 0x73);
+	hwif->OUTB(0x80, hwif->dma_master + 0x73);
 	hwif->OUTB(0x80, hwif->dma_master + 0x77);
 
 	/* Switch clock and reset channels */
 	hwif->OUTB(mode, hwif->dma_master + 0x7b);
 	hwif->OUTB(0xc0, hwif->dma_master + 0x79);
 
-	/* Reset state machines */
-	if (mcr1 & 0x04)
-		hwif->OUTB(0x37, hwif->dma_master + 0x70);
-	hwif->OUTB(0x37, hwif->dma_master + 0x74);
+	/*
+	 * Reset the state machines.
+	 * NOTE: avoid accidentally enabling the disabled channels.
+	 */
+	hwif->OUTB(hwif->INB(hwif->dma_master + 0x70) | 0x32,
+		   hwif->dma_master + 0x70);
+	hwif->OUTB(hwif->INB(hwif->dma_master + 0x74) | 0x32,
+		   hwif->dma_master + 0x74);
 
 	/* Complete reset */
 	hwif->OUTB(0x00, hwif->dma_master + 0x79);
 
 	/* Reconnect channels to bus */
-	if (mcr1 & 0x04)
-		hwif->OUTB(0x00, hwif->dma_master + 0x73);
+	hwif->OUTB(0x00, hwif->dma_master + 0x73);
 	hwif->OUTB(0x00, hwif->dma_master + 0x77);
 }
 
@@ -828,10 +916,7 @@ static void hpt3xxn_set_clock(ide_hwif_t
 
 static void hpt3xxn_rw_disk(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 wantclock		= rq_data_dir(rq) ? 0x23 : 0x21;
-
-	hpt3xxn_set_clock(hwif, wantclock);
+	hpt3xxn_set_clock(HWIF(drive), rq_data_dir(rq) ? 0x23 : 0x21);
 }
 
 /* 
@@ -892,223 +977,293 @@ static int hpt3xx_busproc(ide_drive_t *d
 	return 0;
 }
 
-static void __devinit hpt366_clocking(ide_hwif_t *hwif)
+/**
+ *	hpt37x_calibrate_dpll	-	calibrate the DPLL
+ *	@dev: PCI device
+ *
+ *	Perform a calibration cycle on the DPLL.
+ *	Returns 1 if this succeeds
+ */
+static int __devinit hpt37x_calibrate_dpll(struct pci_dev *dev, u16 f_low, u16 f_high)
 {
-	u32 itr1	= 0;
-	struct hpt_info *info = ide_get_hwifdata(hwif);
+	u32 dpll = (f_high << 16) | f_low | 0x100;
+	u8  scr2;
+	int i;
 
-	pci_read_config_dword(hwif->pci_dev, 0x40, &itr1);
+	pci_write_config_dword(dev, 0x5c, dpll);
 
-	/* detect bus speed by looking at control reg timing: */
-	switch((itr1 >> 8) & 7) {
-		case 5:
-			info->speed = forty_base_hpt36x;
-			break;
-		case 9:
-			info->speed = twenty_five_base_hpt36x;
-			break;
-		case 7:
-		default:
-			info->speed = thirty_three_base_hpt36x;
+	/* Wait for oscillator ready */
+	for(i = 0; i < 0x5000; ++i) {
+		udelay(50);
+		pci_read_config_byte(dev, 0x5b, &scr2);
+		if (scr2 & 0x80)
 			break;
 	}
+	/* See if it stays ready (we'll just bail out if it's not yet) */
+	for(i = 0; i < 0x1000; ++i) {
+		pci_read_config_byte(dev, 0x5b, &scr2);
+		/* DPLL destabilized? */
+		if(!(scr2 & 0x80))
+			return 0;
+	}
+	/* Turn off tuning, we have the DPLL set */
+	pci_read_config_dword (dev, 0x5c, &dpll);
+	pci_write_config_dword(dev, 0x5c, (dpll & ~0x100));
+	return 1;
 }
 
-static void __devinit hpt37x_clocking(ide_hwif_t *hwif)
+static unsigned int __devinit init_chipset_hpt366(struct pci_dev *dev, const char *name)
 {
-	struct hpt_info *info	= ide_get_hwifdata(hwif);
-	struct pci_dev  *dev	= hwif->pci_dev;
-	char *name		= hwif->cds->name;
-	int adjust, i;
-	u16 freq = 0;
-	u32 pll, temp = 0;
-	u8  scr2 = 0, mcr1 = 0;
-	
+	struct hpt_info *info	= kmalloc(sizeof(struct hpt_info), GFP_KERNEL);
+	unsigned long io_base	= pci_resource_start(dev, 4);
+	u8 pci_clk,  dpll_clk	= 0;	/* PCI and DPLL clock in MHz */
+	enum ata_clock	clock;
+
+	if (info == NULL) {
+		printk(KERN_ERR "%s: out of memory!\n", name);
+		return -ENOMEM;
+	}
+
 	/*
-	 * default to pci clock. make sure MA15/16 are set to output
-	 * to prevent drives having problems with 40-pin cables. Needed
-	 * for some drives such as IBM-DTLA which will not enter ready
-	 * state on reset when PDIAG is a input.
-	 *
-	 * ToDo: should we set 0x21 when using PLL mode ?
+	 * Copy everything from a static "template" structure
+	 * to just allocated per-chip hpt_info structure.
 	 */
-	pci_write_config_byte(dev, 0x5b, 0x23);
+	*info = *(struct hpt_info *)pci_get_drvdata(dev);
 
 	/*
-	 * We'll have to read f_CNT value in order to determine
-	 * the PCI clock frequency according to the following ratio:
-	 *
-	 * f_CNT = Fpci * 192 / Fdpll
-	 *
-	 * First try reading the register in which the HighPoint BIOS
-	 * saves f_CNT value before  reprogramming the DPLL from its
-	 * default setting (which differs for the various chips).
-	 * NOTE: This register is only accessible via I/O space.
-	 *
-	 * In case the signature check fails, we'll have to resort to
-	 * reading the f_CNT register itself in hopes that nobody has
-	 * touched the DPLL yet...
+	 * FIXME: Not portable. Also, why do we enable the ROM in the first place?
+	 * We don't seem to be using it.
 	 */
-	temp = inl(pci_resource_start(dev, 4) + 0x90);
-	if ((temp & 0xFFFFF000) != 0xABCDE000) {
-		printk(KERN_WARNING "%s: no clock data saved by BIOS\n", name);
-
-		/* Calculate the average value of f_CNT */
-		for (temp = i = 0; i < 128; i++) {
-			pci_read_config_word(dev, 0x78, &freq);
-			temp += freq & 0x1ff;
-			mdelay(1);
-		}
-		freq = temp / 128;
-	} else
-		freq = temp & 0x1ff;
+	if (dev->resource[PCI_ROM_RESOURCE].start)
+		pci_write_config_dword(dev, PCI_ROM_ADDRESS,
+			dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE);
+
+	pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, (L1_CACHE_BYTES / 4));
+	pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x78);
+	pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
+	pci_write_config_byte(dev, PCI_MAX_LAT, 0x08);
 
 	/*
-	 * HPT3xxN chips use different PCI clock information.
-	 * Currently we always set up the PLL for them.
+	 * First, try to estimate the PCI clock frequency...
 	 */
+	if (info->chip_type >= HPT370) {
+		u8  scr1  = 0;
+		u16 f_cnt = 0;
+		u32 temp  = 0;
+
+		/* Interrupt force enable. */
+		pci_read_config_byte(dev, 0x5a, &scr1);
+		if (scr1 & 0x10)
+			pci_write_config_byte(dev, 0x5a, scr1 & ~0x10);
+
+		/*
+		 * HighPoint does this for HPT372A.
+		 * NOTE: This register is only writeable via I/O space.
+		 */
+		if (info->chip_type == HPT372A)
+			outb(0x0e, io_base + 0x9c);
+
+		/*
+		 * Default to PCI clock. Make sure MA15/16 are set to output
+		 * to prevent drives having problems with 40-pin cables.
+		 */
+		pci_write_config_byte(dev, 0x5b, 0x23);
 
-	if (info->flags & IS_3xxN) {
-		if(freq < 0x55)
-			pll = F_LOW_PCI_33;
-		else if(freq < 0x70)
-			pll = F_LOW_PCI_40;
-		else if(freq < 0x7F)
-			pll = F_LOW_PCI_50;
+		/*
+		 * We'll have to read f_CNT value in order to determine
+		 * the PCI clock frequency according to the following ratio:
+		 *
+		 * f_CNT = Fpci * 192 / Fdpll
+		 *
+		 * First try reading the register in which the HighPoint BIOS
+		 * saves f_CNT value before  reprogramming the DPLL from its
+		 * default setting (which differs for the various chips).
+		 * NOTE: This register is only accessible via I/O space.
+		 *
+		 * In case the signature check fails, we'll have to resort to
+		 * reading the f_CNT register itself in hopes that nobody has
+		 * touched the DPLL yet...
+		 */
+		temp = inl(io_base + 0x90);
+		if ((temp & 0xFFFFF000) != 0xABCDE000) {
+			int i;
+
+			printk(KERN_WARNING "%s: no clock data saved by BIOS\n",
+			       name);
+
+			/* Calculate the average value of f_CNT. */
+			for (temp = i = 0; i < 128; i++) {
+				pci_read_config_word(dev, 0x78, &f_cnt);
+				temp += f_cnt & 0x1ff;
+				mdelay(1);
+			}
+			f_cnt = temp / 128;
+		} else
+			f_cnt = temp & 0x1ff;
+
+		dpll_clk = info->dpll_clk;
+		pci_clk  = (f_cnt * dpll_clk) / 192;
+
+		/* Clamp PCI clock to bands. */
+		if (pci_clk < 40)
+			pci_clk = 33;
+		else if(pci_clk < 45)
+			pci_clk = 40;
+		else if(pci_clk < 55)
+			pci_clk = 50;
 		else
-			pll = F_LOW_PCI_66;
+			pci_clk = 66;
 
+		printk(KERN_INFO "%s: DPLL base: %d MHz, f_CNT: %d, "
+		       "assuming %d MHz PCI\n", name, dpll_clk, f_cnt, pci_clk);
 	} else {
-		if(freq < 0x9C)
-			pll = F_LOW_PCI_33;
-		else if(freq < 0xb0)
-			pll = F_LOW_PCI_40;
-		else if(freq <0xc8)
-			pll = F_LOW_PCI_50;
-		else
-			pll = F_LOW_PCI_66;
-	}
-	printk(KERN_INFO "%s: FREQ: %d, PLL: %d\n", name, freq, pll);
-	
-	if (!(info->flags & IS_3xxN)) {
-		if (pll == F_LOW_PCI_33) {
-			info->speed = thirty_three_base_hpt37x;
-			printk(KERN_DEBUG "%s: using 33MHz PCI clock\n", name);
-		} else if (pll == F_LOW_PCI_40) {
-			/* Unsupported */
-		} else if (pll == F_LOW_PCI_50) {
-			info->speed = fifty_base_hpt37x;
-			printk(KERN_DEBUG "%s: using 50MHz PCI clock\n", name);
-		} else {
-			info->speed = sixty_six_base_hpt37x;
-			printk(KERN_DEBUG "%s: using 66MHz PCI clock\n", name);
+		u32 itr1 = 0;
+
+		pci_read_config_dword(dev, 0x40, &itr1);
+
+		/* Detect PCI clock by looking at cmd_high_time. */
+		switch((itr1 >> 8) & 0x07) {
+			case 0x09:
+				pci_clk = 40;
+			case 0x05:
+				pci_clk = 25;
+			case 0x07:
+			default:
+				pci_clk = 33;
 		}
 	}
 
-	if (pll == F_LOW_PCI_66)
-		info->flags |= PCI_66MHZ;
+	/* Let's assume we'll use PCI clock for the ATA clock... */
+	switch (pci_clk) {
+		case 25:
+			clock = ATA_CLOCK_25MHZ;
+			break;
+		case 33:
+		default:
+			clock = ATA_CLOCK_33MHZ;
+			break;
+		case 40:
+			clock = ATA_CLOCK_40MHZ;
+			break;
+		case 50:
+			clock = ATA_CLOCK_50MHZ;
+			break;
+		case 66:
+			clock = ATA_CLOCK_66MHZ;
+			break;
+	}
 
 	/*
-	 * only try the pll if we don't have a table for the clock
-	 * speed that we're running at. NOTE: the internal PLL will
-	 * result in slow reads when using a 33MHz PCI clock. we also
-	 * don't like to use the PLL because it will cause glitches
-	 * on PRST/SRST when the HPT state engine gets reset.
+	 * Only try the DPLL if we don't have a table for the PCI clock that
+	 * we are running at for HPT370/A, always use it  for anything newer...
 	 *
-	 * ToDo: Use 66MHz PLL when ATA133 devices are present on a
-	 * 372 device so we can get ATA133 support
+	 * NOTE: Using the internal DPLL results in slow reads on 33 MHz PCI.
+	 * We also  don't like using  the DPLL because this causes glitches
+	 * on PRST-/SRST- when the state engine gets reset...
 	 */
-	if (info->speed)
-		goto init_hpt37X_done;
+	if (info->chip_type >= HPT374 || info->settings[clock] == NULL) {
+		u16 f_low, delta = pci_clk < 50 ? 2 : 4;
+		int adjust;
+
+		 /*
+		  * Select 66 MHz DPLL clock only if UltraATA/133 mode is
+		  * supported/enabled, use 50 MHz DPLL clock otherwise...
+		  */
+		if (info->max_mode == 0x04) {
+			dpll_clk = 66;
+			clock = ATA_CLOCK_66MHZ;
+		} else if (dpll_clk) {	/* HPT36x chips don't have DPLL */
+			dpll_clk = 50;
+			clock = ATA_CLOCK_50MHZ;
+		}
 
-	info->flags |= PLL_MODE;
-	
-	/*
-	 * Adjust the PLL based upon the PCI clock, enable it, and
-	 * wait for stabilization...
-	 */
-	adjust = 0;
-	freq = (pll < F_LOW_PCI_50) ? 2 : 4;
-	while (adjust++ < 6) {
-		pci_write_config_dword(dev, 0x5c, (freq + pll) << 16 |
-				       pll | 0x100);
-
-		/* wait for clock stabilization */
-		for (i = 0; i < 0x50000; i++) {
-			pci_read_config_byte(dev, 0x5b, &scr2);
-			if (scr2 & 0x80) {
-				/* spin looking for the clock to destabilize */
-				for (i = 0; i < 0x1000; ++i) {
-					pci_read_config_byte(dev, 0x5b, 
-							     &scr2);
-					if ((scr2 & 0x80) == 0)
-						goto pll_recal;
-				}
-				pci_read_config_dword(dev, 0x5c, &pll);
-				pci_write_config_dword(dev, 0x5c, 
-						       pll & ~0x100);
-				pci_write_config_byte(dev, 0x5b, 0x21);
-
-				info->speed = fifty_base_hpt37x;
-				printk("%s: using 50MHz internal PLL\n", name);
-				goto init_hpt37X_done;
-			}
+		if (info->settings[clock] == NULL) {
+			printk(KERN_ERR "%s: unknown bus timing!\n", name);
+			kfree(info);
+			return -EIO;
 		}
-pll_recal:
-		if (adjust & 1)
-			pll -= (adjust >> 1);
-		else
-			pll += (adjust >> 1);
-	} 
 
-init_hpt37X_done:
-	if (!info->speed)
-		printk(KERN_ERR "%s: unknown bus timing [%d %d].\n",
-		       name, pll, freq);
-	/*
-	 * Reset the state engines.
-	 * NOTE: avoid accidentally enabling the primary channel on HPT371N.
-	 */
-	pci_read_config_byte(dev, 0x50, &mcr1);
-	if (mcr1 & 0x04)
-		pci_write_config_byte(dev, 0x50, 0x37);
-	pci_write_config_byte(dev, 0x54, 0x37);
-	udelay(100);
-}
+		/* Select the DPLL clock. */
+		pci_write_config_byte(dev, 0x5b, 0x21);
+
+		/*
+		 * Adjust the DPLL based upon PCI clock, enable it,
+		 * and wait for stabilization...
+		 */
+		f_low = (pci_clk * 48) / dpll_clk;
+
+		for (adjust = 0; adjust < 8; adjust++) {
+			if(hpt37x_calibrate_dpll(dev, f_low, f_low + delta))
+				break;
+
+			/*
+			 * See if it'll settle at a fractionally different clock
+			 */
+			if (adjust & 1)
+				f_low -= adjust >> 1;
+			else
+				f_low += adjust >> 1;
+		}
+		if (adjust == 8) {
+			printk(KERN_ERR "%s: DPLL did not stabilize!\n", name);
+			kfree(info);
+			return -EIO;
+		}
+
+		printk("%s: using %d MHz DPLL clock\n", name, dpll_clk);
+	} else {
+		/* Mark the fact that we're not using the DPLL. */
+		dpll_clk = 0;
+
+		printk("%s: using %d MHz PCI clock\n", name, pci_clk);
+	}
 
-static unsigned int __devinit init_chipset_hpt366(struct pci_dev *dev, const char *name)
-{
 	/*
-	 * FIXME: Not portable. Also, why do we enable the ROM in the first place?
-	 * We don't seem to be using it.
+	 * Advance the table pointer to a slot which points to the list
+	 * of the register values settings matching the clock being used.
 	 */
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		pci_write_config_dword(dev, PCI_ROM_ADDRESS,
-			dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE);
+	info->settings += clock;
 
-	pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, (L1_CACHE_BYTES / 4));
-	pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x78);
-	pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
-	pci_write_config_byte(dev, PCI_MAX_LAT, 0x08);
+	/* Store the clock frequencies. */
+	info->dpll_clk	= dpll_clk;
+	info->pci_clk	= pci_clk;
 
-	if (hpt_revision(dev) >= 3) {
-		u8 scr1 = 0;
+	/* Point to this chip's own instance of the hpt_info structure. */
+	pci_set_drvdata(dev, info);
 
-		/* Interrupt force enable. */
-		pci_read_config_byte(dev, 0x5a, &scr1);
-		if (scr1 & 0x10)
-			pci_write_config_byte(dev, 0x5a, scr1 & ~0x10);
+	if (info->chip_type >= HPT370) {
+		u8  mcr1, mcr4;
+
+		/*
+		 * Reset the state engines.
+		 * NOTE: Avoid accidentally enabling the disabled channels.
+		 */
+		pci_read_config_byte (dev, 0x50, &mcr1);
+		pci_read_config_byte (dev, 0x54, &mcr4);
+		pci_write_config_byte(dev, 0x50, (mcr1 | 0x32));
+		pci_write_config_byte(dev, 0x54, (mcr4 | 0x32));
+		udelay(100);
 	}
 
+	/*
+	 * On  HPT371N, if ATA clock is 66 MHz we must set bit 2 in
+	 * the MISC. register to stretch the UltraDMA Tss timing.
+	 * NOTE: This register is only writeable via I/O space.
+	 */
+	if (info->chip_type == HPT371N && clock == ATA_CLOCK_66MHZ)
+
+		outb(inb(io_base + 0x9c) | 0x04, io_base + 0x9c);
+
 	return dev->irq;
 }
 
 static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
 {
 	struct pci_dev	*dev		= hwif->pci_dev;
-	struct hpt_info *info		= ide_get_hwifdata(hwif);
+	struct hpt_info *info		= pci_get_drvdata(dev);
 	int serialize			= HPT_SERIALIZE_IO;
 	u8  scr1 = 0, ata66		= (hwif->channel) ? 0x01 : 0x02;
+	u8  chip_type			= info->chip_type;
 	u8  new_mcr, old_mcr 		= 0;
 
 	/* Cache the channel's MISC. control registers' offset */
@@ -1127,7 +1282,7 @@ static void __devinit init_hwif_hpt366(i
 	 * - on 33 MHz PCI we must clock switch
 	 * - on 66 MHz PCI we must NOT use the PCI clock
 	 */
-	if ((info->flags & (IS_3xxN | PCI_66MHZ)) == IS_3xxN) {
+	if (chip_type >= HPT372N && info->dpll_clk && info->pci_clk < 66) {
 		/*
 		 * Clock is shared between the channels,
 		 * so we'll have to serialize them... :-(
@@ -1146,9 +1301,9 @@ static void __devinit init_hwif_hpt366(i
 	 */
 	pci_read_config_byte(dev, hwif->select_data + 1, &old_mcr);
 
-	if (info->revision >= 5)		/* HPT372 and newer   */
+	if (info->chip_type >= HPT374)
 		new_mcr = old_mcr & ~0x07;
-	else if (info->revision >= 3) {		/* HPT370 and HPT370A */
+	else if (info->chip_type >= HPT370) {
 		new_mcr = old_mcr;
 		new_mcr &= ~0x02;
 
@@ -1176,7 +1331,7 @@ static void __devinit init_hwif_hpt366(i
 	 * address lines to access an external EEPROM.  To read valid
 	 * cable detect state the pins must be enabled as inputs.
 	 */
-	if (info->revision >= 8 && (PCI_FUNC(dev->devfn) & 1)) {
+	if (chip_type == HPT374 && (PCI_FUNC(dev->devfn) & 1)) {
 		/*
 		 * HPT374 PCI function 1
 		 * - set bit 15 of reg 0x52 to enable TCBLID as input
@@ -1190,7 +1345,7 @@ static void __devinit init_hwif_hpt366(i
 		/* now read cable id register */
 		pci_read_config_byte (dev, 0x5a, &scr1);
 		pci_write_config_word(dev, mcr_addr, mcr);
-	} else if (info->revision >= 3) {
+	} else if (chip_type >= HPT370) {
 		/*
 		 * HPT370/372 and 374 pcifn 0
 		 * - clear bit 0 of reg 0x5b to enable P/SCBLID as inputs
@@ -1210,10 +1365,10 @@ static void __devinit init_hwif_hpt366(i
 
 	hwif->ide_dma_check		= &hpt366_config_drive_xfer_rate;
 
-	if (info->revision >= 5) {
+	if (chip_type >= HPT374) {
 		hwif->ide_dma_test_irq	= &hpt374_ide_dma_test_irq;
 		hwif->ide_dma_end	= &hpt374_ide_dma_end;
-	} else if (info->revision >= 3) {
+	} else if (chip_type >= HPT370) {
 		hwif->dma_start 	= &hpt370_ide_dma_start;
 		hwif->ide_dma_end	= &hpt370_ide_dma_end;
 		hwif->ide_dma_timeout	= &hpt370_ide_dma_timeout;
@@ -1228,7 +1383,6 @@ static void __devinit init_hwif_hpt366(i
 static void __devinit init_dma_hpt366(ide_hwif_t *hwif, unsigned long dmabase)
 {
 	struct pci_dev	*dev		= hwif->pci_dev;
-	struct hpt_info	*info		= ide_get_hwifdata(hwif);
 	u8 masterdma	= 0, slavedma	= 0;
 	u8 dma_new	= 0, dma_old	= 0;
 	unsigned long flags;
@@ -1236,12 +1390,6 @@ static void __devinit init_dma_hpt366(id
 	if (!dmabase)
 		return;
 		
-	if(info->speed == NULL) {
-		printk(KERN_WARNING "%s: no known IDE timings, disabling DMA.\n",
-		       hwif->cds->name);
-		return;
-	}
-
 	dma_old = hwif->INB(dmabase + 2);
 
 	local_irq_save(flags);
@@ -1260,60 +1408,6 @@ static void __devinit init_dma_hpt366(id
 	ide_setup_dma(hwif, dmabase, 8);
 }
 
-/*
- *	We "borrow" this hook in order to set the data structures
- *	up early enough before dma or init_hwif calls are made.
- */
-
-static void __devinit init_iops_hpt366(ide_hwif_t *hwif)
-{
-	struct hpt_info *info	= kzalloc(sizeof(struct hpt_info), GFP_KERNEL);
-	struct pci_dev  *dev	= hwif->pci_dev;
-	u16 did			= dev->device;
-	u8 mode, rid		= 0;
-
-	if(info == NULL) {
-		printk(KERN_WARNING "%s: out of memory.\n", hwif->cds->name);
-		return;
-	}
-	ide_set_hwifdata(hwif, info);
-
-	/* Avoid doing the same thing twice. */
-	if (hwif->channel && hwif->mate) {
-		memcpy(info, ide_get_hwifdata(hwif->mate), sizeof(struct hpt_info));
-		return;
-	}
-
-	pci_read_config_byte(dev, PCI_REVISION_ID, &rid);
-
-	if (( did == PCI_DEVICE_ID_TTI_HPT366  && rid == 6) ||
-	    ((did == PCI_DEVICE_ID_TTI_HPT372  ||
-	      did == PCI_DEVICE_ID_TTI_HPT302  ||
-	      did == PCI_DEVICE_ID_TTI_HPT371) && rid > 1) ||
-	      did == PCI_DEVICE_ID_TTI_HPT372N)
-		info->flags |= IS_3xxN;
-
-	rid = info->revision = hpt_revision(dev);
-	if (rid >= 8)			/* HPT374 */
-		mode = HPT374_ALLOW_ATA133_6 ? 4 : 3;
-	else if (rid >= 7)		/* HPT371 and HPT371N */
-		mode = HPT371_ALLOW_ATA133_6 ? 4 : 3;
-	else if (rid >= 6)		/* HPT302 and HPT302N */
-		mode = HPT302_ALLOW_ATA133_6 ? 4 : 3;
-	else if (rid >= 5)		/* HPT372, HPT372A, and HPT372N */
-		mode = HPT372_ALLOW_ATA133_6 ? 4 : 3;
-	else if (rid >= 3)		/* HPT370 and HPT370A */
-		mode = HPT370_ALLOW_ATA100_5 ? 3 : 2;
-	else				/* HPT366 and HPT368 */
-		mode = (HPT366_ALLOW_ATA66_4 || HPT366_ALLOW_ATA66_3) ? 2 : 1;
-	info->max_mode = mode;
-
-	if (rid >= 3)
-		hpt37x_clocking(hwif);
-	else
-		hpt366_clocking(hwif);
-}
-
 static int __devinit init_setup_hpt374(struct pci_dev *dev, ide_pci_device_t *d)
 {
 	struct pci_dev *dev2;
@@ -1321,9 +1415,13 @@ static int __devinit init_setup_hpt374(s
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
 
+	pci_set_drvdata(dev, &hpt374);
+
 	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
 		int ret;
 
+		pci_set_drvdata(dev2, &hpt374);
+
 		if (dev2->irq != dev->irq) {
 			/* FIXME: we need a core pci_set_interrupt() */
 			dev2->irq = dev->irq;
@@ -1340,18 +1438,25 @@ static int __devinit init_setup_hpt374(s
 
 static int __devinit init_setup_hpt372n(struct pci_dev *dev, ide_pci_device_t *d)
 {
+	pci_set_drvdata(dev, &hpt372n);
+
 	return ide_setup_pci_device(dev, d);
 }
 
 static int __devinit init_setup_hpt371(struct pci_dev *dev, ide_pci_device_t *d)
 {
+	struct hpt_info *info;
 	u8 rev = 0, mcr1 = 0;
 
 	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
-	if (rev > 1)
+	if (rev > 1) {
 		d->name = "HPT371N";
 
+		info = &hpt371n;
+	} else
+		info = &hpt371;
+
 	/*
 	 * HPT371 chips physically have only one channel, the secondary one,
 	 * but the primary channel registers do exist!  Go figure...
@@ -1362,30 +1467,44 @@ static int __devinit init_setup_hpt371(s
 	if (mcr1 & 0x04)
 		pci_write_config_byte(dev, 0x50, mcr1 & ~0x04);
 
+	pci_set_drvdata(dev, info);
+
 	return ide_setup_pci_device(dev, d);
 }
 
 static int __devinit init_setup_hpt372a(struct pci_dev *dev, ide_pci_device_t *d)
 {
+	struct hpt_info *info;
 	u8 rev = 0;
 
 	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
-	if (rev > 1)
+	if (rev > 1) {
 		d->name = "HPT372N";
 
+		info = &hpt372n;
+	} else
+		info = &hpt372a;
+	pci_set_drvdata(dev, info);
+
 	return ide_setup_pci_device(dev, d);
 }
 
 static int __devinit init_setup_hpt302(struct pci_dev *dev, ide_pci_device_t *d)
 {
+	struct hpt_info *info;
 	u8 rev = 0;
 
 	pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
 
-	if (rev > 1)
+	if (rev > 1) {
 		d->name = "HPT302N";
 
+		info = &hpt302n;
+	} else
+		info = &hpt302;
+	pci_set_drvdata(dev, info);
+
 	return ide_setup_pci_device(dev, d);
 }
 
@@ -1396,6 +1515,9 @@ static int __devinit init_setup_hpt366(s
 	static char   *chipset_names[] = { "HPT366", "HPT366",  "HPT368",
 					   "HPT370", "HPT370A", "HPT372",
 					   "HPT372N" };
+	static struct hpt_info *info[] = { &hpt36x,  &hpt36x,  &hpt36x,
+					   &hpt370,  &hpt370a, &hpt372,
+					   &hpt372n  };
 
 	if (PCI_FUNC(dev->devfn) & 1)
 		return -ENODEV;
@@ -1407,6 +1529,8 @@ static int __devinit init_setup_hpt366(s
 		
 	d->name = chipset_names[rev];
 
+	pci_set_drvdata(dev, info[rev]);
+
 	if (rev > 2)
 		goto init_single;
 
@@ -1416,6 +1540,8 @@ static int __devinit init_setup_hpt366(s
 	  	u8  pin1 = 0, pin2 = 0;
 		int ret;
 
+		pci_set_drvdata(dev2, info[rev]);
+
 		pci_read_config_byte(dev,  PCI_INTERRUPT_PIN, &pin1);
 		pci_read_config_byte(dev2, PCI_INTERRUPT_PIN, &pin2);
 		if (pin1 != pin2 && dev->irq == dev2->irq) {
@@ -1437,40 +1563,39 @@ static ide_pci_device_t hpt366_chipsets[
 		.name		= "HPT366",
 		.init_setup	= init_setup_hpt366,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 		.extra		= 240
 	},{	/* 1 */
 		.name		= "HPT372A",
 		.init_setup	= init_setup_hpt372a,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 		.extra		= 240
 	},{	/* 2 */
 		.name		= "HPT302",
 		.init_setup	= init_setup_hpt302,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 		.extra		= 240
 	},{	/* 3 */
 		.name		= "HPT371",
 		.init_setup	= init_setup_hpt371,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,
@@ -1482,22 +1607,22 @@ static ide_pci_device_t hpt366_chipsets[
 		.name		= "HPT374",
 		.init_setup	= init_setup_hpt374,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,	/* 4 */
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 		.extra		= 240
 	},{	/* 5 */
 		.name		= "HPT372N",
 		.init_setup	= init_setup_hpt372n,
 		.init_chipset	= init_chipset_hpt366,
-		.init_iops	= init_iops_hpt366,
 		.init_hwif	= init_hwif_hpt366,
 		.init_dma	= init_dma_hpt366,
 		.channels	= 2,	/* 4 */
 		.autodma	= AUTODMA,
+		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.bootable	= OFF_BOARD,
 		.extra		= 240
 	}

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

end of thread, other threads:[~2006-06-27 21:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-04-23  8:33 [PATCH][RFT] HPT3xxN clocking fixes Sergei Shtylyov
2006-04-24 22:09 ` [PATCH][RFT] HPT3xxN clocking fixes (take 2) Sergei Shtylyov
2006-05-02 21:55 ` [PATCH][RFT] Fix HPT37x timing tables Sergei Shtylyov
2006-05-06 20:08   ` [PATCH] Optimize " Sergei Shtylyov
2006-05-02 22:14 ` [PATCH][RFT] Fix HPT3xx hotswap support Sergei Shtylyov
2006-05-02 22:26 ` [PATCH] Fix the case of multiple HPT3xx chips present Sergei Shtylyov
2006-05-04 19:46   ` [PATCH] HPT3xx: fix PCI clock detection Sergei Shtylyov
2006-05-16 22:44     ` [PATCH] HPT3xx: rework rate filtering Sergei Shtylyov
2006-05-20  8:51       ` [PATCH] HPT3xx: print the real chip name at startup Sergei Shtylyov
2006-05-27 22:05         ` [PATCH] HPT3xx: switch to using pci_find_slot() Sergei Shtylyov
2006-05-27 22:54           ` Jiri Slaby
2006-05-27 23:01             ` Sergei Shtylyov
2006-05-27 23:30           ` [PATCH] HPT3xx: switch to using pci_get_slot() Sergei Shtylyov
2006-05-28  2:55             ` [PATCH] HPT3xx: cache channel's MCR address Sergei Shtylyov
2006-06-04 22:24               ` [PATCH] HPT3x7: merge speedproc handlers Sergei Shtylyov
2006-06-11 21:18                 ` [PATCH] HPT370: clean up DMA timeout handling Sergei Shtylyov
2006-06-27 21:41                   ` [PATCH][RFT] HPT3xx: init code rewrite Sergei Shtylyov
2006-05-28 13:51             ` [PATCH] HPT3xx: switch to using pci_get_slot() Jiri Slaby
2006-05-29 14:26               ` Sergei Shtylyov
2006-05-20  8:41     ` [PATCH] HPT37x: read f_CNT saved by BIOS from port Sergei Shtylyov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).