Re: [PATCH] libata: cosmetic clean up in ata_eh_reset()

2007-10-25 Thread Jeff Garzik

Tejun Heo wrote:

Local variable @action usage in ata_eh_reset() is a bit confusing.
It's used only to cache ehc-i.action to test reset masks after
clearing it; however, due to the generic name action, it's easy to
misinterpret the local variable as containing the selected reset
method later.  Also, the reason for caching the original value is easy
to miss.

This patch renames @action to @tmp_action and make it buffer newly
selected value instead to improve readability.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
 drivers/ata/libata-eh.c |   12 ++--
 1 file changed, 6 insertions(+), 6 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] libata-core: Don't have screaming fits over DF/ERR combinations

2007-10-25 Thread Jeff Garzik

Alan Cox wrote:

Some hardware seems to get this wrong in a non-harmful way, and there are
some devices that seem to do it deliberately for various reasons.

Just take it as a device error not a catastrophic state machine
explosion. 


Signed-off-by: Alan Cox [EMAIL PROTECTED]


applied to #for-testing branch, let me know when you think its suitable 
for upstream



-
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: [git patches] IDE updates (part 2)

2007-10-25 Thread Jeff Garzik

Al Viro wrote:

Proposed addition to icside part, provided that ARM folks ACK it - gets
icside to build and AFAICS it's correct:

diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index be30923..842fe08 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -332,12 +332,13 @@ static void ata_dummy_noret(struct ata_port *port)


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] ahci: ahci: implement workaround for ASUS P5W-DH Deluxe ahci_broken_hardreset(), take #2

2007-10-25 Thread Jeff Garzik

Tejun Heo wrote:

P5W-DH Deluxe has ICH9 which doesn't have PMP support but SIMG 4726
hardwired to the second port of AHCI controller at PCI device 1f.2.
The 4726 doesn't work as PMP but as a storage processor which can do
hardware RAID on downstream ports.

When no device is attached to the downstream port of the 4726, pseudo
ATA device for configuration appears.  Unfortunately, ATA emulation on
the device is very lousy and causes long hang during boot.

This patch implements workaround for the board.  If the mainboard is
P5W-DH Deluxe (matched using DMI), only hardreset is used on the
second port of AHCI controller @ 1f.2 and the hardreset doesn't depend
on receiving the first FIS and just proceed to IDENTIFY.

This workaround fixes bugzilla #8923.

  http://bugzilla.kernel.org/show_bug.cgi?id=8923

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
Okay, here's improved version.  p5wdh hardreset is sufficiently
different from vt8251, so it now has its own hardreset and ops.  It
now also has two secs + 150ms delay after hardreset so that devices
which are slow to respond after hardreset don't fail.  Most devices
will respond in time unless it's spinning up.  Spin up is handled by
reset retrials.  Also, it sets BSY before initiating BSY and performs
CLO afterward if BSY isn't cleared during the wait.

Change from the first take are minor but make the workaoround fit the
problem better.  Tested on a setup which behaves the same way as
P5W-DH (non-PMP capable ICH AHCI + SIMG 4726) with some number of
harddrives and ATAPI devices.  Everything including hotplug works
fine.

 drivers/ata/ahci.c |  144 +
 1 file changed, 144 insertions(+)


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] libata-core: Be a bit more relaxed about early DMA zero devices

2007-10-25 Thread Jeff Garzik

Alan Cox wrote:

I guess Windows didn't care about the command so neither did they

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 3/4] scsi: expose AN support to user space

2007-10-25 Thread Jeff Garzik

[EMAIL PROTECTED] wrote:

From: Kristen Carlson Accardi [EMAIL PROTECTED]

If a scsi_device supports async notification for media change, then let user
space know this capability exists by creating a new sysfs entry
media_change_notify, which will be 1 if it is supported, and 0 if not
supported.  Create a routine which allows scsi devices to send a uevent when
media change events occur.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
Acked-by: Jeff Garzik [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 drivers/scsi/scsi_lib.c|   83 +++
 drivers/scsi/scsi_scan.c   |1 
 drivers/scsi/scsi_sysfs.c  |   13 +

 include/scsi/scsi_device.h |   15 +-
 4 files changed, 111 insertions(+), 1 deletion(-)


committed to libata-dev.git#an as the attached patch

I changed the interface such that it takes a mask of events.  We only 
have one event right now, but it is now trivial to add more events 
within the same interface.


commit 4e0af19e75e40d71181c4e515009e81f19a0fc04
Author: Jeff Garzik [EMAIL PROTECTED]
Date:   Thu Oct 25 03:13:46 2007 -0400

[SCSI] Emit asynchronous media events

Based originally on a patch by
Kristen Carlson Accardi [EMAIL PROTECTED]

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]

 drivers/scsi/scsi_lib.c|   58 +
 drivers/scsi/scsi_sysfs.c  |   13 ++
 include/scsi/scsi_device.h |   13 +-
 3 files changed, 83 insertions(+), 1 deletion(-)

4e0af19e75e40d71181c4e515009e81f19a0fc04
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 61fdaf0..46a7f50 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2115,6 +2115,64 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)
 EXPORT_SYMBOL(scsi_device_set_state);
 
 /**
+ * scsi_device_event_notify_thread - send a uevent for each scsi event
+ * @work: work struct for scsi_device
+ *
+ * Emit all queued media events as environment variables
+ * sent during a uevent.
+ */
+static void scsi_event_notify_thread(struct work_struct *work)
+{
+   struct scsi_device *sdev;
+   char *envp[3];
+   unsigned long flags, mask;
+   int i, idx;
+
+   /* must match scsi_device_event enum in scsi_device.h */
+   static char *scsi_device_event_strings[] = {
+   [SDEV_MEDIA_CHANGE] = SDEV_MEDIA_CHANGE=1,
+   };
+
+   sdev = container_of(work, struct scsi_device, ew.work);
+
+   spin_lock_irqsave(sdev-list_lock, flags);
+   mask = sdev-event_mask;
+   sdev-event_mask = 0;
+   spin_unlock_irqrestore(sdev-list_lock, flags);
+
+   idx = 0;
+   for (i = 0; i  SDEV_MEDIA_LAST; i++)
+   if (mask  (1UL  i))
+   envp[idx++] = scsi_device_event_strings[i];
+   envp[idx++] = NULL;
+
+   kobject_uevent_env(sdev-sdev_gendev.kobj, KOBJ_CHANGE, envp);
+}
+
+/**
+ * scsi_device_event_notify - store event info and send an event
+ * @sdev: scsi_device event occurred on
+ * @mask: the scsi device event mask
+ *
+ * Store the event information and then switch process context
+ * so that the event may be sent to user space.
+ * This may be called from interrupt context.
+ *
+ * Returns 0 if successful, -ENOMEM otherwise.
+ */
+void scsi_device_event_notify(struct scsi_device *sdev, unsigned long mask)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(sdev-list_lock, flags);
+   sdev-event_mask |= mask;
+   spin_unlock_irqrestore(sdev-list_lock, flags);
+
+   execute_in_process_context(scsi_event_notify_thread, sdev-ew);
+}
+EXPORT_SYMBOL_GPL(scsi_device_event_notify);
+
+/**
  * scsi_device_quiesce - Block user issued commands.
  * @sdev:  scsi device to quiesce.
  *
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d531cee..d6e765c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -614,6 +614,18 @@ sdev_show_modalias(struct device *dev, struct 
device_attribute *attr, char *buf)
 }
 static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
 
+static ssize_t
+sdev_show_media_events(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   if (sdev-media_events)
+   return snprintf(buf, 20, %d\n, 1);
+   else
+   return snprintf(buf, 20, %d\n, 0);
+}
+static DEVICE_ATTR(media_events, S_IRUGO, sdev_show_media_events, NULL);
+
 /* Default template for device attributes.  May NOT be modified */
 static struct attribute *scsi_sdev_attrs[] = {
dev_attr_device_blocked.attr,
@@ -631,6 +643,7 @@ static struct attribute *scsi_sdev_attrs[] = {
dev_attr_iodone_cnt.attr,
dev_attr_ioerr_cnt.attr,
dev_attr_modalias.attr

Re: [patch 4/4] libata: expose AN to user space

2007-10-25 Thread Jeff Garzik

[EMAIL PROTECTED] wrote:

From: Kristen Carlson Accardi [EMAIL PROTECTED]

If Asynchronous Notification of media change events is supported, pass that
information up to the SCSI layer.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
Cc: Jeff Garzik [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 drivers/ata/libata-scsi.c |3 +++
 1 file changed, 3 insertions(+)

diff -puN drivers/ata/libata-scsi.c~libata-expose-an-to-user-space 
drivers/ata/libata-scsi.c
--- a/drivers/ata/libata-scsi.c~libata-expose-an-to-user-space
+++ a/drivers/ata/libata-scsi.c
@@ -773,6 +773,9 @@ static void ata_scsi_dev_config(struct s
blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
}
 
+	if (dev-flags  ATA_DFLAG_AN)

+   sdev-media_change_notify = 1;
+


Methinks this wasn't tested, as you missed removal of the crucial 
OTHER_AN_PATCHES_HAVE_BEEN_APPLIED ifdef-ery... :)


Anyway, I updated the patch for the media_event thingy in the last 
email, and applied the attached patch


commit 6ba2326c804e2d449c2f40a63ba0a9ea650bef47
Author: Jeff Garzik [EMAIL PROTECTED]
Date:   Thu Oct 25 03:20:05 2007 -0400

[libata] Utilize new SCSI media event infrastructure

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]

 drivers/ata/libata-scsi.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

6ba2326c804e2d449c2f40a63ba0a9ea650bef47
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f5d5420..65b6153 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -773,6 +773,9 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
}
 
