Skip to contentSkip to conversation

libata: disable interrupts before soft reset

PATCH/RFC Albert Lee 2 participants 3 messages Mar 31, 2006 – Apr 1, 2006

Subsystems (2)
lore
Share & tools
lore

Albert Lee

author
#0 lore raw

Changes:
- add ata_irq_off() to libata.h
- call ata_irq_off() to disable the interrupt before soft reset

---

When testing an old 1996 Acer 8x CD-ROM drive (CD-787E), an interrupt is generated in ata_devchk(), right after the device is selected. (The unhandled irq cause the IRQ 10 to be disabled.)

Disabling interrupts before doing soft reset fixes the problem. However, this problem is only seen on the specific CD-ROM drive. So, don't know whether the fix is necessary.

Patch against upstream (8b316a3973f05e572b4edeeda9072987f6bbaa44). For your review, thanks.

Albert
===================================================
(Attached detailed dmesg for your reference)
pata_pdc2027x 0000:02:05.0: version 0.74
ACPI: PCI Interrupt Link [LNK1] enabled at IRQ 10
ACPI: PCI Interrupt 0000:02:05.0[A] -> Link [LNK1] -> GSI 10 (level, low) -> IRQ 10 pata_pdc2027x 0000:02:05.0: PLL input clock 16718 kHz
ata3: PATA max UDMA/133 cmd 0x989517C0 ctl 0x98951FDA bmdma 0x98951000 irq 10
ata4: PATA max UDMA/133 cmd 0x989515C0 ctl 0x98951DDA bmdma 0x98951008 irq 10 checking device 0
selecting checking device 0
irq 10: nobody cared (try booting with the "irqpoll" option)
<78139444> __report_bad_irq+0x24/0x7f <78139516> note_interrupt+0x77/0x21d
<78138f0d> handle_IRQ_event+0x2e/0x5a <78139003> __do_IRQ+0xca/0xd7
<78104708> do_IRQ+0x53/0x8b
=======================
<78102f4e> common_interrupt+0x1a/0x20 <7811e589> __do_softirq+0x37/0x95
<7810478f> do_softirq+0x4f/0x60
=======================
<7811e475> irq_exit+0x37/0x39 <7810470f> do_IRQ+0x5a/0x8b
<78102f4e> common_interrupt+0x1a/0x20 <98884ce0> ata_altstatus+0x29/0x2d [libata]
<9887e9d6> ata_std_dev_select+0x23/0x47 [libata] <9887ea2c> ata_devchk+0x32/0x135 [libata]
<9887fde7> ata_std_softreset+0x52/0x11a [libata] <9887fd95> ata_std_softreset+0x0/0x11a [libata]
<9887fac5> ata_std_postreset+0x0/0x12a [libata] <9887d45d> do_probe_reset+0x2b/0x82 [libata]
<9887fd95> ata_std_softreset+0x0/0x11a [libata] <9887fac5> ata_std_postreset+0x0/0x12a [libata]
<9887d4ee> ata_drive_probe_reset+0x3a/0x7d [libata] <989425ae> pdc2027x_probe_reset+0x60/0x65 [pata_pdc2027x]
<9887fac5> ata_std_postreset+0x0/0x12a [libata] <98881b84> ata_device_add+0x428/0x648 [libata]
<98942449> pdc2027x_init_one+0x29f/0x348 [pata_pdc2027x] <781de777> pci_device_probe+0x40/0x5b
<7823fe24> driver_probe_device+0x3b/0xb0 <7823ff88> __driver_attach+0x82/0x84
<7823f457> bus_for_each_dev+0x39/0x57 <7823fd16> driver_attach+0x16/0x1a
<7823ff06> __driver_attach+0x0/0x84 <7823f71c> bus_add_driver+0x63/0x110
<782402da> driver_register+0x41/0xb4 <781de3c8> __pci_register_driver+0x65/0x86
<781317af> sys_init_module+0x114/0x1731 <9887d79a> ata_port_start+0x0/0x7e [libata]
<78102d1b> sysenter_past_esp+0x54/0x75
handlers:
[<98881f1e>] (ata_interrupt+0x0/0x282 [libata])
Disabling IRQ #10
writing pattern 1 device 0
writing pattern 2 device 0
writing pattern 3 device 0
reading pattern 4 device 0
device found 1
checking device 1
selecting checking device 1
writing pattern 1 device 1
writing pattern 2 device 1
writing pattern 3 device 1
reading pattern 4 device 1
device found 1
select dev 0

