Skip to contentSkip to conversation

SATA interrupt handling

PATCH In review Jeff Garzik 3 participants 4 messages Jul 4, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 SATA interrupt handling

Attached is the latest SATA patch (and BK info).

When I turned on the PATA support in libata to do some ATAPI development on my ICH5, I reproduced the behavior that Fabio Coatti was seeing on his box. This patch fixed my PATA interface, here's hoping it fixes Fabio's problem as well.

Tech info for Bart: The ATA_DMA_INTR bit in ATA_DMA_STATUS is apparently set whenever the INTRQ line (emulated under SATA) indicates an interrupt... even for non-DMA commands. I did not expect this, at least :)

Judging from some of the ICH-related bug reports, I'm willing to bet that the IDE driver needs to do what libata is now going, WRT interrupt acknowledgement. Also... for the case in combined mode, where IDE driver and libata driver share the hardware, the IDE driver needs to talk to the BMDMA registers that it is not allowed to access.

IMO once ATAPI is working (very soon), the easiest thing to do would be to let libata handle PATA for the combined mode case. This would eliminate all the nasty ____request_resource stuff (which Marcelo rightly objected to), and should allow for a clean fix to this sharing-hardware problem.

In theory, it is always an ugly situation when two drivers talk to a single PCI device simultaneously...

Jeff

BK users, do a

bk pull bk://gkernel.bkbits.net/libata-2.6

This will update the following files:

 drivers/scsi/ata_piix.c     |    6 ++++++
 drivers/scsi/libata-core.c  |   43 ++++++++++++++++++++++---------------------
 drivers/scsi/libata-scsi.c  |   20 ++++++++++++++++++++
 drivers/scsi/sata_nv.c      |    2 ++
 drivers/scsi/sata_promise.c |   11 +++++++++++
 drivers/scsi/sata_sil.c     |    3 +++
 drivers/scsi/sata_sis.c     |    2 ++
 drivers/scsi/sata_svw.c     |    2 ++
 drivers/scsi/sata_sx4.c     |   13 +++++++++++++
 drivers/scsi/sata_via.c     |    3 +++
 drivers/scsi/sata_vsc.c     |    2 ++
 include/linux/ata.h         |   10 +++++++++-
 include/linux/libata.h      |   18 +++++++++++++++---
 13 files changed, 110 insertions(+), 25 deletions(-)

through these ChangeSets:

<[email protected]> (04/07/03 1.1789)
[ata] add ata_ok() inlined helper, and ATA_{DRDY,DF} bit to linux/ata.h

<[email protected]> (04/07/03 1.1788)
[libata] create, and use, ->irq_clear hook

This is more conservative in general, and so applies to multiple controllers. Specifically it attempts to address irq-related issues on the Intel ICH5/6 hardware. On Intel ICH5/6, the BMDMA 'interrupt' status bit will be set even on non-DMA commands, which software (and I) did not expect.

This change clears pending interrupts once upon initialization, and then each time ata_irq_on() is called.

<[email protected]> (04/07/03 1.1787)
[libata sata_sil] add drive to mod15write quirk list

<[email protected]> (04/07/03 1.1786)
[libata] add ata_queued_cmd completion hook

<[email protected]> (04/07/03 1.1785)
[libata] add ->qc_issue hook

This hook is used when an ATA controller wishes to use hardware-specific methods of taskfile delivery, rather than the standard method of bitbanging the ATA shadow registers.

drivers/scsi/ata_piix.c +6 -0
diff -Nru a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
@@ -138,9 +138,12 @@
 	.bmdma_setup		= ata_bmdma_setup_pio,
 	.bmdma_start		= ata_bmdma_start_pio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+
 	.eng_timeout		= ata_eng_timeout,
 
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
@@ -161,9 +164,12 @@
 	.bmdma_setup		= ata_bmdma_setup_pio,
 	.bmdma_start		= ata_bmdma_start_pio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+
 	.eng_timeout		= ata_eng_timeout,
 
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
drivers/scsi/libata-core.c +22 -21

117 lines not on this page · raw message

drivers/scsi/libata-scsi.c +20 -0
diff -Nru a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
@@ -335,6 +335,24 @@
 	return 1;
 }
 
