[PATCH #sx4] sata_sx4: fixup interrupt handling

[PATCH #sx4] sata_sx4: fixup interrupt handling

[David Milburn]

Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
pdc20621_interrupt ignores command completion since
ATA_TFLAG_POLLING flag is set.

This has already been fixed for sata_promise:

commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
Author: Tejun Heo <htejun@gmail.com>
Date:   Fri Jun 8 13:46:55 2007 -0700

    sata_promise: use TF interface for polling NODATA commands

Also, this patch includes Mikael's original patches:

http://marc.info/?l=linux-ide&m=121135828227724&w=2
http://marc.info/?l=linux-ide&m=121144512109826&w=2

Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
Signed-off-by: David Milburn <dmilburn@redhat.com>
---
 drivers/ata/sata_sx4.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c
index e6a866e..bbcf970 100644
--- a/drivers/ata/sata_sx4.c
+++ b/drivers/ata/sata_sx4.c
@@ -688,8 +688,11 @@ static void pdc20621_packet_start(struct ata_queued_cmd *qc)
 static unsigned int pdc20621_qc_issue(struct ata_queued_cmd *qc)
 {
 	switch (qc->tf.protocol) {
-	case ATA_PROT_DMA:
 	case ATA_PROT_NODATA:
+		if (qc->tf.flags & ATA_TFLAG_POLLING)
+			break;
+		/*FALLTHROUGH*/
+	case ATA_PROT_DMA:
 		pdc20621_packet_start(qc);
 		return 0;
 
@@ -788,12 +791,7 @@ static inline unsigned int pdc20621_host_intr(struct ata_port *ap,
 
 static void pdc20621_irq_clear(struct ata_port *ap)
 {
-	struct ata_host *host = ap->host;
-	void __iomem *mmio = host->iomap[PDC_MMIO_BAR];
-
-	mmio += PDC_CHIP0_OFS;
-
-	readl(mmio + PDC_20621_SEQMASK);
+	ioread8(ap->ioaddr.status_addr);
 }
 
 static irqreturn_t pdc20621_interrupt(int irq, void *dev_instance)
@@ -878,14 +876,12 @@ static void pdc_freeze(struct ata_port *ap)
 static void pdc_thaw(struct ata_port *ap)
 {
 	void __iomem *mmio = ap->ioaddr.cmd_addr;
-	void __iomem *mmio_base;
 	u32 tmp;
 
 	/* FIXME: start HDMA engine, if zero ATA engines running */
 
-	/* reading SEQ mask register clears IRQ */
-	mmio_base = ap->host->iomap[PDC_MMIO_BAR] + PDC_CHIP0_OFS;
-	readl(mmio_base + PDC_20621_SEQMASK);
+	/* clear IRQ */
+	ioread8(ap->ioaddr.status_addr);
 
 	/* turn IRQ back on */
 	tmp = readl(mmio + PDC_CTLSTAT);
@@ -975,7 +971,7 @@ static int pdc_check_atapi_dma(struct ata_queued_cmd *qc)
 static void pdc_tf_load_mmio(struct ata_port *ap, const struct ata_taskfile *tf)
 {
 	WARN_ON(tf->protocol == ATA_PROT_DMA ||
-		tf->protocol == ATA_PROT_NODATA);
+		tf->protocol == ATAPI_PROT_DMA);
 	ata_sff_tf_load(ap, tf);
 }
 
@@ -983,7 +979,7 @@ static void pdc_tf_load_mmio(struct ata_port *ap, const struct ata_taskfile *tf)
 static void pdc_exec_command_mmio(struct ata_port *ap, const struct ata_taskfile *tf)
 {
 	WARN_ON(tf->protocol == ATA_PROT_DMA ||
-		tf->protocol == ATA_PROT_NODATA);
+		tf->protocol == ATAPI_PROT_DMA);
 	ata_sff_exec_command(ap, tf);
 }
 

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[Jeff Garzik]

David Milburn wrote:
> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
> pdc20621_interrupt ignores command completion since
> ATA_TFLAG_POLLING flag is set.
> 
> This has already been fixed for sata_promise:
> 
> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
> Author: Tejun Heo <htejun@gmail.com>
> Date:   Fri Jun 8 13:46:55 2007 -0700
> 
>     sata_promise: use TF interface for polling NODATA commands
> 
> Also, this patch includes Mikael's original patches:
> 
> http://marc.info/?l=linux-ide&m=121135828227724&w=2
> http://marc.info/?l=linux-ide&m=121144512109826&w=2
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> Signed-off-by: David Milburn <dmilburn@redhat.com>
> ---
>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)

Good spotting...

Is this on top of #sx4 or upstream?

	Jeff

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[Jeff Garzik]

David Milburn wrote:
> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
> pdc20621_interrupt ignores command completion since
> ATA_TFLAG_POLLING flag is set.
> 
> This has already been fixed for sata_promise:
> 
> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
> Author: Tejun Heo <htejun@gmail.com>
> Date:   Fri Jun 8 13:46:55 2007 -0700
> 
>     sata_promise: use TF interface for polling NODATA commands
> 
> Also, this patch includes Mikael's original patches:
> 
> http://marc.info/?l=linux-ide&m=121135828227724&w=2
> http://marc.info/?l=linux-ide&m=121144512109826&w=2
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> Signed-off-by: David Milburn <dmilburn@redhat.com>
> ---
>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)

Oh, and:  does vanilla upstream sata_sx4 work at all?

If not, that would justify fast-forwarding the #sx4 branch into 2.6.30 
(with your fixes, obviously)...

Thanks,

	Jeff

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[David Milburn]

Jeff Garzik wrote:
> David Milburn wrote:
>> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
>> pdc20621_interrupt ignores command completion since
>> ATA_TFLAG_POLLING flag is set.
>>
>> This has already been fixed for sata_promise:
>>
>> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
>> Author: Tejun Heo <htejun@gmail.com>
>> Date:   Fri Jun 8 13:46:55 2007 -0700
>>
>>     sata_promise: use TF interface for polling NODATA commands
>>
>> Also, this patch includes Mikael's original patches:
>>
>> http://marc.info/?l=linux-ide&m=121135828227724&w=2
>> http://marc.info/?l=linux-ide&m=121144512109826&w=2
>>
>> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
>> Signed-off-by: David Milburn <dmilburn@redhat.com>
>> ---
>>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> Good spotting...
> 
> Is this on top of #sx4 or upstream?
> 

Jeff, this is on top of #sx4 branch.

Thanks,
David

>     Jeff
> 
> 
> 

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[David Milburn]

Jeff Garzik wrote:
> David Milburn wrote:
>> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
>> pdc20621_interrupt ignores command completion since
>> ATA_TFLAG_POLLING flag is set.
>>
>> This has already been fixed for sata_promise:
>>
>> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
>> Author: Tejun Heo <htejun@gmail.com>
>> Date:   Fri Jun 8 13:46:55 2007 -0700
>>
>>     sata_promise: use TF interface for polling NODATA commands
>>
>> Also, this patch includes Mikael's original patches:
>>
>> http://marc.info/?l=linux-ide&m=121135828227724&w=2
>> http://marc.info/?l=linux-ide&m=121144512109826&w=2
>>
>> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
>> Signed-off-by: David Milburn <dmilburn@redhat.com>
>> ---
>>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> Oh, and:  does vanilla upstream sata_sx4 work at all?

I know it was not working on 2.6.29.2, I will try a more recent
kernel with and without the patch.

Thanks,
David

> 
> If not, that would justify fast-forwarding the #sx4 branch into 2.6.30 
> (with your fixes, obviously)...
> 
> Thanks,
> 
>     Jeff
> 
> 
> 
> 

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[David Milburn]

Jeff Garzik wrote:
> David Milburn wrote:
> 
>> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
>> pdc20621_interrupt ignores command completion since
>> ATA_TFLAG_POLLING flag is set.
>>
>> This has already been fixed for sata_promise:
>>
>> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
>> Author: Tejun Heo <htejun@gmail.com>
>> Date:   Fri Jun 8 13:46:55 2007 -0700
>>
>>     sata_promise: use TF interface for polling NODATA commands
>>
>> Also, this patch includes Mikael's original patches:
>>
>> http://marc.info/?l=linux-ide&m=121135828227724&w=2
>> http://marc.info/?l=linux-ide&m=121144512109826&w=2
>>
>> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
>> Signed-off-by: David Milburn <dmilburn@redhat.com>
>> ---
>>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> 
> Oh, and:  does vanilla upstream sata_sx4 work at all?

Jeff,

It does not work on 2.6.30-rc5

Linux version 2.6.30-rc5
  .
  .
  .
ata1.00: ATA-6: ST3160023AS, 3.05, max UDMA/133
ata1.00: 312581808 sectors, multi 0: LBA48
ata1.00: qc timeout (cmd 0xef)
ata1.00: failed to set xfermode (err_mask=0x4)
ata1.00: ATA-6: ST3160023AS, 3.05, max UDMA/133
ata1.00: 312581808 sectors, multi 0: LBA48
ata1.00: qc timeout (cmd 0xef)
ata1.00: failed to set xfermode (err_mask=0x4)
ata1.00: limiting speed to UDMA/133:PIO3
ata1.00: ATA-6: ST3160023AS, 3.05, max UDMA/133
ata1.00: 312581808 sectors, multi 0: LBA48
ata1.00: qc timeout (cmd 0xef)
ata1.00: failed to set xfermode (err_mask=0x4)
ata1.00: disabled

Same thing if I drop in sata_sx4.c and libata-sff.c
changes from libata-dev sx4 branch.

But, after applying this patch on top of that

ata1.00: ATA-6: ST3160023AS, 3.05, max UDMA/133
ata1.00: 312581808 sectors, multi 0: LBA48
ata1.00: configured for UDMA/133
scsi 0:0:0:0: Direct-Access     ATA      ST3160023AS      3.05 PQ: 0 ANSI: 5
sd 0:0:0:0: [sda] 312581808 512-byte hardware sectors: (160 GB/149 GiB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support
  DPO or FUA
  sda: sda1 sda2
sd 0:0:0:0: [sda] Attached SCSI disk
ata2.00: NODEV after polling detection
ata3.00: NODEV after polling detection
ata4.00: NODEV after polling detection


# cat /proc/scsi/scsi
Attached devices:
Host: scsi0 Channel: 00 Id: 00 Lun: 00
   Vendor: ATA      Model: ST3160023AS      Rev: 3.05
   Type:   Direct-Access                    ANSI  SCSI revision: 05

Thanks,
David

> 
> If not, that would justify fast-forwarding the #sx4 branch into 2.6.30 
> (with your fixes, obviously)...
> 
> Thanks,
> 
>     Jeff
> 
> 
> 
> 

Re: [PATCH #sx4] sata_sx4: fixup interrupt handling

[Jeff Garzik]

David Milburn wrote:
> Issuing ATA_CMD_SET_FEATURES (0xef) times out because 
> pdc20621_interrupt ignores command completion since
> ATA_TFLAG_POLLING flag is set.
> 
> This has already been fixed for sata_promise:
> 
> commit 51b94d2a5a90d4800e74d7348bcde098a28f4fb3
> Author: Tejun Heo <htejun@gmail.com>
> Date:   Fri Jun 8 13:46:55 2007 -0700
> 
>     sata_promise: use TF interface for polling NODATA commands
> 
> Also, this patch includes Mikael's original patches:
> 
> http://marc.info/?l=linux-ide&m=121135828227724&w=2
> http://marc.info/?l=linux-ide&m=121144512109826&w=2
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> Signed-off-by: David Milburn <dmilburn@redhat.com>
> ---
>  drivers/ata/sata_sx4.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)

applied, and forwarding upstream (w/ #sx4 branch) for 2.6.30, since 
sata_sx4 is broken.