Re: libata Intel PIIX/ICH fails to detect both PATA drives, spews ACPI errors
Tejun Heo wrote: Berck E. Nash wrote: Testing the new libata ICH PATA drivers. There's one PATA port on this chip, and I've got two optical drives connected to it. The master drive fails to detect. The slave detects and works properly. Can you test 2.6.20.1 and post full dmesg? Here's 2.6.20.2... No ACPI errors, but still doesn't detect both drives. Please apply the attached patch and see if it works. If it works, please post the result of hdparm -I /dev/srX of the optical drive. Thanks. diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index dc42ba1..6e7775a 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -105,7 +105,8 @@ enum { PIIX_FLAG_AHCI = (1 27), /* AHCI possible */ PIIX_FLAG_CHECKINTR = (1 28), /* make sure PCI INTx enabled */ - PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS, + PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS | + ATA_FLAG_SETXFER_POLLING, PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR, /* combined mode. if set, PATA is channel 0. Since this patch fixes the problem, is there some reason it still hasn't been included? Berck - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: Handle drives that require a spin-up command before first access
Mark Lord wrote: (S)ATA drives can be configured for power-up in standby, a mode whereby a specific spin up now! command is required before the first media access. Currently, a drive with this feature enabled can not be used at all with libata, and once in this mode, the drive becomes a doorstop. The older drivers/ide subsystem at least enumerates the drive, so that it can be woken up after the fact from a userspace HDIO_* command, but not libata. This patch adds support to libata for the power-up in standby mode where a spin up now! command (SET_FEATURES) is needed. With this, libata will recognize such drives, spin them up, and then re-IDENTIFY them if necessary to get a full/complete set of drive features data. Drives in this state are determined by looking for special values in id[2], as documented in the current ATA specs. Signed-off-by: Mark Lord [EMAIL PROTECTED] applied though I'm curious where the verify step went, that was in the previous rev of the patch - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pata_amd: remove contamination added during cable_detect conversion
Tejun Heo wrote: This is added by added by cff63dfceb52c564fe1ba5394d50ab7d599a11b9 - pata: cable methods. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Alan Cox [EMAIL PROTECTED] --- drivers/ata/pata_amd.c |5 - 1 files changed, 0 insertions(+), 5 deletions(-) applied - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.21-rc7] sata_promise: SATAII-150/300 TX4 port numbering fix
Mikael Pettersson wrote: There is a known problem with sata_promise on SATAII-150/300 TX4 controller cards: it enumerates drives in an order that differs from the port numbers printed on the controller cards. However, Promise's BIOS and Linux driver both get the order right. I investigated Promise's Linux driver (v1.01.0.23), and found that it explicitly changes the mapping from logical port number to ATA engine MMIO address on the SATAII TX4 cards. It does this on all SATAII TX4 cards, without inspecting revision etc. The SATAII TX2plus cards continue to use the same mapping that was used for the first-generation chips. This patch updates sata_promise to use the new port number to ATA engine mapping on SATAII TX4 cards, which fixes the drive enumeration order problem on those cards. Tested on 300 TX4, 300 TX2plus, and SATAII-150 TX2plus chips. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] --- This patch should apply to 2.6.21-rc7 and libata#upstream. It won't apply to libata#ALL because of the massive changes for the new init model. I will do a new and cleaner patch for #ALL once I can get it as a patch in -mm (I don't do git). ACK; dropped, awaiting rediff and resend against new init model FWIW: The new init model was applied to #upstream, and subsequently merged into #ALL (which always is a superset of #upstream). So from your description, it sounds like your #upstream may not have been updated. FWIW2: When I note that libata-dev.git has been rebased, this means that you will need to delete the #upstream branch on your side, and download it again. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ata_timing: ensure t-cycle is always correct
Alan Cox wrote: Russell King hit a case where quantisation errors accumulated such that the cycle time was shorter than rather than equal to the active/recovery time. The code already knows how to stretch times to fit the cycle time but does not know about the reverse. Signed-off-by: Alan Cox [EMAIL PROTECTED] applied - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] ahci: consolidate common port flags
Tejun Heo wrote: Consolidate common port flags into AHCI_FLAG_COMMON. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/ahci.c | 29 ++--- 1 files changed, 10 insertions(+), 19 deletions(-) applied patches 1-2 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata-acpi: fix _GTF command protocol for ATAPI devices
Tejun Heo wrote: _GTF command is never ATA_PROT_ATAPI_NODATA whether the device is ATAPI or not. It's always ATA_PROT_NODATA. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-acpi.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 03a0acf..cb3eab6 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -489,8 +489,7 @@ static void taskfile_load_raw(struct ata_port *ap, /* convert gtf to tf */ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; /* TBD */ - tf.protocol = atadev-class == ATA_DEV_ATAPI ? - ATA_PROT_ATAPI_NODATA : ATA_PROT_NODATA; + tf.protocol = ATA_PROT_NODATA; Elaboration? ATA_PROT_ATAPI_NODATA is the ATAPI version of the non-data protocol, so this change is unexpected. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] libata: separate out ata_dev_reread_id()
Tejun Heo wrote: Separate out ata_dev_reread_id() from ata_dev_revalidate(). ata_dev_reread_id() reads IDENTIFY page and determines whether the same device is still there. ata_dev_revalidate() reconfigures after reread completes. This will be used by ACPI update. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-core.c | 47 drivers/ata/libata.h |3 +- 2 files changed, 36 insertions(+), 14 deletions(-) ACK, though I think you'll have to rediff due to a Mark Lord patch - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] libata-acpi: s/CONFIG_SATA_ACPI/CONFIG_ATA_ACPI/
Tejun Heo wrote: ACPI applies to both SATA and PATA. Drop the 'S' from the config variable. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/Kconfig| 26 +- drivers/ata/Makefile |2 +- drivers/ata/libata.h |2 +- include/linux/libata.h |2 +- 4 files changed, 16 insertions(+), 16 deletions(-) ACK patches 4-5 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
Tejun Heo wrote: This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes. * make libata-acpi functions take ata_device instead of ata_port + device index * s/atadev/adev/ * de-indent local variable declarations I prefer 'dev' over 'adev', unless that makes the code in question more ambiguous. Otherwise, ACK - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] libata-acpi: clean up ata_acpi_exec_tfs()
Tejun Heo wrote: This patch cleans up ata_acpi_exec_tfs() and its friends. * Rename taskfile_array to ata_acpi_gtf and make it __packed as it's used as argument to ACPI method, and use pointer to ata_acpi_gtf and number of taskfiles to represent _GTF taskfiles instead of a pointer casted into unsigned long and byte count. This makes argument re-checking in do_drive_set_taskfiles() unnecessary. * Pointer in void * not in unsigned long. * Clean up do_drive_get_GTF() error handling and make do_drive_get_GTF() return number of taskfiles on success, 0 if _GTF doesn't exist or doesn't contain valid ata. -errno on other errors. * Remove superflous check for acpi-buffer.pointer. * Update taskfile_load_raw() such that printed messages look similar to the messages printed by ata_eh_report(). Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-acpi.c | 219 ++--- 1 files changed, 107 insertions(+), 112 deletions(-) ACK As an aside, I hate the do_ prefix on functions. It is utterly redundant. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/13] libata-acpi: miscellaneous cleanups
Tejun Heo wrote: * Add missing LOCKING: and RETURNS: to function comment. * Don't conditionalize warning messages with ata_msg_probe(). Print directly with KERN_WARNING. * Drop duplicate debug messages. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-acpi.c | 51 1 files changed, 23 insertions(+), 28 deletions(-) ACK - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 02/10] sata_nv: add back some verbosity into ADMA error_handler
[EMAIL PROTECTED] wrote: From: Robert Hancock [EMAIL PROTECTED] Some debug output in the ADMA error_handler function was removed recently, but it may be useful in certain cases, like NCQ commands timing out. Add it back in, but make it a bit more intelligent so that it only prints if command(s) are active and only prints the CPBs for those commands. That way it won't spew at inappropriate times like suspend/resume. [EMAIL PROTECTED]: cleanup] Signed-off-by: Robert Hancock [EMAIL PROTECTED] Cc: Jeff Garzik [EMAIL PROTECTED] Cc: Tejun Heo [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- drivers/ata/sata_nv.c | 31 +-- 1 file changed, 29 insertions(+), 2 deletions(-) Robert, is this patch still wanted? It looks OK; I've held because I thought it was created to debug a special situation. But I might be mistaken... Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 07/10] pata_hpt3x2n: Add HPT371N support and other bits
From: Alan Cox [EMAIL PROTECTED] Yes its no longer 3x2n but 3xxn, I can rename it if you want Jeff Choose whatever you feel is the best long term name; I have no preference here. If you do rename, please follow this slightly special procedure: * add a MODULE_ALIAS() for the old name * send me a patch with everything BUT the file move. I will do the rename in git. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 01/10] ata: printk warning fixes
[EMAIL PROTECTED] wrote: From: Andrew Morton [EMAIL PROTECTED] drivers/ata/libata-core.c: In function 'ata_hpa_resize': drivers/ata/libata-core.c:986: warning: format '%lld' expects type 'long long int', but argument 5 has type 'u64' drivers/ata/libata-core.c:986: warning: format '%lld' expects type 'long long int', but argument 6 has type 'u64' drivers/ata/libata-core.c:990: warning: format '%lld' expects type 'long long int', but argument 4 has type 'u64' drivers/ata/libata-core.c:990: warning: format '%lld' expects type 'long long int', but argument 5 has type 'u64' drivers/ata/libata-core.c:1003: warning: format '%lld' expects type 'long long int', but argument 4 has type 'u64' Also fix various 80-col bustage. Cc: Jeff Garzik [EMAIL PROTECTED] Cc: Tejun Heo [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] applied - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 06/10] pata_hpt37x: Further small fixes
[EMAIL PROTECTED] wrote: From: Alan Cox [EMAIL PROTECTED] Further HPT37x changes - No 66MHz 370/370A - Remove dead special case check now we use the DPLL (as per the IDE driver) Pointed out by Sergei Signed-off-by: Alan Cox [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- drivers/ata/pata_hpt37x.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) applied 6-7 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 08/10] drivers/ata/pata_cmd640.c: fix build with CONFIG_PM=n
[EMAIL PROTECTED] wrote: From: Andrew Morton [EMAIL PROTECTED] This is grubby, but all the ata drivers do it this way. Would it not be better to do #define ata_scsi_device_resume NULL in libata.h, remove all those ifdefs? (updated version, ug, ug) Cc: Jeff Garzik [EMAIL PROTECTED] Cc: Alan Cox [EMAIL PROTECTED] Cc: Tejun Heo [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- drivers/ata/pata_cmd640.c |8 1 file changed, 8 insertions(+) applied 8-9 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] bidi support: bidirectional request
From: Boaz Harrosh [EMAIL PROTECTED] Subject: [PATCH 4/4] bidi support: bidirectional request Date: Sun, 15 Apr 2007 20:33:28 +0300 diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 645d24b..16a02ee 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -322,6 +322,7 @@ struct request { void *end_io_data; struct request_io_part uni; +struct request_io_part bidi_read; }; Would be more straightforward to have: struct request_io_part in; struct request_io_part out; /* @@ -600,6 +601,34 @@ static inline struct request_io_part* rq_uni(struct request* req) return req-uni; } +static inline struct request_io_part* rq_out(struct request* req) +{ +WARN_ON_BIDI_FLAG(req); +return req-uni; +} + +static inline struct request_io_part* rq_in(struct request* req) +{ +WARN_ON_BIDI_FLAG(req); +if (likely(rq_dma_dir(req) != DMA_BIDIRECTIONAL)) +return req-uni; + +if (likely(req-cmd_flags REQ_BIDI)) +return req-bidi_read; + +return req-uni; +} + +static inline struct request_io_part* rq_io(struct request* req, +enum dma_data_direction dir) +{ +if (dir == DMA_FROM_DEVICE) +return rq_in(req); + +WARN_ON( (dir != DMA_TO_DEVICE) (dir != DMA_NONE) ); +return req-uni; +} static inline struct request_io_part* rq_io(struct request* req) { return (req is WRITE) ? req-out : req-in; } - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: Handle drives that require a spin-up command before first access
Jeff Garzik wrote: Mark Lord wrote: This patch adds support to libata for the power-up in standby mode where a spin up now! command (SET_FEATURES) is needed. With this, libata will recognize such drives, spin them up, and then re-IDENTIFY them if necessary to get a full/complete set of drive features data. Drives in this state are determined by looking for special values in id[2], as documented in the current ATA specs. Signed-off-by: Mark Lord [EMAIL PROTECTED] applied though I'm curious where the verify step went, that was in the previous rev of the patch Hi Jeff, The difference between this patch and the first RFC version, is that I've dropped the code that attempted to deal with the *other* kind of power-up-in-standby that some drives use. That form is enabled with a Jumper setting on the drive (eg. WD 37GB Raptor drives) rather than through software, and I was unable to figure out exactly what they wanted while I still had them here to experiment on. Those drives are no longer in my possession, so.. no support. The majority of drives use the software enable/disable form, which is (still) fully addressed by the patch you applied. Thanks! -ml - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
libata fails to recover from HSM violation involving DRQ status
Tejun, While working on the new hdparm (version 7.0, released today), I ran into trouble when a buggy SG_IO/ATA_16 packet caused the libata EH to get confused. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. In the IDE driver, we had code to try and cope with stuck DRQ, by just looping and reading from the data port a few times. That could have been done better, but it worked a lot of the time, back in those simpler days. I don't know what you try in libata-eh, but perhaps it can be tweaked? Below is the 'dmesg' from that system before I hit the big red button. I can also supply a program that will generate this lockup on demand, for testing purposes. Cheers Mark sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ata1.00: qc timeout (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata1.00: revalidation failed (errno=-5) ata1: failed to recover some devices, retrying in 5 secs ata1: port is slow to respond, please be patient (Status 0xd8) ata1: port failed to respond (30 secs, Status 0xd8) ata1: soft resetting port ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ata1.00: qc timeout (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata1.00: revalidation failed (errno=-5) ata1.00: limiting speed to UDMA/100:PIO3 ata1: failed to recover some devices, retrying in 5 secs ata1: port is slow to respond, please be patient (Status 0xd8) ata1: port failed to respond (30 secs, Status 0xd8) ata1: soft resetting port ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ATA: abnormal status 0xD8 on port 0x000101f7 ata1.00: qc timeout (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata1.00: revalidation failed (errno=-5) ata1.00: disabled ata1: EH complete sd 0:0:0:0: SCSI error: return code = 0x0004 end_request: I/O error, dev sda, sector 120322555 Buffer I/O error on device sda1, logical block 15018230 lost page write due to I/O error on sda1 sd 0:0:0:0: SCSI error: return code = 0x0004 end_request: I/O error, dev sda, sector 127883035 Buffer I/O error on device sda1, logical block 15963290 lost page write due to I/O error on sda1 sd 0:0:0:0: SCSI error: return code = 0x0004 end_request: I/O error, dev sda, sector 31060987 Buffer I/O error on device sda1, logical block 3860534 lost page write due to I/O error on sda1 sd 0:0:0:0: SCSI error: return code = 0x0004 end_request: I/O error, dev sda, sector 31060995 Buffer I/O error on device sda1, logical block 3860535 lost page write due to I/O error on sda1 Buffer I/O error on device sda1, logical block 3860536 lost page write due to I/O error on sda1 Buffer I/O error on device sda1, logical block 3860537 lost page write due to I/O error on sda1 sd 0:0:0:0: SCSI error: return code = 0x0004 end_request: I/O error, dev sda, sector 31780267 Buffer I/O error on device sda1, logical block 3950444 lost page write due to I/O error on sda1 Buffer I/O error on device sda1, logical block 3950445 lost page write due to I/O error on sda1 sd 0:0:0:0: SCSI error: return code = 0x0004
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: Tejun, While working on the new hdparm (version 7.0, released today), I ran into trouble when a buggy SG_IO/ATA_16 packet caused the libata EH to get confused. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. ... This was on 2.6.21, with ata_piix. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
In the IDE driver, we had code to try and cope with stuck DRQ, by just looping and reading from the data port a few times. That could have been done better, but it worked a lot of the time, back in those simpler days. It works very well. The current old IDE has some changes in the area but those are basically to handle one or two controllers whose internal state machine flushes the data queue so we don't hang the box solid trying to flush it ourselves. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: Tejun, While working on the new hdparm (version 7.0, released today), I ran into trouble when a buggy SG_IO/ATA_16 packet caused the libata EH to get confused. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. In the IDE driver, we had code to try and cope with stuck DRQ, by just looping and reading from the data port a few times. That could have been done better, but it worked a lot of the time, back in those simpler days. I don't know what you try in libata-eh, but perhaps it can be tweaked? Below is the 'dmesg' from that system before I hit the big red button. I am reluctant to do anything about this. All manner of things can go wrong, if the taskfile protocol specified disagrees with the taskfile contents. At that point you are in undefined territory, since libata will happily ARM a DMA controller or otherwise program controller registers in preparation for the requested taskfile protocol. Data corruption, hard locks, anything could happen at that point. Maybe we do need to recover from a stuck DRQ bit, but I'll wait until that symptom shows up with a different catalyst. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Jeff Garzik wrote: Mark Lord wrote: .. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. .. Maybe we do need to recover from a stuck DRQ bit, but I'll wait until that symptom shows up with a different catalyst. It's a failure mode that occurs very often (as far as failures go) with the IDE driver. *Lots* of occurance. So as more things migrate to libata, we'll eventually have to deal with it here, too. I'm just trying to give us a chance to fix it before somebody loses data over it. Actually, I'm not so sure that this problem hasn't *already* been posted to this very mailing list. http://lkml.org/lkml/2006/10/1/264 http://www.mail-archive.com/linux-ide@vger.kernel.org/msg05078.html ... Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: Actually, I'm not so sure that this problem hasn't *already* been posted to this very mailing list. http://lkml.org/lkml/2006/10/1/264 http://www.mail-archive.com/linux-ide@vger.kernel.org/msg05078.html ... What Tejun said at the end of that thread :) That one is a phy-level problem, when it starts complaining about 10b-to-8b decode and non-recoverable communication errors. I'm keeping an open mind, but with the drivers being different from each other, I want to see how libata encounters that failure mode in the field. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
I am reluctant to do anything about this. This one does need dealing with. It happens in the real world and the old IDE paths for this do get triggered and used now and then (we know this because bugs in them were found). All it takes is a device and a controller disagreeing about the length of a data transfer to get in a mess. In theory resetting the bus should get you out of this, I'm suprised we didn't get out that way. All manner of things can go wrong, if the taskfile protocol specified disagrees with the taskfile contents. True but at the point you are trying to do error recovery and DRQ is wedged on its a good idea to pull remaining data out of the fifo. At that point you are in undefined territory, since libata will happily ARM a DMA controller or otherwise program controller registers in preparation for the requested taskfile protocol. Data corruption, hard locks, anything could happen at that point. SG_IO and other userspace interfaces can mean we issue a command that ends up causing variants of this kind of confusion. Alan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Alan Cox wrote: I am reluctant to do anything about this. This one does need dealing with. It happens in the real world and the old IDE paths for this do get triggered and used now and then (we know this because bugs in them were found). All it takes is a device and a controller disagreeing about the length of a data transfer to get in a mess. In theory resetting the bus should get you out of this, I'm suprised we didn't get out that way. .. SG_IO and other userspace interfaces can mean we issue a command that ends up causing variants of this kind of confusion. That last one doesn't really worry me -- it has to be deliberately done by the sysadmin. But the history of real-world cases are definitely of concern, especially since it's quite likely a rather simple fix. I think failed WRITE_DMA requests (IDNF or ECC faults) were one source. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Alan Cox wrote: I am reluctant to do anything about this. This one does need dealing with. It happens in the real world and the old IDE paths for this do get triggered and used now and then (we know this because bugs in them were found). All it takes is a device and a controller disagreeing about the length of a data transfer to get in a How would they disagree (excluding human error)? mess. In theory resetting the bus should get you out of this, I'm suprised we didn't get out that way. Indeed. All manner of things can go wrong, if the taskfile protocol specified disagrees with the taskfile contents. True but at the point you are trying to do error recovery and DRQ is wedged on its a good idea to pull remaining data out of the fifo. It's not really a good idea for SATA. The FIFO often co-emulated by the SATA controller and SATA phy. You just want to kick SATA really hard (i.e. bus reset and friends). Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Jeff Garzik wrote: It's not really a good idea for SATA. The FIFO often co-emulated by the SATA controller and SATA phy. You just want to kick SATA really hard (i.e. bus reset and friends). Sure. So why don't we do that now? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH: libata: add support for ATA_16 on ATAPI
resending.. again.. with a Subject line this time :) This patch adds support for issuing ATA_16 passthru commands to ATAPI devices managed by libata. It requires the previous CDB length fix patch. A boot/module parameter, ata16_passthru=1 can be used to globally disable this feature, if ever desired. Signed-Off-By: Mark Lord [EMAIL PROTECTED] --- diff -u --recursive --new-file --exclude-from=old/Documentation/dontdiff old/drivers/ata/libata-core.c new/drivers/ata/libata-core.c --- old/drivers/ata/libata-core.c 2007-02-02 12:29:10.0 -0500 +++ new/drivers/ata/libata-core.c 2007-02-02 12:29:03.0 -0500 @@ -82,6 +82,10 @@ module_param(atapi_dmadir, int, 0444); MODULE_PARM_DESC(atapi_dmadir, Enable ATAPI DMADIR bridge support (0=off, 1=on)); +int ata16_passthru = 0; +module_param(ata16_passthru, int, 0444); +MODULE_PARM_DESC(ata16_passthru, Enable passthru of SCSI opcode 0x85 to ATAPI devices (0=off, 1=on)); + int libata_fua = 0; module_param_named(fua, libata_fua, int, 0444); MODULE_PARM_DESC(fua, FUA support (0=off, 1=on)); diff -u --recursive --new-file --exclude-from=old/Documentation/dontdiff old/drivers/ata/libata-scsi.c new/drivers/ata/libata-scsi.c --- old/drivers/ata/libata-scsi.c 2007-02-02 12:29:10.0 -0500 +++ new/drivers/ata/libata-scsi.c 2007-02-02 12:29:25.0 -0500 @@ -2688,6 +2688,10 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd) { + if (dev-class == ATA_DEV_ATAPI) + if (cmd != ATA_16 || ata16_passthru) + return atapi_xlat; + switch (cmd) { case READ_6: case READ_10: @@ -2746,27 +2750,28 @@ void (*done)(struct scsi_cmnd *), struct ata_device *dev) { - int rc = 0; - - if (unlikely(!scmd-cmd_len || scmd-cmd_len dev-cdb_len)) { - DPRINTK(bad CDB len=%u, max=%u\n, - scmd-cmd_len, dev-cdb_len); + ata_xlat_func_t xlat_func; + int rc = 0, max_len; + u8 scsi_op = scmd-cmnd[0]; + + if (scsi_op == ATA_16 dev-class == ATA_DEV_ATAPI !ata16_passthru) + max_len = 16; + else + max_len = dev-cdb_len; + + if (unlikely(!scmd-cmd_len || scmd-cmd_len max_len)) { + DPRINTK(bad CDB len=%u, max=%u\n, + scmd-cmd_len, max_len); scmd-result = DID_ERROR 16; done(scmd); return 0; } - if (dev-class == ATA_DEV_ATA) { - ata_xlat_func_t xlat_func = ata_get_xlat_func(dev, - scmd-cmnd[0]); - - if (xlat_func) - rc = ata_scsi_translate(dev, scmd, done, xlat_func); - else - ata_scsi_simulate(dev, scmd, done); - } else - rc = ata_scsi_translate(dev, scmd, done, atapi_xlat); - + xlat_func = ata_get_xlat_func(dev, scsi_op); + if (xlat_func) + rc = ata_scsi_translate(dev, scmd, done, xlat_func); + else + ata_scsi_simulate(dev, scmd, done); return rc; } diff -u --recursive --new-file --exclude-from=old/Documentation/dontdiff old/drivers/ata/libata.h new/drivers/ata/libata.h --- old/drivers/ata/libata.h2007-02-02 12:26:28.0 -0500 +++ new/drivers/ata/libata.h2007-02-02 12:29:03.0 -0500 @@ -47,6 +47,7 @@ extern struct workqueue_struct *ata_aux_wq; extern int atapi_enabled; extern int atapi_dmadir; +extern int ata16_passthru; extern int libata_fua; extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev); extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: .. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. ... sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete ... (over and over) Say.. is this problem as simple as excessive retries for an SG_IO command? There shouldn't really be *any* retries here, and it should eventually just fail the command rather than shut down the port. Or am I just reading the logs wrong? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
This one does need dealing with. It happens in the real world and the old IDE paths for this do get triggered and used now and then (we know this because bugs in them were found). All it takes is a device and a controller disagreeing about the length of a data transfer to get in a How would they disagree (excluding human error)? Human error with SG_IO is quite sufficient, or controller bugs. It happens: we see it happen and stuff gets stuck that way sometimes when you get a timeout. For some controllers a failure gets stuck because of the FIFO magic they do. It's not really a good idea for SATA. The FIFO often co-emulated by the SATA controller and SATA phy. You just want to kick SATA really hard (i.e. bus reset and friends). Possibly we need a per controller -drain_fifo method if available. It's precisely because the FIFO is magic that some of the PATA controllers get stuck in a mess (eg they hold off IRQ until the FIFO is drained) Alan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.21-rc7-mm2 1/2] sata_promise: SATAII-150/300 TX4 port numbering fix
There is a known problem with sata_promise on SATAII-150/300 TX4 controller cards: it enumerates drives in an order that differs from the port numbers printed on the controller cards. However, Promise's BIOS and Linux driver both get the order right. I investigated Promise's Linux driver (v1.01.0.23), and found that it explicitly changes the mapping from logical port number to ATA engine MMIO address on the SATAII TX4 cards. It does this on all SATAII TX4 cards, without inspecting revision etc. The SATAII TX2plus cards continue to use the same mapping that was used for the first-generation chips. This patch updates sata_promise to use the new port number to ATA engine mapping on SATAII TX4 cards, which fixes the drive enumeration order problem on those cards. Tested on SATA300 TX4, SATA300 TX2plus, SATAII-150 TX2plus, and TX4000. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] --- Patch updated to apply to current libata#upstream/#ALL. Testing updated to include TX4000. drivers/ata/sata_promise.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) --- linux-2.6.21-rc7-mm2/drivers/ata/sata_promise.c.~1~ 2007-04-28 23:16:57.0 +0200 +++ linux-2.6.21-rc7-mm2/drivers/ata/sata_promise.c 2007-04-29 01:44:42.0 +0200 @@ -45,7 +45,7 @@ #include sata_promise.h #define DRV_NAME sata_promise -#define DRV_VERSION2.05 +#define DRV_VERSION2.06 enum { @@ -924,6 +924,7 @@ struct ata_host *host; void __iomem *base; int n_ports, i, rc; + int is_sataii_tx4; if (!printed_version++) dev_printk(KERN_DEBUG, pdev-dev, version DRV_VERSION \n); @@ -962,10 +963,23 @@ } host-iomap = pcim_iomap_table(pdev); - for (i = 0; i host-n_ports; i++) + is_sataii_tx4 = 0; + if ((pi-flags (PDC_FLAG_GEN_II|PDC_FLAG_4_PORTS)) == (PDC_FLAG_GEN_II|PDC_FLAG_4_PORTS)) { + is_sataii_tx4 = 1; + dev_printk(KERN_INFO, pdev-dev, applying SATAII TX4 port numbering workaround\n); + } + for (i = 0; i host-n_ports; i++) { + static const unsigned char sataii_tx4_port_remap[4] = { 3, 1, 0, 2}; + int ata_nr; + + ata_nr = i; + if (is_sataii_tx4) + ata_nr = sataii_tx4_port_remap[i]; + pdc_ata_setup_port(host-ports[i], - base + 0x200 + i * 0x80, - base + 0x400 + i * 0x100); + base + 0x200 + ata_nr * 0x80, + base + 0x400 + ata_nr * 0x100); + } /* initialize adapter */ pdc_host_init(host); - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.21-rc7-mm2 2/2] sata_promise: move port reset from error intr to EH prereset
When sata_promise detects an error, it resets the port and triggers EH. This is how it has always done things. New-style EH however appears to prefer (based on ahci and sata_sil24) error detection to just freeze the port or otherwise render it harmless, and for the reset to be delayed until EH runs. This patch updates sata_promise EH according to this principle. When an error is detected in pdc_error_intr, the port is frozen but not reset. Later in the error handler, the prereset procedure performs the necessary Promise-specific reset of the port. Notes: - Since pdc_error_intr no longer resets the port, ata_do_eh() can do its error analysis on the current register contents, so pdc_error_intr doesn't need to copy SCR_ERROR to ehi-serror. - The reset is done in prereset since that is the one point that (a) comes after libata's error analysis, and (b) dominates (in a control-flow sense) all subsequent reset parts of EH. Tested on SATA300 TX4, SATA300 TX2plus, SATAII-150 TX2plus, SATA150 TX2plus, and TX4000. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] --- drivers/ata/sata_promise.c | 19 ++- 1 files changed, 10 insertions(+), 9 deletions(-) --- linux-2.6.21-rc7-mm2/drivers/ata/sata_promise.c.~1~ 2007-04-28 23:16:57.0 +0200 +++ linux-2.6.21-rc7-mm2/drivers/ata/sata_promise.c 2007-04-28 23:55:49.0 +0200 @@ -45,7 +45,7 @@ #include sata_promise.h #define DRV_NAME sata_promise -#define DRV_VERSION2.06 +#define DRV_VERSION2.07 enum { @@ -598,13 +598,16 @@ static void pdc_thaw(struct ata_port *ap readl(mmio + PDC_CTLSTAT); /* flush */ } -static void pdc_common_error_handler(struct ata_port *ap, ata_reset_fn_t hardreset) +static int pdc_prereset(struct ata_port *ap, unsigned long deadline) { - if (!(ap-pflags ATA_PFLAG_FROZEN)) - pdc_reset_port(ap); + pdc_reset_port(ap); + return ata_std_prereset(ap, deadline); +} +static void pdc_common_error_handler(struct ata_port *ap, ata_reset_fn_t hardreset) +{ /* perform recovery */ - ata_do_eh(ap, ata_std_prereset, ata_std_softreset, hardreset, + ata_do_eh(ap, pdc_prereset, ata_std_softreset, hardreset, ata_std_postreset); } @@ -647,12 +650,10 @@ static void pdc_error_intr(struct ata_po | PDC_PCI_SYS_ERR | PDC1_PCI_PARITY_ERR)) ac_err_mask |= AC_ERR_HOST_BUS; - if (sata_scr_valid(ap)) - ehi-serror |= pdc_sata_scr_read(ap, SCR_ERROR); - + ehi-action |= ATA_EH_SOFTRESET; qc-err_mask |= ac_err_mask; - pdc_reset_port(ap); + ata_port_freeze(ap); } static inline unsigned int pdc_host_intr( struct ata_port *ap, - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata-acpi: fix _GTF command protocol for ATAPI devices
Jeff Garzik wrote: Tejun Heo wrote: _GTF command is never ATA_PROT_ATAPI_NODATA whether the device is ATAPI or not. It's always ATA_PROT_NODATA. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-acpi.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 03a0acf..cb3eab6 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -489,8 +489,7 @@ static void taskfile_load_raw(struct ata_port *ap, /* convert gtf to tf */ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; /* TBD */ -tf.protocol = atadev-class == ATA_DEV_ATAPI ? -ATA_PROT_ATAPI_NODATA : ATA_PROT_NODATA; +tf.protocol = ATA_PROT_NODATA; Elaboration? ATA_PROT_ATAPI_NODATA is the ATAPI version of the non-data protocol, so this change is unexpected. ATA_PROT_ATAPI_NODATA is used for PACKET command without CDB. ACPI _GTF never contains PACKET command. It's always ATA commands. So, without the change, it basically ends up issuing an ATA command than tries to transmit non-existent CDB and gets a HSM violation. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] libata: separate out ata_dev_reread_id()
Jeff Garzik wrote: Tejun Heo wrote: Separate out ata_dev_reread_id() from ata_dev_revalidate(). ata_dev_reread_id() reads IDENTIFY page and determines whether the same device is still there. ata_dev_revalidate() reconfigures after reread completes. This will be used by ACPI update. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-core.c | 47 drivers/ata/libata.h |3 +- 2 files changed, 36 insertions(+), 14 deletions(-) ACK, though I think you'll have to rediff due to a Mark Lord patch No problem. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
Jeff Garzik wrote: Tejun Heo wrote: This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes. * make libata-acpi functions take ata_device instead of ata_port + device index * s/atadev/adev/ * de-indent local variable declarations I prefer 'dev' over 'adev', unless that makes the code in question more ambiguous. Alan is preferring adev over dev and I thought that might be better in the spirit of 'ap'. I don't really care about the naming tho. Will convert to dev. It won't increase ambiguity. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
Jeff Garzik wrote: Tejun Heo wrote: Whether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA, or it supports slave device or not. This patch adds ATA_FLAG_ACPI_SATA port flags which tells libata-acpi that the port needs SATA ACPI nodes, and sets the flag for ahci and sata_sil24. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/ahci.c|3 ++- drivers/ata/libata-acpi.c | 10 +- drivers/ata/sata_sil24.c |3 ++- include/linux/libata.h|1 + 4 files changed, 10 insertions(+), 7 deletions(-) I don't think the situation is as static as a compiled-in driver flag implies. And I'm not really convinced a driver flag is needed, or wanted. If anything, the only flag we /may/ need could be a ATA_FLAG_NEVER_EVER_DO_ACPI_FOR_THIS_CONTROLLER. Can you please elaborate a bit? As I wrote while talking with Alan, I really don't know how to do auto-matching. Personally, I don't think there is a way to do that safely but will be happy to implement it if someone can enlighten me. :-) -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: Mark Lord wrote: .. I triggered this by accident, issuing an IDENTIFY command which incorrectly specified ATA_PROT_NODATA. My error, for sure, but libata never recovered from the stuck DRQ bit that resulted. ... sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: enabled, read cache: enabled, doesn't support DPO or FUA ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata1.00: cmd ec/00:00:00:00:00/00:00:00:00:00/00 tag 0 cdb 0x0 data 0 res 58/00:00:00:00:00/00:00:00:00:00/00 Emask 0x2 (HSM violation) ata1: soft resetting port ata1.00: configured for UDMA/100 ata1: EH complete ... (over and over) Say.. is this problem as simple as excessive retries for an SG_IO command? There shouldn't really be *any* retries here, and it should eventually just fail the command rather than shut down the port. Or am I just reading the logs wrong? libata EH isn't trying to retry the command. It's trying to revalidate the device after resetting it to make sure that the device is still there and listening to commands. As the device fails to respond to reset and the following IDENTIFY, libata EH assumes that the device is dead one way or the other and gives up on the device after a few reset/revalidate retries. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
Tejun Heo wrote: Jeff Garzik wrote: Tejun Heo wrote: This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes. * make libata-acpi functions take ata_device instead of ata_port + device index * s/atadev/adev/ * de-indent local variable declarations I prefer 'dev' over 'adev', unless that makes the code in question more ambiguous. Alan is preferring adev over dev and I thought that might be better in the spirit of 'ap'. I don't really care about the naming tho. Will convert to dev. It won't increase ambiguity. Cool. You will see 'dev' used universally in the code I wrote. It also matches well with ata_dev_ prefixes, which are a bit better than ata_adev_ prefix if one were to apply the alternate policy. Yes, I sometimes spend way too much time pay attention to trivialities :) Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata-acpi: fix _GTF command protocol for ATAPI devices
Tejun Heo wrote: ATA_PROT_ATAPI_NODATA is used for PACKET command without CDB. ACPI _GTF never contains PACKET command. It's always ATA commands. Ah yes. Thanks for the reminder about the code I wrote :) Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Mark Lord wrote: Jeff Garzik wrote: It's not really a good idea for SATA. The FIFO often co-emulated by the SATA controller and SATA phy. You just want to kick SATA really hard (i.e. bus reset and friends). Sure. So why don't we do that now? We do that. It's just that ata_piix is lacking SControl access so all we can do is SRST not PHY hardreset. I don't think draining FIFO/whatever on most SATA controllers would be unnecessary as PHY hardreset would make most drives forget what they were doing. I thought SRST would have similar effect. It's supposed to reset the device's HSM and thus clear DRQ, right? Stuck DRQ after SRST seems odd to me. One more thing to note is that there might be no way to drain data safely on non-SFF (ahci/sil24...) interfaces and some controllers lock the machine up hard when TF registers are accessed in certain unexpected way (unsurprisingly, sata_nv), so if we do this, it needs to be configurable per-driver. Another question is how would SATA controllers emulating TF interface react when data port is polled after a DMA command. I'm pretty sure many of them would behave erratically. Anyways, can you try to hack it into ata_bmdma_error_handler() and see whether it actually works? You can check for AC_ERR_HSM there and drain data port if DRQ is set. After HSM, ATA_NIEN is set and the port should be quiescent at that point. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Tejun Heo wrote: and thus clear DRQ, right? Stuck DRQ after SRST seems odd to me. Unfortunately not odd on ata_piix, which can get stuck DRQ-on somewhere deep inside its IDE emulation engine. And neither draining the FIFO nor SRST nor a couple other tricks ever helped. The only thing that seemed to make any difference was an enable/disable reset via our beloved PCS register. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata fails to recover from HSM violation involving DRQ status
Tejun Heo wrote: Mark Lord wrote: Jeff Garzik wrote: It's not really a good idea for SATA. The FIFO often co-emulated by the SATA controller and SATA phy. You just want to kick SATA really hard (i.e. bus reset and friends). Sure. So why don't we do that now? We do that. It's just that ata_piix is lacking SControl access so all we can do is SRST not PHY hardreset. I don't think draining FIFO/whatever on most SATA controllers would be unnecessary as PHY hardreset would make most drives forget what they were doing. I thought SRST would have similar effect. It's supposed to reset the device's HSM and thus clear DRQ, right? Stuck DRQ after SRST seems odd to me. One more thing to note is that there might be no way to drain data safely on non-SFF (ahci/sil24...) interfaces and some controllers lock the machine up hard when TF registers are accessed in certain unexpected way (unsurprisingly, sata_nv), so if we do this, it needs to be configurable per-driver. Another question is how would SATA controllers emulating TF interface react when data port is polled after a DMA command. I'm pretty sure many of them would behave erratically. Anyways, can you try to hack it into ata_bmdma_error_handler() and see whether it actually works? You can check for AC_ERR_HSM there and drain data port if DRQ is set. After HSM, ATA_NIEN is set and the port should be quiescent at that point. Oh, and one more thing, was the drive SATA or PATA? I think it could be the SATA SFF emulation which doesn't reset itself on SRST. Testing whether SRST clears DRQ on actual PATA devices would be worthwhile. If it's really the controller's emulation HSM that's not getting reset, the fix definitely should be applied per-driver. Ah.. one more thing, is this draining also needed after DMA commands or only after PIO commands? Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html