+static int ata_scsi_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat)
+{
+	struct scsi_cmnd *cmd = qc->scsicmd;
+
+	if (unlikely(drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ))) {
+		if (is_atapi_taskfile(&qc->tf))
+			cmd->result = SAM_STAT_CHECK_CONDITION;
+		else
+			ata_to_sense_error(qc);
+	} else {
+		cmd->result = SAM_STAT_GOOD;
+	}
+
+	qc->scsidone(cmd);
+
+	return 0;
+}
+
 /**
  *	ata_scsi_translate - Translate then issue SCSI command to ATA device
  *	@ap: ATA port to which the command is addressed
@@ -378,6 +396,8 @@
 
 		qc->flags |= ATA_QCFLAG_SG; /* data is present; dma-map it */
 	}
+
+	qc->complete_fn = ata_scsi_qc_complete;
 
 	if (xlat_func(qc, scsicmd))
 		goto err_out;
drivers/scsi/sata_nv.c +2 -0
diff -Nru a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
@@ -139,8 +139,10 @@
 	.bmdma_setup		= ata_bmdma_setup_pio,
 	.bmdma_start		= ata_bmdma_start_pio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.irq_handler		= nv_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
 	.port_start		= ata_port_start,
drivers/scsi/sata_promise.c +11 -0
diff -Nru a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
@@ -86,6 +86,7 @@
 static void pdc_exec_command_mmio(struct ata_port *ap, struct ata_taskfile *tf);
 static inline void pdc_dma_complete (struct ata_port *ap,
                                      struct ata_queued_cmd *qc, int have_err);
+static void pdc_irq_clear(struct ata_port *ap);
 
 static Scsi_Host_Template pdc_sata_sht = {
 	.module			= THIS_MODULE,
@@ -115,8 +116,10 @@
 	.bmdma_setup            = pdc_dma_setup,
 	.bmdma_start            = pdc_dma_start,
 	.qc_prep		= pdc_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= pdc_eng_timeout,
 	.irq_handler		= pdc_interrupt,
+	.irq_clear		= pdc_irq_clear,
 	.scr_read		= pdc_sata_scr_read,
 	.scr_write		= pdc_sata_scr_write,
 	.port_start		= pdc_port_start,
@@ -376,6 +379,14 @@
         }
 
         return handled;
+}
+
+static void pdc_irq_clear(struct ata_port *ap)
+{
+	struct ata_host_set *host_set = ap->host_set;
+	void *mmio = host_set->mmio_base;
+
+	readl(mmio + PDC_INT_SEQMASK);
 }
 
 static irqreturn_t pdc_interrupt (int irq, void *dev_instance, struct pt_regs *regs)
drivers/scsi/sata_sil.c +3 -0
diff -Nru a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
@@ -86,6 +86,7 @@
 	{ "ST360015AS",		SIL_QUIRK_MOD15WRITE },
 	{ "ST380023AS",		SIL_QUIRK_MOD15WRITE },
 	{ "ST3120023AS",	SIL_QUIRK_MOD15WRITE },
+	{ "ST3160023AS",	SIL_QUIRK_MOD15WRITE },
 	{ "ST340014ASL",	SIL_QUIRK_MOD15WRITE },
 	{ "ST360014ASL",	SIL_QUIRK_MOD15WRITE },
 	{ "ST380011ASL",	SIL_QUIRK_MOD15WRITE },
@@ -132,8 +133,10 @@
 	.bmdma_setup            = ata_bmdma_setup_mmio,
 	.bmdma_start            = ata_bmdma_start_mmio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sil_scr_read,
 	.scr_write		= sil_scr_write,
 	.port_start		= ata_port_start,
drivers/scsi/sata_sis.c +2 -0
diff -Nru a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
@@ -101,8 +101,10 @@
 	.bmdma_setup            = ata_bmdma_setup_pio,
 	.bmdma_start            = ata_bmdma_start_pio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sis_scr_read,
 	.scr_write		= sis_scr_write,
 	.port_start		= ata_port_start,
drivers/scsi/sata_svw.c +2 -0
diff -Nru a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
@@ -234,8 +234,10 @@
 	.bmdma_setup            = ata_bmdma_setup_mmio,
 	.bmdma_start            = ata_bmdma_start_mmio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= k2_sata_scr_read,
 	.scr_write		= k2_sata_scr_write,
 	.port_start		= ata_port_start,
drivers/scsi/sata_sx4.c +13 -0
diff -Nru a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
@@ -171,6 +171,7 @@
 #endif
 static void pdc20621_put_to_dimm(struct ata_probe_ent *pe, 
 				 void *psource, u32 offset, u32 size);
