[PATCH] (pata-2.6 fix queue) cmd64x: fix primary channel address setup time

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The driver used to merge the address setup timing for the secondary channel but
this must have been done for both channels actually despite the primary channel
had separate registers for each drive (I'afraid that this is too common mistake
in both hardware and software -- address setup timing must be the same accross
an IDE channel).  Thus, do a number of changes to the program_drive_counts():

- store the address setup count in the 'drive_data' to select the slowest one
  for the primary channel's drives, and write the selected value into *both*
  ARRTIM0 and ARTTIM1 registers (just writing 0 to their reserved bits 0 to 5);

- when writing to ARTTIM23 register for the secondary channel, preserve the
  interrupt status bit; elminate the local_irq_{save|restore}() calls as there
  is *no* race with the interrupt handler;

- make drwtim_regs[] single-dimensional, indexing it with drive->dn, eliminate
  now useless index variables;

- replace 'temp_b' variable with 'arttim' and 'drwtim';

- rename {setup|recovery}_counts[] into more fitting {setup|recovery}_values[];

- replace the multiple HWIF() invocation with a single one...

And in addition to those changes also do remove:

- needless and misplaced timing registers initialization in the init_chipset()
  method;

- meaningless register #define's...

Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

---
Warning: this has only been compile-tested, as usual.

I've finally decided to make this a separate patch, so the MWDMA support
restoring patch is delayed again. :-<

 drivers/ide/pci/cmd64x.c |  106 +++++++++++++++++------------------------------
 1 files changed, 40 insertions(+), 66 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
@@ -1,6 +1,6 @@
 /* $Id: cmd64x.c,v 1.21 2000/01/30 23:23:16
  *
- * linux/drivers/ide/pci/cmd64x.c		Version 1.47	Feb 16, 2007
+ * linux/drivers/ide/pci/cmd64x.c		Version 1.48	Feb 26, 2007
  *
  * cmd64x.c: Enable interrupts at initialization time on Ultra/PCI machines.
  *           Note, this driver is not used at all on other systems because
@@ -55,9 +55,6 @@
 #define   ARTTIM23_DIS_RA2	0x04
 #define   ARTTIM23_DIS_RA3	0x08
 #define   ARTTIM23_INTR_CH1	0x10
-#define ARTTIM2		0x57
-#define ARTTIM3		0x57
-#define DRWTIM23	0x58
 #define DRWTIM2		0x58
 #define BRST		0x59
 #define DRWTIM3		0x5b
@@ -172,73 +169,62 @@ static int cmd64x_get_info (char *buffer
  * This routine writes the prepared setup/active/recovery counts
  * for a drive into the cmd646 chipset registers to active them.
  */
