From 62877f6b0c4943c2231b84b49182a078eb02a777 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 22 Jun 2007 14:17:28 +0100 Subject: [PATCH 01/11] HPT374 is UDMA100 not UDMA133 Propogate change from drivers/ide Signed-off-by: Alan Cox Signed-off-by: Jeff Garzik --- drivers/ata/pata_hpt37x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ata/pata_hpt37x.c b/drivers/ata/pata_hpt37x.c index 6446735a46e0..a8c0cbeca399 100644 --- a/drivers/ata/pata_hpt37x.c +++ b/drivers/ata/pata_hpt37x.c @@ -931,13 +931,13 @@ static int hpt37x_init_one(struct pci_dev *dev, const struct pci_device_id *id) .udma_mask = 0x7f, .port_ops = &hpt372_port_ops }; - /* HPT374 - UDMA133 */ + /* HPT374 - UDMA100 */ static const struct ata_port_info info_hpt374 = { .sht = &hpt37x_sht, .flags = ATA_FLAG_SLAVE_POSS|ATA_FLAG_SRST, .pio_mask = 0x1f, .mwdma_mask = 0x07, - .udma_mask = 0x7f, + .udma_mask = 0x3f, .port_ops = &hpt374_port_ops }; From 55f3952d45a439cecc36fd845a87026d04c82931 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 21:31:05 +0900 Subject: [PATCH 02/11] libata: kill the infamous abnormal status message The infamous abnormal status message triggers on not so abnormal cases including empty port and even when it's being triggered on actual errors the info it provides is redundant and out of context - higher level functions will print the info in better safe later anyway. Also, by being triggered all the time, it leads people to think that the abnormality is somehow related to all ATA and system problems they're experiencing and gives owners of healthy systems unfounded doubts about the integrity of the universe. Make it a DPRINTK and save the universe. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- include/linux/libata.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/include/linux/libata.h b/include/linux/libata.h index 745c4f9b4caa..a3380f808630 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1088,11 +1088,9 @@ static inline u8 ata_wait_idle(struct ata_port *ap) { u8 status = ata_busy_wait(ap, ATA_BUSY | ATA_DRQ, 1000); - if (status != 0xff && (status & (ATA_BUSY | ATA_DRQ))) { - if (ata_msg_warn(ap)) - printk(KERN_WARNING "ATA: abnormal status 0x%X on port 0x%p\n", - status, ap->ioaddr.status_addr); - } + if (status != 0xff && (status & (ATA_BUSY | ATA_DRQ))) + DPRINTK("ATA: abnormal status 0x%X on port 0x%p\n", + status, ap->ioaddr.status_addr); return status; } From 8af500bc7f8f1a8822ff451596f818ecb6968f38 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 21:11:13 +0900 Subject: [PATCH 03/11] libata: kill non-sense warning message prereset() is now allowed to set flag for unsupported reset method. EH layer is responsible for selecting the fallback. Remove non-sense warning message. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index d8070989a39f..376f0044f134 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1665,8 +1665,6 @@ static int ata_eh_reset(struct ata_port *ap, int classify, /* did prereset() screw up? if so, fix up to avoid oopsing */ if (!reset) { - ata_port_printk(ap, KERN_ERR, "BUG: prereset() requested " - "invalid reset type\n"); if (softreset) reset = softreset; else From 37301a559d87494614fb843b96b7528532236f82 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 20:45:54 +0900 Subject: [PATCH 04/11] libata: be less verbose about hpa There's no reason to print out hpa related messages when HPA is not active. Kill the unconditional message and add a warning message which is printed if HPA size is smaller than the current size. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index adfae9d1ceb1..deda68446b43 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -983,11 +983,6 @@ static u64 ata_hpa_resize(struct ata_device *dev) else hpa_sectors = ata_read_native_max_address(dev); - /* if no hpa, both should be equal */ - ata_dev_printk(dev, KERN_INFO, "%s 1: sectors = %lld, " - "hpa_sectors = %lld\n", - __FUNCTION__, (long long)sectors, (long long)hpa_sectors); - if (hpa_sectors > sectors) { ata_dev_printk(dev, KERN_INFO, "Host Protected Area detected:\n" @@ -1009,7 +1004,11 @@ static u64 ata_hpa_resize(struct ata_device *dev) return hpa_sectors; } } - } + } else if (hpa_sectors < sectors) + ata_dev_printk(dev, KERN_WARNING, "%s 1: hpa sectors (%lld) " + "is smaller than sectors (%lld)\n", __FUNCTION__, + (long long)hpa_sectors, (long long)sectors); + return sectors; } From 8b5bb2fa3d1c2a90ca921b6bfbb7e2de1e6dd273 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 21:43:04 +0900 Subject: [PATCH 05/11] libata: remove unused variable from ata_eh_reset() Removed unused variable did_followup_srst from ata_eh_reset(). Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 376f0044f134..45f81add150e 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1616,7 +1616,7 @@ static int ata_eh_reset(struct ata_port *ap, int classify, unsigned long deadline; unsigned int action; ata_reset_fn_t reset; - int i, did_followup_srst, rc; + int i, rc; /* about to reset */ ata_eh_about_to_do(ap, NULL, ehc->i.action & ATA_EH_RESET_MASK); @@ -1687,11 +1687,9 @@ static int ata_eh_reset(struct ata_port *ap, int classify, rc = ata_do_reset(ap, reset, classes, deadline); - did_followup_srst = 0; if (reset == hardreset && ata_eh_followup_srst_needed(rc, classify, classes)) { /* okay, let's do follow-up softreset */ - did_followup_srst = 1; reset = softreset; if (!reset) { From 112cc2b510156494918abdf877111dfd56e5643b Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Mon, 25 Jun 2007 10:42:22 -0700 Subject: [PATCH 06/11] pata_it821x: fix section mismatch warning Fix section mismatch when CONFIG_HOTPLUG=n (but functions are used for resume): WARNING: drivers/ata/pata_it821x.o(.text+0x3f): Section mismatch: reference to .init.text: (between 'it821x_reinit_one' and 'it821x_program_udma') WARNING: drivers/ata/pata_it821x.o(.text+0x691): Section mismatch: reference to .init.text: (between 'it821x_init_one' and 'it821x_passthru_set_dmamode') Signed-off-by: Randy Dunlap Signed-off-by: Jeff Garzik --- drivers/ata/pata_it821x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/pata_it821x.c b/drivers/ata/pata_it821x.c index dab4e7cf8cda..12c6e08cc4d1 100644 --- a/drivers/ata/pata_it821x.c +++ b/drivers/ata/pata_it821x.c @@ -690,7 +690,7 @@ static struct ata_port_operations it821x_passthru_port_ops = { .port_start = it821x_port_start, }; -static void __devinit it821x_disable_raid(struct pci_dev *pdev) +static void it821x_disable_raid(struct pci_dev *pdev) { /* Reset local CPU, and set BIOS not ready */ pci_write_config_byte(pdev, 0x5E, 0x01); From 09d7f9b0658072485a93247e1b6e15e661f860d2 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 23:34:02 +0900 Subject: [PATCH 07/11] libata: fix ata_dev_disable() Fix silly condition check bug in ata_dev_disable(). Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index deda68446b43..642097a7d60d 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -600,8 +600,9 @@ static const char *sata_spd_string(unsigned int spd) void ata_dev_disable(struct ata_device *dev) { - if (ata_dev_enabled(dev) && ata_msg_drv(dev->ap)) { - ata_dev_printk(dev, KERN_WARNING, "disabled\n"); + if (ata_dev_enabled(dev)) { + if (ata_msg_drv(dev->ap)) + ata_dev_printk(dev, KERN_WARNING, "disabled\n"); ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 | ATA_DNXFER_QUIET); dev->class++; From 914616a3c2a54504f3b0eda0b67fcd32226b3e83 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 25 Jun 2007 21:47:11 +0900 Subject: [PATCH 08/11] libata: fix infinite EH waiting bug When EH gives up after repeated exceptions, it doesn't't clear the PENDING bit on exit which leaves PENDING bit set without EH actually scheduled. This makes ata_port_wait_eh() to wait forever makes rmmod hang on such port. Fix it by clearing the flag. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 45f81add150e..f7582c9c320e 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -336,6 +336,7 @@ void ata_scsi_error(struct Scsi_Host *host) } ata_port_printk(ap, KERN_ERR, "EH pending after %d " "tries, giving up\n", ATA_EH_MAX_REPEAT); + ap->pflags &= ~ATA_PFLAG_EH_PENDING; } /* this run is complete, make sure EH info is clear */ From e00f1ff3c8977eff07d0214d2f3478ac947bda0f Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 27 Jun 2007 02:47:35 +0900 Subject: [PATCH 09/11] libata: call ata_check_atapi_dma() with qc better prepared In atapi_xlat(), prepare qc better before calling ata_check_atapi_dma() such that ata_check_atapi_dma() can use info from qc. While at it, reformat weird looking if/else block in the function. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-scsi.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index c228df298bd8..4ddf00c8c5f5 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2384,11 +2384,6 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc) int using_pio = (dev->flags & ATA_DFLAG_PIO); int nodata = (scmd->sc_data_direction == DMA_NONE); - if (!using_pio) - /* Check whether ATAPI DMA is safe */ - if (ata_check_atapi_dma(qc)) - using_pio = 1; - memset(qc->cdb, 0, dev->cdb_len); memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len); @@ -2401,19 +2396,22 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc) } qc->tf.command = ATA_CMD_PACKET; + qc->nbytes = scmd->request_bufflen; + + /* check whether ATAPI DMA is safe */ + if (!using_pio && ata_check_atapi_dma(qc)) + using_pio = 1; - /* no data, or PIO data xfer */ if (using_pio || nodata) { + /* no data, or PIO data xfer */ if (nodata) qc->tf.protocol = ATA_PROT_ATAPI_NODATA; else qc->tf.protocol = ATA_PROT_ATAPI; qc->tf.lbam = (8 * 1024) & 0xff; qc->tf.lbah = (8 * 1024) >> 8; - } - - /* DMA data xfer */ - else { + } else { + /* DMA data xfer */ qc->tf.protocol = ATA_PROT_ATAPI_DMA; qc->tf.feature |= ATAPI_PKT_DMA; @@ -2422,8 +2420,6 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc) qc->tf.feature |= ATAPI_DMADIR; } - qc->nbytes = scmd->request_bufflen; - return 0; } From b9a4197e266a40d5d1d16c9fb2a852cf10743afe Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 27 Jun 2007 02:48:43 +0900 Subject: [PATCH 10/11] libata: use PIO for non-16 byte aligned ATAPI commands The IDE driver used DMA for ATAPI commands if READ/WRITE command is multiple of sector size or sg command is multiple of 16 bytes. For libata, READ/WRITE sector alignment is guaranteed by the high level driver (sr), so we only have to worry about the 16 byte alignment. This patch makes ata_check_atapi_dma() always request PIO for all data transfer commands which are not multiple of 16 bytes. The following reports are related to this problem. http://bugzilla.kernel.org/show_bug.cgi?id=8605 (confirmed) http://thread.gmane.org/gmane.linux.kernel/476620 (confirmed) https://bugzilla.novell.com/show_bug.cgi?id=229260 (probably) Albert first pointed out the difference between IDE and libata. Kudos to him. Signed-off-by: Tejun Heo Cc: Albert Lee Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 642097a7d60d..094b51891dbf 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4109,6 +4109,7 @@ static void ata_fill_sg(struct ata_queued_cmd *qc) if (idx) ap->prd[idx - 1].flags_len |= cpu_to_le32(ATA_PRD_EOT); } + /** * ata_check_atapi_dma - Check whether ATAPI DMA can be supported * @qc: Metadata associated with taskfile to check @@ -4126,33 +4127,19 @@ static void ata_fill_sg(struct ata_queued_cmd *qc) int ata_check_atapi_dma(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; - int rc = 0; /* Assume ATAPI DMA is OK by default */ - /* some drives can only do ATAPI DMA on read/write */ - if (unlikely(qc->dev->horkage & ATA_HORKAGE_DMA_RW_ONLY)) { - struct scsi_cmnd *cmd = qc->scsicmd; - u8 *scsicmd = cmd->cmnd; - - switch (scsicmd[0]) { - case READ_10: - case WRITE_10: - case READ_12: - case WRITE_12: - case READ_6: - case WRITE_6: - /* atapi dma maybe ok */ - break; - default: - /* turn off atapi dma */ - return 1; - } - } + /* Don't allow DMA if it isn't multiple of 16 bytes. Quite a + * few ATAPI devices choke on such DMA requests. + */ + if (unlikely(qc->nbytes & 15)) + return 1; if (ap->ops->check_atapi_dma) - rc = ap->ops->check_atapi_dma(qc); + return ap->ops->check_atapi_dma(qc); - return rc; + return 0; } + /** * ata_qc_prep - Prepare taskfile for submission * @qc: Metadata associated with taskfile to be prepared From 40a1d531f6c894b298e784fd2090d87633e4989a Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 27 Jun 2007 02:49:38 +0900 Subject: [PATCH 11/11] libata: kill ATA_HORKAGE_DMA_RW_ONLY ATA_HORKAGE_DMA_RW_ONLY for TORiSAN is verified to be subset of using DMA for ATAPI commands which aren't aligned to 16 bytes. As libata now doesn't use DMA for unaligned ATAPI commands, the horkage is redundant. Kill it. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 7 +------ include/linux/libata.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 094b51891dbf..bfc59a104728 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2046,10 +2046,6 @@ int ata_dev_configure(struct ata_device *dev) dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128, dev->max_sectors); - /* limit ATAPI DMA to R/W commands only */ - if (ata_device_blacklisted(dev) & ATA_HORKAGE_DMA_RW_ONLY) - dev->horkage |= ATA_HORKAGE_DMA_RW_ONLY; - if (ap->ops->dev_config) ap->ops->dev_config(dev); @@ -3780,8 +3776,7 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = { { "IOMEGA ZIP 250 ATAPI", NULL, ATA_HORKAGE_NODMA }, /* temporary fix */ /* Weird ATAPI devices */ - { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_128 | - ATA_HORKAGE_DMA_RW_ONLY }, + { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_128 }, /* Devices we expect to fail diagnostics */ diff --git a/include/linux/libata.h b/include/linux/libata.h index a3380f808630..620da7be07b7 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -298,7 +298,6 @@ enum { ATA_HORKAGE_NODMA = (1 << 1), /* DMA problems */ ATA_HORKAGE_NONCQ = (1 << 2), /* Don't use NCQ */ ATA_HORKAGE_MAX_SEC_128 = (1 << 3), /* Limit max sects to 128 */ - ATA_HORKAGE_DMA_RW_ONLY = (1 << 4), /* ATAPI DMA for RW only */ }; enum hsm_task_states {