+static void pdc20621_irq_clear(struct ata_port *ap);
 
 
 static Scsi_Host_Template pdc_sata_sht = {
@@ -201,8 +202,10 @@
 	.bmdma_setup            = pdc20621_dma_setup,
 	.bmdma_start            = pdc20621_dma_start,
 	.qc_prep		= pdc20621_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= pdc_eng_timeout,
 	.irq_handler		= pdc20621_interrupt,
+	.irq_clear		= pdc20621_irq_clear,
 	.port_start		= pdc_port_start,
 	.port_stop		= pdc_port_stop,
 	.host_stop		= pdc20621_host_stop,
@@ -700,6 +703,16 @@
 	}
 
 	return handled;
+}
+
+static void pdc20621_irq_clear(struct ata_port *ap)
+{
+	struct ata_host_set *host_set = ap->host_set;
+	void *mmio = host_set->mmio_base;
+
+	mmio += PDC_CHIP0_OFS;
+
+	readl(mmio + PDC_20621_SEQMASK);
 }
 
 static irqreturn_t pdc20621_interrupt (int irq, void *dev_instance, struct pt_regs *regs)
drivers/scsi/sata_via.c +3 -0
diff -Nru a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
@@ -109,9 +109,12 @@
 	.bmdma_setup            = ata_bmdma_setup_pio,
 	.bmdma_start            = ata_bmdma_start_pio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+
 	.eng_timeout		= ata_eng_timeout,
 
 	.irq_handler		= ata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 
 	.scr_read		= svia_scr_read,
 	.scr_write		= svia_scr_write,
drivers/scsi/sata_vsc.c +2 -0
diff -Nru a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
@@ -216,8 +216,10 @@
 	.bmdma_setup            = ata_bmdma_setup_mmio,
 	.bmdma_start            = ata_bmdma_start_mmio,
 	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.irq_handler		= vsc_sata_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= vsc_sata_scr_read,
 	.scr_write		= vsc_sata_scr_write,
 	.port_start		= ata_port_start,
include/linux/ata.h +9 -1
diff -Nru a/include/linux/ata.h b/include/linux/ata.h
@@ -78,9 +78,11 @@
 	ATA_NIEN		= (1 << 1),	/* disable-irq flag */
 	ATA_LBA			= (1 << 6),	/* LBA28 selector */
 	ATA_DEV1		= (1 << 4),	/* Select Device 1 (slave) */
-	ATA_BUSY		= (1 << 7),	/* BSY status bit */
 	ATA_DEVICE_OBS		= (1 << 7) | (1 << 5), /* obs bits in dev reg */
 	ATA_DEVCTL_OBS		= (1 << 3),	/* obsolete bit in devctl reg */
+	ATA_BUSY		= (1 << 7),	/* BSY status bit */
+	ATA_DRDY		= (1 << 6),	/* device ready */
+	ATA_DF			= (1 << 5),	/* device fault */
 	ATA_DRQ			= (1 << 3),	/* data request i/o */
 	ATA_ERR			= (1 << 0),	/* have an error */
 	ATA_SRST		= (1 << 2),	/* software reset */
@@ -222,6 +224,12 @@
 {
 	return (tf->protocol == ATA_PROT_ATAPI) ||
 	       (tf->protocol == ATA_PROT_ATAPI_DMA);
+}
+
+static inline int ata_ok(u8 status)
+{
+	return ((status & (ATA_BUSY | ATA_DRDY | ATA_DF | ATA_DRQ | ATA_ERR))
+			== ATA_DRDY);
 }
 
 #endif /* __LINUX_ATA_H__ */
include/linux/libata.h +15 -3
diff -Nru a/include/linux/libata.h b/include/linux/libata.h
@@ -158,7 +158,7 @@
 struct ata_queued_cmd;
 
 /* typedefs */
-typedef void (*ata_qc_cb_t) (struct ata_queued_cmd *qc, unsigned int flags);
+typedef int (*ata_qc_cb_t) (struct ata_queued_cmd *qc, u8 drv_stat);
 
 struct ata_ioports {
 	unsigned long		cmd_addr;
@@ -202,6 +202,7 @@
 	void			*mmio_base;
 	unsigned int		n_ports;
 	void			*private_data;
+	struct ata_port_operations *ops;
 	struct ata_port *	ports[0];
 };
 