+   if (dev-flags  ATA_DFLAG_AN)
+   sdev-media_events = 1;
+
if (dev-flags  ATA_DFLAG_NCQ) {
int depth;
 
@@ -3248,10 +3251,8 @@ static void ata_scsi_handle_link_detach(struct ata_link 
*link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-#ifdef OTHER_AN_PATCHES_HAVE_BEEN_APPLIED
if (dev-sdev)
-   scsi_device_event_notify(dev-sdev, SDEV_MEDIA_CHANGE);
-#endif
+   scsi_device_event_notify(dev-sdev, 1UL  SDEV_MEDIA_CHANGE);
 }
 
 /**


[git patches] libata updates

2007-10-25 Thread Jeff Garzik

A couple small cleanups, the rest fixes.

Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git 
upstream-linus

to receive the following updates:

 drivers/ata/ahci.c|  144 +
 drivers/ata/libata-core.c |   40 ++--
 drivers/ata/libata-eh.c   |   12 ++--
 drivers/ata/pata_icside.c |   42 +++--
 drivers/ata/sata_nv.c |6 +-
 5 files changed, 195 insertions(+), 49 deletions(-)

Adrian Bunk (1):
  libata-core.c: make 2 functions static

Al Viro (1):
  Fix pata_icside build for recent libata API changes

Alan Cox (1):
  libata-core: Be a bit more relaxed about early DMA zero devices

Jeff Garzik (1):
  [libata] Create [and use -ed.] internal helper ata_dev_set_feature()

Kuan Luo (1):
  [libata] sata_nv: SWNCQ should not apply to MCP61

Tejun Heo (2):
  libata: cosmetic clean up in ata_eh_reset()
  ahci: ahci: implement workaround for ASUS P5W-DH Deluxe 
ahci_broken_hardreset(), take #2

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 95229e7..49cf4cf 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -41,6 +41,7 @@
 #include linux/interrupt.h
 #include linux/dma-mapping.h
 #include linux/device.h
+#include linux/dmi.h
 #include scsi/scsi_host.h
 #include scsi/scsi_cmnd.h
 #include linux/libata.h
@@ -241,6 +242,7 @@ static void ahci_pmp_attach(struct ata_port *ap);
 static void ahci_pmp_detach(struct ata_port *ap);
 static void ahci_error_handler(struct ata_port *ap);
 static void ahci_vt8251_error_handler(struct ata_port *ap);
+static void ahci_p5wdh_error_handler(struct ata_port *ap);
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc);
 static int ahci_port_resume(struct ata_port *ap);
 static unsigned int ahci_fill_sg(struct ata_queued_cmd *qc, void *cmd_tbl);
@@ -339,6 +341,40 @@ static const struct ata_port_operations ahci_vt8251_ops = {
.port_stop  = ahci_port_stop,
 };
 
+static const struct ata_port_operations ahci_p5wdh_ops = {
+   .check_status   = ahci_check_status,
+   .check_altstatus= ahci_check_status,
+   .dev_select = ata_noop_dev_select,
+
+   .tf_read= ahci_tf_read,
+
+   .qc_defer   = sata_pmp_qc_defer_cmd_switch,
+   .qc_prep= ahci_qc_prep,
+   .qc_issue   = ahci_qc_issue,
+
+   .irq_clear  = ahci_irq_clear,
+
+   .scr_read   = ahci_scr_read,
+   .scr_write  = ahci_scr_write,
+
+   .freeze = ahci_freeze,
+   .thaw   = ahci_thaw,
+
+   .error_handler  = ahci_p5wdh_error_handler,
+   .post_internal_cmd  = ahci_post_internal_cmd,
+
+   .pmp_attach = ahci_pmp_attach,
+   .pmp_detach = ahci_pmp_detach,
+
+#ifdef CONFIG_PM
+   .port_suspend   = ahci_port_suspend,
+   .port_resume= ahci_port_resume,
+#endif
+
+   .port_start = ahci_port_start,
+   .port_stop  = ahci_port_stop,
+};
+
 #define AHCI_HFLAGS(flags) .private_data   = (void *)(flags)
 
 static const struct ata_port_info ahci_port_info[] = {
@@ -1213,6 +1249,53 @@ static int ahci_vt8251_hardreset(struct ata_link *link, 
unsigned int *class,
return rc ?: -EAGAIN;
 }
 
+static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
+   unsigned long deadline)
+{
+   struct ata_port *ap = link-ap;
+   struct ahci_port_priv *pp = ap-private_data;
+   u8 *d2h_fis = pp-rx_fis + RX_FIS_D2H_REG;
+   struct ata_taskfile tf;
+   int rc;
+
+   ahci_stop_engine(ap);
+
+   /* clear D2H reception area to properly wait for D2H FIS */
+   ata_tf_init(link-device, tf);
+   tf.command = 0x80;
+   ata_tf_to_fis(tf, 0, 0, d2h_fis);
+
+   rc = sata_link_hardreset(link, sata_ehc_deb_timing(link-eh_context),
+deadline);
+
+   ahci_start_engine(ap);
+
+   if (rc || ata_link_offline(link))
+   return rc;
+
+   /* spec mandates = 2ms before checking status */
+   msleep(150);
+
+   /* The pseudo configuration device on SIMG4726 attached to
+* ASUS P5W-DH Deluxe doesn't send signature FIS after
+* hardreset if no device is attached to the first downstream
+* port  the pseudo device locks up on SRST w/ PMP==0.  To
+* work around this, wait for !BSY only briefly.  If BSY isn't
+* cleared, perform CLO and proceed to IDENTIFY (achieved by
+* ATA_LFLAG_NO_SRST and ATA_LFLAG_ASSUME_ATA).
+*
+* Wait for two seconds.  Devices attached to downstream port
+* which can't process the following IDENTIFY after this will
+* have to be reset again.  For most cases, this should
+* suffice while making probing snappish

Re: [PATCH 0/5]: Resolve MSI vs. INTX_DISABLE quirks, V2.

2007-10-25 Thread Jeff Garzik

David Miller wrote:

Ok, I've respun the patches including all of the feedback I've
obtained.  Again, it's at:

kernel.org:/pub/scm/linux/kernel/git/davem/msiquirk-2.6.git

Greg, I think this stuff is ready to go so if you would pull
them in I would really appreciate it.

These changes clean up the handling of the common quirk wherein
setting INTX_DISABLE will mistakedly disable MSI generation for some
devices.

For devices without that problem, we want to keep the pci_intx() calls
in drivers/pci/msi.c because those help protect against devices with
the opposite problem.  Such devices always generate INTX interrupts
even when MSI is enabled, unless INTX_DISABLE is set.

In addition to the Tigon3 cases, I added quirk entries for the
SB700/800 SATA chips and the IXP SB400 USB controllers.  And as
a result of the latter we can remove several AMD full-chipset
MSI disable quirks which are no longer necessary.

Signed-off-by: David S. Miller [EMAIL PROTECTED]


[corrected subject line s/4/5/.  the actual patches are OK]

Acked-by: Jeff Garzik [EMAIL PROTECTED]

-
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: Deal with ATA8-ACS proposed Trusted/Treacherous Computing features

2007-10-25 Thread Jeff Garzik

Alan Cox wrote:

- Make dword_io check the ATA version


speaking of dword_io, it would be nice to add 32-bit functions for I/O 
rather than hand-rolling like pdc_data_xfer_vlb() does.


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 #upstream] libata: relocate and fix post-command processing

2007-10-25 Thread Jeff Garzik

Tejun Heo wrote:

Some commands need post-processing after successful completion.  This
was done in ata_scsi_qc_complete() till now but this has the following
problems.

* Post-command processing gets executed when qc is completed from EH.
  Some qc's are retried from EH with zero err_mask and thus triggers
  unnecessary/incorrect post-command processing.

* Command post processing doesn't belong to SAT layer.

* Link-wide revalidation was scheduled where device revalidation
  suffices.

This patch moves post-command processing to success completion path of
ata_qc_complete() which is travelled iff the command is going to be
completed without passing through EH and updates post-command
processing such that device-specific action is used.  While at it,
restructure code a bit for readability.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]


applied

now if only I had a way for the SAT to issue CHECK POWER MODE (used in 
TEST UNIT READY translation), and inspect qc-err_mask before EH gets 
its hands on it...  :)



-
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 #upstream] libata: track SLEEP state and issue SRST to wake it up

2007-10-25 Thread Jeff Garzik

Tejun Heo wrote:

ATA devices in SLEEP mode don't respond to any commands.  SRST is
necessary to wake it up.  Till now, when a command is issued to a
device in SLEEP mode, the command times out, which makes EH reset the
device and retry the command after that, causing a long delay.

This patch makes libata track SLEEP state and issue SRST automatically
if a command is about to be issued to a device in SLEEP.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Bruce Allen [EMAIL PROTECTED]
Cc: Andrew Paprocki [EMAIL PROTECTED]
---
This patch is against

 #upstream
 + relocate-and-fix-post-command-processing patch
   (http://article.gmane.org/gmane.linux.ide/24001)

The infinite loop with the previous patch wasn't caused by bug in
sleep state tracking.  It was happening because post-command
processing was being performed for commands which are being retried
from EH, so what happened was.

1. device is flagged SLEEPING

2. SLEEP is issued, as device is sleeping, EH is rescheduled against
   the qc for SLEEP.

3. EH kicks in and wakes up the device and clears SLEEPING.

4. EH finishes up failed qc which triggers post-command processing and
   sets SLEEPING.

5. SCSI command is retried, go back to #2.

relocate-and-fix-post-command-processing patch fixes the original bug
in post-command processing and this patch doesn't cause infinite loop
anymore.

 drivers/ata/libata-core.c |   12 
 drivers/ata/libata-eh.c   |4 +++-
 include/linux/ata.h   |1 +
 include/linux/libata.h|1 +
 4 files changed, 17 insertions(+), 1 deletion(-)


applied, both of these changes were nice improvements, as well as fixing 
problems



-
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


What's left in libata-dev.git?

2007-10-25 Thread Jeff Garzik


This is a summary of items and issues that remain outstanding for 
libata.  Whenever a git branch is mentioned, it is referring to a branch 
at git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git



Current push just sent to Linus: 
http://marc.info/?l=linux-idem=119329974119307w=2



Changes in 'upstream' branch pending for 2.6.24-rc, but not included in 
the above push:

Tejun Heo (2):
libata: relocate and fix post-command processing
libata: track SLEEP state and issue SRST to wake it up


Active libata branches:
  ALL   Superset branch for -mm testing
  alpm  Link power management (2.6.24-rc hopefully)
  anAsync notify (2.6.24-rc hopefully)
  for-testing   Interesting-but-not-ready stuff
  masterVanilla linux-2.6.git clone
  mv-ahci-pata  Marvell 6141 PATA support (incomplete)
  mv-ncqsata_mv NCQ support (incomplete)
  new-ehsata_qstore, sata_sx4 new-EH conversions
(TEST ME! they have been in -mm for a while)
  pmask proto_mask introduction
  sii-lbt   sata_sil large block transfer enablement
  upstream  pending for 2.6.24-rc
  upstream-linusjust pushed upstream for 2.6.24-rc


Patches and tasks in mbox queue, not yet in git:

1) Axboe: kill sg_last(), libata is last remaining user

2) Bart points out bugs in Re: [git patches] libata update

3) Tejun: libata: implement ata_wait_after_reset()

^^^  Tejun, what case does this solve?  Still needed?

4) Alan: [PATCH] libata: Deal with ATA8-ACS proposed Trusted/Treacherous 
Computing features


5) Need a pass through bz.kernel.org


Please speak up if there is something missing.

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: What's left in libata-dev.git?

2007-10-25 Thread Jeff Garzik

Jeff Garzik wrote:

Active libata branches:
  ALLSuperset branch for -mm testing
  alpmLink power management (2.6.24-rc hopefully)
  anAsync notify (2.6.24-rc hopefully)
  for-testingInteresting-but-not-ready stuff
  masterVanilla linux-2.6.git clone
  mv-ahci-pataMarvell 6141 PATA support (incomplete)
  mv-ncqsata_mv NCQ support (incomplete)
  new-ehsata_qstore, sata_sx4 new-EH conversions
(TEST ME! they have been in -mm for a while)
  pmaskproto_mask introduction
  sii-lbtsata_sil large block transfer enablement
  upstreampending for 2.6.24-rc
  upstream-linusjust pushed upstream for 2.6.24-rc



FWIW, current contents of ALL meta-branch:

upstream
an
alpm
for-testing
new-eh

i.e. that is what akpm will get on his next pull for -mm.

-
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/4] libata: implement ata_wait_after_reset()

2007-10-25 Thread Jeff Garzik

[EMAIL PROTECTED] wrote:

From: Tejun Heo [EMAIL PROTECTED]

On certain device/controller combination, 0xff status is asserted
after reset and doesn't get cleared during 150ms post-reset wait.  As
0xff status is interpreted as no device (for good reasons), this can
lead to misdetection on such cases.

This patch implements ata_wait_after_reset() which replaces the 150ms
sleep and waits upto ATA_TMOUT_FF_WAIT if status is 0xff.
ATA_TMOUT_FF_WAIT is currently 800ms which is enough for
HHD424020F7SV00 to get detected but not enough for Quantum GoVault
drive which is known to take upto 2s.

Without parallel probing, spending 2s on 0xff port would incur too
much delay on ata_piix's which use 0xff to indicate empty port and
doesn't have SCR register, so GoVault needs to wait till parallel
probing.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 drivers/ata/ahci.c  |   11 +
 drivers/ata/libata-core.c   |   67 +++---
 drivers/ata/pata_scc.c  |   13 +-
 drivers/ata/sata_inic162x.c |2 -
 include/linux/libata.h  |8 
 5 files changed, 67 insertions(+), 34 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: PMP device port link speed issues

2007-10-25 Thread Jeff Garzik

Kalra Ashish-B00888 wrote:

While testing the sata_fsl driver with a Sil3726 based PMP, we had a
specific configuration where the host port to PMP link speed was
1.5Gbps, while the PMP has configured it's device port(s) link speed to
3Gbps. This configuration causes NCQ hangs on certain Seagate drives,
probably because of the link speed difference between host and PMP
device  PMP device ports and drives.
 
Does it makes sense to limit the PMP device port link speeds to the host
port link speed ? 
 
I believe currently sata_pmp_attach() is calling sata_link_init_spd()

for each PMP device port and thus causing PMP device port link speeds
being configured independent of host port link speed. Probably the PMP
device port links should be limited to the host link speed. 


Interesting question.  I'm sure Tejun will chime in, but I'm wondering 
if there is any real use -- even theoretical -- for running downstream 
links faster than the host-PMP link?


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: 2.6.24-rc1, sata_nv: MCP51 is boned with SWNCQ too

2007-10-25 Thread Jeff Garzik

Andrew wrote:

Hi,

I've noticed a thread reporting that SWNCQ can't be disabled
on the sata_nv.

Gerhard Dirschl
*   BUG: sata_nv swncq cannot be disabled

and another with a patch switching MCP61 to GENERIC instead
of SWNCQ

Kuan Luo
*   [PATCH] ata: sata_nv MCP61 using GENERIC instead of SWNCQ


I would like to to report that the MCP51 on my Asus A8NVM CSM
mainboard is dead due to timeouts with SWNCQ. Reverting sata_nv.c
to the version prior to git commit
f140f0f12fc8dc7264d2f97cbe663564e7d24f6d works around the problem.

My drives are all Seagate drives, ST3320620AS, ST3500630AS,
ST3300831AS.



Really?  That's a showstopper bug, as swncq is supposed to be disabled 
by default.


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 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop

2007-10-25 Thread Jeff Garzik

Bartlomiej Zolnierkiewicz wrote:

On Thursday 25 October 2007, Jeff Garzik wrote:

Store our hwif indices at probe time, in order to eliminate a needless
and ugly loop across all hwifs, searching for our pci device.


It seems that we can simplify it even further and remove knowledge about
hwifs altogether from suspend/resume methods.


Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---
 drivers/ide/pci/sc1200.c |   76 +++---
 1 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
index d2c8b55..17e58d6 100644
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -41,6 +41,8 @@
 #define PCI_CLK_66 0x02
 #define PCI_CLK_33A0x03
 
+#define SC1200_IFS	4

+
 static unsigned short sc1200_get_pci_clock (void)
 {
unsigned char chip_id, silicon_revision;
@@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const 
u8 pio)
 }
 
 #ifdef CONFIG_PM

-static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
-{
-   int h;
-
-   for (h = 0; h  MAX_HWIFS; h++) {
-   ide_hwif_t *hwif = ide_hwifs[h];
-   if (prev) {
-   if (hwif == prev)
-   prev = NULL;// found previous, now look for 
next match
-   } else {
-   if (hwif  hwif-pci_dev == dev)
-   return hwif;// found next match
-   }
-   }
-   return NULL;// not found
-}
-
 typedef struct sc1200_saved_state_s {
__u32   regs[4];
 } sc1200_saved_state_t;
 
+static unsigned int pack_hwif_idx(u8 *idx)

+{
+   return  (((unsigned int) idx[0])  0) |
+   (((unsigned int) idx[1])  8) |
+   (((unsigned int) idx[2])  16) |
+   (((unsigned int) idx[3])  24);
+}
+
+static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
+{
+   unsigned int packed_hwifs, idx;
+
+   packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
+   idx = (packed_hwifs  (iface * 8))  0xff;
+
+   return (idx == 0xff) ? NULL : ide_hwifs[idx];
+}
 
 static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)

 {
-   ide_hwif_t  *hwif = NULL;
+   ide_hwif_t  *hwif;
+   int i;
 
 	printk(SC1200: suspend(%u)\n, state.event);
 
@@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)

//
// Loop over all interfaces that are part of this PCI device:
//
-   while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
+   for (i = 0; i  SC1200_IFS; i++) {
sc1200_saved_state_t*ss;
unsigned intbasereg, r;
+
+   hwif = sc1200_hwif(dev, i);
+   if (!hwif)
+   continue;
+
//
// allocate a permanent save area, if not already 
allocated
//
@@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, 
pm_message_t state)
}
ss = (sc1200_saved_state_t *)hwif-config_data;
//
-			// Save timing registers:  this may be unnecessary if 
+			// Save timing registers:  this may be unnecessary if

// BIOS also does it
//
basereg = hwif-channel ? 0x50 : 0x40;


Please take a close look at the line above and the next three lines:

for (r = 0; r  4; ++r) {
pci_read_config_dword (hwif-pci_dev, basereg + (r2), ss-regs[r]);
}

It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
the offset 0x40 (for the primary port) and puts them in the corresponding
struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
offset 0x50 (for the secondary port) and puts it in the another buffer.

In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
and the exactly reverse operation happens in sc1200_resume().

Given that and the fact that struct sc1200_save_state_s buffers are used
_only_ by sc1200_{suspend,resume}() we may safely convert the code to use
one buffer for both ports (the whole PCI device).  We just need to bump
the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
pci_{get,set}_drvdata() instead of hwif-config_data.  Then we can remove
looping over interfaces completely from sc1200_{suspend,resume}()! :)


May I assume you'll handle that task?  :)  It sounds like you have a far 
better idea than mine, and my main goal -- fixing bugs and warning -- is 
accomplished anyway with the merging of patch #3.


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More

Re: Questions about SATA hotplug in linux 2.6

2007-10-25 Thread Jeff Garzik

Shane Huang wrote:

1. If users unplug one SATA HDD(no-root partition) or SATA ODD when
the system is running, then plug it back to the same SATA port,
Should the system and SATA HDD/ODD still work well?


Yes.


2. How about users plug the SATA HDD/ODD in a different SATA port?
Should it still work?


Yes.

For all hotplug-aware libata drivers, you should be able to unplug a 
SATA device _while_ it is actively reading or writing data, with no ill 
effects to the kernel.


You might lose cached and in-flight data of course, and userspace 
applications may or may not handle the disappearance of their underlying 
filesystem with grace and aplomb :)


But device hotplug should be reliable from a kernel standpoint [assuming 
driver support].




These questions come up when our QA test our SB700 SATA drivers,
but I don't know the SATA hotplug support in linux 2.6.
Is there any guy who can give some official confirmation? :-)


The main thing of note with regards to hotplug is that the associated 
device (/dev/sdb, /dev/scd0, etc.) may change between plug and unplug. 
For example, if you unplug a SATA HDD then plug it back in, the user 
might see /dev/sdb disappear, and /dev/sdd appear -- even if it is the 
exact same HDD, on the exact same port.


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-RFC] Promise TX4 implement hw-bug workaround

2007-10-28 Thread Jeff Garzik

Alexander Sabourenkov wrote:

Alan Cox wrote:

I can't think of a way to avoid second pass over scatterlist without
duplicating code (ata_qc_prep() and ata_fill_sg() from libata-core.c).

This appears to be incomplete:



[...]


What guarantees you have enough PRD entries to do this without changing
the limit in the structures ?

Otherwise looks good


PRD entries count is 256
include/linux/ata.h:
ATA_MAX_PRD = 256,
ATA_PRD_TBL_SZ  = (ATA_MAX_PRD * ATA_PRD_SZ),

drivers/ata/libata-core.c:
 ap-prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ, ap-prd_dma,

sata_promise Scsi_Host declares support for half of that:

include/linux/libata.h:
LIBATA_MAX_PRD  = ATA_MAX_PRD / 2,

drivers/ata/sata_promise.c
.sg_tablesize   = LIBATA_MAX_PRD,


Alan's point was that the existing code will give you up to 
LIBATA_MAX_PRD entries.  After the post-virtual-merge splitting code in 
ata_fill_sg() executes, the worst case result is ATA_MAX_PRD entries.


Thus, since your code has the potential to increase the number of s/g 
entries above that, it can potentially corrupt memory, lock up the 
machine, all the wonderful things that can happen when you run off the 
end of the s/g list.


The fix is to decrease .sg_tablesize (LIBATA_MAX_PRD - 2 perhaps?) so 
that you guarantee this worst case never occurs, by guaranteeing that 
the system never sends you enough s/g entries to cause your code to go 
out of bounds.


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-RFC] Promise SATA300 TX4

2007-10-28 Thread Jeff Garzik

Alexander Sabourenkov wrote:

Hmm. sata_promise.c says:

*  Maintained by:  Jeff Garzik [EMAIL PROTECTED]


That should be changed :)  I wrote the code but Mikael is now the 
primary maintainer.




Meanwhile I'll check the Promise driver(s) to see if there's
something about SG table formatting restrictions there.


Take a look at:

cam_ata.c:6190
cam_ata.c:6259

The fix is also conditioned on sg segment length == 0, which I did not
implement. Is this at all possible in libata ?


The block layer should never pass us a scatterlist element of length zero.

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-RFC] Promise TX4 implement hw-bug workaround

2007-10-28 Thread Jeff Garzik

BTW, looking at the Promise code I see


cam_con.h:
/* for ASIC bug, limit the last element of SG byteCount must  32 Dword */
#define SG_COUNT_ASIC_BUG   32
//#define SG_COUNT_ASIC_BUG 128


and in the code itself


/* check PRD table, last element = (32 Dword), fix ASIC bug */


(though the code obviously uses SG_COUNT_ASIC_BUG==32, as the first 
paste indicates)


so it seems like Promise first used 128 (32 dwords), but then backed 
down to 32 (8 dwords).


Either way, we definitely have an ASIC bug to work around, it seems...

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-RFC] Promise TX4 implement hw-bug workaround

2007-10-28 Thread Jeff Garzik

Alexander Sabourenkov wrote:

Jeff Garzik wrote:

BTW, looking at the Promise code I see


cam_con.h:
/* for ASIC bug, limit the last element of SG byteCount must  32
Dword */
#define SG_COUNT_ASIC_BUG   32
//#define SG_COUNT_ASIC_BUG 128

and in the code itself


/* check PRD table, last element = (32 Dword), fix ASIC bug */

(though the code obviously uses SG_COUNT_ASIC_BUG==32, as the first
paste indicates)

so it seems like Promise first used 128 (32 dwords), but then backed
down to 32 (8 dwords).



Which version is this define from?

Both versions that are available now from their website define it at 41*4:


Mikael Pettersson wrote:

You're looking at the old pdc-ultra2 driver. The newer unified
sataii150-300 driver (v1.01.0.23) upped the value to 41*4.



I was looking at pdc-ulsata2_1.00.0.15.tgz, which was the latest driver 
that Promise's website gave me to when I listed SATA300 TX4 as my product.


Sounds like that is outdated information, thanks for the correction!

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 #upstream 1/2] libata: relocate forcing PIO0 on reset

2007-10-29 Thread Jeff Garzik

Tejun Heo wrote:

Forcing PIO0 on reset was done inside ata_bus_softreset(), which is a
bit out of place as it should be applied to all resets - hard, soft
and implementation which don't use ata_bus_softreset().  Relocate it
such that...

* For new EH, it's done in ata_eh_reset() before calling prereset.

* For old EH, it's done before calling ap-ops-phy_reset() in
  ata_bus_probe().

