From: matthieu castet On via controller(vt8235) and pata via, some ATAPI devices (CDR-6S48) fail in the xfer mode initialisation. This make the boot slow and the drive unsuable. It seems the interrupt for xfer mode is done before the drive is ready, so the current libata code skip the interrupt. But no new interrupt is done by the controller, so the xfer mode fails. A quirk is to wait for 10 us (by step of 1 us) and see if the device becomes ready in the case of SETFEATURES_XFER feature. The problem seems pata_via only, so the quirk is put in the pata_via interrupt handler as Tejun Heo request. It won't affect working devices, as we don't wait if the device is already ready. It will adds an extra ata_altstatus in order to avoid to duplicate ata_host_intr, but only in the case of SETFEATURES_XFER (with should happen only few times). Sorry for the lack of changelog in my previous mail, I tought the old changelog + move it in pata via interrupt was enought. And sorry again, I sent you a bad patch (I forgot a quitl refresh). Signed-off-by: Matthieu CASTET Cc: Alan Cox Cc: Bartlomiej Zolnierkiewicz Cc: Jeff Garzik Cc: Tejun Heo Jeff Garzik wrote: > Data point #1: > > Here I quote from drivers/ide generic code ide_config_drive_speed() in > ide-iops.c: > > /* > * Allow status to settle, then read it again. > * A few rare drives vastly violate the 400ns spec here, > * so we'll wait up to 10usec for a "good" status > * rather than expensively fail things immediately. > * This fix courtesy of Matthew Faupel & Niccolo Rigacci. > */ > for (i = 0; i < 10; i++) { > udelay(1); > if (OK_STAT((stat = hwif->INB(IDE_STATUS_REG)), > DRIVE_READY, BUSY_STAT|DRQ_STAT|ERR_STAT)) { > error = 0; > break; > } > } > > And there is similar logic in ide_wait_stat(). wait_drive_not_busy() in > ide-taskfile.c waits for up to 1 ms. > > > Data point #2: > > libata was originally based on the "highly correct" (or one version > thereof) programming sequences found in Hale Landis's free ATADRVR > (http://www.ata-atapi.com/). Hale's ATADRVR, which was MS-DOS based and > not threaded or asynchronous at all, did the following after every > command execution (sent taskfile to hardware): > > if (device is ATAPI) > sleep(150 ms) > > > Overall, I've no clue what to do on older PATA hardware, so my > "insightful" suggestions are largely (a) follow drivers/ide code and/or > (b) listen to any old fogies with deep historical knowledge. Jeff Garzik wrote: > Data point #3 (or #0...): > > This appears to be a _device_ that sends its interrupt early. > > If that is the case, the device may appear on any controller, not just > VIA, and we would have to handle it globally via a device special-case > in libata-core. > Alan Cox wrote: > SETFEATURES_XFER stuff is magic anyway because some controllers snoop. > Also of course like the old IDE we send some rather iffy PIO requests > and that might have something to do with the problem of course. > > I've got two reports that look similar and are from ALi controllers so > trying the 10uS version might be worth doing, at least for PATA Signed-off-by: Andrew Morton --- drivers/ata/pata_via.c | 77 +++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff -puN drivers/ata/pata_via.c~via-pata-controller-xfer-fixes drivers/scsi/pata_via.c --- a/drivers/ata/pata_via.c~via-pata-controller-xfer-fixes +++ a/drivers/ata/pata_via.c @@ -283,6 +283,79 @@ static void via_set_dmamode(struct ata_p via_do_set_mode(ap, adev, adev->dma_mode, tclock[mode], set_ast, udma[mode]); } +/** + * pata_via_interrupt - ATA host interrupt handler for pata via + * @irq: irq line (unused) + * @dev_instance: pointer to our ata_host_set information structure + * @regs: unused + * + * Interrupt handler for via controller that is the default interrupt + * handler with a quirk for some ATAPI devices. + * Calls ata_host_intr() for each port that is not disabled. + * + * LOCKING: + * Obtains host_set lock during operation. + * + * RETURNS: + * IRQ_NONE or IRQ_HANDLED. + */ + +static irqreturn_t pata_via_interrupt(int irq, void *dev_instance, + struct pt_regs *regs) +{ + struct ata_host_set *host_set = dev_instance; + unsigned int i; + unsigned int handled = 0; + unsigned long flags; + + /* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */ + spin_lock_irqsave(&host_set->lock, flags); + for (i = 0; i < host_set->n_ports; i++) { + struct ata_port *ap; + + ap = host_set->ports[i]; + if (ap && !(ap->flags & ATA_FLAG_DISABLED)) { + struct ata_queued_cmd *qc; + + qc = ata_qc_from_tag(ap, ap->active_tag); + if (!qc) + continue; + if (qc->tf.flags & ATA_TFLAG_POLLING) + continue; + if (!(qc->flags & ATA_QCFLAG_ACTIVE)) + continue; + if (qc->tf.command == ATA_CMD_SET_FEATURES && + qc->tf.feature == SETFEATURES_XFER) { + /* + * With some ATAPI devices (CDR-6S48, ...), the + * ata_altstatus take some times (~ 2us) to + * become not busy. Without this quirk, it is + * impossible to set the xfer mode, and the + * libata-core disable the device. + */ + int i; + + for (i = 0; i < 10; i++) { + if (!(ata_altstatus(ap) & ATA_BUSY)) + break; + udelay(1); + } + + if (ata_altstatus(ap) & ATA_BUSY) + continue; /* It's stuck */ + + if (i) + ata_port_printk(ap, KERN_WARNING, + "ata_altstatus take %dus\n", i); + } + handled |= ata_host_intr(ap, qc); + } + } + spin_unlock_irqrestore(&host_set->lock, flags); + + return IRQ_RETVAL(handled); +} + static struct scsi_host_template via_sht = { .module = THIS_MODULE, .name = DRV_NAME, @@ -328,7 +401,7 @@ static struct ata_port_operations via_po .eng_timeout = ata_eng_timeout, .data_xfer = ata_pio_data_xfer, - .irq_handler = ata_interrupt, + .irq_handler = pata_via_interrupt, .irq_clear = ata_bmdma_irq_clear, .port_start = ata_port_start, @@ -363,7 +436,7 @@ static struct ata_port_operations via_po .eng_timeout = ata_eng_timeout, .data_xfer = ata_pio_data_xfer_noirq, - .irq_handler = ata_interrupt, + .irq_handler = pata_via_interrupt, .irq_clear = ata_bmdma_irq_clear, .port_start = ata_port_start, _