@@ -224,7 +225,7 @@
 
 	struct scatterlist	*sg;
 
-	ata_qc_cb_t		callback;
+	ata_qc_cb_t		complete_fn;
 
 	struct completion	*waiting;
 
@@ -311,10 +312,14 @@
 
 	void (*bmdma_setup) (struct ata_queued_cmd *qc);
 	void (*bmdma_start) (struct ata_queued_cmd *qc);
+
 	void (*qc_prep) (struct ata_queued_cmd *qc);
+	int (*qc_issue) (struct ata_queued_cmd *qc);
+
 	void (*eng_timeout) (struct ata_port *ap);
 
 	irqreturn_t (*irq_handler)(int, void *, struct pt_regs *);
+	void (*irq_clear) (struct ata_port *);
 
 	u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg);
 	void (*scr_write) (struct ata_port *ap, unsigned int sc_reg,
@@ -372,12 +377,14 @@
 extern void ata_port_stop (struct ata_port *ap);
 extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
 extern void ata_qc_prep(struct ata_queued_cmd *qc);
+extern int ata_qc_issue_prot(struct ata_queued_cmd *qc);
 extern void ata_dev_id_string(struct ata_device *dev, unsigned char *s,
 			      unsigned int ofs, unsigned int len);
 extern void ata_bmdma_setup_mmio (struct ata_queued_cmd *qc);
 extern void ata_bmdma_start_mmio (struct ata_queued_cmd *qc);
 extern void ata_bmdma_setup_pio (struct ata_queued_cmd *qc);
 extern void ata_bmdma_start_pio (struct ata_queued_cmd *qc);
+extern void ata_bmdma_irq_clear(struct ata_port *ap);
 extern int pci_test_config_bits(struct pci_dev *pdev, struct pci_bits *bits);
 extern void ata_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat);
 extern void ata_eng_timeout(struct ata_port *ap);
@@ -480,6 +487,7 @@
 static inline u8 ata_irq_on(struct ata_port *ap)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
+	u8 tmp;
 
 	ap->ctl &= ~ATA_NIEN;
 	ap->last_ctl = ap->ctl;
@@ -488,7 +496,11 @@
 		writeb(ap->ctl, ioaddr->ctl_addr);
 	else
 		outb(ap->ctl, ioaddr->ctl_addr);
-	return ata_wait_idle(ap);
+	tmp = ata_wait_idle(ap);
+
+	ap->ops->irq_clear(ap);
+
+	return tmp;
 }
 
 static inline u8 ata_irq_ack(struct ata_port *ap, unsigned int chk_drq)
#1 lore raw

Alle 05:02, domenica 4 luglio 2004, Jeff Garzik ha scritto:

Quoted 6 lines

Attached is the latest SATA patch (and BK info).

When I turned on the PATA support in libata to do some ATAPI development on my ICH5, I reproduced the behavior that Fabio Coatti was seeing on his box. This patch fixed my PATA interface, here's hoping it fixes Fabio's problem as well.

Yes, I can confirm that this patch fixes my problem as well. (tested against 2.6.7-mm5).
Now my box boots just fine, good work!

Signature · 4 lines
Fabio Coatti       http://members.ferrara.linux.it/cova     
Ferrara Linux Users Group           http://ferrara.linux.it
GnuPG fp:9765 A5B6 6843 17BC A646  BE8C FA56 373A 5374 C703
Old SysOps never die... they simply forget their password.
#2 lore raw
In reply to Jeff Garzik
Quoted 1 line · Jeff Garzik

Attached is the latest SATA patch (and BK info).

2.6.7-bk17 & your patches works fine on the ICH5 which had problems with kernels above -bk6 (unless i used acpi=off).

Thanks!

Danny

Signature · 3 lines
"If Microsoft had been the innovative company that it calls itself, it 
would have taken the opportunity to take a radical leap beyond the Mac, 
instead of producing a feeble, me-too implementation." - Douglas Adams -
#3 lore raw
Quoted 7 lines · Danny ter Haar
Quoted 1 line · Jeff Garzik

Attached is the latest SATA patch (and BK info).

2.6.7-bk17 & your patches works fine on the ICH5
which had problems with kernels above -bk6 (unless i used acpi=off).

Linus just committed this, thanks for testing.

Jeff