This makes PIO0 forced after all resets.  Another difference is that
reset itself is done after PIO0 is forced.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]


applied 1-2 to #upstream-fixes, after renaming existing #upstream to 
#upstream-fixes (thereby assuring those previous changesets will go 
upstream sooner)



-
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: [RFC/PATCH] libata and bogus LBA48 drives

2007-10-29 Thread Jeff Garzik

Geert Uytterhoeven wrote:

Hi Jeff,

A colleague noticed recent versions of Ubuntu no longer detect his 80 GB
ST380020ACE drive. This drive is special in that it advertises LBA48 support,
but has the lba_capacity_2 field set to zero (cfr.
http://lkml.org/lkml/2004/3/30/163).

Upon closer look, libata indeed doesn't seem to handle this case yet.
Below is an (untested) fix.

---
Subject: libata: Ignore bogus lba48 drives

Some drives (e.g. the 80 GB ST380020ACE) advertise they support LBA48, but have
lba_capacity_2 field set to zero. This causes the drive not being detected by
the libata driver.

Add a check for this to ata_id_has_lba48(), cfr. what is done in
idedisk_supports_lba48().

Signed-off-by: Geert Uytterhoeven [EMAIL PROTECTED]
---
NOTE: Untested due to the lack of hardware

 include/linux/ata.h |2 ++
 1 file changed, 2 insertions(+)

--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -402,6 +402,8 @@ static inline int ata_id_has_lba48(const
 {
if ((id[83]  0xC000) != 0x4000)
return 0;
+   if (!ata_id_u64(id, 100))
+   return 0;
return id[83]  (1  10);
 }


Is there any hope of getting a dump of the IDENTIFY DEVICE page?

'hdparm --Istdout /dev/DEVICE' should do the trick, for either IDE 
driver or libata.


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: Should be Acard ATP8620 2SATA / 1 IDE driver

2007-10-30 Thread Jeff Garzik

jameshsu wrote:

Should be in TEXT/PLAIN mode. --- resend


Hello,

This driver has nothing changed since last time submit.
However recently we download 2.6.23.1 kernel and found Acard ATP8620 SATA
driver is still not there.
Resend this message to submit the same driver again.
Please help Acard to build in this SATA driver with latest Linux kernel.
If any reason not able to add in, please let us know.


We would be glad to help!

This driver is an ATA driver, which duplicates the existing SCSI-ATA 
translation code we already have.  It also fails to work around the 
problems (quirks) found in a large number of ATA devices.


As such, it would be preferred that this ATA hardware use the existing 
ATA driver API.


You can see example drivers for advanced controllers such a 
drivers/ata/ahci.c and drivers/ata/sata_sil24.c, which are both 
FIS-based controllers with full NCQ support, hotplug support, port 
multiplier support, and many others.


It is important in Linux that we do not duplicate effort by merging 
drivers that duplicate the existing SCSI-ATA translation layer, or 
fail to use our existing ATA API.


Overall, we wish to avoid adding another ATA driver inside the SCSI 
layer, without using the existing ATA-SCSI code.


Is your hardware documentation public?  I can help provide a sample 
driver for your hardware, or help guide your engineers in this effort.


Regards,

Jeff, the Linux ATA maintainer


-
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/pata_it821x: Improve handling of poorly compatible emulations

2007-10-30 Thread Jeff Garzik

Alan Cox wrote:

Some it821x RAID firmwares return 0 for the err return off both devices.
A similar issue occurs with the slave returning 0 not 1 if you plug a
gigabyte sata ramdisk into a controller that fakes two SATA ports as
master/slave on an SFF channel.

The patch does the following

- Allow the 'failed diagnostics' case on both master and slave
- Move the HORKAGE_DIAGNOSTIC check after -dev_config

This second change also allows IT821x to fix up a problem where we report
drive diagnostic failures when in fact the drive is fine but the
microcontroller firmware doesn't appear to get it right. IT821x clears
the flag again to avoid giving the user bogus warnings about their disk.

The other IT821x change is a bit ugly, we slightly abuse the cable type
hook to fiddle with the identify data for the devices. We could add a new
hook for this but as we have only one offender and no more seeming likely
it seems better to keep libata-core clean.

Please let this sit in -mm briefly, just in case the relaxed checking
breaks some other emulated interface.

Signed-off-by: Alan Cox [EMAIL PROTECTED]


Two questions:

1) should I queue for 2.6.24-rc?

2) why is -dev_config() insufficient?


-
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 1/3] libata: flush is an IO command

2007-10-30 Thread Jeff Garzik

Tejun Heo wrote:

ATA_QCFLAG_IO is used to mark commands which are used to perform
regluar IO transfers via block layer.  These commands are assumed to
be valid and taken more seriously during error handling.  Cache flush
is used by regular IO path and necessary for data integrity.  Mark it
with ATA_QCFLAG_IO.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
 drivers/ata/libata-scsi.c |3 +++
 1 file changed, 3 insertions(+)


patch series looks fine... ok for 2.6.24-rc?


-
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/pata_it821x: Improve handling of poorly compatible emulations

2007-10-30 Thread Jeff Garzik

Alan Cox wrote:

The other IT821x change is a bit ugly, we slightly abuse the cable type
hook to fiddle with the identify data for the devices. We could add a new
hook for this but as we have only one offender and no more seeming likely
it seems better to keep libata-core clean.

Please let this sit in -mm briefly, just in case the relaxed checking
breaks some other emulated interface.

Signed-off-by: Alan Cox [EMAIL PROTECTED]

Two questions:

1) should I queue for 2.6.24-rc?


Can we leave it once cycle in -mm and aim for 2.6.25-rc. There is a tiny
possibility we will find someone who finds slave returning 0 produces a
ghost drive or decode problem. Probably paranoia.


into #for-testing it goes



2) why is -dev_config() insufficient?


It is called (I think correctly) after we have performed identify
dependant actions including HPA sizing.


h.  It certainly seems like we should have a hook that permits 
touching up dev-id[] before we go through all the actions based on 
IDENTIFY DEVICE results.


It sounds like such a hook would be more appropriate here...  can you 
think of any other situation or driver that could make use of a 
pre-dev-config hook?


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: Deal with ATA8-ACS proposed Trusted/Treacherous Computing features

2007-10-30 Thread Jeff Garzik

Alan Cox wrote:

Historically word 48 in the identify data was used to mean 32bit I/O was
supported for VLB IDE etc. ATA8 reassigns this word to the Trusted
Computing Group, where it is used for TCG features. This means that an
ATA8 TCG drive is going to trigger 32bit I/O on some systems which will
be funny. Perhaps thats why T13 gave them the word.

Anyway we need to sort this out ready for ATA8 so:
- Reorder the ata.h header a bit so the ata_version function occurs early
in it
- Make dword_io check the ATA version
- Add an ATA8 version checking TCG presence test

While we are at it the current drafts have a flaw where it may not be
possible to disable TCG features at boot (and opt out of the trusted
model) as TCG intends because it relies on presence of a different
optional feature (DCS). Handle this in software by refusing the TCG
commands if libata.allow_tpm is not set. (We must make it possible as
some environments such as proprietary VDR devices will doubtless want to
use it to lock up content)

Finally as with CPRM print a warning so that the user knows they may not
be able to full access and use the device.

Alan

Signed-off-by: Alan Cox [EMAIL PROTECTED]


seems fairly reasonable... 2.6.24-rc?


-
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.24-rc1 1/2] sata_promise: ASIC PRD table bug workaround, take 2

2007-10-30 Thread Jeff Garzik

Mikael Pettersson wrote:

Second-generation Promise SATA controllers have an ASIC bug
which can trigger if the last PRD entry is larger than 164 bytes,
resulting in intermittent errors and possible data corruption.

Work around this by replacing calls to ata_qc_prep() with a
private version that fills the PRD, checks the size of the
last entry, and if necessary splits it to avoid the bug.
Also reduce sg_tablesize by 1 to accommodate the new entry.

Tested on the second-generation SATA300 TX4 and SATA300 TX2plus,
and the first-generation PDC20378.

Thanks to Alexander Sabourenkov for verifying the bug by
studying the vendor driver, and for writing the initial patch
upon which this one is based.

Signed-off-by: Mikael Pettersson [EMAIL PROTECTED]
--
Changes since previous version:
* use new PDC_MAX_PRD constant to initialise sg_tablesize

 drivers/ata/sata_promise.c |   87 ++---
 1 files changed, 83 insertions(+), 4 deletions(-)


applies 1-2 to #upstream-fixes


-
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/pata_it821x: Improve handling of poorly compatible emulations

2007-10-30 Thread Jeff Garzik

Alan Cox wrote:

Some it821x RAID firmwares return 0 for the err return off both devices.
A similar issue occurs with the slave returning 0 not 1 if you plug a
gigabyte sata ramdisk into a controller that fakes two SATA ports as
master/slave on an SFF channel.

The patch does the following

- Allow the 'failed diagnostics' case on both master and slave
- Move the HORKAGE_DIAGNOSTIC check after -dev_config

This second change also allows IT821x to fix up a problem where we report
drive diagnostic failures when in fact the drive is fine but the
microcontroller firmware doesn't appear to get it right. IT821x clears
the flag again to avoid giving the user bogus warnings about their disk.

The other IT821x change is a bit ugly, we slightly abuse the cable type
hook to fiddle with the identify data for the devices. We could add a new
hook for this but as we have only one offender and no more seeming likely
it seems better to keep libata-core clean.

Please let this sit in -mm briefly, just in case the relaxed checking
breaks some other emulated interface.

Signed-off-by: Alan Cox [EMAIL PROTECTED]


applied to #for-testing


-
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: Deal with ATA8-ACS proposed Trusted/Treacherous Computing features

2007-10-30 Thread Jeff Garzik

Alan Cox wrote:

Historically word 48 in the identify data was used to mean 32bit I/O was
supported for VLB IDE etc. ATA8 reassigns this word to the Trusted
Computing Group, where it is used for TCG features. This means that an
ATA8 TCG drive is going to trigger 32bit I/O on some systems which will
be funny. Perhaps thats why T13 gave them the word.

Anyway we need to sort this out ready for ATA8 so:
- Reorder the ata.h header a bit so the ata_version function occurs early
in it
- Make dword_io check the ATA version
- Add an ATA8 version checking TCG presence test

While we are at it the current drafts have a flaw where it may not be
possible to disable TCG features at boot (and opt out of the trusted
model) as TCG intends because it relies on presence of a different
optional feature (DCS). Handle this in software by refusing the TCG
commands if libata.allow_tpm is not set. (We must make it possible as
some environments such as proprietary VDR devices will doubtless want to
use it to lock up content)

Finally as with CPRM print a warning so that the user knows they may not
be able to full access and use the device.

Alan

Signed-off-by: Alan Cox [EMAIL PROTECTED]


would you rediff against latest torvalds/linux-2.6.git pretty please?


-
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: [RFC 0/3] [SCSI/libata] libata EH conversion for ipr SAS

2007-10-30 Thread Jeff Garzik

Brian King wrote:

The following three patches convert ipr to use the new libata EH APIs.
In the process of doing this, I first looked into implementing this
in a similar manner to how libata SAS is done today, which is hooking
into target_alloc/target_destroy to allocate/delete sata ports. While
I was able to get this working after writing my own eh_strategy_handler,
I was doubtful this was the best long term solution. One problem with this
implementation I didn't solve was the fact that libata now invokes EH
for each and every error received. For some devices, such as optical
devices, each not ready response received during a media reload would
result in all the attached SAS devices getting quiesced as well.

My second approach is the attached patch set. In this series I have
created a new libata API which can be used by SAS LLDDs. It introduces
an ata_sas_rphy device object which is created/destroyed by the following API:

ata_sas_rphy_alloc
ata_sas_rphy_add
ata_sas_rphy_delete

When using this API in ipr, I made ipr's scsi_host the parent device of the
SATA rphy. The SATA rphy is then the parent of the allocated scsi_hosts. This
means that each SATA rphy in the SAS topology will have its own scsi_host, 
making
SAS *much* more like all the SATA LLDDs in how it uses libata.

The only issue I ran into with this implementation is that since each SATA
port has its own scsi_host, the adapter cannot rely on scsi core to manage
any LLDD or adapter imposed queue depth. To solve this I added some code to
ipr. Longer term, block layer queue groups might be another way to do this.

I'm still polishing this up, but it is up and running and seems to work with
what testing I've done so far.


Like I said on IRC... thanks for taking care of this!  After this we are 
down to three old-EH users:  libsas, sata_qstor (patch exists, waiting 
on testing) and sata_sx4 (patch exists, waiting on testing).


I'll take a good look in the next day or so.  Overall the coupling 
between SAS (ipr and libsas) and libata definitely needs some thought.


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: Fix ATAPI transfer lengths causes CD writing regression

2007-10-30 Thread Jeff Garzik

Daniel Drake wrote:

Hi Alan,

In 2.6.23 and previous, CD writing works fine on my system. I'm using 
ata_piix on:


00:1f.2 IDE interface: Intel Corporation 82801GBM/GHM (ICH7 Family) SATA 
IDE Controller (rev 01)


When I'm running CD writing utilities, I sometimes see this message in 
the kernel logs:

ata2.00: 66 bytes trailing data
Things do work fine though.

With 2.6.24-rc1, I can't write CDs. Instead of the above message, I get:
ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
ata2.00: cmd a0/00:00:00:0a:00/00:00:00:00:00/a0 tag 0 cdb 0x5a data 10 in
 res 58/00:02:00:0a:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
ata2.00: status: { DRDY DRQ }
ata2: soft resetting link
ata2.00: configured for UDMA/33
ata2: EH complete

and the software acts oddly - for example Brasero tells me that the 
blank CD I have inserted has a capacity of 17,179,869,184gb and it 
doesn't let me write CDs as it says the medium is not writable.


git bisect lead me to commit 2db78dd302d26d242d3e8e5c4c5024b6c3ea93c2 as 
the culprit.


Author: Alan Cox [EMAIL PROTECTED]
Date:   Tue Oct 2 13:53:04 2007 -0700

libata_scsi: Fix ATAPI transfer lengths

Some controller variants snoop the ATAPI length value for Packet
transfers to do state machine and FIFO management. Thus we want to
set it properly, even for cases where it is otherwise meaningless.

Any ideas?


hrm

we may need to make that controller-specific, given that the code prior 
to 2db78dd302d26d242d3e8e5c4c5024b6c3ea93c2 was working for most...


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


[git patches] libata fixes

2007-10-30 Thread Jeff Garzik

Of particular note is the sata_promise fix, which works around a
nasty hw errata.  I need to push that to stable@

Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git 
upstream-linus

to receive the following updates:

 drivers/ata/libata-eh.c|   20 ++---
 drivers/ata/libata-scsi.c  |7 ++-
 drivers/ata/sata_promise.c |  104 ++-
 drivers/ata/sata_sil24.c   |6 +-
 include/linux/libata.h |1 +
 5 files changed, 115 insertions(+), 23 deletions(-)

Mikael Pettersson (2):
  sata_promise: ASIC PRD table bug workaround, take 2
  sata_promise: cleanups

Tejun Heo (3):
  libata: flush is an IO command
  libata: stop being overjealous about non-IO commands
  libata: implement and use ATA_QCFLAG_QUIET

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index fefea74..8d64f8f 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1800,10 +1800,8 @@ static void ata_eh_link_autopsy(struct ata_link *link)
qc-err_mask = ~AC_ERR_OTHER;
 
/* SENSE_VALID trumps dev/unknown error and revalidation */
-   if (qc-flags  ATA_QCFLAG_SENSE_VALID) {
+   if (qc-flags  ATA_QCFLAG_SENSE_VALID)
qc-err_mask = ~(AC_ERR_DEV | AC_ERR_OTHER);
-   ehc-i.action = ~ATA_EH_REVALIDATE;
-   }
 
/* accumulate error info */
ehc-i.dev = qc-dev;
@@ -1816,7 +1814,8 @@ static void ata_eh_link_autopsy(struct ata_link *link)
if (ap-pflags  ATA_PFLAG_FROZEN ||
all_err_mask  (AC_ERR_HSM | AC_ERR_TIMEOUT))
ehc-i.action |= ATA_EH_SOFTRESET;
-   else if (all_err_mask)
+   else if ((is_io  all_err_mask) ||
+(!is_io  (all_err_mask  ~AC_ERR_DEV)))
ehc-i.action |= ATA_EH_REVALIDATE;
 
/* if we have offending qcs and the associated failed device */
@@ -1879,7 +1878,9 @@ static void ata_eh_link_report(struct ata_link *link)
for (tag = 0; tag  ATA_MAX_QUEUE; tag++) {
struct ata_queued_cmd *qc = __ata_qc_from_tag(ap, tag);
 
-   if (!(qc-flags  ATA_QCFLAG_FAILED) || qc-dev-link != link)
+   if (!(qc-flags  ATA_QCFLAG_FAILED) || qc-dev-link != link ||
+   ((qc-flags  ATA_QCFLAG_QUIET) 
+qc-err_mask == AC_ERR_DEV))
continue;
if (qc-flags  ATA_QCFLAG_SENSE_VALID  !qc-err_mask)
continue;
@@ -2697,8 +2698,15 @@ void ata_eh_finish(struct ata_port *ap)
/* FIXME: Once EH migration is complete,
 * generate sense data in this function,
 * considering both err_mask and tf.
+*
+* There's no point in retrying invalid
+* (detected by libata) and non-IO device
+* errors (rejected by device).  Finish them
+* immediately.
 */
-   if (qc-err_mask  AC_ERR_INVALID)
+   if ((qc-err_mask  AC_ERR_INVALID) ||
+   (!(qc-flags  ATA_QCFLAG_IO) 
+qc-err_mask == AC_ERR_DEV))
ata_eh_qc_complete(qc);
else
ata_eh_qc_retry(qc);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 93bd36c..fc89590 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1108,6 +1108,9 @@ static unsigned int ata_scsi_flush_xlat(struct 
ata_queued_cmd *qc)
else
tf-command = ATA_CMD_FLUSH;
 
+   /* flush is critical for IO integrity, consider it an IO command */
+   qc-flags |= ATA_QCFLAG_IO;
+
return 0;
 }
 