======================================

drivers/scsi/libata-core.c +8 -0
diff -Nrup upstream0/drivers/scsi/libata-core.c reset_irq_off/drivers/scsi/libata-core.c
@@ -2069,6 +2069,10 @@ void ata_bus_reset(struct ata_port *ap)
 
 	DPRINTK("ENTER, host %u, port %u\n", ap->id, ap->port_no);
 
+	/* disable interrupts */
+	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
+		ata_irq_off(ap);
+
 	/* determine if device 0/1 are present */
 	if (ap->flags & ATA_FLAG_SATA_RESET)
 		dev0 = 1;
@@ -2199,6 +2203,10 @@ int ata_std_softreset(struct ata_port *a
 		goto out;
 	}
 
+	/* disable interrupts */
+	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
+		ata_irq_off(ap);
+
 	/* determine if device 0/1 are present */
 	if (ata_devchk(ap, 0))
 		devmask |= (1 << 0);
include/linux/libata.h +29 -0
diff -Nrup upstream0/include/linux/libata.h reset_irq_off/include/linux/libata.h
@@ -840,6 +840,35 @@ static inline u8 ata_irq_on(struct ata_p
 	return tmp;
 }
 
+/**
+ *	ata_irq_off - Disable interrupts on a port.
+ *	@ap: Port on which interrupts are disabled.
+ *
+ *	Disable interrupts on a legacy IDE device using MMIO or PIO,
+ *	wait for idle, clear any pending interrupts.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+
+static inline u8 ata_irq_off(struct ata_port *ap)
+{
+	struct ata_ioports *ioaddr = &ap->ioaddr;
+	u8 tmp;
+
+	ap->ctl |= ATA_NIEN;
+	ap->last_ctl = ap->ctl;
+
+	if (ap->flags & ATA_FLAG_MMIO)
+		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
+	else
+		outb(ap->ctl, ioaddr->ctl_addr);
+	tmp = ata_wait_idle(ap);
+
+	ap->ops->irq_clear(ap);
+
+	return tmp;
+}
 
 /**
  *	ata_irq_ack - Acknowledge a device interrupt.
#1 lore raw
Quoted 11 lines · Albert Lee

Changes:
- add ata_irq_off() to libata.h
- call ata_irq_off() to disable the interrupt before soft reset

---

When testing an old 1996 Acer 8x CD-ROM drive (CD-787E), an interrupt is generated in ata_devchk(), right after the device is selected. (The unhandled irq cause the IRQ 10 to be disabled.)

Quoted 7 lines

Disabling interrupts before doing soft reset fixes the problem. However, this problem is only seen on the specific CD-ROM drive. So, don't know whether the fix is necessary.

Patch against upstream (8b316a3973f05e572b4edeeda9072987f6bbaa44). For your review, thanks.

Hello, Albert.

Please hold this off. This issue is handled by new EH.

This problem is bigger than just during reset. Any error condition which leaves the device and/or controller has potential to cause similar problems or worse (e.g. data corruption, screaming interrupts). New EH handles this by freezing ports when device/controller enters unknown state. The controller is thawed after EH reset is complete. The same mechanism applies to probing. All ports are started frozen. Only after initial reset is complete, ports get thawed.

It's been a loooooong week but I'm splitting the patches now. :)

Signature · 1 line
tejun
author
#2 lore raw
Quoted 12 lines · Tejun Heo

Please hold this off. This issue is handled by new EH.

This problem is bigger than just during reset. Any error condition which leaves the device and/or controller has potential to cause similar problems or worse (e.g. data corruption, screaming interrupts). New EH handles this by freezing ports when device/controller enters unknown state. The controller is thawed after EH reset is complete. The same mechanism applies to probing. All ports are started frozen. Only after initial reset is complete, ports get thawed.

Quoted 2 lines

It's been a loooooong week but I'm splitting the patches now. :)

Great. :)
Will test the drive again when the new EH is out.

Thanks,

Albert