Re: libata Intel PIIX/ICH fails to detect both PATA drives, spews ACPI errors

2007-04-28 Thread Berck E. Nash
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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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()

2007-04-28 Thread Jeff Garzik

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/

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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()

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

[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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

[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

2007-04-28 Thread Jeff Garzik

[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

2007-04-28 Thread Jeff Garzik

[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

2007-04-28 Thread FUJITA Tomonori
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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Alan Cox
 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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Alan Cox
 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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Mark Lord

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

2007-04-28 Thread Alan Cox
  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

2007-04-28 Thread Mikael Pettersson
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

2007-04-28 Thread Mikael Pettersson
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

2007-04-28 Thread Tejun Heo
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()

2007-04-28 Thread Tejun Heo
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

2007-04-28 Thread Tejun Heo
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

2007-04-28 Thread Tejun Heo
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

2007-04-28 Thread Tejun Heo
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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Tejun Heo
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

2007-04-28 Thread Jeff Garzik

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

2007-04-28 Thread Tejun Heo
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