@@ -2764,8 +2767,8 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
 */
qc-nbytes = scsi_bufflen(scmd);
 
-   /* request result TF */
-   qc-flags |= ATA_QCFLAG_RESULT_TF;
+   /* request result TF and be quiet about device error */
+   qc-flags |= ATA_QCFLAG_RESULT_TF | ATA_QCFLAG_QUIET;
 
return 0;
 
diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
index deb26f0..825e717 100644
--- a/drivers/ata/sata_promise.c
+++ b/drivers/ata/sata_promise.c
@@ -2,6 +2,7 @@
  *  sata_promise.c - Promise SATA
  *
  *  Maintained by:  Jeff Garzik [EMAIL PROTECTED]
+ * Mikael Pettersson [EMAIL PROTECTED]
  * Please ALWAYS copy linux-ide@vger.kernel.org
  * on emails.
  *
@@ -45,11 +46,12 @@
 #include sata_promise.h
 
 #define DRV_NAME   sata_promise
-#define DRV_VERSION2.10
+#define DRV_VERSION2.11
 
 enum {
PDC_MAX_PORTS   = 4,
PDC_MMIO_BAR= 3

Re: [git patches] libata fixes

2007-10-30 Thread Jeff Garzik

Linus Torvalds wrote:


On Tue, 30 Oct 2007, Jeff Garzik wrote:

Mikael Pettersson (2):
  sata_promise: ASIC PRD table bug workaround, take 2
  sata_promise: cleanups


You and Mikael need to sort out the way you send/accept patches. 


Both of these commits had stuff like this:

Signed-off-by: Mikael Pettersson [EMAIL PROTECTED]
--
Changes since previous version:
* use new PDC_MAX_PRD constant to initialise sg_tablesize

 drivers/ata/sata_promise.c |   87 ++---

 1 files changed, 83 insertions(+), 4 deletions(-)
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]

which seems to be because Mikael uses two dashes instead of three to 
separate his real message from the stuff you have.


So either you need to teach Mikael to use the proper separators, or you 
need to edit these things down to be something readable instead of keeping 
the extraneous commentary around...


Pulled, but I'm hoping for cleaner commit messages in the future..


Can we change git-am to accept two dashes as well as three?  :)

It seems pretty common, not just with Mikael but several others who send 
patches to me.


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


[PATCH] killing sg_last(), and discussion

2007-10-31 Thread Jeff Garzik
I looked into killing sg_last(), but really, this is the best its gonna
get (moving sg_last to libata-core.c).

You could maybe kill one use with caching, but in the other sg_last()
callsites there isn't another s/g loop we can stick a last_sg = sg;
into.

libata is stuck because we undertake the highly unusual operation of
fiddling with the final S/G element, to enforce 32-bit alignment.

Of course we could eliminate all that nasty fiddling/padding
completely, including sg_last(), if other areas of the kernel would
guarantee ahead of time that buffer lengths are always a multiple
of 4

Jeff





[the obvious patch follows, moving sg_last()...]

 drivers/ata/libata-core.c   |   24 
 drivers/ata/sata_nv.c   |1 -
 drivers/ata/sata_promise.c  |1 -
 drivers/ata/sata_qstor.c|1 -
 include/linux/scatterlist.h |   34 --
 5 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 63035d7..4f0acc5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4480,6 +4480,30 @@ static unsigned int ata_dev_init_params(struct 
ata_device *dev,
 }
 
 /**
+ * sg_last - return the last scatterlist entry in a list
+ * @sgl:   First entry in the scatterlist
+ * @nents: Number of entries in the scatterlist
+ *
+ * Description:
+ *   Should only be used casually, it (currently) scan the entire list
+ *   to get the last entry.
+ *
+ *   Note that the @sgl@ pointer passed in need not be the first one,
+ *   the important bit is that @nents@ denotes the number of entries that
+ *   exist from @[EMAIL PROTECTED]
+ *
+ **/
+static inline struct scatterlist *sg_last(struct scatterlist *sgl,
+ unsigned int nents)
+{
+   struct scatterlist *sg, *ret = NULL;
+   unsigned int i;
+
+   for_each_sg(sgl, sg, nents, i)
+   ret = sg;
+}
+
+/**
  * ata_sg_clean - Unmap DMA memory associated with command
  * @qc: Command containing DMA memory to be released
  *
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 35b2df2..6546913 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1985,7 +1985,6 @@ static void nv_swncq_fill_sg(struct ata_queued_cmd *qc)
struct nv_swncq_port_priv *pp = ap-private_data;
struct ata_prd *prd;
 
-   WARN_ON(qc-__sg == NULL);
WARN_ON(qc-n_elem == 0  qc-pad_len == 0);
 
prd = pp-prd + ATA_MAX_PRD * qc-tag;
diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
index 825e717..1fba9d4 100644
--- a/drivers/ata/sata_promise.c
+++ b/drivers/ata/sata_promise.c
@@ -547,7 +547,6 @@ static void pdc_fill_sg(struct ata_queued_cmd *qc)
if (!(qc-flags  ATA_QCFLAG_DMAMAP))
return;
 
-   WARN_ON(qc-__sg == NULL);
WARN_ON(qc-n_elem == 0  qc-pad_len == 0);
 
idx = 0;
diff --git a/drivers/ata/sata_qstor.c b/drivers/ata/sata_qstor.c
index 6d43ba7..664e3f7 100644
--- a/drivers/ata/sata_qstor.c
+++ b/drivers/ata/sata_qstor.c
@@ -276,7 +276,6 @@ static unsigned int qs_fill_sg(struct ata_queued_cmd *qc)
unsigned int nelem;
u8 *prd = pp-pkt + QS_CPB_BYTES;
 
-   WARN_ON(qc-__sg == NULL);
WARN_ON(qc-n_elem == 0  qc-pad_len == 0);
 
nelem = 0;
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 32326c2..ab94464 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -130,40 +130,6 @@ static inline struct scatterlist *sg_next(struct 
scatterlist *sg)
for (__i = 0, sg = (sglist); __i  (nr); __i++, sg = sg_next(sg))
 
 /**
- * sg_last - return the last scatterlist entry in a list
- * @sgl:   First entry in the scatterlist
- * @nents: Number of entries in the scatterlist
- *
- * Description:
- *   Should only be used casually, it (currently) scan the entire list
- *   to get the last entry.
- *
- *   Note that the @sgl@ pointer passed in need not be the first one,
- *   the important bit is that @nents@ denotes the number of entries that
- *   exist from @[EMAIL PROTECTED]
- *
- **/
-static inline struct scatterlist *sg_last(struct scatterlist *sgl,
- unsigned int nents)
-{
-#ifndef ARCH_HAS_SG_CHAIN
-   struct scatterlist *ret = sgl[nents - 1];
-#else
-   struct scatterlist *sg, *ret = NULL;
-   unsigned int i;
-
-   for_each_sg(sgl, sg, nents, i)
-   ret = sg;
-
-#endif
-#ifdef CONFIG_DEBUG_SG
-   BUG_ON(sgl[0].sg_magic != SG_MAGIC);
-   BUG_ON(!sg_is_last(ret));
-#endif
-   return ret;
-}
-
-/**
  * sg_chain - Chain two sglists together
  * @prv:   First scatterlist
  * @prv_nents: Number of entries in prv
-
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: [git patches] libata fixes

2007-10-31 Thread Jeff Garzik

Mikael Pettersson wrote:

That's my fault for misremembering the rule about the
number of dashes before the other comments part :-(
I'll remember better in the future.



Well, I should have caught it and hand-edited it on my side too...

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] killing sg_last(), and discussion

2007-10-31 Thread Jeff Garzik

Boaz Harrosh wrote:

On Wed, Oct 31 2007 at 10:49 +0200, Jeff Garzik [EMAIL PROTECTED] wrote:

I looked into killing sg_last(), but really, this is the best its gonna
get (moving sg_last to libata-core.c).

You could maybe kill one use with caching, but in the other sg_last()
callsites there isn't another s/g loop we can stick a last_sg = sg;
into.

libata is stuck because we undertake the highly unusual operation of
fiddling with the final S/G element, to enforce 32-bit alignment.

Of course we could eliminate all that nasty fiddling/padding
completely, including sg_last(), if other areas of the kernel would
guarantee ahead of time that buffer lengths are always a multiple
of 4

Jeff

OK Now I'm confused. I thought that ULD's can give you SG's 
that are actually longer than bufflen and that, at the end, the 
bufflen should govern the transfer length.


Now FS_PC commands are sector aligned so you do not have
problems with that.

The BLOCK_PC commands have 2 main sources that I know of
one is sg  bsg from user mode that can easily enforce
4 bytes alignment. The second is kernel services which 80%
of these are done by scsi_execute(). All These can be found
and fixed. Starting with scsi_execute(). Another place can be
blk_rq_map_sg(), since all IO's are bio based. It can enforce 
alignment too.


I would start by sticking a WARN_ON(qc-pad_len) and
see if it triggers, what are the sources of that.


The whole qc-pad_len etc. machinery was added because it solved 
problems in the field with ATAPI devices.  So sr or some userland 
application is sending lengths that are not padded to 32-bit boundary, 
probably because plenty of trivial commands can send or return odd 
amounts of data.


This used to be irrelevant, but now with SATA, even PIO data xfer 
(normally what is used for non-READ/WRITE CDBs) must be 32-bit aligned 
because both SATA DMA and SATA PIO are converted into dword-based SATA 
FIS's on the wire.


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: Fix ATAPI transfer lengths causes CD writing regression

2007-10-31 Thread Jeff Garzik

Jens Axboe wrote:

On Wed, Oct 31 2007, Alan Cox wrote:

On Tue, 30 Oct 2007 19:21:29 +
Daniel Drake [EMAIL PROTECTED] wrote:


Alan Cox wrote:

I would guess Brasero is issuing a command with the length of data
wrongly set. In the old code that might well just produce errors of the
Umm wtf is this data left over for ?, with the new code the drive is
likely to change state as it knows the transfer size and that will
*correctly* cause an HSM error and what follows.

Now the question is who gets the length wrong - Brasero or the ata
translation code in libata
Brasero does exactly the same as my test app which I attached to my last 
mail. Is my test app wrong?

Would need to double check the SCSI specificatons to be sure but I think
you are asking for less data than the drive wishes to provide. You
aren't allowed to do that with ATA.


ide-cd handles this by throwing the excess away, which I think is the
sane way to do this.


That's easy for the PIO case.  But CD writing is normally DMA, which 
means you will get a DMA engine exception if the device wants to give 
you more data than the scatter/gather entries permit.


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: Fix ATAPI transfer lengths causes CD writing regression

2007-10-31 Thread Jeff Garzik

Jens Axboe wrote:

Right, that's of course problematic... There has to be a way to recover
that situation though, or you can't export any user command issue
facility.


You cannot hope to handle all possible effects arising from an app 
providing an invalid sg header / cdb.


Once you start talking recovery you are already screwed:  we are 
talking about low-level hardware commands that are passed straight to 
the hardware.  It is trivial to lock up hardware, brick hardware, and 
corrupt data at that level.



If this is NOT a privileged app, we must update the command validation 
to ensure that invalid commands are not transported to the hardware.


If this is a privileged app, our work is done.  Fix the app.  We gave 
root rope, and he took it.



I even venture to say that accept anything, clean up afterwards is 
/impossible/ to implement, in addition to being dangerous.


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: Other Problems with Marvell Driver - 7042 (2.6.23)...

2007-10-31 Thread Jeff Garzik

Morrison, Tom wrote:

I am running the 'latest' kernel retrieved from Kumar Gala's
Powerpc git tree (mainly because I am running on a MPC8548 board) 
and it appears to be in the full 2.6.23 version while the sata_mv 
driver version seems to be 1.01. 


I have searched the archives, and there has been some discussion
of a regression in the Marvell driver, but I seem to have a 
different symptom from Olaf's (I am not running a 64bit kernel - 
and I am *not* running with large (36bit) physical and resources - 
so I wouldn't have the same type of IOMMU issues)


Below in the error output (using ATA_DEBUG) the driver sees the 
two drives (3 partitions on /dev/sda  2 partitions on /dev/sdb)

and apparently sets up everything correctly for standard access...

But, when I try to mount a partition - it freezes up and gets I/O
errors?
Can someone give me any hints of things to try?


First you need to try 2.6.23.1 rather than 2.6.23, because it contains a 
critical sata_mv fix.


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


[PATCH] libata ATAPI transfer size cleanups

2007-11-01 Thread Jeff Garzik

This is purely for comment and testing, not for merging (yet?).

A common recipe in several vendor drivers (either GPL'd, or I have NDA'd
access to use them as a documentation-like reference) for ATAPI was
slightly different from ours.  This recipe can be found in
atapi_tf_xfer_size(), and it's slightly different from Alan's.

This patch also adds some byte count checks, consolidated
ata_pio_use_silly() use, and adds a dmadir-related FIXME.

The 'xfer-size' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git xfer-size

to receive the following updates:

 drivers/ata/libata-core.c |   53 +
 drivers/ata/libata-eh.c   |   11 +
 drivers/ata/libata-scsi.c |   48 +---
 drivers/ata/libata.h  |7 ++
 4 files changed, 72 insertions(+), 47 deletions(-)

Jeff Garzik (1):
  [libata] Standardize ATAPI byte count [limit] handling

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 63035d7..5c616a8 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5258,6 +5258,55 @@ next_sg:
goto next_sg;
 }
 
+void atapi_tf_xfer_size(struct ata_taskfile *tf,
+   unsigned int total_xfer_len,
+   bool dma)
+{
+   if (dma) {
+   tf-protocol = ATA_PROT_ATAPI_DMA;
+   tf-feature |= ATAPI_PKT_DMA;
+   tf-lbam = 0;
+   tf-lbah = 0;
+   } else {
+   tf-protocol = ATA_PROT_ATAPI;
+   tf-feature = ~ATAPI_PKT_DMA;
+   if (total_xfer_len = 0x) {
+   tf-lbam = total_xfer_len;
+   tf-lbah = total_xfer_len  8;
+   } else {
+   tf-lbam = 0xff;
+   tf-lbah = 0xff;
+   }
+   }
+}
+
+static bool atapi_valid_bcount(const struct ata_taskfile *tf,
+  const struct ata_taskfile *result_tf)
+{
+   unsigned int ibyte, obyte, lo, hi;
+
+   lo = tf-lbam;
+   hi = tf-lbam;
+   ibyte = (hi  8) | lo;
+
+   lo = result_tf-lbam;
+   hi = result_tf-lbam;
+   obyte = (hi  8) | lo;
+
+   if (unlikely(obyte  ibyte))
+   return false;
+   if (unlikely(obyte == 0))
+   return false;
+   if (unlikely(obyte  0xfffe))
+   return false;
+
+   /* another test, omitted due to lack of data point:
+* obyte should be even, if it is not the last DRQ block
+*/
+
+   return true;
+}
+
 /**
  * atapi_pio_bytes - Transfer data from/to the ATAPI device.
  * @qc: Command on going
@@ -5282,6 +5331,10 @@ static void atapi_pio_bytes(struct ata_queued_cmd *qc)
 * So, the correctness of qc-result_tf is not affected.
 */
ap-ops-tf_read(ap, qc-result_tf);
+
+   if (!atapi_valid_bcount(qc-tf, qc-result_tf))
+   goto err_out;
+
ireason = qc-result_tf.nsect;
bc_lo = qc-result_tf.lbam;
bc_hi = qc-result_tf.lbah;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 8d64f8f..505b2e9 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1352,16 +1352,7 @@ static unsigned int atapi_eh_request_sense(struct 
ata_queued_cmd *qc)
 
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
tf.command = ATA_CMD_PACKET;
-
-   /* is it pointless to prefer PIO for safety reasons? */
-   if (ap-flags  ATA_FLAG_PIO_DMA) {
-   tf.protocol = ATA_PROT_ATAPI_DMA;
-   tf.feature |= ATAPI_PKT_DMA;
-   } else {
-   tf.protocol = ATA_PROT_ATAPI;
-   tf.lbam = (8 * 1024)  0xff;
-   tf.lbah = (8 * 1024)  8;
-   }
+   atapi_tf_xfer_size(tf, SCSI_SENSE_BUFFERSIZE, ata_pio_use_silly(ap));
 
return ata_exec_internal(dev, tf, cdb, DMA_FROM_DEVICE,
 sense_buf, SCSI_SENSE_BUFFERSIZE, 0);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index fc89590..bec28c2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2313,12 +2313,6 @@ static void atapi_sense_complete(struct ata_queued_cmd 
*qc)
ata_qc_free(qc);
 }
 
