linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH pata-2.6] cmd64x: procfs code fixes/cleanups (take 2)
@ 2007-04-14 20:11 Mikael Pettersson
  2007-04-14 20:16 ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Mikael Pettersson @ 2007-04-14 20:11 UTC (permalink / raw)
  To: bzolnier, sshtylyov; +Cc: linux-ide

On Sat, 14 Apr 2007 23:41:16 +0400, Sergei Shtylyov wrote:
> While at it, also perform the following cleanups:
...
> - correct the chipset names (from CMDxxx to PCI-xxx)

Please explain why this rename is a correction.

Did the company making these chips change name at some point?
Or did they change the names of the chips?

CMD64x is an established name, while PCI-xxx is something
I've never heard of (and it sounds awfully generic).

/Mikael

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup
@ 2007-02-03 20:09 Sergei Shtylyov
  2007-02-03 21:04 ` [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2) Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2007-02-03 20:09 UTC (permalink / raw)
  To: bzolnier; +Cc: linux-ide

The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to cmd64x_tune_pio(), making it return the mode set,
and "wrapping" the new tuneproc() method around it; while at it, also get rid
of the non-working prefetch control code, remove redundant PIO5 mode limitation,
and make cmdprintk() give more sensible mode info.  Also, get rid of the broken
config_chipset_for_pio() which always tried to set PIO4 instead auto-tuning PIO.
Note that removing a call from config_chipset_for_dma() breaks "rudimentary"
MWDMA2 support which can only work because of the timing registers being pre-
setup for PIO4 since the code in the speedproc() method which sets up the chip
for SW/MW DMA is completely bogus (!) and I'm going to remove it for the time
being in the next patch...
Oh, and add the missing PIO5 support to the speedproc() method while at it. :-)

Warning: compile tested only -- getting to the real hardware isn't that easy...

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

 drivers/ide/pci/cmd64x.c |   92 ++++++++++++++++-------------------------------
 1 files changed, 32 insertions(+), 60 deletions(-)

Index: linux-2.6/drivers/ide/pci/cmd64x.c
===================================================================
--- linux-2.6.orig/drivers/ide/pci/cmd64x.c
+++ linux-2.6/drivers/ide/pci/cmd64x.c
@@ -262,43 +262,25 @@ static void program_drive_counts (ide_dr
 }
 
 /*
- * Attempts to set the interface PIO mode.
- * The preferred method of selecting PIO modes (e.g. mode 4) is 
- * "echo 'piomode:4' > /proc/ide/hdx/settings".  Special cases are
- * 8: prefetch off, 9: prefetch on, 255: auto-select best mode.
- * Called with 255 at boot time.
+ * This routine selects drive's best PIO mode, calculates setup/active/recovery
+ * counts, and writes them into the chipset registers.
  */
-
-static void cmd64x_tuneproc (ide_drive_t *drive, u8 mode_wanted)
+static u8 cmd64x_tune_pio (ide_drive_t *drive, u8 mode_wanted)
 {
 	int setup_time, active_time, recovery_time;
 	int clock_time, pio_mode, cycle_time;
 	u8 recovery_count2, cycle_count;
 	int setup_count, active_count, recovery_count;
 	int bus_speed = system_bus_clock();
-	/*byte b;*/
 	ide_pio_data_t  d;
 
-	switch (mode_wanted) {
-		case 8: /* set prefetch off */
-		case 9: /* set prefetch on */
-			mode_wanted &= 1;
-			/*set_prefetch_mode(index, mode_wanted);*/
-			cmdprintk("%s: %sabled cmd640 prefetch\n",
-				drive->name, mode_wanted ? "en" : "dis");
-			return;
-	}
-
-	mode_wanted = ide_get_best_pio_mode (drive, mode_wanted, 5, &d);
-	pio_mode = d.pio_mode;
+	pio_mode = ide_get_best_pio_mode(drive, mode_wanted, 5, &d);
 	cycle_time = d.cycle_time;
 
 	/*
 	 * I copied all this complicated stuff from cmd640.c and made a few
 	 * minor changes.  For now I am just going to pray that it is correct.
 	 */
-	if (pio_mode > 5)
-		pio_mode = 5;
 	setup_time  = ide_pio_timings[pio_mode].setup_time;
 	active_time = ide_pio_timings[pio_mode].active_time;
 	recovery_time = cycle_time - (setup_time + active_time);
@@ -320,22 +302,27 @@ static void cmd64x_tuneproc (ide_drive_t
 	if (active_count > 16)
 		active_count = 16; /* maximum allowed by cmd646 */
 
-	/*
-	 * In a perfect world, we might set the drive pio mode here
-	 * (using WIN_SETFEATURE) before continuing.
-	 *
-	 * But we do not, because:
-	 *	1) this is the wrong place to do it
-	 *		(proper is do_special() in ide.c)
-	 * 	2) in practice this is rarely, if ever, necessary
-	 */
 	program_drive_counts (drive, setup_count, active_count, recovery_count);
 
-	cmdprintk("%s: selected cmd646 PIO mode%d : %d (%dns)%s, "
+	cmdprintk("%s: PIO mode wanted %d, selected %d (%dns)%s, "
 		"clocks=%d/%d/%d\n",
-		drive->name, pio_mode, mode_wanted, cycle_time,
+		drive->name, mode_wanted, pio_mode, cycle_time,
 		d.overridden ? " (overriding vendor mode)" : "",
 		setup_count, active_count, recovery_count);
+
+	return pio_mode;
+}
+
+/*
+ * Attempts to set drive's PIO mode.
+ * The preferred method of selecting PIO modes (e.g. mode 4) is
+ * "echo 'piomode:4' > /proc/ide/hdx/settings".
+ * Special case is 255: auto-select best mode (used at boot time).
+ */
+static void cmd64x_tune_drive (ide_drive_t *drive, u8 pio)
+{
+	pio = cmd64x_tune_pio(drive, pio);
+	(void) ide_config_drive_speed(drive, XFER_PIO_0 + pio);
 }
 
 static u8 cmd64x_ratemask (ide_drive_t *drive)
@@ -387,22 +374,6 @@ static u8 cmd64x_ratemask (ide_drive_t *
 	return mode;
 }
 
-static void config_cmd64x_chipset_for_pio (ide_drive_t *drive, u8 set_speed)
-{
-	u8 speed	= 0x00;
-	u8 set_pio	= ide_get_best_pio_mode(drive, 4, 5, NULL);
-
-	cmd64x_tuneproc(drive, set_pio);
-	speed = XFER_PIO_0 + set_pio;
-	if (set_speed)
-		(void) ide_config_drive_speed(drive, speed);
-}
-
-static void config_chipset_for_pio (ide_drive_t *drive, u8 set_speed)
-{
-	config_cmd64x_chipset_for_pio(drive, set_speed);
-}
-
 static int cmd64x_tune_chipset (ide_drive_t *drive, u8 xferspeed)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
@@ -414,7 +385,7 @@ static int cmd64x_tune_chipset (ide_driv
 
 	u8 speed	= ide_rate_filter(cmd64x_ratemask(drive), xferspeed);
 
-	if (speed > XFER_PIO_4) {
+	if (speed >= XFER_SW_DMA_0) {
 		(void) pci_read_config_byte(dev, pciD, &regD);
 		(void) pci_read_config_byte(dev, pciU, &regU);
 		regD &= ~(unit ? 0x40 : 0x20);
@@ -438,17 +409,20 @@ static int cmd64x_tune_chipset (ide_driv
 		case XFER_SW_DMA_2:	regD |= (unit ? 0x40 : 0x10); break;
 		case XFER_SW_DMA_1:	regD |= (unit ? 0x80 : 0x20); break;
 		case XFER_SW_DMA_0:	regD |= (unit ? 0xC0 : 0x30); break;
-		case XFER_PIO_4:	cmd64x_tuneproc(drive, 4); break;
-		case XFER_PIO_3:	cmd64x_tuneproc(drive, 3); break;
-		case XFER_PIO_2:	cmd64x_tuneproc(drive, 2); break;
-		case XFER_PIO_1:	cmd64x_tuneproc(drive, 1); break;
-		case XFER_PIO_0:	cmd64x_tuneproc(drive, 0); break;
+		case XFER_PIO_5:
+		case XFER_PIO_4:
+		case XFER_PIO_3:
+		case XFER_PIO_2:
+		case XFER_PIO_1:
+		case XFER_PIO_0:
+			(void) cmd64x_tune_pio(drive, speed - XFER_PIO_0);
+			break;
 
 		default:
 			return 1;
 	}
 
-	if (speed > XFER_PIO_4) {
+	if (speed >= XFER_SW_DMA_0) {
 		(void) pci_write_config_byte(dev, pciU, regU);
 		regD |= (unit ? 0x40 : 0x20);
 		(void) pci_write_config_byte(dev, pciD, regD);
@@ -461,8 +435,6 @@ static int config_chipset_for_dma (ide_d
 {
 	u8 speed	= ide_dma_speed(drive, cmd64x_ratemask(drive));
 
-	config_chipset_for_pio(drive, !speed);
-
 	if (!speed)
 		return 0;
 
@@ -491,7 +463,7 @@ static int cmd64x_config_drive_for_dma (
 
 	} else if ((id->capability & 8) || (id->field_valid & 2)) {
 fast_ata_pio:
-		config_chipset_for_pio(drive, 1);
+		cmd64x_tune_drive(drive, 255);
 		return hwif->ide_dma_off_quietly(drive);
 	}
 	/* IORDY not supported */
@@ -694,7 +666,7 @@ static void __devinit init_hwif_cmd64x(i
 	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
 	class_rev &= 0xff;
 
-	hwif->tuneproc  = &cmd64x_tuneproc;
+	hwif->tuneproc  = &cmd64x_tune_drive;
 	hwif->speedproc = &cmd64x_tune_chipset;
 
 	if (!hwif->dma_base) {


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

end of thread, other threads:[~2007-04-23 22:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-14 20:11 [PATCH pata-2.6] cmd64x: procfs code fixes/cleanups (take 2) Mikael Pettersson
2007-04-14 20:16 ` Sergei Shtylyov
2007-04-14 22:18   ` Mark Lord
2007-04-16 12:15     ` Sergei Shtylyov
2007-04-17 17:59       ` Mark Lord
2007-04-17 18:04         ` Sergei Shtylyov
2007-04-17 14:07   ` Alan Cox
2007-04-17 14:09     ` Sergei Shtylyov
  -- strict thread matches above, loose matches on Subject: below --
2007-02-03 20:09 [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup Sergei Shtylyov
2007-02-03 21:04 ` [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2) Sergei Shtylyov
2007-02-15 19:17   ` [PATCH] (pata-2.6 fix queue) cmd64x: procfs code fixes/cleanups Sergei Shtylyov
2007-04-14 19:41     ` [PATCH pata-2.6] cmd64x: procfs code fixes/cleanups (take 2) Sergei Shtylyov
2007-04-23 21:58       ` Bartlomiej Zolnierkiewicz

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).