Patchwork pata_bf54x: fix BMIDE status register emulation

login
register
mail settings
Submitter Sergei Shtylyov
Date Dec. 28, 2011, 3:36 p.m.
Message ID <201112281836.45834.sshtylyov@ru.mvista.com>
Download mbox | patch
Permalink /patch/133441/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Sergei Shtylyov - Dec. 28, 2011, 3:36 p.m.
The author of this driver clearly wasn't familiar with the BMIDE specification
(also known as SFF-8038i) when he implemented the bmdma_status() method: first,
the interrupt bit of the BMIDE status register corresponds to nothing else but
INTRQ signal (ATAPI_DEV_INT here); second, the error bit is only set if the
controller encounters issue doing the bus master transfers, not on the DMA burst
termination interrupts like here (moreover, setting the error bit doesn't cause
an interrupt).

(The only thing I couldn't figure out is how to flush the FIFO to memory once
the interrupt happens as required by the mentioned spec.)

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

---
The patch is against the current Linus' tree.
Sonic, if you still work in Analog Devices, please give this a try.

I looked over the driver, and it left pretty bad impression. In particular,
I highly doubt that the transfers with more than one S/G item can work. And
this is after the driver has been in the kernel for 4 years already...
Unfortunately, I have neither hardware nor much time to work on improving it...

 drivers/ata/pata_bf54x.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Dec. 28, 2011, 5:56 p.m.
Hello.

On 12/28/2011 06:36 PM, Sergei Shtylyov wrote:

> The author of this driver clearly wasn't familiar with the BMIDE specification
> (also known as SFF-8038i) when he implemented the bmdma_status() method: first,
> the interrupt bit of the BMIDE status register corresponds to nothing else but
> INTRQ signal (ATAPI_DEV_INT here); second, the error bit is only set if the
> controller encounters issue doing the bus master transfers, not on the DMA burst
> termination interrupts like here

    Forgot to say that burst terminations are happening on the IDE side.

> (moreover, setting the error bit doesn't cause an interrupt).

> (The only thing I couldn't figure out is how to flush the FIFO to memory once
> the interrupt happens as required by the mentioned spec.)

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

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

Patch

Index: linux-2.6/drivers/ata/pata_bf54x.c
===================================================================
--- linux-2.6.orig/drivers/ata/pata_bf54x.c
+++ linux-2.6/drivers/ata/pata_bf54x.c
@@ -1153,15 +1153,11 @@  static unsigned char bfin_bmdma_status(s
 {
 	unsigned char host_stat = 0;
 	void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr;
-	unsigned short int_status = ATAPI_GET_INT_STATUS(base);
 
-	if (ATAPI_GET_STATUS(base) & (MULTI_XFER_ON|ULTRA_XFER_ON))
+	if (ATAPI_GET_STATUS(base) & (MULTI_XFER_ON | ULTRA_XFER_ON))
 		host_stat |= ATA_DMA_ACTIVE;
-	if (int_status & (MULTI_DONE_INT|UDMAIN_DONE_INT|UDMAOUT_DONE_INT|
-		ATAPI_DEV_INT))
+	if (ATAPI_GET_INT_STATUS(base) & ATAPI_DEV_INT)
 		host_stat |= ATA_DMA_INTR;
-	if (int_status & (MULTI_TERM_INT|UDMAIN_TERM_INT|UDMAOUT_TERM_INT))
-		host_stat |= ATA_DMA_ERR|ATA_DMA_INTR;
 
 	dev_dbg(ap->dev, "ATAPI: host_stat=0x%x\n", host_stat);