-/* is it pointless to prefer PIO for safety reasons? */
-static inline int ata_pio_use_silly(struct ata_port *ap)
-{
-   return (ap-flags  ATA_FLAG_PIO_DMA);
-}
-
 static void atapi_request_sense(struct ata_queued_cmd *qc)
 {
struct ata_port *ap = qc-ap;
@@ -2346,15 +2340,9 @@ static void atapi_request_sense(struct ata_queued_cmd 
*qc)
 
qc-tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
qc-tf.command = ATA_CMD_PACKET;
+   atapi_tf_xfer_size(qc-tf, SCSI_SENSE_BUFFERSIZE,
+  ata_pio_use_silly(ap));
 
-   if (ata_pio_use_silly(ap)) {
-   qc-tf.protocol = ATA_PROT_ATAPI_DMA;
-   qc-tf.feature |= ATAPI_PKT_DMA

Re: Fix ATAPI transfer lengths causes CD writing regression

2007-11-01 Thread Jeff Garzik

Ok, gave this a hard look.

This is basically a behavior change with regards to how we program the 
bcount(low) and bcount(high) registers.


Issues about FIFO draining and devices returning too-much data are 
ultimately tangential.  Furthermore, this is an ATAPI PIO issue, as 
demonstrated by (a) Alan's patch did not change DMA lbam/lbah 
programming and (b) Daniel's report of the message ata2.00: 66 bytes 
trailing data which occurs in the PIO state machine.


To survey the behaviors for ATAPI PIO:

ide-cd: read/write commands,blimit = 32k
others cmds,blimit = xfer_len

old libata (pre-Alan):  blimit = 8k

new libata behavior:blimit = xfer_len

thus Alan's patch was moving us CLOSER to ide-cd's behavior (if we 
ignore read/write commands for the moment, which are not at issue).


and the end result is that the change from old-libata to new-libata 
behavior broke Daniel's case, which is a device known to ignore the SCSI 
command CDB's allocation length field.


Additionally, let's add some background:

libata was originally written for first-gen SATA controllers (incl. 
first-gen SATA bridges), most notably ata_piix, the controller Daniel is 
using.


I chose blimit 8k because I felt that matches the maximum size of a SATA 
Data FIS.  I felt -- this was a wild-added guess -- that setting blimit 
thusly would properly configure all the magic internal FIFOs and data 
buffers in silicon that handled these crazy ATAPI devices with random 
transfer lengths.


I am not drawing any conclusions yet, but I'm thinking that blimit=8k 
may be a better choice for SATA ATAPI.


Other comment:

* as Tejun noted in IRC, we don't have a clear idea of what triggered 
the HSM violation.  turning on debugging printk's would help, but 
unfortunately that means turning them on for everything, including hard 
drives.


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 ATAPI transfer size cleanups

2007-11-01 Thread Jeff Garzik

Alan Cox wrote:

On Thu, 1 Nov 2007 05:07:33 -0400
Jeff Garzik [EMAIL PROTECTED] wrote:


This is purely for comment and testing, not for merging (yet?).

A common recipe in several vendor drivers (either GPL'd, or I have NDA'd
access to use them as a documentation-like reference) for ATAPI was
slightly different from ours.  This recipe can be found in
atapi_tf_xfer_size(), and it's slightly different from Alan's.


Looks mostly good. Might cause breakage on one or two controllers by
setting lbam/lbah to 0 for DMA but we can test that and find out.


DMA always zeroed lbam/lbah thanks to memset(), my patch merely made it 
explicit.




Note however - the scheme in use now has been tested for over ten years
in drivers/ide. The scheme below has not...


See my other email just posted, differences and errata abound :)

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: Fix ATAPI transfer lengths causes CD writing regression

2007-11-01 Thread Jeff Garzik

Alan Cox wrote:
demonstrated by (a) Alan's patch did not change DMA lbam/lbah 
programming and (b) Daniel's report of the message ata2.00: 66 bytes 


(a) did. Well the original did, dunno about your version.


We are both half-right.  I reverted my version of that completely, 
applied your version verbatim, and pushed it upstream.  The result: 
atapi_xlat _did_ start programming lbam/lbah for DMA (another behavior 
change), but request-sense DMA path was not changed to program lba[mh] 
for DMA.



I am not drawing any conclusions yet, but I'm thinking that blimit=8k 
may be a better choice for SATA ATAPI.


Only if your transfer is actually 8K or more.


No, that's precisely the problem cause for what Daniel is reporting.  We 
are setting blimit=xfer_len(==10), when the device wants to return more 
than 10 bytes.  When set to the old limit (8k), the problem didn't occur.


Prior to this change, __atapi_pio_bytes() happily discarded trailing 
data, so the software already knows how to eat trailing data left to us 
by the ATAPI device.  The printk (and lack of problem) indicated that 
this code was active and working fine.


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: Suspend to ram regression (2.6.24-rc1-git)

2007-11-01 Thread Jeff Garzik

Jens Axboe wrote:

Reverting just the default AHCI flags makes it work again. IOW, with the
below patch I can suspend properly with current -git.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ed9b407..77f7631 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -190,8 +190,7 @@ enum {
 
 	AHCI_FLAG_COMMON		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |

  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
- ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
- ATA_FLAG_IPM,
+ ATA_FLAG_ACPI_SATA | ATA_FLAG_AN,
AHCI_LFLAG_COMMON   = ATA_LFLAG_SKIP_D2H_BSY,



sounds like the easy thing to do, in light of this breakage, might be to 
default it to off, add a module parameter turning it on by setting that 
flag.


-
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: Suspend to ram regression (2.6.24-rc1-git)

2007-11-01 Thread Jeff Garzik

Jens Axboe wrote:

On Thu, Nov 01 2007, Jeff Garzik wrote:

Jens Axboe wrote:

Reverting just the default AHCI flags makes it work again. IOW, with the
below patch I can suspend properly with current -git.
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ed9b407..77f7631 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -190,8 +190,7 @@ enum {
AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
- ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
- ATA_FLAG_IPM,
+ ATA_FLAG_ACPI_SATA | ATA_FLAG_AN,
AHCI_LFLAG_COMMON   = ATA_LFLAG_SKIP_D2H_BSY,


sounds like the easy thing to do, in light of this breakage, might be to 
default it to off, add a module parameter turning it on by setting that 
flag.


Wouldn't it be better to just get this bug fixed? IOW, is there a reason
for disabling ALPM if it's Bug Free?

I'd suggest committing the patch disabling IPM, then Kristen can debug
the thing in piece in quiet. Once confident it works with ahci again, we
can revert that commit.


Right -- if you are going to commit a patch disabling it, it is best 
to do so via a simple module option, which allows users to easily try 
the feature in parallel with Intel's debugging.


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: sata_nv: ata2 errors

2007-11-01 Thread Jeff Garzik

Andrew Morton wrote:

On Thu, 25 Oct 2007 13:18:50 +0530
Amit Shah [EMAIL PROTECTED] wrote:


On 2.6.24-rc1 (AMD Opteron 1216), my sata_nv driver produces this
output on bootup:

ata2: EH in SWNCQ mode,QC:qc_active 0x1 sactive 0x1
ata2: SWNCQ:qc_active 0x1 defer_bits 0x0 last_issue_tag 0x0
ata2: ATA_REG 0x40 ERR_REG 0x0
ata2: tag : dhfis dmafis sdbfis sacitve
ata2: tag 0x0: 0 1 0 0
ata2.00: exception Emask 0x0 SAct 0x1 SErr 0x0 action 0x6 frozen
...
ata2.00: status: { DRDY }



Are any other problems observeable?



2.6.24-rc1 is missing the fix for this that is upstream,
360737a982b1ae09e1659e0bb27085c03f02f404

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 ATAPI transfer size cleanups

2007-11-01 Thread Jeff Garzik

Torsten Kaiser wrote:

On 11/1/07, Jeff Garzik [EMAIL PROTECTED] wrote:

+   lo = tf-lbam;
+   hi = tf-lbam;
+   ibyte = (hi  8) | lo;
+
+   lo = result_tf-lbam;
+   hi = result_tf-lbam;


That doesn't look right.
I suspect this was intended:

lo = tf-lbam;
hi = tf-lbah;


Agreed, will correct.

Thanks,

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 1/3] libata: implement dev-acpi_init_gtm

2007-11-02 Thread Jeff Garzik

Tejun Heo wrote:

Add dev-acpi_init_gtm and store initial GTM values on host
initialization.  If the field is valid, ATA_PFLAG_INIT_GTM_VALID flag
is set.  This is to remember BIOS/firmware programmed initial timing
for later use before reset and mode configuration modify it.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]


It sounds like pata_via and pata_amd need a foo_save_initial_config() 
much like AHCI, during which they would fill ppriv-init_gtm rather than 
ap-init_gtm.  Thoughts?  Both drivers are calling ata_acpi_cbl() 
themselves, permitting the possibility of ppriv-init_gtm.


This avoids forcing everyone else to bear the memory cost in ata_port 
for just these two drivers.


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: pata_serverworks hangs

2007-11-02 Thread Jeff Garzik

Krzysztof Oledzki wrote:

Hello,

I just tried to switch from old IDE to libata but unfortunately kernel 
(2.6.22.10) hangs during boot:


does 2.6.23.1 or 2.6.24-rc1 work?

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: Fix ATAPI transfer lengths causes CD writing regression

2007-11-02 Thread Jeff Garzik

Daniel Drake wrote:
Again, ignore me if I'm not contributing anything useful, but I'm 
increasingly thinking that the SG_IO command block in question is 
perfectly valid, and doing a short read of the mode page in question 
(and probably others too) is in fact required before you can know its 
true size to do a full read anyway.



Correct -- I do not see anything wrong with the SCSI command being 
passed to the device.


It is quite normal to request an allocation length less than the amount 
of data to be returned -- often this is even a requirement, in order for 
the SCSI client to see how much data buffer is must allocate for the 
returned data.


This technique is used all over SCSI, both in userland and in the 
kernel.  Nothing wrong with the test app AFAICS.


This behavior can be adequated summarized as a drive firmware bug, where 
the device will puke when both SCSI allocation length and ATA byte count 
limit are below the amount of data it wishes to return.


At this point I think we're interested to see the output from Tejun's 
debugging patch, posted in this thread.


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 1/3] libata: implement dev-acpi_init_gtm

2007-11-03 Thread Jeff Garzik

Tejun Heo wrote:

Tejun Heo wrote:

Jeff Garzik wrote:

Tejun Heo wrote:

Add dev-acpi_init_gtm and store initial GTM values on host
initialization.  If the field is valid, ATA_PFLAG_INIT_GTM_VALID flag
is set.  This is to remember BIOS/firmware programmed initial timing
for later use before reset and mode configuration modify it.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]

It sounds like pata_via and pata_amd need a foo_save_initial_config()
much like AHCI, during which they would fill ppriv-init_gtm rather than
ap-init_gtm.  Thoughts?  Both drivers are calling ata_acpi_cbl()
themselves, permitting the possibility of ppriv-init_gtm.

This avoids forcing everyone else to bear the memory cost in ata_port
for just these two drivers.

The memory overhead is pretty small and more importantly there isn't a
good place to load initial GTM.  ACPI is associated with the host during
host registration after all private host initialization is complete.
When the EH gets invoked, the first thing which is done is forcing PIO0
before initial reset.

Unless we add another hook, the only place to put this is in private
-error_handler().  A driver can check whether it's being called for the
first time and record it in private structure, which isn't too pretty,
so I thought doing it this way was fair tradeoff.


Jeff, do you agree or still think it's better done in LLDs?  I'm okay
either way.


I trust your judgement...  My reply was more of a gut feeling sort of 
thing.


In general, I want to keep libata as close as possible to the LLD 
call-out model as possible:  LLD registers itself with system services 
[perhaps using generic helpers to do so].  LLD fills in its own scsi 
host template and ata_port_operations [perhaps using generic helpers to 
do so].  LLD implements specific helpers that control LLD-specific 
behavior, calling out to libata helpers as need arises.


So I tend to prefer adding direct LLD control points to ATA_FLAG_xxx 
control points or other solutions, if the situation permits me to choose.


If you have to ask...  I will always prefer it (for some value of...) 
to be done in the LLD.  ;-)


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: Fix ATAPI transfer lengths causes CD writing regression

2007-11-03 Thread Jeff Garzik

Tejun Heo wrote:

Does this patch fix the problem?

[snip]

I hope it does...  that patch removes a call to sg_last() and in general 
cleans up the function a bit, which is nice.


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 01/12] ata/sata_fsl: Update for ata_link introduction

2007-11-03 Thread Jeff Garzik

Li Yang wrote:

Update the driver to use the newly added ata_link structure.

Signed-off-by: Li Yang [EMAIL PROTECTED]
---
 drivers/ata/sata_fsl.c |   31 ++-
 1 files changed, 18 insertions(+), 13 deletions(-)


applied patches 1-12 to #upstream-fixes (2.6.24), thanks much!


-
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: suppress two warnings

2007-11-03 Thread Jeff Garzik

Stephen Rothwell wrote:

drivers/ata/libata-core.c:768: warning: 'ata_lpm_enable' defined but not used
drivers/ata/libata-core.c:784: warning: 'ata_lpm_disable' defined but not used

Signed-off-by: Stephen Rothwell [EMAIL PROTECTED]
---
 drivers/ata/libata-core.c |4 
 1 files changed, 4 insertions(+), 0 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: [2.6 patch] make ata_scsi_lpm_get() static

2007-11-03 Thread Jeff Garzik

Adrian Bunk wrote:

ata_scsi_lpm_get() can become static.

Signed-off-by: Adrian Bunk [EMAIL PROTECTED]

---
380046f657271be470566bb5c762c1599569bac6 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c

index 93bd36c..c4f0c6c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -120,7 +120,7 @@ static const struct {
{ MEDIUM_POWER, medium_power },
 };
 
-const char *ata_scsi_lpm_get(enum link_pm policy)

+static const char *ata_scsi_lpm_get(enum link_pm policy)
 {
int i;
 


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.24-rc1] sata_promise: fix endianess bug in ASIC PRD bug workaround

2007-11-03 Thread Jeff Garzik

Mikael Pettersson wrote:

The original workaround for the Promise ASIC PRD bug
contained an endianess bug which I failed to detect:
the adjustment of the last PRD entry's length field
applied host arithmetic to little-endian data, which
is incorrect on big-endian machines.

We have the length available in host-endian format, so
do the adjustment on host-endian data and then convert
and store it in the PRD entry's little-endian data field.

Thanks to an anonymous reviewer for detecting this bug.

Signed-off-by: Mikael Pettersson [EMAIL PROTECTED]
---
Jeff: please include this fix in any backport(s) of the
ASIC PRD bug workaround.

 drivers/ata/sata_promise.c |2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -rupN linux-2.6.24-rc1/drivers/ata/sata_promise.c 
linux-2.6.24-rc1.sata_promise-endianess-fix/drivers/ata/sata_promise.c
--- linux-2.6.24-rc1/drivers/ata/sata_promise.c 2007-10-31 11:47:13.0 
+0100
+++ linux-2.6.24-rc1.sata_promise-endianess-fix/drivers/ata/sata_promise.c  
2007-10-31 11:47:25.0 +0100
@@ -585,7 +585,7 @@ static void pdc_fill_sg(struct ata_queue
VPRINTK(Splitting last PRD.\n);
 
 			addr = le32_to_cpu(ap-prd[idx - 1].addr);

-   ap-prd[idx - 1].flags_len -= 
cpu_to_le32(SG_COUNT_ASIC_BUG);
+   ap-prd[idx - 1].flags_len = cpu_to_le32(len - 
SG_COUNT_ASIC_BUG);
VPRINTK(PRD[%u] = (0x%X, 0x%X)\n, idx - 1, addr, 
SG_COUNT_ASIC_BUG);
 
 			addr = addr + len - SG_COUNT_ASIC_BUG;

-


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] libata: increase 128 KB / cmd limit for ATAPI tape drives

2007-11-03 Thread Jeff Garzik

Tony Battersby wrote:

Commands sent to ATAPI tape drives via the SCSI generic (sg) driver are
limited in the amount of data that they can transfer by the max_sectors
value.  The max_sectors value is currently calculated according to the
command set for disk drives, which doesn't apply to tape drives.  The
default max_sectors value of 256 limits ATAPI tape drive commands to
128 KB.  This patch against 2.6.24-rc1 increases the max_sectors value
for tape drives to 65535, which permits tape drive commands to transfer
just under 32 MB.

Tested with a SuperMicro PDSME motherboard, AHCI, and a Sony SDX-570V
SATA tape drive.

Note that some of the chipset drivers also set their own max_sectors
value, which may override the value set in libata-core.  I don't have
any of these chipsets to test, so I didn't go messing with them.  Also,
ATAPI devices other than tape drives may benefit from similar changes,
but I have only tape drives and disk drives to test.

Signed-off-by: Tony Battersby [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 1/6] libata: fix timing computation in ata_eh_reset()

2007-11-03 Thread Jeff Garzik

Tejun Heo wrote:

As jiffies changes asynchronously, it needs to be cached if unchanging
timestamp is needed.  The code in ata_eh_reset() intended to do that
with @now but never actually did it.  Fix it.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
 drivers/ata/libata-eh.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 496edaf..f1c0117 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2184,7 +2184,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
unsigned long now = jiffies;
 
 		if (time_before(now, deadline)) {

-   unsigned long delta = deadline - jiffies;
+   unsigned long delta = deadline - now;


applied 1-6 to #upstream-fixes


-
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: Bart's efforts?

2007-11-03 Thread Jeff Garzik

Greg Freemyer wrote:

Jeff / Alan,

I mostly lurk here, but I know there is a long term effort/ToDo to
move libata away from the SCSI infrastructure.

I've also seen that Bart has been making a large number of
improvements to drivers/ide over the last little while.  From my
limited perspective, many changes appear to be to the core
infrastructure.

My question is if the drivers/ide infrastructure is slowly moving in
the direction of being leverageable by libata when/if it moves out of
scsi.  Or does the drivers/ide code simply have the wrong kind of
plumbing for libata to ever use.


One of my key goals with libata was to make a driver look like a 
driver and greatly improve upon the IDE driver API, which was a 
complete and utter piece of garbage (this is no reflection on Bart, he 
inherited it).  As a result, each driver is a fully fledged 
PCI/SCSI/ATA/platform driver, without anything getting in the way.  That 
made things like controller hotplug trivial to support from day one.


Bart has definitely made many good improvements, but I don't think 
people would be surprised that I feel CONFIG_IDE is legacy...


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: add ACPI _PSx method

2007-11-03 Thread Jeff Garzik

Shaohua Li wrote:

Not sure, this is just to call a BIOS routine, but a check should be
safer. Thanks!

ACPI spec (ver 3.0a, p289) requires IDE power on/off executes ACPI _PSx
methods. As recently most PATA drivers use libata, this patch adds _PSx
method support in libata. ACPI spec doesn't mention if SATA requires the
same _PSx method.

Signed-off-by: Shaohua Li [EMAIL PROTECTED]
Acked-by: Len Brown [EMAIL PROTECTED]



What is the safety factor on this patch?  :)

Since we are into 2.6.24-rc, my preference would be to let this get some 
testing in -mm, and push it upstream for 2.6.25.  Comments?


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


[git patches] libata fixes

2007-11-03 Thread Jeff Garzik
Fixes, fixes and more fixes.

Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git 
upstream-linus

to receive the following updates:

 drivers/ata/libata-core.c  |   38 ---
 drivers/ata/libata-eh.c|  148 ++--
 drivers/ata/libata-scsi.c  |2 +-
 drivers/ata/sata_fsl.c |  159 +---
 drivers/ata/sata_promise.c |2 +-
 include/linux/ata.h|6 ++
 6 files changed, 149 insertions(+), 206 deletions(-)

Adrian Bunk (1):
  make ata_scsi_lpm_get() static

Jeff Garzik (4):
  ata/sata_fsl: Remove unnecessary SCR cases
  ata/sata_fsl: cleanup needless casts to/from void __iomem *
  ata/sata_fsl: remove unneeded on-stack copy of FIS
  ata/sata_fsl: remove unneeded sata_fsl_hardreset()

Li Yang (5):
  ata/sata_fsl: Update for ata_link introduction
  ata/sata_fsl: Remove deprecated hooks
  ata/sata_fsl: save irq in private data for irq unmapping
  ata/sata_fsl: Kill ata_sg_is_last()
  ata/sata_fsl: cleanup style problem

Mikael Pettersson (1):
  sata_promise: fix endianess bug in ASIC PRD bug workaround

Stephen Hemminger (1):
  libata: fix docbook

Stephen Rothwell (1):
  libata: suppress two warnings

Tejun Heo (6):
  libata: fix timing computation in ata_eh_reset()
  libata: cosmetic clean up / reorganization of ata_eh_reset()
  libata: more robust reset failure handling
  libata: consider errors not associated with commands for speed down
  libata: request PHY speed configuration on SControl access failure
  libata: don't configure downstream links faster than the upstream link

Tony Battersby (1):
  libata: increase 128 KB / cmd limit for ATAPI tape drives

ashish kalra (3):
  ata/sata_fsl: Move MPC8315DS link speed limit workaround to specific ifdef
  ata/sata_fsl: Remove sending LOG EXT command in sata_fsl_softreset()
  ata/sata_fsl: Remove ata_scsi_suspend/resume callbacks

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 63035d7..164c7d9 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -704,8 +704,8 @@ static int ata_dev_set_dipm(struct ata_device *dev, enum 
link_pm policy)
 
 /**
  * ata_dev_enable_pm - enable SATA interface power management
- * @device - device to enable ipm for
- * @policy - the link power management policy
+ * @dev:  device to enable power management
+ * @policy: the link power management policy
  *
  * Enable SATA Interface power management.  This will enable
  * Device Interface Power Management (DIPM) for min_power
@@ -735,9 +735,10 @@ enable_pm_out:
return /* rc */;/* hopefully we can use 'rc' eventually */
 }
 
+#ifdef CONFIG_PM
 /**
  * ata_dev_disable_pm - disable SATA interface power management
- * @device - device to enable ipm for
+ * @dev: device to disable power management
  *
  * Disable SATA Interface power management.  This will disable
  * Device Interface Power Management (DIPM) without changing
@@ -755,6 +756,7 @@ static void ata_dev_disable_pm(struct ata_device *dev)
if (ap-ops-disable_pm)
ap-ops-disable_pm(ap);
 }
+#endif /* CONFIG_PM */
 
 void ata_lpm_schedule(struct ata_port *ap, enum link_pm policy)
 {
@@ -764,6 +766,7 @@ void ata_lpm_schedule(struct ata_port *ap, enum link_pm 
policy)
ata_port_schedule_eh(ap);
 }
 
+#ifdef CONFIG_PM
 static void ata_lpm_enable(struct ata_host *host)
 {
struct ata_link *link;
@@ -789,6 +792,7 @@ static void ata_lpm_disable(struct ata_host *host)
ata_lpm_schedule(ap, ap-pm_policy);
}
 }
+#endif /* CONFIG_PM */
 
 
 /**
@@ -2300,6 +2304,10 @@ int ata_dev_configure(struct ata_device *dev)
dev-max_sectors = ATA_MAX_SECTORS;
}
 
+   if ((dev-class == ATA_DEV_ATAPI) 
+   (atapi_command_packet_set(id) == TYPE_TAPE))
+   dev-max_sectors = ATA_MAX_SECTORS_TAPE;
+
if (dev-horkage  ATA_HORKAGE_MAX_SEC_128)
dev-max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
 dev-max_sectors);
@@ -2743,17 +2751,27 @@ int sata_down_spd_limit(struct ata_link *link)
 
 static int __sata_set_spd_needed(struct ata_link *link, u32 *scontrol)
 {
-   u32 spd, limit;
+   struct ata_link *host_link = link-ap-link;
+   u32 limit, target, spd;
+
+   limit = link-sata_spd_limit;
 
-   if (link-sata_spd_limit == UINT_MAX)
-   limit = 0;
+   /* Don't configure downstream link faster than upstream link.
+* It doesn't speed up anything and some PMPs choke on such
+* configuration.
+*/
+   if (!ata_is_host_link(link)  host_link-sata_spd)
+   limit = (1  host_link-sata_spd) - 1;
+
+   if (limit == UINT_MAX)
+   target = 0;
else

[git patches] Add and use media change notification

2007-11-04 Thread Jeff Garzik

The end to CD-ROM polling...  newer SATA ATAPI hardware will emit
'asynchronous notification' events when media is changed.  This adds
support.

The libata change /using/ the infrastructure is tiny, encompassed
entirely in the first file patched, drivers/ata/libata-scsi.c.

Been working since August (and in -mm), but has gone through various
revisions to try and make SCSI maintainer happy.  Every step of the way,
it was upstream-mergeable in my opinion.

Everyone agrees this should go upstream; indeed, the key change from
me [SCSI] add asynchronous event notification API is even found
in jejb/scsi-misc-2.6.git (db0e2c76011f25f22f71e9c0d69ce95d15f21e8a).

There are minor disagreements with regards to userspace API, with a
more complex solve all your problems change currently being discussed,
blocked behind sysfs changes that don't exist yet (and are still being
discussed).

I am ready and waiting to address further feedback -- each feedback
I received so far was addressed in under 24 hours -- but let us not
delay this anymore.

Changes can and should be iterated upstream... that is what the bug fix
process is for...  getting final feedback and polish on working, testing
changes that have been waiting in the wings for months.

Please pull from 'an' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git an

to receive the following updates:

 drivers/ata/libata-scsi.c  |8 ++-
 drivers/scsi/scsi_lib.c|  136 
 drivers/scsi/scsi_scan.c   |3 +
 drivers/scsi/scsi_sysfs.c  |   47 +++
 include/scsi/scsi_device.h |   25 
 5 files changed, 216 insertions(+), 3 deletions(-)


commit f26792d5c63344e14540ced4b19deb29e360bb8d
Author: Jeff Garzik [EMAIL PROTECTED]
Date:   Mon Oct 29 17:18:39 2007 -0400

[libata] Utilize new SCSI event infrastructure

An end to CD-ROM polling (if you have a device that supports AN)...
hooray!

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]

commit a341cd0f6a0fde1f85fec9aa8f81f824ea4a3f92
Author: Jeff Garzik [EMAIL PROTECTED]
Date:   Mon Oct 29 17:15:22 2007 -0400

SCSI: add asynchronous event notification API

Originally based on a patch by Kristen Carlson Accardi @ Intel.
Copious input from James Bottomley.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 245057d..94144ed 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -841,6 +841,9 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
}
 