-static void program_drive_counts (ide_drive_t *drive, int setup_count, int active_count, int recovery_count)
+static void program_drive_counts (ide_drive_t *drive, int setup_count,
+				  int active_count, int recovery_count)
 {
-	unsigned long flags;
-	struct pci_dev *dev = HWIF(drive)->pci_dev;
-	ide_drive_t *drives = HWIF(drive)->drives;
-	u8 temp_b;
-	static const u8 setup_counts[] = {0x40, 0x40, 0x40, 0x80, 0, 0xc0};
-	static const u8 recovery_counts[] =
+	ide_hwif_t *hwif	= HWIF(drive);
+	struct pci_dev *dev	= hwif->pci_dev;
+	ide_drive_t *drives	= hwif->drives;
+	u8 drwtim, arttim	= 0;
+	static const u8 setup_values[] = {0x40, 0x40, 0x40, 0x80, 0, 0xc0};
+	static const u8 recovery_values[] =
 		{15, 15, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 0};
-	static const u8 arttim_regs[2][2] = {
-			{ ARTTIM0, ARTTIM1 },
-			{ ARTTIM23, ARTTIM23 }
-		};
-	static const u8 drwtim_regs[2][2] = {
-			{ DRWTIM0, DRWTIM1 },
-			{ DRWTIM2, DRWTIM3 }
-		};
-	int channel = (int) HWIF(drive)->channel;
-	int slave = (drives != drive);  /* Is this really the best way to determine this?? */
+	static const u8 drwtim_regs[] = {DRWTIM0, DRWTIM1, DRWTIM2, DRWTIM3};
 
 	cmdprintk("program_drive_count parameters = s(%d),a(%d),r(%d),p(%d)\n",
 		setup_count, active_count, recovery_count, drive->present);
+
 	/*
-	 * Set up address setup count registers.
-	 * Primary interface has individual count/timing registers for
-	 * each drive.  Secondary interface has one common set of registers,
-	 * for address setup so we merge these timings, using the slowest
-	 * value.
+	 * The primary channel has individual address setup timing registers
+	 * for each drive, while the secondary channel has one common register.
+	 * We must merge these timings in any case and use the slowest value.
 	 */
-	if (channel) {
-		drive->drive_data = setup_count;
-		setup_count = max(drives[0].drive_data,
-					drives[1].drive_data);
-		cmdprintk("Secondary interface, setup_count = %d\n",
-					setup_count);
-	}
+	drive->drive_data = setup_count;
+	setup_count = max(drives[0].drive_data,	drives[1].drive_data);
+	cmdprintk("Slowest address setup count = %d\n", setup_count);
 
 	/*
 	 * Convert values to internal chipset representation
 	 */
-	setup_count = (setup_count > 5) ? 0xc0 : (int) setup_counts[setup_count];
+	setup_count = (setup_count > 5) ? 0xc0 : setup_values[setup_count];
 	active_count &= 0xf; /* Remember, max value is 16 */
-	recovery_count = (int) recovery_counts[recovery_count];
-
+	recovery_count = recovery_values[recovery_count];
 	cmdprintk("Final values = %d,%d,%d\n",
 		setup_count, active_count, recovery_count);
 
 	/*
-	 * Now that everything is ready, program the new timings
+	 * Program the address setup clocks into the ARTTIM registers.
+	 * Avoid clearing the secondary channel's interrupt bit.
 	 */
-	local_irq_save(flags);
-	/*
-	 * Program the address_setup clocks into ARTTIM reg,
-	 * and then the active/recovery counts into the DRWTIM reg
-	 */
-	(void) pci_read_config_byte(dev, arttim_regs[channel][slave], &temp_b);
-	(void) pci_write_config_byte(dev, arttim_regs[channel][slave],
-		((u8) setup_count) | (temp_b & 0x3f));
-	(void) pci_write_config_byte(dev, drwtim_regs[channel][slave],
-		(u8) ((active_count << 4) | recovery_count));
-	cmdprintk ("Write %x to %x\n",
-		((u8) setup_count) | (temp_b & 0x3f),
-		arttim_regs[channel][slave]);
-	cmdprintk ("Write %x to %x\n",
-		(u8) ((active_count << 4) | recovery_count),
-		drwtim_regs[channel][slave]);
-	local_irq_restore(flags);
+	if (hwif->channel) {
+		(void) pci_read_config_byte (dev, ARTTIM23, &arttim);
+		arttim &= ~ARTTIM23_INTR_CH1;
+		arttim &= ~0xc0;
+		arttim |= setup_count;
+		(void) pci_write_config_byte(dev, ARTTIM23, arttim);
+		cmdprintk ("Write %x to %x\n", arttim, ARTTIM23);
+	} else {
+		arttim = setup_count;
+		(void) pci_write_config_byte(dev, ARTTIM0, arttim);
+		cmdprintk ("Write %x to %x\n", arttim, ARTTIM0);
+		(void) pci_write_config_byte(dev, ARTTIM1, arttim);
+		cmdprintk ("Write %x to %x\n", arttim, ARTTIM1);
+	}
+
+	/* Program the active/recovery counts into the DRWTIM register */
+	drwtim = (active_count << 4) | recovery_count;
+	(void) pci_write_config_byte(dev, drwtim_regs[drive->dn], drwtim);
+	cmdprintk ("Write %x to %x\n", drwtim, drwtim_regs[drive->dn]);
 }
 
 static u8 quantize_timing(int timing, int quant)
@@ -575,18 +561,6 @@ static unsigned int __devinit init_chips
 	 */
 	(void) pci_write_config_byte(dev, MRDMODE, mrdmode | 0x02);
 
-	/* Set reasonable active/recovery/address-setup values. */
-	(void) pci_write_config_byte(dev, ARTTIM0,  0x40);
-	(void) pci_write_config_byte(dev, DRWTIM0,  0x3f);
-	(void) pci_write_config_byte(dev, ARTTIM1,  0x40);
-	(void) pci_write_config_byte(dev, DRWTIM1,  0x3f);
-#ifdef __i386__
-	(void) pci_write_config_byte(dev, ARTTIM23, 0x1c);
-#else
-	(void) pci_write_config_byte(dev, ARTTIM23, 0x5c);
-#endif
-	(void) pci_write_config_byte(dev, DRWTIM23, 0x3f);
-	(void) pci_write_config_byte(dev, DRWTIM3,  0x3f);
 #ifdef CONFIG_PPC
 	(void) pci_write_config_byte(dev, UDIDETCR0, 0xf0);
 #endif /* CONFIG_PPC */

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux