Skip to contentSkip to conversation

atang tree: fix UDMA mode for older Promise controllers

PATCH New Bartlomiej Zolnierkiewicz 2 participants 7 messages Feb 13, 2010 libata

lore
Share & tools
lore

Bartlomiej Zolnierkiewicz

author
#0 lore raw

PATCH 1/1 atang tree: fix UDMA mode for older Promise controllers

DISCLAIMER: the fact of getting patches merged into atang tree means that from now on they will be getting updates for changes happening in atang tree and it should not be treated as an indication regarding decisions taken by 'upstream' kernel trees

Russell King did some great debugging work last month on the root source of UDMA problems that have been consistenly reported against pata_pdc202xx_old driver for the last couple of years which should allow us to hopefully fix them all in atang tree.

On top of atang-v4.4.

The following changes since commit bcf33ade797072b4dcd51dd13b353a6e052036c6:
Thomas Chou (1):
drivers/staging: New PATA driver for Altera CompactFlash.

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git atang-v4.5

Bartlomiej Zolnierkiewicz (3):
pata_pdc202xx_old: fix UDMA mode for Promise UDMA33 cards
pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets
pata_pdc202xx_old: update documentation

 drivers/ata/Kconfig             |    1 -
 drivers/ata/pata_pdc202xx_old.c |   13 ++++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
author
#1 lore raw

PATCH 1/3 pata_pdc202xx_old: fix UDMA mode for Promise UDMA33 cards

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_pdc202xx_old: fix UDMA mode for Promise UDMA33 cards

Quoted 20 lines · Russell King

Found the problem - getting rid of the read of the alt status register after the command has been written fixes the UDMA CRC errors on write:

(quoted diff) +2 -1
@@ -676,7 +676,8 @@ void ata_sff_exec_command(struct ata_port *ap, const struct
 ata_taskfile *tf)
        DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
 
        iowrite8(tf->command, ap->ioaddr.command_addr);
-       ata_sff_pause(ap);
+       ndelay(400);
+//     ata_sff_pause(ap);
 }
 EXPORT_SYMBOL_GPL(ata_sff_exec_command);

This rather makes sense. The PDC20247 handles the UDMA part of the protocol. It has no way to tell the PDC20246 to wait while it suspends UDMA, so that a normal register access can take place - the 246 ploughs on with the register access without any regard to the state of the 247.

Quoted 4 lines

If the drive immediately starts the UDMA protocol after a write to the command register (as it probably will for the DMA WRITE command), then we'll be accessing the taskfile in the middle of the UDMA setup, which can't be good. It's certainly a violation of the ATA specs.

Fix it by adding custom ->sff_exec_command method for UDMA33 chipsets.

Debugged-by Russell King <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ata/pata_pdc202xx_old.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/ata/pata_pdc202xx_old.c +12 -1
Index: b/drivers/ata/pata_pdc202xx_old.c
===================================================================
@@ -2,7 +2,7 @@
  * pata_pdc202xx_old.c 	- Promise PDC202xx PATA for new ATA layer
  *			  (C) 2005 Red Hat Inc
  *			  Alan Cox <[email protected]>
- *			  (C) 2007,2009 Bartlomiej Zolnierkiewicz
+ *			  (C) 2007,2009,2010 Bartlomiej Zolnierkiewicz
  *
  * Based in part on linux/drivers/ide/pci/pdc202xx_old.c
  *

@@ -26,6 +26,15 @@
 
 #include "pata_pdc202xx_old.h"
 
+static void pdc20246_exec_command(struct ata_port *ap,
+				  const struct ata_taskfile *tf)
+{
+	DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
+
+	iowrite8(tf->command, ap->ioaddr.command_addr);
+	ndelay(400);
+}
+
 /**
  *	pdc2026x_bmdma_start		-	DMA engine begin
  *	@qc: ATA command

@@ -171,6 +180,8 @@ static struct ata_port_operations pdc202
 	.cable_detect		= ata_cable_40wire,
 	.set_piomode		= pdc202xx_set_piomode,
 	.set_dmamode		= pdc202xx_set_dmamode,
+
+	.sff_exec_command	= pdc20246_exec_command,
 };
 
 static struct ata_port_operations pdc2026x_port_ops = {
Applied
#2 lore raw

Re: [PATCH 1/3] pata_pdc202xx_old: fix UDMA mode for Promise UDMA33 cards

Quoted 32 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz<[email protected]>
Subject: [PATCH] pata_pdc202xx_old: fix UDMA mode for Promise UDMA33 cards

Quoted 20 lines · Russell King

Found the problem - getting rid of the read of the alt status register after the command has been written fixes the UDMA CRC errors on write:

(quoted diff) +2 -1
@@ -676,7 +676,8 @@ void ata_sff_exec_command(struct ata_port *ap, const struct
 ata_taskfile *tf)
         DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
 
         iowrite8(tf->command, ap->ioaddr.command_addr);
-       ata_sff_pause(ap);
+       ndelay(400);
+//     ata_sff_pause(ap);
  }
  EXPORT_SYMBOL_GPL(ata_sff_exec_command);

This rather makes sense. The PDC20247 handles the UDMA part of the protocol. It has no way to tell the PDC20246 to wait while it suspends UDMA, so that a normal register access can take place - the 246 ploughs on with the register access without any regard to the state of the 247.

Quoted 4 lines

If the drive immediately starts the UDMA protocol after a write to the command register (as it probably will for the DMA WRITE command), then we'll be accessing the taskfile in the middle of the UDMA setup, which can't be good. It's certainly a violation of the ATA specs.

Fix it by adding custom ->sff_exec_command method for UDMA33 chipsets.

Quoted 5 lines
Debugged-by Russell King<[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz<[email protected]>
---
  drivers/ata/pata_pdc202xx_old.c |   13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

applied

author
#3 lore raw

PATCH 2/3 pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets

PDC2026x chipsets need the same treatment as PDC20246 one.

This is completely untested but will hopefully fix UDMA issues that people have been reporting against pata_pdc202xx_old for the last couple of years.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ata/pata_pdc202xx_old.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ata/pata_pdc202xx_old.c +2 -2
Index: b/drivers/ata/pata_pdc202xx_old.c
===================================================================
@@ -26,7 +26,7 @@
 
 #include "pata_pdc202xx_old.h"
 
-static void pdc20246_exec_command(struct ata_port *ap,
+static void pdc202xx_exec_command(struct ata_port *ap,
 				  const struct ata_taskfile *tf)
 {
 	DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);

@@ -181,7 +181,7 @@ static struct ata_port_operations pdc202
 	.set_piomode		= pdc202xx_set_piomode,
 	.set_dmamode		= pdc202xx_set_dmamode,
 
-	.sff_exec_command	= pdc20246_exec_command,
+	.sff_exec_command	= pdc202xx_exec_command,
 };
 
 static struct ata_port_operations pdc2026x_port_ops = {
author
#4 lore raw

Re: [PATCH 2/3] pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets

Quoted 28 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets

PDC2026x chipsets need the same treatment as PDC20246 one.

This is completely untested but will hopefully fix UDMA issues that people have been reporting against pata_pdc202xx_old for the last couple of years.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ata/pata_pdc202xx_old.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/drivers/ata/pata_pdc202xx_old.c
===================================================================

drivers/ata/pata_pdc202xx_old.c +1 -1
@@ -26,7 +26,7 @@
 
 #include "pata_pdc202xx_old.h"
 
-static void pdc20246_exec_command(struct ata_port *ap,
+static void pdc202xx_exec_command(struct ata_port *ap,
 				  const struct ata_taskfile *tf)
 {
 	DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);

drivers/ata/pata_pdc202xx_old.c static struct ata_port_operations pdc202 · line 187

Quoted 8 lines
drivers/ata/pata_pdc202xx_old.c +1 -1
@@ -181,7 +181,7 @@ static struct ata_port_operations pdc202
 	.set_piomode		= pdc202xx_set_piomode,
 	.set_dmamode		= pdc202xx_set_dmamode,
 
-	.sff_exec_command	= pdc20246_exec_command,
+	.sff_exec_command	= pdc202xx_exec_command,
 };
 
 static struct ata_port_operations pdc2026x_port_ops = {

the missing chunk :)

@@ -195,6 +195,8 @@
.dev_config = pdc2026x_dev_config,

.port_start = pdc2026x_port_start,
+
+ .sff_exec_command = pdc202xx_exec_command,
};

static int pdc202xx_init_one(struct pci_dev *dev, const struct pci_device_id *id)

#5 lore raw

Re: [PATCH 2/3] pata_pdc202xx_old: fix UDMA mode for PDC2026x chipsets

Quoted 1 line · Bartlomiej Zolnierkiewicz

.sff_exec_command = pdc202xx_exec_command,

applied patch #2 + this missing chunk

author
#6 lore raw

PATCH 3/3 pata_pdc202xx_old: update documentation

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_pdc202xx_old: update documentation

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ata/Kconfig |    1 -
 1 file changed, 1 deletion(-)
drivers/ata/Kconfig +0 -1
Index: b/drivers/ata/Kconfig
===================================================================
@@ -599,7 +599,6 @@ config PATA_PDC_OLD
 	  20265 and 20267 adapters.
 
 	  Known issues:
-	  - UDMA transfers fail mysteriously on some chipsets
 	  - ATAPI DMA is unsupported currently
 
 	  If unsure, say N.