+   if (dev-flags  ATA_DFLAG_AN)
+   set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);
+
if (dev-flags  ATA_DFLAG_NCQ) {
int depth;
 
@@ -3296,10 +3299,9 @@ static void ata_scsi_handle_link_detach(struct ata_link 
*link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-#ifdef OTHER_AN_PATCHES_HAVE_BEEN_APPLIED
if (dev-sdev)
-   scsi_device_event_notify(dev-sdev, SDEV_MEDIA_CHANGE);
-#endif
+   sdev_evt_send_simple(dev-sdev, SDEV_EVT_MEDIA_CHANGE,
+GFP_ATOMIC);
 }
 
 /**
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 88de771..0e81e4c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2115,6 +2115,142 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)
 EXPORT_SYMBOL(scsi_device_set_state);
 
 /**
+ * sdev_evt_emit - emit a single SCSI device uevent
+ * @sdev: associated SCSI device
+ * @evt: event to emit
+ *
+ * Send a single uevent (scsi_event) to the associated scsi_device.
+ */
+static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
+{
+   int idx = 0;
+   char *envp[3];
+
+   switch (evt-evt_type) {
+   case SDEV_EVT_MEDIA_CHANGE:
+   envp[idx++] = SDEV_MEDIA_CHANGE=1;
+   break;
+
+   default:
+   /* do nothing */
+   break;
+   }
+
+   envp[idx++] = NULL;
+
+   kobject_uevent_env(sdev-sdev_gendev.kobj, KOBJ_CHANGE, envp);
+}
+
+/**
+ * sdev_evt_thread - send a uevent for each scsi event
+ * @work: work struct for scsi_device
+ *
+ * Dispatch queued events to their associated scsi_device kobjects
+ * as uevents.
+ */
+void scsi_evt_thread(struct work_struct *work)
+{
+   struct scsi_device *sdev;
+   LIST_HEAD(event_list);
+
+   sdev = container_of(work, struct scsi_device, event_work);
+
+   while (1) {
+   struct scsi_event *evt;
+   struct list_head *this, *tmp;
+   unsigned long flags;
+
+   spin_lock_irqsave(sdev-list_lock, flags);
+   list_splice_init(sdev-event_list, event_list);
+   spin_unlock_irqrestore(sdev-list_lock, flags

Re: Suspend to ram regression (2.6.24-rc1-git)

2007-11-05 Thread Jeff Garzik

Jens Axboe wrote:

On Fri, Nov 02 2007, Kristen Carlson Accardi wrote:

On Thu, 1 Nov 2007 09:41:46 +0100
Jens Axboe [EMAIL PROTECTED] wrote:


On Wed, Oct 31 2007, Jens Axboe wrote:

Hi,

My x60 stopped suspending about two days ago. It just freezes after
printing

Suspending console(s)

where it would normally turn everything off and the 'moon' light would
go on. Posting this message in case somebody else knows what is up, if
not I'll do a bisect on it tomorrow.

Did the bisect, it points to this commit:

1556594f913fa81d008cecfe46d7211c919a853 is first bad commit
commit 31556594f913fa81d008cecfe46d7211c919a853
Author: Kristen Carlson Accardi [EMAIL PROTECTED]
Date:   Thu Oct 25 01:33:26 2007 -0400

[libata] AHCI: add hw link power management support

Booting any kernel after this commit fails suspending to ram, it just
sits there forever.

--
Jens Axboe


Does this patch fix your problem?  It seems to get hung up while disabling
DIPM, and after thinking about this a bit, I don't think we really need
to do this anyway.


Yep, works for me! Can we get this expedited upstream?


If everybody's happy with it, I've got it ready for upstream in 
libata-dev.git#dlpm-fix


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: [git patches] Add and use media change notification

2007-11-05 Thread Jeff Garzik

Linus Torvalds wrote:


On Sun, 4 Nov 2007, Jeff Garzik wrote:

The end to CD-ROM polling...  newer SATA ATAPI hardware will emit
'asynchronous notification' events when media is changed.  This adds
support.


I *really* didn't want to pull this.

Not only is it after the -rc1 period, but I also think you pushed this ina 
really offensive manner, and I don't like how you and James have made this 
whole thing personal. You guys need to sort it out, and there is no way 
you can blame James for all your troubles, since I've heard the same kind 
of complaints about every single maintainer out there (including you) 
about some driver or other infrastructure issue.


So I'm unhappy about pulling this.

That said, I did finally decide to just pull it. Partly because the 
concept apparently has been in -mm for a while anyway (even if the final 
patch has not - but the patch itself isn't that large, I'd worry more 
about thngs like certain hardware simply not doing things right), but 
mostly because I hate it when others hold up driver features, and I 
decided that in this case this really is something that is better done 
earlier rather than later, to get exposure as soon as possible.


But I really think you need to lay off James, and help him rather than 
make every single complaint a big flame-war!


Please, Jeff?


Fair enough...

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


[git patches] libata fixes

2007-11-05 Thread Jeff Garzik

Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git 
upstream-linus

to receive the following updates:

 drivers/ata/ata_piix.c |1 +
 drivers/ata/libata-core.c  |   39 ---
 drivers/ata/pata_hpt37x.c  |   49 +--
 drivers/ata/pata_serverworks.c |   11 -
 include/linux/ata.h|   11 +
 include/linux/libata.h |1 +
 6 files changed, 84 insertions(+), 28 deletions(-)

Alan Cox (4):
  pata_serverworks: Fix problem with some drive combinations
  ata_piix: Add additional PCI identifier for 40 wire short cable
  pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable 
detect
  libata: handle broken cable reporting

Geert Uytterhoeven (1):
  libata and bogus LBA48 drives

Kristen Carlson Accardi (1):
  libata: Don't disable dipm with SET FEATURES

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index a4b2cb2..f08cca2 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -621,6 +621,7 @@ struct ich_laptop {
 static const struct ich_laptop ich_laptop[] = {
/* devid, subvendor, subdev */
{ 0x27DF, 0x0005, 0x0280 }, /* ICH7 on Acer 5602WLMi */
+   { 0x27DF, 0x1025, 0x0102 }, /* ICH7 on Acer 5602aWLMi */
{ 0x27DF, 0x1025, 0x0110 }, /* ICH7 on Acer 3682WLMi */
{ 0x27DF, 0x1043, 0x1267 }, /* ICH7 on Asus W5F */
{ 0x27DF, 0x103C, 0x30A1 }, /* ICH7 on HP Compaq nc2400 */
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 164c7d9..ec3ce12 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -676,10 +676,11 @@ static int ata_dev_set_dipm(struct ata_device *dev, enum 
link_pm policy)
if (rc)
return rc;
 
-   /* disable DIPM */
-   if (ata_dev_enabled(dev)  (dev-flags  ATA_DFLAG_DIPM))
-   err_mask = ata_dev_set_feature(dev,
-   SETFEATURES_SATA_DISABLE, SATA_DIPM);
+   /*
+* we don't have to disable DIPM since IPM flags
+* disallow transitions to SLUMBER, which effectively
+* disable DIPM if it does not support PARTIAL
+*/
break;
case NOT_AVAILABLE:
case MAX_PERFORMANCE:
@@ -689,10 +690,11 @@ static int ata_dev_set_dipm(struct ata_device *dev, enum 
link_pm policy)
if (rc)
return rc;
 
-   /* disable DIPM */
-   if (ata_dev_enabled(dev)  (dev-flags  ATA_DFLAG_DIPM))
-   err_mask = ata_dev_set_feature(dev,
-   SETFEATURES_SATA_DISABLE, SATA_DIPM);
+   /*
+* we don't have to disable DIPM since IPM flags
+* disallow all transitions which effectively
+* disable DIPM anyway.
+*/
break;
}
 
@@ -4239,6 +4241,10 @@ static const struct ata_blacklist_entry 
ata_device_blacklist [] = {
{ ST340823A,  NULL,   ATA_HORKAGE_HPA_SIZE, },
{ ST320413A,  NULL,   ATA_HORKAGE_HPA_SIZE, },
 
+   /* Devices which get the IVB wrong */
+   { QUANTUM FIREBALLlct10 05, A03.0900, ATA_HORKAGE_IVB, },
+   { TSSTcorp CDDVDW SH-S202J, SB00, ATA_HORKAGE_IVB, },
+
/* End Marker */
{ }
 };
@@ -4300,6 +4306,21 @@ static int ata_dma_blacklisted(const struct ata_device 
*dev)
 }
 
 /**
+ * ata_is_40wire   -   check drive side detection
+ * @dev: device
+ *
+ * Perform drive side detection decoding, allowing for device vendors
+ * who can't follow the documentation.
+ */
+
+static int ata_is_40wire(struct ata_device *dev)
+{
+   if (dev-horkage  ATA_HORKAGE_IVB)
+   return ata_drive_40wire_relaxed(dev-id);
+   return ata_drive_40wire(dev-id);
+}
+
+/**
  * ata_dev_xfermask - Compute supported xfermask of the given device
  * @dev: Device to compute xfermask for
  *
@@ -4368,7 +4389,7 @@ static void ata_dev_xfermask(struct ata_device *dev)
if (xfer_mask  (0xF8  ATA_SHIFT_UDMA))
/* UDMA/44 or higher would be available */
if ((ap-cbl == ATA_CBL_PATA40) ||
-   (ata_drive_40wire(dev-id) 
+   (ata_is_40wire(dev) 
(ap-cbl == ATA_CBL_PATA_UNK ||
 ap-cbl == ATA_CBL_PATA80))) {
ata_dev_printk(dev, KERN_WARNING,
diff --git a/drivers/ata/pata_hpt37x.c b/drivers/ata/pata_hpt37x.c
index e61cb1f..3816b86 100644
--- a/drivers/ata/pata_hpt37x.c
+++ b/drivers/ata/pata_hpt37x.c
@@ -295,7 +295,7 @@ static unsigned long hpt370_filter(struct ata_device *adev, 
unsigned long mask)
 
 static unsigned long hpt370a_filter(struct ata_device 

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

2007-11-06 Thread Jeff Garzik

Tejun Heo wrote:

Alan Cox wrote:

On Tue,  6 Nov 2007 14:39:03 +0900
Tejun Heo [EMAIL PROTECTED] wrote:


xfer_mask is unsigned int not unsigned long.  Change -mode_filter to
take and return unsigned int.

While at it, rename @adev of ata_pci_default_filter() to @dev for
consistency.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]

The filter type was purposefully unsigned long to allow for expansion (eg
for SWDMA) without breaking drivers. No problem with it changing but I'd
say unsigned int was the worst possible choice - its now shorter (no
room for expansion) and size dependant on arch. u32 would be shorter and
consistent across all platforms, ulong would have more room for expansion.


I agree it should be one of u* types and am happy to convert.  This
patch is primarily for consistency as in libata-core, xfer_mask is
unsigned int.  My first proposal was u32 too but Jeff vetoed it.  IIRC,
Jeff has affection for machine-independent integral types.  :-)

Anyways, I think ulong is worse than uint because ulong differs in size
between 32 and 64 archs.  What's the good of extra 32bits in flags space
if it's not there on 32bit archs?  Also, we currently only use 20bits of
xfermask, 12 extra bits seem enough for the time, especially as
everything is moving over to SATA where xfermode doesn't really matter, no?

Jeff, are you okay with u32 or 64?


unsigned long is IMO the best choice for bitmaps.

* it is a machine int on all(?) architectures

* it is 32-bit on 32-bit architectures

* it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces

* conversion to test_bit() and lib/bitmap.c interfaces as a future step 
is trivial


* your structs inflate on 64-bit due to pointers anyway, so I see little 
real consequence to using the lower 32 bits of an unsigned long as a 
portable meme.


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: Should be Acard ATP8620 2SATA / 1 IDE driver

2007-11-07 Thread Jeff Garzik
On Tue, Nov 06, 2007 at 07:25:46PM +0800, jameshsu wrote:
 Hi Jeff,
 
 Please help Acard to add this chip spec on the web site in your earlier
 conveniance.
 http://gkernel.sourceforge.net/specs/
 http://linux-ata.org/driver-status.html#open_chipsets

Updated, thanks much!


 By the way, once you complete the SATA sample driver , please inform us , so
 we could modify, test and submit in the near future.
 If any chip info still missing or need us to involve, please let me know.

I began working on a sample driver, but looking at your document, it
appears you are AHCI-compatible?

If so, we would prefer to modify drivers/ata/ahci.c to support your
hardware.  This already supports AHCI variants from Intel, NVIDIA, ATI,
VIA, JMicron and Marvell.

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: Should be Acard ATP8620 2SATA / 1 IDE driver

2007-11-07 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 05:13:55PM -0500, Jeff Garzik wrote:
 On Tue, Nov 06, 2007 at 07:25:46PM +0800, jameshsu wrote:
  Please help Acard to add this chip spec on the web site in your earlier
  conveniance.
  http://gkernel.sourceforge.net/specs/

BTW, I put the doc in 

http://gkernel.sourceforge.net/specs/acard/

Regards,

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


[PATCH] Re: Should be Acard ATP8620 2SATA / 1 IDE driver

2007-11-07 Thread Jeff Garzik
On Tue, Nov 06, 2007 at 07:25:46PM +0800, jameshsu wrote:
 By the way, once you complete the SATA sample driver , please inform us , so
 we could modify, test and submit in the near future.
 If any chip info still missing or need us to involve, please let me know.

Any chance the patch below works?

Jeff


 drivers/ata/ahci.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ed9b407..7a42b4e 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -553,6 +553,11 @@ static const struct pci_device_id ahci_pci_tbl[] = {
/* Marvell */
{ PCI_VDEVICE(MARVELL, 0x6145), board_ahci_mv },/* 6145 */
 
+   /* Acard */
+   { PCI_VDEVICE(ARTOP, 0x000D), board_ahci },
+   { PCI_VDEVICE(ARTOP, 0x000E), board_ahci },
+   { PCI_VDEVICE(ARTOP, 0x000F), board_ahci },
+
/* Generic, PCI class code for AHCI */
{ PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
  PCI_CLASS_STORAGE_SATA_AHCI, 0xff, board_ahci },
-
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: SC1200 failure in 2.6.23 and 2.6.24-rc1-git10

2007-11-07 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 02:12:55PM -0500, Mark Lord wrote:
 That cannot be correct (??).  Is this with hdparm-7.7 (latest sourceforge) 
 ??
 Can you show us the hdparm --Istdout output as well, please.

If this is applicable...  FWIW hdparm was only recently (in past 72
hours) updated from 6.9 to 7.7 in Fedora...

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 4/4] libata sata_qstor conversion to new error handling (EH).

2007-11-07 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 10:54:15AM -0500, Mark Lord wrote:
 sata_qstor conversion to new error handling (EH).
 
 Convert sata_qstor to use the newer libata EH mechanisms.
 Based on earlier work by Jeff Garzik.
 
 This belongs in 2.6.24.
 
 Signed-off-by:  Mark Lord [EMAIL PROTECTED]

Did you test this with both device probe (it works) and at least
one device error?

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 4/4] libata sata_qstor conversion to new error handling (EH).

2007-11-07 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 06:36:13PM -0500, Mark Lord wrote:
 Mark Lord wrote:
 ..
 and SG_IO WRITEs don't work.  That's really no different from
 the existing 2.6.24 version.  Regular I/O looks fine.
 ...
 
 I should qualify that:  SG_IO *PIO* WRITEs don't work.
 SG_IO PIO READs do work, and SG_IO DMA R/W both work.

multi-sector or single-sector PIO?

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] Re: Should be Acard ATP8620 2SATA / 1 IDE driver

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 07:28:38PM +0800, jameshsu wrote:
 Not really understand your patch listed below. 
 We do not provide ahci.c file to you, so we don't know where the patch came 
 from(diff from where)?? 
 Who create this and how can we get ahci.c file?? Can you tell us??

The file is found in the latest kernel source code.  Go to

http://www.kernel.org/

and download the latest full kernel version, 2.6.23:

http://www.kernel.org/pub/linux/kernel/v2.6/linux-2.6.23.tar.bz2

and then to get the latest kernel, apply this patch:

http://www.kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.24-rc2.bz2

Let me know if you need help unpacking and patching the kernel!

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: Acard ATP8620 2SATA / 1 IDE driver - AHCI.C Nov082007

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 07:34:22PM +0800, jameshsu wrote:
 From: LaurenceWu

 We didn't study about ata/ahci.c, but it should be based on AHCI1.0 or 1.1
 spec. That is, NO P.M. FIS base switching, but supports both non-NCQ and NCQ
 protocols.
 
 For NCQ or nonNCQ, 8620 is very AHCI-like, although not fully compatible,
 programmer can easily modify standard ahci.c
 for 8620. The main differences between 8620 and AHCI are :
 
 1. PRD table format changed, (please compare AHCI 1.x section 4.2.3.3 and
 8620 datasheet section 7.3), 'I' bit in 8620 is defined as 'EOT' and NO
 PRDTL value are available in the
 Command List Structure.
 
 2. For NCQ transfer, PxIS bit 3(SDBS) is changed. ATP8620 add the Reg_144h
 to accumulate 32 Sactive bits in each SDB FIS.
 The Reg_144h is RWC and all its 32 bits are 'ORed'  to form the PxIS
 bit3 and interrupt, if PxIE bit 3 enabled.
 
 Yes.  Modifying the ata/ahci.c is OK to support atp8620.

This is good information, thanks.

After studying the datasheet I also noted a couple differences:

1) Port Multiplier support appears different from standard AHCI.

2) This chip includes target mode support.  Very nice, well done!
I hope that standard AHCI eventually supports this nice feature!

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: SC1200 failure in 2.6.23 and 2.6.24-rc1-git10

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 06:44:31PM +0200, Denys Fedoryshchenko wrote:
 Doesn't help
 
 WRAP ~ #cat /proc/cmdline
 console=ttyS0,38400n8 libata.dma_mask=3

It's libata.dma if its built into the kernel, or 'dma' module option
if built as a kernel module.

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: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
  On Thu, 08 Nov 2007 17:43:59 +0100 Roberto Oppedisano [EMAIL PROTECTED] 
  wrote:
  Andrew Morton wrote, On 11/07/2007 09:13 PM:
   On Wed, 07 Nov 2007 15:15:07 +0100 Roberto Oppedisano [EMAIL 
   PROTECTED] wrote:
   Hello.
   I noticed that after suspending to ram the DVD-ROM/CDRW
   drive in no more recognized on my laptop. Looking at dmesg
   after suspend i found:
  
   [5.313446] ata2.00: _GTF unexpected object type 0x1
   [5.313453] ata2.00: ACPI on devcfg failed the second time, disabling 
   (errno=-22)
   [5.313457] ata2.00: revalidation failed (errno=1)
   [5.313459] ata2.00: disabled
  
  
   Not sure when the bug was introduced or if it has been always been there
   (but I can investigate if needed).
  
   More details on request.
  
   Following dmesg pre and after suspend.
   Yes, it would be useful if you could work ot whether this worked OK in an
   earlier kernel, thanks.
  Hello Andrew.
  The bug has been recently added.
  
  I did a git-bisect, but the result is probably not completely useful,
  because many kernel did non build with my config, and I marked them as bad.
 
 Those build errors are bad.  Please report them.  They directly prevented
 you from finding the commit which caused this regression.
 
 The only way in which we'll stop people doing crap like this is to rub
 their noses (and the person who committed its nose) in it.
 
  Anyway here's the output of git-bisect:
  
  [EMAIL PROTECTED]:/usr/src/linux-2.6$ git bisect bad
  99874d50481c093adfe74e796436024d88b6a48c is first bad commit
  commit 99874d50481c093adfe74e796436024d88b6a48c
  Author: Jens Axboe [EMAIL PROTECTED]
  Date:   Fri Oct 12 12:50:41 2007 +0200
  
  [BLOCK] Only include the compat ioctl code if CONFIG_BLOCK is set
  
  Add an extra CONFIG_BLOCK_COMPAT that we can use in the Makefile
  
  Signed-off-by: Jens Axboe [EMAIL PROTECTED]
  
  :04 04 f88b7b0e496edb8fbdd4bc74abd1a742a6a1d6d9
  32ead3bd57b52a664cc8ccb662195041868d7632 M  block
  
  ...
  
  If needed I can do further investigation, changing the assumption of
  efefc6eb38d43b8e5daef482f575d767b002004e to good and see if the bisect
  points to a different commit.
 
 Yes, it'll be some other commit.  Sorry.
 
 It would help you (and me) if an ata developer could pay some attention.
 
 Rafael, please track this as an ata regression.

We unfortunately need to bounce this ACPI people.

We recently turned on ACPI by default in libata, which INTRODUCES the
ability of the BIOS to pass random, unknown-quality ATA commands to the
device.

I highly expect some breakage related to this...  if ACPI folks
determine it is not an ACPI bug, then its a firmware bug and we will
have to avoid that firmware on that platform.

Set module option noacpi to 1, to disable ACPI and see if that fixes
the problem.

This will be a common diagnosis and workaround, FWIW...

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_piix: add SATELLITE PRO U200 to broken suspend list

2007-11-08 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 12:02:27PM +0900, Tejun Heo wrote:
 From: Yann Chachkoff [EMAIL PROTECTED]
 
 Please warmly welcome the PRO variant of Satellite U200 to the broken
 suspend list.
 
 Original patch is from Yann Chachkoff.  Patch reformatted and
 forwarded by Tejun Heo.
 
 Signed-off-by: Yann Chachkoff [EMAIL PROTECTED]
 Signed-off-by: Tejun Heo [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 2/4] libata sata_qstor nuke idle state

2007-11-08 Thread Jeff Garzik
On Wed, Nov 07, 2007 at 10:52:55AM -0500, Mark Lord wrote:
 sata_qstor nuke idle state.
 
 We're really only ever in one of two hardware states:  packet, or mmio.
 Get rid of unnecessary qs_state_idle state.
 
 This belongs in 2.6.24.
 
 Signed-off-by:  Mark Lord [EMAIL PROTECTED]

applied patches 2-4

Please put this belongs in 2.6.24 comment behind the --- separator,
as described in Documentation/SubmittingPatches, so that Linus's patch
merging tools automatically snip such comments.

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 1/2 take #2] libata: Support PIO polling-only hosts.

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 11:14:56AM +0900, Paul Mundt wrote:
 By default ata_host_activate() expects a valid IRQ in order to
 successfully register the host. This patch enables a special case
 for registering polling-only hosts that either don't have IRQs
 or have buggy IRQ generation (either in terms of handling or
 sensing), which otherwise work fine.
 
 Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
 and pass in an invalid IRQ.
 
 Signed-off-by: Paul Mundt [EMAIL PROTECTED]
 

applied 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 #upstream-fixes] libata: skip 0xff polling for PATA controllers

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 11:20:18AM +0900, Tejun Heo wrote:
 In a presentation of true workmanship, pata_ali asserts IRQ
 permanantly if the TF status register is read more than once when
 there's no device attached to the port.
 
 Avoid waiting polling for !0xff if it's PATA.  It's needed only for
 some rare SATA devices anyway.
 
 This problem is reported by Luca Tettamanti in bugzilla bug 9298.
 
 Signed-off-by: Tejun Heo [EMAIL PROTECTED]
 Tested-By: Luca Tettamanti [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 #upstream-fixes] libata: port and host should be stopped before hardware resources are released

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 01:09:00PM +0900, Tejun Heo wrote:
 Port / host stop calls used to be made from ata_host_release() which
 is called after all hardware resources acquired after host allocation
 are released.  This is wrong as port and host stop routines often
 access the hardware.
 
 Add separate devres for port / host stop which is invoked right after
 IRQ is released but with all other hardware resources intact.  The
 devres is added iff -host_stop and/or -port_stop exist.
 
 This problem has been spotted by Mark Lord.
 
 Signed-off-by: Tejun Heo [EMAIL PROTECTED]
 Cc: Mark Lord [EMAIL PROTECTED]
 ---
 Jeff, this really needs to get into 2.6.24.  Although it hasn't caused
 any real problem (yet), ahci is one of the affected drivers and it's
 poking released iomems in its port_ops.

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: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:
  On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik [EMAIL PROTECTED] wrote:
  On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
On Thu, 08 Nov 2007 17:43:59 +0100 Roberto Oppedisano [EMAIL 
PROTECTED] wrote:
Andrew Morton wrote, On 11/07/2007 09:13 PM:
 On Wed, 07 Nov 2007 15:15:07 +0100 Roberto Oppedisano [EMAIL 
 PROTECTED] wrote:
 Hello.
 I noticed that after suspending to ram the DVD-ROM/CDRW
 drive in no more recognized on my laptop. Looking at dmesg
 after suspend i found:

 [5.313446] ata2.00: _GTF unexpected object type 0x1
 [5.313453] ata2.00: ACPI on devcfg failed the second time, 
 disabling 
 (errno=-22)
 [5.313457] ata2.00: revalidation failed (errno=1)
 [5.313459] ata2.00: disabled


 Not sure when the bug was introduced or if it has been always been 
 there
 (but I can investigate if needed).

 More details on request.

 Following dmesg pre and after suspend.
 Yes, it would be useful if you could work ot whether this worked OK 
 in an
 earlier kernel, thanks.
Hello Andrew.
The bug has been recently added.

I did a git-bisect, but the result is probably not completely useful,
because many kernel did non build with my config, and I marked them as 
bad.
   
   Those build errors are bad.  Please report them.  They directly prevented
   you from finding the commit which caused this regression.
   
   The only way in which we'll stop people doing crap like this is to rub
   their noses (and the person who committed its nose) in it.
   
Anyway here's the output of git-bisect:

[EMAIL PROTECTED]:/usr/src/linux-2.6$ git bisect bad
99874d50481c093adfe74e796436024d88b6a48c is first bad commit
commit 99874d50481c093adfe74e796436024d88b6a48c
Author: Jens Axboe [EMAIL PROTECTED]
Date:   Fri Oct 12 12:50:41 2007 +0200

[BLOCK] Only include the compat ioctl code if CONFIG_BLOCK is set

Add an extra CONFIG_BLOCK_COMPAT that we can use in the Makefile

Signed-off-by: Jens Axboe [EMAIL PROTECTED]

:04 04 f88b7b0e496edb8fbdd4bc74abd1a742a6a1d6d9
32ead3bd57b52a664cc8ccb662195041868d7632 M  block

...

If needed I can do further investigation, changing the assumption of
efefc6eb38d43b8e5daef482f575d767b002004e to good and see if the bisect
points to a different commit.
   
   Yes, it'll be some other commit.  Sorry.
   
   It would help you (and me) if an ata developer could pay some attention.
   
   Rafael, please track this as an ata regression.
  
  We unfortunately need to bounce this ACPI people.
  
  We recently turned on ACPI by default in libata, which INTRODUCES the
  ability of the BIOS to pass random, unknown-quality ATA commands to the
  device.
  
  I highly expect some breakage related to this...  if ACPI folks
  determine it is not an ACPI bug, then its a firmware bug and we will
  have to avoid that firmware on that platform.
  
  Set module option noacpi to 1, to disable ACPI and see if that fixes
  the problem.
  
  This will be a common diagnosis and workaround, FWIW...
  
 
 I suspect it wold be best to disable the feature for the 2.6.24 release,
 then reenable it afterwards and keep doing this until the code is
 sufficiently stable.

Re-read my message :)

The code is stable.  Behavior _by definition_ will vary by BIOS.

This feature (a) enables suspend/resume, but (b) now sends random
unvalidated shite to the device that we hope will work.

Look at all the messages where turning on ACPI in libata _fixed_
suspend/resume (because its obviously required for many, including
laptops).

So it's not an easy turn it off answer, you break shitloads of
suspend/resume that way, that we just fixed.

The message _GTF unexpected object type indicates a broken BIOS, so
IMO we should proceed in that direction, blacklisting that platform.

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: Acard ATP8620 2SATA / 1 IDE driver - AHCI.C Nov082007

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 03:19:52PM -0500, Mark Lord wrote:
 Jeff Garzik wrote:
 On Thu, Nov 08, 2007 at 02:52:26PM -0500, Mark Lord wrote:
 Jeff Garzik wrote:
 ..
 2) This chip includes target mode support.  Very nice, well done!
 I hope that standard AHCI eventually supports this nice feature!
 ..
 
 Speaking of which.  Do we have a strategy as to how to implement/support
 the target side of target mode on controllers which can do it?
 
 The Marvell chips also have a target mode feature, and I'd like to add
 support for it soon-ish, but it's now clear how you would like it plumbed
 into libata.
 
 It's almost like a separate driver/subsystem, except that would be very 
 silly.
 
 I'm letting the SCSI folks do the heavy lifting, implementing SCSI
 target mode -- an effort already quite well along.
 
 We should be able to piggyback off of that work.
 ..
 
 MMmm..  I wonder what the most common use case is for target mode?
 
 Everybody I've dealt with thus far uses it as a high-speed local comms 
 interface,
 which would suggest that it might be done as a network interface (ethernet 
 emulation).
 
 But that would confusingly go across driver subsystems,
 despite that this is how it actually is used.

The low-level driver itself will just be a dumb DMA send/receive engine,
with submit/completion APIs highly similar to the existing ones.  Then
you can easily provide a network interface interface (not a typo) on top
of that.

The biggest use case I've seen is in the embedded space, where you
really are creating a SCSI (or ATA) target, that appears to the
initiator/client to be a real SCSI-or-ATA device.

There are certainly other uses:  networking, creating a cheap SATA bus
analyzer, creating a cheap SATA bridge, ...

My main goal is to ensure that the low-level driver is as simple as
possible, which permits upper layers to actually figure out what
purposes it shall use.

Modern SATA is just a DMA engine with PHY control anyway (just like
networking), so we really just need to be sure to abstract away
initiator-mode (aka host mode) specifics in drivers that support target
mode.

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: Acard ATP8620 2SATA / 1 IDE driver - AHCI.C Nov082007

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 02:52:26PM -0500, Mark Lord wrote:
 Jeff Garzik wrote:
 ..
 2) This chip includes target mode support.  Very nice, well done!
 I hope that standard AHCI eventually supports this nice feature!
 ..
 
 Speaking of which.  Do we have a strategy as to how to implement/support
 the target side of target mode on controllers which can do it?
 
 The Marvell chips also have a target mode feature, and I'd like to add
 support for it soon-ish, but it's now clear how you would like it plumbed
 into libata.
 
 It's almost like a separate driver/subsystem, except that would be very 
 silly.

I'm letting the SCSI folks do the heavy lifting, implementing SCSI
target mode -- an effort already quite well along.

We should be able to piggyback off of that work.

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: SATA Target mode libata

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 03:49:17PM -0500, Mark Lord wrote:
 Jeff Garzik wrote:
 On Thu, Nov 08, 2007 at 03:19:52PM -0500, Mark Lord wrote:
 ..
 MMmm..  I wonder what the most common use case is for target mode?
 
 Everybody I've dealt with thus far uses it as a high-speed local comms 
 interface,
 which would suggest that it might be done as a network interface 
 (ethernet emulation).
 
 But that would confusingly go across driver subsystems,
 despite that this is how it actually is used.
 
 The low-level driver itself will just be a dumb DMA send/receive engine,
 with submit/completion APIs highly similar to the existing ones.  Then
 you can easily provide a network interface interface (not a typo) on top
 of that.
 ..
 
 The obvious BIG difference is that in host mode, *we* initiate 
 communcations,
 whereas in target mode, it has to just sit there waiting for a host to say 
 something.
 
 That's a pretty big change from how libata operates today,
 in just about every respect.

Not at all -- both initiator and target modes have the exact same tasks:  
1) send stuff to hardware, 2) receive hardware responses.

On modern SAS/SATA hardware, you have both command and response queues.

On modern SATA hardware, you have command queue and a received FIS list,
which provides essentially the same services.

Thus SATA target mode will simply need a here is data I just received
from the wire hook, and the rest of the infrastructure already exists.

Target mode will re-use -qc_issue, and need a new ata_receive_fis()
function for asynchronously received FIS's (H2D FIS, etc.)

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] Add quirk to set AHCI mode on ICH boards

2007-11-08 Thread Jeff Garzik
On Fri, Nov 09, 2007 at 09:02:35AM +0700, Riki Oktarianto wrote:
 Some BIOSen map AHCI ABAR but lock the SATA controller to IDE mode.
 This patch add quirk to set AHCI mode on ICH board with such case.
 
 Tested on Macbook2,1 (ICH7M)

Intel will complain but it's awful tempting...

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] Add quirk to set AHCI mode on ICH boards

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 10:29:37PM -0500, Mark Lord wrote:
 And I might even privately patch my own kernels to map the ACHI BAR
 in the cases where the BIOS didn't...

The inability to do this in the general case is the main reason why
AHCI was not unconditionally enabled, even in IDE mode, when it was
originally added...  :/

Jeff, taking a trip down memory lane



-
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: FIS structure and Command List structure for AHCI SATA controller

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 10:38:25PM -0500, mike zheng wrote:
 I am working on an AHCI SATA controller. For each port, there is one
 FIS descriptor and one Command List, which points to a Received FIS
 structure and Command List structure. So what is Received FIS
 structure? The Command List structure points to Command Table, that
 has Command FIS field and Physical Region Descriptor table. I assume
 the Command table is used for the communication between processor and
 the SATA controller. Then why do we need the Received FIS structure?

The driver actually doesn't need it at all, though a few ideas are
floating around for using it.

However, it's required by the hardware by definition -- you must
provide a buffer for incoming FIS's (P0FB), before enabling FIS
reception (P0CMD bit 4, FRE).

If I had to guess, the hardware dumps the RX FIS into host memory rather
than having additional internal buffers/FIFOs...

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] Add quirk to set AHCI mode on ICH boards

2007-11-08 Thread Jeff Garzik
On Thu, Nov 08, 2007 at 11:44:22PM -0500, Mark Lord wrote:
 Jeff Garzik wrote:
 On Thu, Nov 08, 2007 at 10:29:37PM -0500, Mark Lord wrote:
 And I might even privately patch my own kernels to map the ACHI BAR
 in the cases where the BIOS didn't...
 
 The inability to do this in the general case is the main reason why
 AHCI was not unconditionally enabled, even in IDE mode, when it was
 originally added...  :/
 ..
 
 Yeah, that one's always puzzled me.
 It's just software,  so why don't we do it?  In the PCI layer, that is?

Ah, but it's not just software:  when trying to find bus address
space for the BAR, we don't know if we are stomping on magic hardware
resources the BIOS has conveniently failed to tell us about.

So while in all likelihood you will have no problem finding a
suitable bus address to use, as a generalized rule it is a far more
difficult proposition.

Mind you, I would /love/ to be proven wrong here.  In additional to AHCI
BAR, modern ata_piix includes SATA PHY registers that we could make use
of, but cannot...

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] Add quirk to set AHCI mode on ICH boards

2007-11-09 Thread Jeff Garzik

Alan Cox wrote:

On Thu, 8 Nov 2007 22:46:22 -0500
Jeff Garzik [EMAIL PROTECTED] wrote:


On Thu, Nov 08, 2007 at 10:29:37PM -0500, Mark Lord wrote:

And I might even privately patch my own kernels to map the ACHI BAR
in the cases where the BIOS didn't...

The inability to do this in the general case is the main reason why
AHCI was not unconditionally enabled, even in IDE mode, when it was
originally added...  :/


We've done it all the time for various devices without problems (eg S3
video cards). I'd like to see it go in - although perhaps attached to a
force_ahci boot param initially


By forcing AHCI, your PATA devices will be inaccessible, in a common 
configuration.  It also means shuffling users from one driver to 
another, which induces breakage.


I was speaking wishfully.  Real life intrudes, alas.

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] Add quirk to set AHCI mode on ICH boards

2007-11-09 Thread Jeff Garzik

Matthias Schniedermeyer wrote:
And on the topic of broken BIOSes. I have a little empathy for the MB 
manufactures as non-RAID AHCI royaly screws Windos, so not supporting it 
reduces their support costs enough to overlook screwing the non-windos 
faction.


non-RAID AHCI works just fine on Windows.

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] sata_nv,ahci: add the ahci legacy mode support to sata_nv

2007-11-09 Thread Jeff Garzik

Jeff Garzik wrote:
The proposed sata_nv patch does the opposite -- guarantees we must 
support the continually problematic legacy IDE interface ad infinitum. 
Such patches are OK for the test lab, but in this specific case users 
/suffer/ when not running AHCI mode.


Just to reinforce...

sata_nv support and bug fixes are primarily done right now through the 
valiant efforts of Robert Hancock (with assists from Alan, Tejun, and 
others).


Robert's job is difficult, because he has no hardware documentation[1], 
and NVIDIA does not seem to be helping out much with driver bug reports 
on the lists or in bugzillas.


As far as I know, I am the only one in the universe outside of NVIDIA 
with any SATA docs at all, and those docs _only_ cover ADMA registers 
and DMA structures, no PCI config info, no errata, nothing on SWNCQ or 
legacy IDE (well, half a page).


NVIDIA has indeed become more engaged in sata_nv in recent times, and 
that's a positive sign.  You, Kuon and Ayaz have all been noticeably 
more responsive in email.  Thanks.  Users have definitely benefited, 
particularly from your help addressing a couple SWNCQ issues.


But at this point in time, being asked to choose between sata_nv and 
ahci is no choice at all.  One has public documentation, wide industry 
support and little-or-no bugs.  The other has several open issues, no 
documentation, and support obstacles.


Jeff


[1] Robert, please correct me if I'm 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: [PATCH] Add quirk to set AHCI mode on ICH boards

2007-11-10 Thread Jeff Garzik

Allen Martin wrote:

At least for NVIDIA controllers, loading the AHCI driver when the BIOS
is set to IDE mode is not recommended by NVIDIA.  Any AHCI workarounds
in the BIOS are likely to be disabled when set to IDE mode.  In practice


What workarounds, if any, are needed?

We need those in the driver not BIOS anyway, in order to fully support 
suspend/resume and host controller reset during runtime operation.




we don't expect to see a lot of users running an AHCI controller in IDE
mode unless they have explicitly disabled AHCI from the BIOS or the
system builder has some specific reason for shipping IDE mode by default
(like support for some legacy DOS or Win9x tools)


That's the situation we run into the most:  the system is in IDE mode 
not because AHCI doesn't work, but for legacy compatibility.  We even 
run into cases (MacBook, Dell servers) where the user is never offered 
the option to turn on AHCI, even though the silicon supports it.


As such, the user winds up being forced to into the inferior mode for no 
reason related to their own setup (since they are obviously running Linux).


In Linux at least, we have a bunch of open sata_nv issues, so forcing 
users' interface into AHCI mode as a default future policy seems like 
the most stable choice on NVIDIA AHCI platforms.


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


linux-ata.org contributions

2007-11-10 Thread Jeff Garzik

(cc'd to main potential content contributors)

What do you guys think about a git tree for linux-ata.org web pages, 
rather than a wiki?  A git tree would be my preference.


Whenever you want to update or add a page, just send a 'git pull' 
request to me...


As I noted to Tejun in IRC, the main caveat towards contributing to 
linux-ata.org -- in any fashion, git or wiki or whatever -- is that I 
would like to continue to have google adsense ads there.  They help pay 
for the non-Red Hat dedicated server that handles these web pages.


Additional disclosures follow, so that you may align or avoid as your 
ethics dictates...


Disclosure #1:  income from linux-ata.org: Aug US$31, Sep US$38, Oct US$37

Disclosure #2:  the dedicated server in question also handles my email 
(which is intentionally not on Red Hat machines) and other sites like 
linux.yyz.us.


Disclosure #3:  All this is done under the copyright and onus of 
Dunvegan Media, Inc. which is really me and nobody else.  The company 
was started to put a corporate face on the lulu.com and AdSense stuff, 
and to give myself practice at running a business.  It also does a bit 
of email and DNS hosting for some friends, but the vast majority of 
usage is my linux stuff.  The company has always been [if only slightly] 
cash flow negative, so its not like I'm getting rich off this.  :)


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: cd/dvd inaccessible in 2.6.24-rc2

2007-11-10 Thread Jeff Garzik

Alan Cox wrote:

ata9.00: qc timeout (cmd 0xa0)
ata9.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
ata9.00: cmd a0/00:00:00:02:00/00:00:00:00:00/a0 tag 0 cdb 0x5a data 2
in
 res 51/54:03:00:02:00/00:00:00:00:00/a0 Emask 0x5 (timeout)
ata9.00: status: { DRDY ERR }


Could be an IRQ/ACPI regression, and in fact to me it looks more like
that, than an IDE one. Probably worth trying the various IRQ routing
options and seeing if they help.



Agreed, though the output is indeed signalling an error...  IMO the EH 
should handle the error if the device is signalling an error, upon 
timeout, rather than just going ahead and resetting the device.


Its similar to where ATA devices on PCI SFF controllers signal DMA error 
via timeout, where EH must inspect BMDMA Status register to determine if 
it's a DMA error signalled by hardware, or something that requires 
additional autopsy.


EH for ATAPI is quite different from EH for ATA, so there may be some 
areas where we don't handle things the right way for ATAPI.


Decoding the error message we have:

cdb 0x5a ==
MODE SENSE(10)
status 0x51 ==
DRDY
command-specific flag (aka SERV, in !overlap case)
CHK (check condition, aka error)
error 0x54 ==
ABRT (command aborted or command parameter invalid)
sense key 0x5 (illegal request)
ireason 0x3 ==
the hardcoded values (bits 0 and 1) remain hardcoded, all good

Since BSY is not set in the Status register, and given the other 
information derived from the decoded values, it looks like the device is 
otherwise happy and ready to accept additional commands.


It appears to have chewed on an ATAPI command, spit it out, but failed 
to send a completion interrupt.


So its an open question whether it's a device not completing this 
errored-out command, or whether its IRQ/ACPI stuff infecting libata.


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: cd/dvd inaccessible in 2.6.24-rc2

2007-11-10 Thread Jeff Garzik

Will Trives wrote:

Hello,

Motherboard: Gigabyte GA-P35-DS4 (rev. 1.1)
Chipset: Intel P35 + ICH9R 
PATA port runs off JMicron controller

CD/DVD Device: BENQ DW1640 16X

I cannot access my dvd burner under 2.6.24-rc2, I have no problems under
2.6.23. Basically the drive is detected OK, everything looks ok but as
soon as I go to use it errors like this occur:


Is 2.6.24-rc1 also broken?

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


<    8   9   10   11   12   13   14   15   16   >