[PATCHSET 03/03] add hotplug support, take 4

[PATCHSET 03/03] add hotplug support, take 4

[Tejun Heo]

Hello,

This is This is part of patchset series described in [T].

This is the fourth take of add-hotplug-support patchset.  Changes from
the last take[L] are.

* ata_eh_scsi_hotplug() renamed to ata_scsi_hotplug() and moved to
  libata-scsi.c

* ata_port_detach() now automatically calls scsi_remove_host()

* debug stuff removed from convert-sil24 patch

This patchset is against

  upstream (ef2824073fba9def3cf122e89cc485f66dd71f70)
  + [1] set-PIO-0-after-successful-EH-reset
  + [2] prep-for-hotplug-support patchset
  + [3] prep-LLDDs-for-hotplug-support patchset

Thanks.

--
tejun

[T] http://article.gmane.org/gmane.linux.ide/10891
[L] http://article.gmane.org/gmane.linux.ide/10588
[1] http://article.gmane.org/gmane.linux.ide/10890
[2] http://article.gmane.org/gmane.linux.ide/10892
[3] http://article.gmane.org/gmane.linux.ide/10905

[PATCH 02/13] libata-hp: implement hotplug

[Tejun Heo]

Implement ATA part of hotplug.  To avoid probing broken devices over
and over again, disabled devices are not automatically detached.  They
are detached only if probing is requested for the device or the
associated port is offline.  Also, to avoid infinite probing loop,
Each device is probed only once per EH run.

As SATA PHY status is fragile, devices are detached only after it has
used up its recovery chances unless explicitly requested by LLDD or
user (LLDD may request direct detach if, for example, it supports cold
presence detection).

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-eh.c |  123 ++++++++++++++++++++++++++++++++++++++--------
 include/linux/libata.h   |   13 +++++
 2 files changed, 115 insertions(+), 21 deletions(-)

978da67a282e2bb044129f117e6d5468cbe99cfa
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 695375b..beaba8f 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -894,10 +894,8 @@ static void ata_eh_analyze_serror(struct
 		err_mask |= AC_ERR_SYSTEM;
 		action |= ATA_EH_SOFTRESET;
 	}
-	if (serror & (SERR_PHYRDY_CHG | SERR_DEV_XCHG)) {
-		err_mask |= AC_ERR_ATA_BUS;
-		action |= ATA_EH_HARDRESET;
-	}
+	if (serror & (SERR_PHYRDY_CHG | SERR_DEV_XCHG))
+		ata_ehi_hotplugged(&ehc->i);
 
 	ehc->i.err_mask |= err_mask;
 	ehc->i.action |= action;
@@ -1449,11 +1447,12 @@ static int ata_eh_reset(struct ata_port 
 	return rc;
 }
 
-static int ata_eh_revalidate(struct ata_port *ap,
-			     struct ata_device **r_failed_dev)
+static int ata_eh_revalidate_and_attach(struct ata_port *ap,
+					struct ata_device **r_failed_dev)
 {
 	struct ata_eh_context *ehc = &ap->eh_context;
 	struct ata_device *dev;
+	unsigned long flags;
 	int i, rc = 0;
 
 	DPRINTK("ENTER\n");
@@ -1475,6 +1474,23 @@ static int ata_eh_revalidate(struct ata_
 				break;
 
 			ehc->i.action &= ~ATA_EH_REVALIDATE;
+		} else if (dev->class == ATA_DEV_UNKNOWN &&
+			   ehc->tries[dev->devno] &&
+			   ata_class_enabled(ehc->classes[dev->devno])) {
+			dev->class = ehc->classes[dev->devno];
+
+			rc = ata_dev_read_id(dev, &dev->class, 1, dev->id);
+			if (rc == 0)
+				rc = ata_dev_configure(dev, 1);
+
+			if (rc) {
+				dev->class = ATA_DEV_UNKNOWN;
+				break;
+			}
+
+			spin_lock_irqsave(&ap->host_set->lock, flags);
+			ap->flags |= ATA_FLAG_SCSI_HOTPLUG;
+			spin_unlock_irqrestore(&ap->host_set->lock, flags);
 		}
 	}
 
@@ -1495,6 +1511,36 @@ static int ata_port_nr_enabled(struct at
 	return cnt;
 }
 
+static int ata_port_nr_vacant(struct ata_port *ap)
+{
+	int i, cnt = 0;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (ap->device[i].class == ATA_DEV_UNKNOWN)
+			cnt++;
+	return cnt;
+}
+
+static int ata_eh_skip_recovery(struct ata_port *ap)
+{
+	struct ata_eh_context *ehc = &ap->eh_context;
+	int i;
+
+	if (ap->flags & ATA_FLAG_FROZEN || ata_port_nr_enabled(ap))
+		return 0;
+
+	/* skip if class codes for all vacant slots are ATA_DEV_NONE */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+
+		if (dev->class == ATA_DEV_UNKNOWN &&
+		    ehc->classes[dev->devno] != ATA_DEV_NONE)
+			return 0;
+	}
+
+	return 1;
+}
+
 /**
  *	ata_eh_recover - recover host port after error
  *	@ap: host port to recover
@@ -1505,9 +1551,10 @@ static int ata_port_nr_enabled(struct at
  *
  *	This is the alpha and omega, eum and yang, heart and soul of
  *	libata exception handling.  On entry, actions required to
- *	recover each devices are recorded in eh_context.  This
- *	function executes all the operations with appropriate retrials
- *	and fallbacks to resurrect failed devices.
+ *	recover the port and hotplug requests are recorded in
+ *	eh_context.  This function executes all the operations with
+ *	appropriate retrials and fallbacks to resurrect failed
+ *	devices, detach goners and greet newcomers.
  *
  *	LOCKING:
  *	Kernel thread context (may sleep).
@@ -1530,6 +1577,19 @@ static int ata_eh_recover(struct ata_por
 		dev = &ap->device[i];
 
 		ehc->tries[dev->devno] = ATA_EH_DEV_TRIES;
+
+		/* process hotplug request */
+		if (dev->flags & ATA_DFLAG_DETACH)
+			ata_eh_detach_dev(dev);
+
+		if (!ata_dev_enabled(dev) &&
+		    ((ehc->i.probe_mask & (1 << dev->devno)) &&
+		     !(ehc->did_probe_mask & (1 << dev->devno)))) {
+			ata_eh_detach_dev(dev);
+			ata_dev_init(dev);
+			ehc->did_probe_mask |= (1 << dev->devno);
+			ehc->i.action |= ATA_EH_SOFTRESET;
+		}
 	}
 
  retry:
@@ -1537,15 +1597,18 @@ static int ata_eh_recover(struct ata_por
 	rc = 0;
 
 	/* skip EH if possible. */
-	if (!ata_port_nr_enabled(ap) && !(ap->flags & ATA_FLAG_FROZEN))
+	if (ata_eh_skip_recovery(ap))
 		ehc->i.action = 0;
 
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		ehc->classes[i] = ATA_DEV_UNKNOWN;
+
 	/* reset */
 	if (ehc->i.action & ATA_EH_RESET_MASK) {
 		ata_eh_freeze_port(ap);
 
-		rc = ata_eh_reset(ap, 0, prereset, softreset, hardreset,
-				  postreset);
+		rc = ata_eh_reset(ap, ata_port_nr_vacant(ap), prereset,
+				  softreset, hardreset, postreset);
 		if (rc) {
 			ata_port_printk(ap, KERN_ERR,
 					"reset failed, giving up\n");
@@ -1555,8 +1618,8 @@ static int ata_eh_recover(struct ata_por
 		ata_eh_thaw_port(ap);
 	}
 
-	/* revalidate existing devices */
-	rc = ata_eh_revalidate(ap, &dev);
+	/* revalidate existing devices and attach new ones */
+	rc = ata_eh_revalidate_and_attach(ap, &dev);
 	if (rc)
 		goto dev_fail;
 
@@ -1574,6 +1637,8 @@ static int ata_eh_recover(struct ata_por
  dev_fail:
 	switch (rc) {
 	case -ENODEV:
+		/* device missing, schedule probing */
+		ehc->i.probe_mask |= (1 << dev->devno);
 	case -EINVAL:
 		ehc->tries[dev->devno] = 0;
 		break;
@@ -1586,15 +1651,31 @@ static int ata_eh_recover(struct ata_por
 			ehc->tries[dev->devno] = 0;
 	}
 
-	/* disable device if it has used up all its chances */
-	if (ata_dev_enabled(dev) && !ehc->tries[dev->devno])
+	if (ata_dev_enabled(dev) && !ehc->tries[dev->devno]) {
+		/* disable device if it has used up all its chances */
 		ata_dev_disable(dev);
 
-	/* soft didn't work?  be haaaaard */
-	if (ehc->i.flags & ATA_EHI_DID_RESET)
-		ehc->i.action |= ATA_EH_HARDRESET;
-	else
-		ehc->i.action |= ATA_EH_SOFTRESET;
+		/* detach if offline */
+		if (ata_port_offline(ap))
+			ata_eh_detach_dev(dev);
+
+		/* probe if requested */
+		if ((ehc->i.probe_mask & (1 << dev->devno)) &&
+		    !(ehc->did_probe_mask & (1 << dev->devno))) {
+			ata_eh_detach_dev(dev);
+			ata_dev_init(dev);
+
+			ehc->tries[dev->devno] = ATA_EH_DEV_TRIES;
+			ehc->did_probe_mask |= (1 << dev->devno);
+			ehc->i.action |= ATA_EH_SOFTRESET;
+		}
+	} else {
+		/* soft didn't work?  be haaaaard */
+		if (ehc->i.flags & ATA_EHI_DID_RESET)
+			ehc->i.action |= ATA_EH_HARDRESET;
+		else
+			ehc->i.action |= ATA_EH_SOFTRESET;
+	}
 
 	if (ata_port_nr_enabled(ap)) {
 		ata_port_printk(ap, KERN_WARNING, "failed to recover some "
diff --git a/include/linux/libata.h b/include/linux/libata.h
index a506b2f..ef070b4 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -822,6 +822,19 @@ #define ata_ehi_clear_desc(ehi) do { \
 	(ehi)->desc_len = 0; \
 } while (0)
 
+static inline void ata_ehi_hotplugged(struct ata_eh_info *ehi)
+{
+	if (ehi->flags & ATA_EHI_HOTPLUGGED)
+		return;
+
+	ehi->flags |= ATA_EHI_HOTPLUGGED;
+	ehi->hotplug_timestamp = jiffies;
+
+	ehi->err_mask |= AC_ERR_ATA_BUS;
+	ehi->action |= ATA_EH_SOFTRESET;
+	ehi->probe_mask |= (1 << ATA_MAX_DEVICES) - 1;
+}
+
 /*
  * qc helpers
  */
-- 
1.3.2

Re: [PATCH 02/13] libata-hp: implement hotplug

[Jeff Garzik]

Tejun Heo wrote:
\> +		} else if (dev->class == ATA_DEV_UNKNOWN &&
> +			   ehc->tries[dev->devno] &&
> +			   ata_class_enabled(ehc->classes[dev->devno])) {
> +			dev->class = ehc->classes[dev->devno];
> +
> +			rc = ata_dev_read_id(dev, &dev->class, 1, dev->id);
> +			if (rc == 0)
> +				rc = ata_dev_configure(dev, 1);
> +
> +			if (rc) {
> +				dev->class = ATA_DEV_UNKNOWN;
> +				break;
> +			}

ACK, with a comment:  It seems like this small snippet of logic appears 
in more than one place, and could be made common

Re: [PATCH 02/13] libata-hp: implement hotplug

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> \> +        } else if (dev->class == ATA_DEV_UNKNOWN &&
>> +               ehc->tries[dev->devno] &&
>> +               ata_class_enabled(ehc->classes[dev->devno])) {
>> +            dev->class = ehc->classes[dev->devno];
>> +
>> +            rc = ata_dev_read_id(dev, &dev->class, 1, dev->id);
>> +            if (rc == 0)
>> +                rc = ata_dev_configure(dev, 1);
>> +
>> +            if (rc) {
>> +                dev->class = ATA_DEV_UNKNOWN;
>> +                break;
>> +            }
> 
> ACK, with a comment:  It seems like this small snippet of logic appears 
> in more than one place, and could be made common
> 

If you're referring to ata_bus_probe(), it will be removed as soon as 
all drivers are converted to new EH with a lot of other codes, so I 
don't think factoring above part is necessary.

-- 
tejun

[PATCH 03/13] libata-hp: implement SCSI part of hotplug

[Tejun Heo]

Implement SCSI part of hotplug.

This must be done in a separate context as SCSI makes use of EH during
probing.  Unfortunately, SCSI probing fails silently if EH is active.
ata_eh_scsi_hotplug() does its best to avoid such conditions but,
theoretically, it may fail to associate SCSI device to newly found ATA
device; however, the chance is very slim and I haven't experienced any
such event during testing.

Device removal synchronization is somewhat complex but I think I've
got it right and haven't seen it malfunction.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    1 
 drivers/scsi/libata-eh.c   |    6 ++
 drivers/scsi/libata-scsi.c |  124 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libata.h      |    1 
 include/linux/libata.h     |    2 -
 5 files changed, 132 insertions(+), 2 deletions(-)

9b6e334e9835c4941ff6dd39ddebaa56ff29ecdd
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 261fcd4..d9a664c 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5365,6 +5365,7 @@ static void ata_host_init(struct ata_por
 	ap->msg_enable = ATA_MSG_DRV;
 
 	INIT_WORK(&ap->port_task, NULL, NULL);
+	INIT_WORK(&ap->hotplug_task, ata_scsi_hotplug, ap);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 
 	/* set cable type */
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index beaba8f..b37575b 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -286,9 +286,13 @@ void ata_scsi_error(struct Scsi_Host *ho
 	/* clean up */
 	spin_lock_irqsave(hs_lock, flags);
 
+	if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
+		queue_work(ata_scsi_wq, &ap->hotplug_task);
+
 	if (ap->flags & ATA_FLAG_RECOVERED)
 		ata_port_printk(ap, KERN_INFO, "EH complete\n");
-	ap->flags &= ~ATA_FLAG_RECOVERED;
+
+	ap->flags &= ~(ATA_FLAG_SCSI_HOTPLUG | ATA_FLAG_RECOVERED);
 
 	spin_unlock_irqrestore(hs_lock, flags);
 
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index acfa26c..f5095cb 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2786,3 +2786,127 @@ int ata_scsi_offline_dev(struct ata_devi
 	}
 	return 0;
 }
+
+/**
+ *	ata_scsi_remove_dev - remove attached SCSI device
+ *	@dev: ATA device to remove attached SCSI device for
+ *
+ *	This function is called from ata_eh_scsi_hotplug() and
+ *	responsible for removing the SCSI device attached to @dev.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+static void ata_scsi_remove_dev(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->ap;
+	struct scsi_device *sdev;
+	unsigned long flags;
+
+	/* Alas, we need to grab scan_mutex to ensure SCSI device
+	 * state doesn't change underneath us and thus
+	 * scsi_device_get() always succeeds.  The mutex locking can
+	 * be removed if there is __scsi_device_get() interface which
+	 * increments reference counts regardless of device state.
+	 */
+	mutex_lock(&ap->host->scan_mutex);
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	/* clearing dev->sdev is protected by host_set lock */
+	sdev = dev->sdev;
+	dev->sdev = NULL;
+
+	if (sdev) {
+		/* If user initiated unplug races with us, sdev can go
+		 * away underneath us after the host_set lock and
+		 * scan_mutex are released.  Hold onto it.
+		 */
+		if (scsi_device_get(sdev) == 0) {
+			/* The following ensures the attached sdev is
+			 * offline on return from ata_scsi_offline_dev()
+			 * regardless it wins or loses the race
+			 * against this function.
+			 */
+			scsi_device_set_state(sdev, SDEV_OFFLINE);
+		} else {
+			WARN_ON(1);
+			sdev = NULL;
+		}
+	}
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	mutex_unlock(&ap->host->scan_mutex);
+
+	if (sdev) {
+		ata_dev_printk(dev, KERN_INFO, "detaching (SCSI %s)\n",
+			       sdev->sdev_gendev.bus_id);
+
+		scsi_remove_device(sdev);
+		scsi_device_put(sdev);
+	}
+}
+
+/**
+ *	ata_scsi_hotplug - SCSI part of hotplug
+ *	@data: Pointer to ATA port to perform SCSI hotplug on
+ *
+ *	Perform SCSI part of hotplug.  It's executed from a separate
+ *	workqueue after EH completes.  This is necessary because SCSI
+ *	hot plugging requires working EH and hot unplugging is
+ *	synchronized with hot plugging with a mutex.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+void ata_scsi_hotplug(void *data)
+{
+	struct ata_port *ap = data;
+	unsigned long timeout;
+	int i, requeue = 0;
+
+	DPRINTK("ENTER\n");
+
+	/* SCSI hotplug is requested.  EH might still be running and
+	 * we wanna scan the bus after EH is complete; otherwise,
+	 * scsi_host_scan_allowed() test fails and SCSI scan is
+	 * skipped silently.  scsi_block_when_processing_errors()
+	 * cannot be used because we might not have a sdev to wait on.
+	 * Poll for !scsi_host_in_recovery() for 2 secs.
+	 */
+	timeout = jiffies + 2 * HZ;
+	do {
+		if (!scsi_host_in_recovery(ap->host))
+			break;
+		msleep(100);
+	} while (time_before(jiffies, timeout));
+
+	if (scsi_host_in_recovery(ap->host))
+		requeue = 1;
+
+	/* unplug detached devices */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+		unsigned long flags;
+
+		if (!(dev->flags & ATA_DFLAG_DETACHED))
+			continue;
+
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		dev->flags &= ~ATA_DFLAG_DETACHED;
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+		ata_scsi_remove_dev(dev);
+	}
+
+	/* scan for new ones */
+	ata_scsi_scan_host(ap);
+
+	if (requeue || scsi_host_in_recovery(ap->host)) {
+		/* we might have scanned while EH is active.  Repeat
+		 * scan after a sec.
+		 */
+		queue_delayed_work(ata_scsi_wq, &ap->hotplug_task, HZ);
+	}
+
+	DPRINTK("EXIT\n");
+}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 15628b8..2d8cc05 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -76,6 +76,7 @@ extern struct scsi_transport_template at
 
 extern void ata_scsi_scan_host(struct ata_port *ap);
 extern int ata_scsi_offline_dev(struct ata_device *dev);
+extern void ata_scsi_hotplug(void *data);
 extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
 			       unsigned int buflen);
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ef070b4..e20831c 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -520,7 +520,7 @@ struct ata_port {
 	struct ata_host_set	*host_set;
 	struct device 		*dev;
 
-	struct work_struct	port_task;
+	struct work_struct	port_task, hotplug_task;
 
 	unsigned int		hsm_task_state;
 
-- 
1.3.2

[PATCH 01/13] libata-hp: implement ata_eh_detach_dev()

[Tejun Heo]

Implement ata_eh_detach_dev().  This function is responsible for
detaching an ATA device and offlining the associated SCSI device
atomically so that the detached device is not accessed after ATA
detach is complete.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-eh.c   |   28 ++++++++++++++++++++++++++++
 drivers/scsi/libata-scsi.c |   24 ++++++++++++++++++++++++
 drivers/scsi/libata.h      |    1 +
 3 files changed, 53 insertions(+), 0 deletions(-)

f42d6c9dfcfd3f3f720b2bee67ab83046ae9e855
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 4189bb4..695375b 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -629,6 +629,34 @@ void ata_eh_qc_retry(struct ata_queued_c
 }
 
 /**
+ *	ata_eh_detach_dev - detach ATA device
+ *	@dev: ATA device to detach
+ *
+ *	Detach @dev.
+ *
+ *	LOCKING:
+ *	None.
+ */
+static void ata_eh_detach_dev(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->ap;
+	unsigned long flags;
+
+	ata_dev_disable(dev);
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	dev->flags &= ~ATA_DFLAG_DETACH;
+
+	if (ata_scsi_offline_dev(dev)) {
+		dev->flags |= ATA_DFLAG_DETACHED;
+		ap->flags |= ATA_FLAG_SCSI_HOTPLUG;
+	}
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+}
+
+/**
  *	ata_eh_about_to_do - about to perform eh_action
  *	@ap: target ATA port
  *	@action: action about to be performed
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index a1caf3f..acfa26c 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2762,3 +2762,27 @@ void ata_scsi_scan_host(struct ata_port 
 		}
 	}
 }
+
+/**
+ *	ata_scsi_offline_dev - offline attached SCSI device
+ *	@dev: ATA device to offline attached SCSI device for
+ *
+ *	This function is called from ata_eh_hotplug() and responsible
+ *	for taking the SCSI device attached to @dev offline.  This
+ *	function is called with host_set lock which protects dev->sdev
+ *	against clearing.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ *
+ *	RETURNS:
+ *	1 if attached SCSI device exists, 0 otherwise.
+ */
+int ata_scsi_offline_dev(struct ata_device *dev)
+{
+	if (dev->sdev) {
+		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
+		return 1;
+	}
+	return 0;
+}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index e3f7406..15628b8 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -75,6 +75,7 @@ extern int ata_cmd_ioctl(struct scsi_dev
 extern struct scsi_transport_template ata_scsi_transport_template;
 
 extern void ata_scsi_scan_host(struct ata_port *ap);
+extern int ata_scsi_offline_dev(struct ata_device *dev);
 extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
 			       unsigned int buflen);
 
-- 
1.3.2

Re: [PATCH 01/13] libata-hp: implement ata_eh_detach_dev()

[Jeff Garzik]

Tejun Heo wrote:
> Implement ata_eh_detach_dev().  This function is responsible for
> detaching an ATA device and offlining the associated SCSI device
> atomically so that the detached device is not accessed after ATA
> detach is complete.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK

[PATCH 06/13] libata-hp: implement bootplug

[Tejun Heo]

Implement bootplug - boot probing via hotplug path.  While loading,
ata_host_add() simply schedules probing and invokes EH.  After EH
completes, ata_host_add() scans and assicates them with SCSI devices.
EH path is slightly modified to handle this (e.g. no autopsy during
bootplug).  The SCSI part is left in ata_host_add() because it's
shared with legacy path and to keep probing order as before (ATA scan
all ports in host_set then attach all).

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   58 +++++++++++++++++++++++++++++++++-----------
 drivers/scsi/libata-eh.c   |   29 +++++++++++++++-------
 2 files changed, 63 insertions(+), 24 deletions(-)

fe25a0b404d5a3f12fe68201e719e6b7d75ada66
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 96b0ffe..f45e04f 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5413,7 +5413,7 @@ static struct ata_port * ata_host_add(co
 
 	DPRINTK("ENTER\n");
 
-	if (!ent->port_ops->probe_reset &&
+	if (!ent->port_ops->probe_reset && !ent->port_ops->error_handler &&
 	    !(ent->host_flags & (ATA_FLAG_SATA_RESET | ATA_FLAG_SRST))) {
 		printk(KERN_ERR "ata%u: no reset mechanism available\n",
 		       port_no);
@@ -5441,6 +5441,12 @@ err_out:
 	return NULL;
 }
 
+static int ata_boot_probe_wait(void *dummy)
+{
+	schedule();
+	return 0;
+}
+
 /**
  *	ata_device_add - Register hardware device with ATA and SCSI layers
  *	@ent: Probe information describing hardware device to be registered
@@ -5459,7 +5465,6 @@ err_out:
  *	RETURNS:
  *	Number of ports registered.  Zero on error (no ports registered).
  */
-
 int ata_device_add(const struct ata_probe_ent *ent)
 {
 	unsigned int count = 0, i;
@@ -5536,19 +5541,6 @@ int ata_device_add(const struct ata_prob
 		}
 		ap->sata_spd_limit = ap->hw_sata_spd_limit;
 
-		DPRINTK("ata%u: bus probe begin\n", ap->id);
-		rc = ata_bus_probe(ap);
-		DPRINTK("ata%u: bus probe end\n", ap->id);
-
-		if (rc) {
-			/* FIXME: do something useful here?
-			 * Current libata behavior will
-			 * tear down everything when
-			 * the module is removed
-			 * or the h/w is unplugged.
-			 */
-		}
-
 		rc = scsi_add_host(ap->host, dev);
 		if (rc) {
 			ata_port_printk(ap, KERN_ERR, "scsi_add_host failed\n");
@@ -5558,6 +5550,42 @@ int ata_device_add(const struct ata_prob
 			 * at the very least
 			 */
 		}
+
+		if (!ap->ops->probe_reset) {
+			int bit = fls(ATA_FLAG_LOADING) - 1;
+			unsigned long flags;
+
+			ata_port_probe(ap);
+
+			spin_lock_irqsave(&ap->host_set->lock, flags);
+
+			ap->eh_info.probe_mask = (1 << ATA_MAX_DEVICES) - 1;
+			ap->eh_info.action |= ATA_EH_SOFTRESET;
+
+			set_bit(bit, &ap->flags);
+			ata_port_schedule_eh(ap);
+
+			spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+			wait_on_bit(&ap->flags, bit, ata_boot_probe_wait,
+				    TASK_UNINTERRUPTIBLE);
+
+			while (scsi_host_in_recovery(ap->host))
+				msleep(10);
+		} else {
+			DPRINTK("ata%u: bus probe begin\n", ap->id);
+			rc = ata_bus_probe(ap);
+			DPRINTK("ata%u: bus probe end\n", ap->id);
+
+			if (rc) {
+				/* FIXME: do something useful here?
+				 * Current libata behavior will
+				 * tear down everything when
+				 * the module is removed
+				 * or the h/w is unplugged.
+				 */
+			}
+		}
 	}
 
 	/* probes are done, now scan each port's disk(s) */
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index b37575b..7b5cb02 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -286,11 +286,16 @@ void ata_scsi_error(struct Scsi_Host *ho
 	/* clean up */
 	spin_lock_irqsave(hs_lock, flags);
 
-	if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
-		queue_work(ata_scsi_wq, &ap->hotplug_task);
-
-	if (ap->flags & ATA_FLAG_RECOVERED)
-		ata_port_printk(ap, KERN_INFO, "EH complete\n");
+	if (ap->flags & ATA_FLAG_LOADING) {
+		int bit = fls(ATA_FLAG_LOADING) - 1;
+		clear_bit(bit, &ap->flags);
+		wake_up_bit(&ap->flags, bit);
+	} else {
+		if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
+			queue_work(ata_scsi_wq, &ap->hotplug_task);
+		if (ap->flags & ATA_FLAG_RECOVERED)
+			ata_port_printk(ap, KERN_INFO, "EH complete\n");
+	}
 
 	ap->flags &= ~(ATA_FLAG_SCSI_HOTPLUG | ATA_FLAG_RECOVERED);
 
@@ -1329,6 +1334,7 @@ static int ata_eh_reset(struct ata_port 
 	struct ata_eh_context *ehc = &ap->eh_context;
 	unsigned int *classes = ehc->classes;
 	int tries = ATA_EH_RESET_TRIES;
+	int verbose = !(ap->flags & ATA_FLAG_LOADING);
 	unsigned int action;
 	ata_reset_fn_t reset;
 	int i, did_followup_srst, rc;
@@ -1376,8 +1382,10 @@ static int ata_eh_reset(struct ata_port 
 	}
 
  retry:
-	ata_port_printk(ap, KERN_INFO, "%s resetting port\n",
-			reset == softreset ? "soft" : "hard");
+	/* shut up during boot probing */
+	if (verbose)
+		ata_port_printk(ap, KERN_INFO, "%s resetting port\n",
+				reset == softreset ? "soft" : "hard");
 
 	/* reset */
 	ata_eh_about_to_do(ap, ATA_EH_RESET_MASK);
@@ -1761,8 +1769,11 @@ void ata_do_eh(struct ata_port *ap, ata_
 	       ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
 	       ata_postreset_fn_t postreset)
 {
-	ata_eh_autopsy(ap);
-	ata_eh_report(ap);
+	if (!(ap->flags & ATA_FLAG_LOADING)) {
+		ata_eh_autopsy(ap);
+		ata_eh_report(ap);
+	}
+
 	ata_eh_recover(ap, prereset, softreset, hardreset, postreset);
 	ata_eh_finish(ap);
 }
-- 
1.3.2

Re: [PATCH 06/13] libata-hp: implement bootplug

[Jeff Garzik]

Tejun Heo wrote:
> +static int ata_boot_probe_wait(void *dummy)
> +{
> +	schedule();

schedule_timeout(1) is preferred

> +			spin_unlock_irqrestore(&ap->host_set->lock, flags);
> +
> +			wait_on_bit(&ap->flags, bit, ata_boot_probe_wait,
> +				    TASK_UNINTERRUPTIBLE);
> +
> +			while (scsi_host_in_recovery(ap->host))
> +				msleep(10);

Is this seemingly infinite loop _guaranteed_ to stop eventually?


> +		} else {
> +			DPRINTK("ata%u: bus probe begin\n", ap->id);
> +			rc = ata_bus_probe(ap);
> +			DPRINTK("ata%u: bus probe end\n", ap->id);
> +
> +			if (rc) {
> +				/* FIXME: do something useful here?
> +				 * Current libata behavior will
> +				 * tear down everything when
> +				 * the module is removed
> +				 * or the h/w is unplugged.
> +				 */
> +			}
> +		}
>  	}
>  
>  	/* probes are done, now scan each port's disk(s) */
> diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
> index b37575b..7b5cb02 100644
> --- a/drivers/scsi/libata-eh.c
> +++ b/drivers/scsi/libata-eh.c
> @@ -286,11 +286,16 @@ void ata_scsi_error(struct Scsi_Host *ho
>  	/* clean up */
>  	spin_lock_irqsave(hs_lock, flags);
>  
> -	if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
> -		queue_work(ata_scsi_wq, &ap->hotplug_task);
> -
> -	if (ap->flags & ATA_FLAG_RECOVERED)
> -		ata_port_printk(ap, KERN_INFO, "EH complete\n");
> +	if (ap->flags & ATA_FLAG_LOADING) {
> +		int bit = fls(ATA_FLAG_LOADING) - 1;
> +		clear_bit(bit, &ap->flags);
> +		wake_up_bit(&ap->flags, bit);
> +	} else {
> +		if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
> +			queue_work(ata_scsi_wq, &ap->hotplug_task);
> +		if (ap->flags & ATA_FLAG_RECOVERED)
> +			ata_port_printk(ap, KERN_INFO, "EH complete\n");
> +	}
>  
>  	ap->flags &= ~(ATA_FLAG_SCSI_HOTPLUG | ATA_FLAG_RECOVERED);
>  
> @@ -1329,6 +1334,7 @@ static int ata_eh_reset(struct ata_port 
>  	struct ata_eh_context *ehc = &ap->eh_context;
>  	unsigned int *classes = ehc->classes;
>  	int tries = ATA_EH_RESET_TRIES;
> +	int verbose = !(ap->flags & ATA_FLAG_LOADING);
>  	unsigned int action;
>  	ata_reset_fn_t reset;
>  	int i, did_followup_srst, rc;
> @@ -1376,8 +1382,10 @@ static int ata_eh_reset(struct ata_port 
>  	}
>  
>   retry:
> -	ata_port_printk(ap, KERN_INFO, "%s resetting port\n",
> -			reset == softreset ? "soft" : "hard");
> +	/* shut up during boot probing */
> +	if (verbose)
> +		ata_port_printk(ap, KERN_INFO, "%s resetting port\n",
> +				reset == softreset ? "soft" : "hard");
>  
>  	/* reset */
>  	ata_eh_about_to_do(ap, ATA_EH_RESET_MASK);
> @@ -1761,8 +1769,11 @@ void ata_do_eh(struct ata_port *ap, ata_
>  	       ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
>  	       ata_postreset_fn_t postreset)
>  {
> -	ata_eh_autopsy(ap);
> -	ata_eh_report(ap);
> +	if (!(ap->flags & ATA_FLAG_LOADING)) {
> +		ata_eh_autopsy(ap);
> +		ata_eh_report(ap);
> +	}
> +
>  	ata_eh_recover(ap, prereset, softreset, hardreset, postreset);
>  	ata_eh_finish(ap);
>  }

Re: [PATCH 06/13] libata-hp: implement bootplug

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> +static int ata_boot_probe_wait(void *dummy)
>> +{
>> +    schedule();
> 
> schedule_timeout(1) is preferred

Can you elaborate?  AFAICS, the function is entered in 
TASK_UNINTERRUPTIBLE state.  No busy looping there.  It's called from 
wait_on_bit() which takes care of sleep/wakeup conditions.

>> +            spin_unlock_irqrestore(&ap->host_set->lock, flags);
>> +
>> +            wait_on_bit(&ap->flags, bit, ata_boot_probe_wait,
>> +                    TASK_UNINTERRUPTIBLE);
>> +
>> +            while (scsi_host_in_recovery(ap->host))
>> +                msleep(10);
> 
> Is this seemingly infinite loop _guaranteed_ to stop eventually?

Yeap.  Main thread is woken up after EH completes and EH is guaranteed 
to complete.

-- 
tejun

Re: [PATCH 06/13] libata-hp: implement bootplug

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> +static int ata_boot_probe_wait(void *dummy)
>>> +{
>>> +    schedule();
>>
>> schedule_timeout(1) is preferred
> 
> Can you elaborate?  AFAICS, the function is entered in 
> TASK_UNINTERRUPTIBLE state.  No busy looping there.  It's called from 
> wait_on_bit() which takes care of sleep/wakeup conditions.

Well, the purpose is to _wait, and so schedule_timeout() is more 
appropriate than schedule().  I also that an unconditional schedule() is 
a bit unfriendly if there is nothing useful to schedule at the moment.

	Jeff

Re: [PATCH 06/13] libata-hp: implement bootplug

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> Tejun Heo wrote:
>>>> +static int ata_boot_probe_wait(void *dummy)
>>>> +{
>>>> +    schedule();
>>>
>>> schedule_timeout(1) is preferred
>>
>> Can you elaborate?  AFAICS, the function is entered in 
>> TASK_UNINTERRUPTIBLE state.  No busy looping there.  It's called from 
>> wait_on_bit() which takes care of sleep/wakeup conditions.
> 
> Well, the purpose is to _wait, and so schedule_timeout() is more 
> appropriate than schedule().  I also that an unconditional schedule() is 
> a bit unfriendly if there is nothing useful to schedule at the moment.

Hmmm... I'm not very sure whether using schedule_timeout() inside 
wait_on_bit() is allowed or not.  I think it should work but I don't 
think it's supposed to be used that way.

Actually, I'm thinking about dropping the wait_on_bit() and implement 
generic helper to wait for EH completion as it's done in more than one 
places (during boot probing and unload unplugging and suspend/resume in 
future).  It probably will use completion.  Would that be better?

-- 
tejun

Re: [PATCH 06/13] libata-hp: implement bootplug

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Jeff Garzik wrote:
>>>> Tejun Heo wrote:
>>>>> +static int ata_boot_probe_wait(void *dummy)
>>>>> +{
>>>>> +    schedule();
>>>>
>>>> schedule_timeout(1) is preferred
>>>
>>> Can you elaborate?  AFAICS, the function is entered in 
>>> TASK_UNINTERRUPTIBLE state.  No busy looping there.  It's called from 
>>> wait_on_bit() which takes care of sleep/wakeup conditions.
>>
>> Well, the purpose is to _wait, and so schedule_timeout() is more 
>> appropriate than schedule().  I also that an unconditional schedule() 
>> is a bit unfriendly if there is nothing useful to schedule at the moment.
> 
> Hmmm... I'm not very sure whether using schedule_timeout() inside 
> wait_on_bit() is allowed or not.  I think it should work but I don't 
> think it's supposed to be used that way.

If schedule() can work, so can schedule_timeout()


> Actually, I'm thinking about dropping the wait_on_bit() and implement 
> generic helper to wait for EH completion as it's done in more than one 
> places (during boot probing and unload unplugging and suspend/resume in 
> future).  It probably will use completion.  Would that be better?

Sure, that's what completions are for :)

	Jeff

[PATCH 05/13] libata-hp: hook warmplug

[Tejun Heo]

Hook transportt->user_scan() and hostt->slave_destroy().

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ahci.c         |    1 +
 drivers/scsi/ata_piix.c     |    1 +
 drivers/scsi/libata-scsi.c  |    1 +
 drivers/scsi/pdc_adma.c     |    1 +
 drivers/scsi/sata_mv.c      |    1 +
 drivers/scsi/sata_nv.c      |    1 +
 drivers/scsi/sata_promise.c |    1 +
 drivers/scsi/sata_qstor.c   |    1 +
 drivers/scsi/sata_sil.c     |    1 +
 drivers/scsi/sata_sil24.c   |    1 +
 drivers/scsi/sata_sis.c     |    1 +
 drivers/scsi/sata_svw.c     |    1 +
 drivers/scsi/sata_sx4.c     |    1 +
 drivers/scsi/sata_uli.c     |    1 +
 drivers/scsi/sata_via.c     |    1 +
 drivers/scsi/sata_vsc.c     |    1 +
 16 files changed, 16 insertions(+), 0 deletions(-)

506098b74882b8fde1f9904cd5050a63160529ff
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 8493b02..afb3805 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -228,6 +228,7 @@ static struct scsi_host_template ahci_sh
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= AHCI_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index ad41dfd..54c2e52 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -219,6 +219,7 @@ static struct scsi_host_template piix_sh
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 	.resume			= ata_scsi_device_resume,
 	.suspend		= ata_scsi_device_suspend,
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 230cd20..bf6802b 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -107,6 +107,7 @@ static const u8 def_control_mpage[CONTRO
 struct scsi_transport_template ata_scsi_transport_template = {
 	.eh_strategy_handler	= ata_scsi_error,
 	.eh_timed_out		= ata_scsi_timed_out,
+	.user_scan		= ata_scsi_user_scan,
 };
 
 
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index a341fa8..eb910e4 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -152,6 +152,7 @@ static struct scsi_host_template adma_at
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ADMA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index 624983c..634bab1 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -390,6 +390,7 @@ static struct scsi_host_template mv_sht 
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= MV_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index d93513e..9055124 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -216,6 +216,7 @@ static struct scsi_host_template nv_sht 
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index 0111159..b2b6ed5 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -121,6 +121,7 @@ static struct scsi_host_template pdc_ata
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index 68737ca..98ddc25 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -142,6 +142,7 @@ static struct scsi_host_template qs_ata_
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= QS_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 695c06c..d420495 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -174,6 +174,7 @@ static struct scsi_host_template sil_sht
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 4a83090..d4ca6d6 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -370,6 +370,7 @@ static struct scsi_host_template sil24_s
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 82a07bf..a07e6e5 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -96,6 +96,7 @@ static struct scsi_host_template sis_sht
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index 7a4703b..d9b5168 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -299,6 +299,7 @@ static struct scsi_host_template k2_sata
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 #ifdef CONFIG_PPC_OF
 	.proc_info		= k2_sata_proc_info,
 #endif
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index c4db6bf..7f86441 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -191,6 +191,7 @@ static struct scsi_host_template pdc_sat
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index 7fae3e0..e69ba22 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -90,6 +90,7 @@ static struct scsi_host_template uli_sht
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 1c9e2f3..c6975c5 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -103,6 +103,7 @@ static struct scsi_host_template svia_sh
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index 438e7c6..22ca7b8 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -279,6 +279,7 @@ static struct scsi_host_template vsc_sat
 	.proc_name		= DRV_NAME,
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
 };
 
-- 
1.3.2

Re: [PATCH 05/13] libata-hp: hook warmplug

[Jeff Garzik]

Tejun Heo wrote:
> Hook transportt->user_scan() and hostt->slave_destroy().
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK 3-5

[PATCH 08/13] ata_piix: convert ata_piix to new probing mechanism

[Tejun Heo]

Convert ata_piix to new probing mechanism.  Automatic hotplug is not
supported due to hardware limitation (no PHY event interrupt), but
warm plugging works.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ata_piix.c |   87 +++++++++++++++++------------------------------
 1 files changed, 32 insertions(+), 55 deletions(-)

b5bcb83ff6ea2ec505c54d91f8fa806521750eae
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 54c2e52..521b718 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -146,11 +146,10 @@ struct piix_map_db {
 
 static int piix_init_one (struct pci_dev *pdev,
 				    const struct pci_device_id *ent);
-
-static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes);
-static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
+static void piix_pata_error_handler(struct ata_port *ap);
+static void piix_sata_error_handler(struct ata_port *ap);
 
 static unsigned int in_module_init = 1;
 
@@ -237,8 +236,6 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.probe_reset		= piix_pata_probe_reset,
-
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
@@ -249,7 +246,7 @@ static const struct ata_port_operations 
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= piix_pata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -269,8 +266,6 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.probe_reset		= piix_sata_probe_reset,
-
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
@@ -281,7 +276,7 @@ static const struct ata_port_operations 
 
 	.freeze			= ata_bmdma_freeze,
 	.thaw			= ata_bmdma_thaw,
-	.error_handler		= ata_bmdma_error_handler,
+	.error_handler		= piix_sata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
@@ -466,59 +461,51 @@ cbl40:
 }
 
 /**
- *	piix_pata_probeinit - probeinit for PATA host controller
+ *	piix_pata_prereset - prereset for PATA host controller
  *	@ap: Target port
  *
- *	Probeinit including cable detection.
- *
- *	LOCKING:
- *	None (inherited from caller).
- */
-static void piix_pata_probeinit(struct ata_port *ap)
-{
-	piix_pata_cbl_detect(ap);
-	ata_std_probeinit(ap);
-}
-
-/**
- *	piix_pata_probe_reset - Perform reset on PATA port and classify
- *	@ap: Port to reset
- *	@classes: Resulting classes of attached devices
- *
- *	Reset PATA phy and classify attached devices.
+ *	Prereset including cable detection.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
-static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes)
+static int piix_pata_prereset(struct ata_port *ap)
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
 
 	if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->hard_port_no])) {
 		ata_port_printk(ap, KERN_INFO, "port disabled. ignoring.\n");
+		ap->eh_context.i.action &= ~ATA_EH_RESET_MASK;
 		return 0;
 	}
 
-	return ata_drive_probe_reset(ap, piix_pata_probeinit,
-				     ata_std_softreset, NULL,
-				     ata_std_postreset, classes);
+	piix_pata_cbl_detect(ap);
+
+	return ata_std_prereset(ap);
+}
+
+static void piix_pata_error_handler(struct ata_port *ap)
+{
+	ata_bmdma_drive_eh(ap, piix_pata_prereset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 /**
- *	piix_sata_probe - Probe PCI device for present SATA devices
- *	@ap: Port associated with the PCI device we wish to probe
+ *	piix_sata_prereset - prereset for SATA host controller
+ *	@ap: Target port
  *
  *	Reads and configures SATA PCI device's PCI config register
  *	Port Configuration and Status (PCS) to determine port and
- *	device availability.
+ *	device availability.  Return -ENODEV to skip reset if no
+ *	device is present.
  *
  *	LOCKING:
  *	None (inherited from caller).
  *
  *	RETURNS:
- *	Mask of avaliable devices on the port.
+ *	0 if device is present, -ENODEV otherwise.
  */
-static unsigned int piix_sata_probe (struct ata_port *ap)
+static int piix_sata_prereset(struct ata_port *ap)
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
 	const unsigned int *map = ap->host_set->private_data;
@@ -560,29 +547,19 @@ static unsigned int piix_sata_probe (str
 	DPRINTK("ata%u: LEAVE, pcs=0x%x present_mask=0x%x\n",
 		ap->id, pcs, present_mask);
 
-	return present_mask;
-}
-
-/**
- *	piix_sata_probe_reset - Perform reset on SATA port and classify
- *	@ap: Port to reset
- *	@classes: Resulting classes of attached devices
- *
- *	Reset SATA phy and classify attached devices.
- *
- *	LOCKING:
- *	None (inherited from caller).
- */
-static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes)
-{
-	if (!piix_sata_probe(ap)) {
+	if (!present_mask) {
 		ata_port_printk(ap, KERN_INFO, "SATA port has no device.\n");
+		ap->eh_context.i.action &= ~ATA_EH_RESET_MASK;
 		return 0;
 	}
 
-	return ata_drive_probe_reset(ap, ata_std_probeinit,
-				     ata_std_softreset, NULL,
-				     ata_std_postreset, classes);
+	return ata_std_prereset(ap);
+}
+
+static void piix_sata_error_handler(struct ata_port *ap)
+{
+	ata_bmdma_drive_eh(ap, piix_sata_prereset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 /**
-- 
1.3.2

[PATCH 04/13] libata-hp: implement warmplug

[Tejun Heo]

Implement warmplug.  User-initiated unplug can be detected by
hostt->slave_destroy() and plug by transportt->user_scan().  This
patch only implements the two callbacks.  The next function will hook
them.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    1 
 drivers/scsi/libata-scsi.c |   89 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |    1 
 3 files changed, 91 insertions(+), 0 deletions(-)

9a57b5b1dd36ac035559ad01dd69d1ddb6d6481f
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index d9a664c..96b0ffe 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5936,6 +5936,7 @@ EXPORT_SYMBOL_GPL(ata_port_queue_task);
 EXPORT_SYMBOL_GPL(ata_scsi_ioctl);
 EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
 EXPORT_SYMBOL_GPL(ata_scsi_slave_config);
+EXPORT_SYMBOL_GPL(ata_scsi_slave_destroy);
 EXPORT_SYMBOL_GPL(ata_scsi_change_queue_depth);
 EXPORT_SYMBOL_GPL(ata_scsi_release);
 EXPORT_SYMBOL_GPL(ata_host_intr);
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index f5095cb..230cd20 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -57,6 +57,8 @@ static struct ata_device * __ata_scsi_fi
 					const struct scsi_device *scsidev);
 static struct ata_device * ata_scsi_find_dev(struct ata_port *ap,
 					    const struct scsi_device *scsidev);
+static int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
+			      unsigned int id, unsigned int lun);
 
 
 #define RW_RECOVERY_MPAGE 0x1
@@ -727,6 +729,40 @@ int ata_scsi_slave_config(struct scsi_de
 }
 
 /**
+ *	ata_scsi_slave_destroy - SCSI device is about to be destroyed
+ *	@sdev: SCSI device to be destroyed
+ *
+ *	@sdev is about to be destroyed for hot/warm unplugging.  If
+ *	this unplugging was initiated by libata as indicated by NULL
+ *	dev->sdev, this function doesn't have to do anything.
+ *	Otherwise, SCSI layer initiated warm-unplug is in progress.
+ *	Clear dev->sdev, schedule the device for ATA detach and invoke
+ *	EH.
+ *
+ *	LOCKING:
+ *	Defined by SCSI layer.  We don't really care.
+ */
+void ata_scsi_slave_destroy(struct scsi_device *sdev)
+{
+	struct ata_port *ap = ata_shost_to_port(sdev->host);
+	unsigned long flags;
+	struct ata_device *dev;
+
+	if (!ap->ops->error_handler)
+		return;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+	dev = __ata_scsi_find_dev(ap, sdev);
+	if (dev && dev->sdev) {
+		/* SCSI device already in CANCEL state, no need to offline it */
+		dev->sdev = NULL;
+		dev->flags |= ATA_DFLAG_DETACH;
+		ata_port_schedule_eh(ap);
+	}
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+}
+
+/**
  *	ata_scsi_change_queue_depth - SCSI callback for queue depth config
  *	@sdev: SCSI device to configure queue depth for
  *	@queue_depth: new queue depth
@@ -2910,3 +2946,56 @@ void ata_scsi_hotplug(void *data)
 
 	DPRINTK("EXIT\n");
 }
+
+/**
+ *	ata_scsi_user_scan - indication for user-initiated bus scan
+ *	@shost: SCSI host to scan
+ *	@channel: Channel to scan
+ *	@id: ID to scan
+ *	@lun: LUN to scan
+ *
+ *	This function is called when user explicitly requests bus
+ *	scan.  Set probe pending flag and invoke EH.
+ *
+ *	LOCKING:
+ *	SCSI layer (we don't care)
+ *
+ *	RETURNS:
+ *	Zero.
+ */
+static int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
+			      unsigned int id, unsigned int lun)
+{
+	struct ata_port *ap = ata_shost_to_port(shost);
+	unsigned long flags;
+	int rc = 0;
+
+	if (!ap->ops->error_handler)
+		return -EOPNOTSUPP;
+
+	if ((channel != SCAN_WILD_CARD && channel != 0) ||
+	    (lun != SCAN_WILD_CARD && lun != 0))
+		return -EINVAL;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	if (id == SCAN_WILD_CARD) {
+		ap->eh_info.probe_mask |= (1 << ATA_MAX_DEVICES) - 1;
+		ap->eh_info.action |= ATA_EH_SOFTRESET;
+	} else {
+		struct ata_device *dev = ata_find_dev(ap, id);
+
+		if (dev) {
+			ap->eh_info.probe_mask |= 1 << dev->devno;
+			ap->eh_info.action |= ATA_EH_SOFTRESET;
+		} else
+			rc = -EINVAL;
+	}
+
+	if (rc == 0)
+		ata_port_schedule_eh(ap);
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+	return rc;
+}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e20831c..b216400 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -732,6 +732,7 @@ extern int ata_std_bios_param(struct scs
 			      struct block_device *bdev,
 			      sector_t capacity, int geom[]);
 extern int ata_scsi_slave_config(struct scsi_device *sdev);
+extern void ata_scsi_slave_destroy(struct scsi_device *sdev);
 extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
 				       int queue_depth);
 extern struct ata_device *ata_dev_pair(struct ata_device *adev);
-- 
1.3.2

[PATCH 09/13] sata_sil: convert to new probing mechanism and add hotplug support

[Tejun Heo]

Convert to new probing mechanism and add hotplug support by enabling
SATA IRQ for SError.N, marking ehi for hotplug and scheduling EH on
SATA IRQs.

Sil3112/3512/3114 family of controllers use COMRESET as TF clearing
point and can reliably wait for FIS34 after COMRESET whether the FIS
is the first FIS34 after POR or in response to the COMRESET.  Thus,
setting ATA_FLAG_HRST_TO_RESUME is enough for device detection after
hotplug.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/sata_sil.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

1068249a03081de450a306060e8a740d639920b3
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index d420495..9503fd2 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -56,7 +56,7 @@ enum {
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
 
 	SIL_DFL_HOST_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO,
+				  ATA_FLAG_MMIO | ATA_FLAG_HRST_TO_RESUME,
 
 	/*
 	 * Controller IDs
@@ -186,7 +186,6 @@ static const struct ata_port_operations 
 	.check_status		= ata_check_status,
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
-	.probe_reset		= ata_std_probe_reset,
 	.post_set_mode		= sil_post_set_mode,
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
@@ -344,6 +343,11 @@ static void sil_host_intr(struct ata_por
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
 	u8 status;
 
+	if (unlikely(bmdma2 & SIL_DMA_SATA_IRQ)) {
+		ata_ehi_hotplugged(&ap->eh_info);
+		goto freeze;
+	}
+
 	if (unlikely(!qc || qc->tf.ctl & ATA_NIEN))
 		goto freeze;
 
@@ -416,7 +420,7 @@ static irqreturn_t sil_interrupt(int irq
 		if (unlikely(!ap || ap->flags & ATA_FLAG_DISABLED))
 			continue;
 
-		if (!(bmdma2 & SIL_DMA_COMPLETE))
+		if (!(bmdma2 & (SIL_DMA_COMPLETE | SIL_DMA_SATA_IRQ)))
 			continue;
 
 		sil_host_intr(ap, bmdma2);
@@ -433,6 +437,9 @@ static void sil_freeze(struct ata_port *
 	void __iomem *mmio_base = ap->host_set->mmio_base;
 	u32 tmp;
 
+	/* global IRQ mask doesn't block SATA IRQ, turn off explicitly */
+	writel(0, mmio_base + sil_port[ap->port_no].sien);
+
 	/* plug IRQ */
 	tmp = readl(mmio_base + SIL_SYSCFG);
 	tmp |= SIL_MASK_IDE0_INT << ap->port_no;
@@ -449,6 +456,9 @@ static void sil_thaw(struct ata_port *ap
 	ata_chk_status(ap);
 	ata_bmdma_irq_clear(ap);
 
+	/* turn on SATA IRQ */
+	writel(SIL_SIEN_N, mmio_base + sil_port[ap->port_no].sien);
+
 	/* turn on IRQ */
 	tmp = readl(mmio_base + SIL_SYSCFG);
 	tmp &= ~(SIL_MASK_IDE0_INT << ap->port_no);
@@ -622,11 +632,6 @@ static int sil_init_one (struct pci_dev 
 			       mmio_base + sil_port[2].bmdma);
 	}
 
-	/* mask all SATA phy-related interrupts */
-	/* TODO: unmask bit 6 (SError N bit) for hotplug */
-	for (i = 0; i < probe_ent->n_ports; i++)
-		writel(0, mmio_base + sil_port[i].sien);
-
 	pci_set_master(pdev);
 
 	/* FIXME: check ata_device_add return value */
-- 
1.3.2

Re: [PATCH 09/13] sata_sil: convert to new probing mechanism and add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> Convert to new probing mechanism and add hotplug support by enabling
> SATA IRQ for SError.N, marking ehi for hotplug and scheduling EH on
> SATA IRQs.
> 
> Sil3112/3512/3114 family of controllers use COMRESET as TF clearing
> point and can reliably wait for FIS34 after COMRESET whether the FIS
> is the first FIS34 after POR or in response to the COMRESET.  Thus,
> setting ATA_FLAG_HRST_TO_RESUME is enough for device detection after
> hotplug.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

What is FIS34?  I've never heard of that...   :)

ACK patch 8, and the content of this patch (#9)

[PATCH 10/13] ahci: convert to new probing mechanism and add hotplug support

[Tejun Heo]

Convert to new probing mechanism and add hotplug support by enabling
PORT_IRQ_PHYRDY, marking ehi for hotplug and scheduling EH on
CONNECT/PHYRDY interrupts.

Unfortunately, ahci cannot reliably wait for the first FIS34 after
hotplug.  It sometimes succeeds but times out more often than not, so
ATA_FLAG_CANT_WAIT_FIS34 is used.

This patch also fixes ahci_hardreset() such that D2H Register FIS RX
area is cleared before issuing COMRESET.  Without this,
ata_busy_sleep() after COMRESET might prematually finish if the
previous TF contains DRDY && !BSY.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ahci.c |   47 +++++++++++++++++++++++++++--------------------
 1 files changed, 27 insertions(+), 20 deletions(-)

8cf0cb52955e53d055c16c85b648c5b9b9ef2679
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 60f455b..1fd1aa0 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -136,6 +136,7 @@ enum {
 	PORT_IRQ_FREEZE		= PORT_IRQ_HBUS_ERR |
 				  PORT_IRQ_IF_ERR |
 				  PORT_IRQ_CONNECT |
+				  PORT_IRQ_PHYRDY |
 				  PORT_IRQ_UNK_FIS,
 	PORT_IRQ_ERROR		= PORT_IRQ_FREEZE |
 				  PORT_IRQ_TF_ERR |
@@ -200,7 +201,6 @@ static void ahci_scr_write (struct ata_p
 static int ahci_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 static irqreturn_t ahci_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
-static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void ahci_irq_clear(struct ata_port *ap);
 static int ahci_port_start(struct ata_port *ap);
 static void ahci_port_stop(struct ata_port *ap);
@@ -241,8 +241,6 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
-	.probe_reset		= ahci_probe_reset,
-
 	.qc_prep		= ahci_qc_prep,
 	.qc_issue		= ahci_qc_issue,
 
@@ -267,7 +265,8 @@ static const struct ata_port_info ahci_p
 	{
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  ATA_FLAG_SKIP_D2H_BSY,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
@@ -277,6 +276,7 @@ static const struct ata_port_info ahci_p
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  ATA_FLAG_SKIP_D2H_BSY |
 				  AHCI_FLAG_RESET_NEEDS_CLO,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
@@ -569,6 +569,17 @@ static int ahci_clo(struct ata_port *ap)
 	return 0;
 }
 
+static int ahci_prereset(struct ata_port *ap)
+{
+	if ((ap->flags & AHCI_FLAG_RESET_NEEDS_CLO) &&
+	    (ata_busy_wait(ap, ATA_BUSY, 1000) & ATA_BUSY)) {
+		/* ATA_BUSY hasn't cleared, so send a CLO */
+		ahci_clo(ap);
+	}
+
+	return ata_std_prereset(ap);
+}
+
 static int ahci_softreset(struct ata_port *ap, unsigned int *class)
 {
 	struct ahci_port_priv *pp = ap->private_data;
@@ -678,12 +689,22 @@ static int ahci_softreset(struct ata_por
 
 static int ahci_hardreset(struct ata_port *ap, unsigned int *class)
 {
+	struct ahci_port_priv *pp = ap->private_data;
+	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
+	struct ata_taskfile tf;
 	int rc;
 
 	DPRINTK("ENTER\n");
 
 	ahci_stop_engine(ap);
+
+	/* clear D2H reception area to properly wait for FIS34 */
+	ata_tf_init(ap->device, &tf);
+	tf.command = 0xff;
+	ata_tf_to_fis(&tf, d2h_fis, 0);
+
 	rc = sata_std_hardreset(ap, class);
+
 	ahci_start_engine(ap);
 
 	if (rc == 0 && ata_port_online(ap))
@@ -714,19 +735,6 @@ static void ahci_postreset(struct ata_po
 	}
 }
 
-static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
-{
-	if ((ap->flags & AHCI_FLAG_RESET_NEEDS_CLO) &&
-	    (ata_busy_wait(ap, ATA_BUSY, 1000) & ATA_BUSY)) {
-		/* ATA_BUSY hasn't cleared, so send a CLO */
-		ahci_clo(ap);
-	}
-
-	return ata_drive_probe_reset(ap, ata_std_probeinit,
-				     ahci_softreset, ahci_hardreset,
-				     ahci_postreset, classes);
-}
-
 static u8 ahci_check_status(struct ata_port *ap)
 {
 	void __iomem *mmio = (void __iomem *) ap->ioaddr.cmd_addr;
@@ -839,8 +847,7 @@ static void ahci_error_intr(struct ata_p
 	}
 
 	if (irq_stat & (PORT_IRQ_CONNECT | PORT_IRQ_PHYRDY)) {
-		err_mask |= AC_ERR_ATA_BUS;
-		action |= ATA_EH_SOFTRESET;
+		ata_ehi_hotplugged(ehi);
 		ata_ehi_push_desc(ehi, ", %s", irq_stat & PORT_IRQ_CONNECT ?
 			"connection status changed" : "PHY RDY changed");
 	}
@@ -1027,7 +1034,7 @@ static void ahci_error_handler(struct at
 	}
 
 	/* perform recovery */
-	ata_do_eh(ap, ata_std_prereset, ahci_softreset, ahci_hardreset,
+	ata_do_eh(ap, ahci_prereset, ahci_softreset, ahci_hardreset,
 		  ahci_postreset);
 }
 
-- 
1.3.2

Re: [PATCH 10/13] ahci: convert to new probing mechanism and add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> Convert to new probing mechanism and add hotplug support by enabling
> PORT_IRQ_PHYRDY, marking ehi for hotplug and scheduling EH on
> CONNECT/PHYRDY interrupts.
> 
> Unfortunately, ahci cannot reliably wait for the first FIS34 after
> hotplug.  It sometimes succeeds but times out more often than not, so
> ATA_FLAG_CANT_WAIT_FIS34 is used.
> 
> This patch also fixes ahci_hardreset() such that D2H Register FIS RX
> area is cleared before issuing COMRESET.  Without this,
> ata_busy_sleep() after COMRESET might prematually finish if the
> previous TF contains DRDY && !BSY.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

NAK all mention of FIS34, ACK everything else

[PATCH 12/13] libata-hp: killl ops->probe_reset

[Tejun Heo]

Now that all drivers implementing new EH are converted to new probing
mechanism, ops->probe_reset doesn't have any user.  Kill it.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |  187 +++-----------------------------------------
 include/linux/libata.h     |    8 --
 2 files changed, 13 insertions(+), 182 deletions(-)

9fb8ab9ef597845ec62798779071cf7a868b5fab
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index b622c61..c1a10ac 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1479,31 +1479,21 @@ static int ata_bus_probe(struct ata_port
 	down_xfermask = 0;
 
 	/* reset and determine device classes */
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		classes[i] = ATA_DEV_UNKNOWN;
+	ap->ops->phy_reset(ap);
 
-	if (ap->ops->probe_reset) {
-		rc = ap->ops->probe_reset(ap, classes);
-		if (rc) {
-			ata_port_printk(ap, KERN_ERR,
-					"reset failed (errno=%d)\n", rc);
-			return rc;
-		}
-	} else {
-		ap->ops->phy_reset(ap);
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		dev = &ap->device[i];
 
-		for (i = 0; i < ATA_MAX_DEVICES; i++) {
-			if (!(ap->flags & ATA_FLAG_DISABLED))
-				classes[i] = ap->device[i].class;
-			ap->device[i].class = ATA_DEV_UNKNOWN;
-		}
+		if (!(ap->flags & ATA_FLAG_DISABLED) &&
+		    dev->class != ATA_DEV_UNKNOWN)
+			classes[dev->devno] = dev->class;
+		else
+			classes[dev->devno] = ATA_DEV_NONE;
 
-		ata_port_probe(ap);
+		dev->class = ATA_DEV_UNKNOWN;
 	}
 
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		if (classes[i] == ATA_DEV_UNKNOWN)
-			classes[i] = ATA_DEV_NONE;
+	ata_port_probe(ap);
 
 	/* after the reset the device state is PIO 0 and the controller
 	   state is undefined. Record the mode */
@@ -2605,37 +2595,11 @@ int ata_std_prereset(struct ata_port *ap
 }
 
 /**
- *	ata_std_probeinit - initialize probing
- *	@ap: port to be probed
- *
- *	@ap is about to be probed.  Initialize it.  This function is
- *	to be used as standard callback for ata_drive_probe_reset().
- *
- *	NOTE!!! Do not use this function as probeinit if a low level
- *	driver implements only hardreset.  Just pass NULL as probeinit
- *	in that case.  Using this function is probably okay but doing
- *	so makes reset sequence different from the original
- *	->phy_reset implementation and Jeff nervous.  :-P
- */
-void ata_std_probeinit(struct ata_port *ap)
-{
-	static const unsigned long deb_timing[] = { 5, 100, 5000 };
-
-	/* resume link */
-	sata_phy_resume(ap, deb_timing);
-
-	/* wait for device */
-	if (ata_port_online(ap))
-		ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
-}
-
-/**
  *	ata_std_softreset - reset host port via ATA SRST
  *	@ap: port to reset
  *	@classes: resulting classes of attached devices
  *
- *	Reset host port using ATA SRST.  This function is to be used
- *	as standard callback for ata_drive_*_reset() functions.
+ *	Reset host port using ATA SRST.
  *
  *	LOCKING:
  *	Kernel thread context (may sleep)
@@ -2690,8 +2654,6 @@ int ata_std_softreset(struct ata_port *a
  *	@class: resulting class of attached device
  *
  *	SATA phy-reset host port using DET bits of SControl register.
- *	This function is to be used as standard callback for
- *	ata_drive_*_reset().
  *
  *	LOCKING:
  *	Kernel thread context (may sleep)
@@ -2770,9 +2732,6 @@ int sata_std_hardreset(struct ata_port *
  *	the device might have been reset more than once using
  *	different reset methods before postreset is invoked.
  *
- *	This function is to be used as standard callback for
- *	ata_drive_*_reset().
- *
  *	LOCKING:
  *	Kernel thread context (may sleep)
  */
@@ -2819,32 +2778,6 @@ void ata_std_postreset(struct ata_port *
 	DPRINTK("EXIT\n");
 }
 
-/**
- *	ata_std_probe_reset - standard probe reset method
- *	@ap: prot to perform probe-reset
- *	@classes: resulting classes of attached devices
- *
- *	The stock off-the-shelf ->probe_reset method.
- *
- *	LOCKING:
- *	Kernel thread context (may sleep)
- *
- *	RETURNS:
- *	0 on success, -errno otherwise.
- */
-int ata_std_probe_reset(struct ata_port *ap, unsigned int *classes)
-{
-	ata_reset_fn_t hardreset;
-
-	hardreset = NULL;
-	if (sata_scr_valid(ap))
-		hardreset = sata_std_hardreset;
-
-	return ata_drive_probe_reset(ap, ata_std_probeinit,
-				     ata_std_softreset, hardreset,
-				     ata_std_postreset, classes);
-}
-
 int ata_do_reset(struct ata_port *ap, ata_reset_fn_t reset,
 		 unsigned int *classes)
 {
@@ -2874,97 +2807,6 @@ int ata_do_reset(struct ata_port *ap, at
 }
 
 /**
- *	ata_drive_probe_reset - Perform probe reset with given methods
- *	@ap: port to reset
- *	@probeinit: probeinit method (can be NULL)
- *	@softreset: softreset method (can be NULL)
- *	@hardreset: hardreset method (can be NULL)
- *	@postreset: postreset method (can be NULL)
- *	@classes: resulting classes of attached devices
- *
- *	Reset the specified port and classify attached devices using
- *	given methods.  This function prefers softreset but tries all
- *	possible reset sequences to reset and classify devices.  This
- *	function is intended to be used for constructing ->probe_reset
- *	callback by low level drivers.
- *
- *	Reset methods should follow the following rules.
- *
- *	- Return 0 on sucess, -errno on failure.
- *	- If classification is supported, fill classes[] with
- *	  recognized class codes.
- *	- If classification is not supported, leave classes[] alone.
- *
- *	LOCKING:
- *	Kernel thread context (may sleep)
- *
- *	RETURNS:
- *	0 on success, -EINVAL if no reset method is avaliable, -ENODEV
- *	if classification fails, and any error code from reset
- *	methods.
- */
-int ata_drive_probe_reset(struct ata_port *ap, ata_probeinit_fn_t probeinit,
-			  ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
-			  ata_postreset_fn_t postreset, unsigned int *classes)
-{
-	int rc = -EINVAL;
-
-	ata_eh_freeze_port(ap);
-
-	if (probeinit)
-		probeinit(ap);
-
-	if (softreset && !sata_set_spd_needed(ap)) {
-		rc = ata_do_reset(ap, softreset, classes);
-		if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
-			goto done;
-		ata_port_printk(ap, KERN_INFO, "softreset failed, "
-				"will try hardreset in 5 secs\n");
-		ssleep(5);
-	}
-
-	if (!hardreset)
-		goto done;
-
-	while (1) {
-		rc = ata_do_reset(ap, hardreset, classes);
-		if (rc == 0) {
-			if (classes[0] != ATA_DEV_UNKNOWN)
-				goto done;
-			break;
-		}
-
-		if (sata_down_spd_limit(ap))
-			goto done;
-
-		ata_port_printk(ap, KERN_INFO, "hardreset failed, "
-				"will retry in 5 secs\n");
-		ssleep(5);
-	}
-
-	if (softreset) {
-		ata_port_printk(ap, KERN_INFO,
-				"hardreset succeeded without classification, "
-				"will retry softreset in 5 secs\n");
-		ssleep(5);
-
-		rc = ata_do_reset(ap, softreset, classes);
-	}
-
- done:
-	if (rc == 0) {
-		if (postreset)
-			postreset(ap, classes);
-
-		ata_eh_thaw_port(ap);
-
-		if (classes[0] == ATA_DEV_UNKNOWN)
-			rc = -ENODEV;
-	}
-	return rc;
-}
-
-/**
  *	ata_dev_same_device - Determine whether new ID matches configured device
  *	@dev: device to compare against
  *	@new_class: class of the new device
@@ -5413,7 +5255,7 @@ static struct ata_port * ata_host_add(co
 
 	DPRINTK("ENTER\n");
 
-	if (!ent->port_ops->probe_reset && !ent->port_ops->error_handler &&
+	if (!ent->port_ops->error_handler &&
 	    !(ent->host_flags & (ATA_FLAG_SATA_RESET | ATA_FLAG_SRST))) {
 		printk(KERN_ERR "ata%u: no reset mechanism available\n",
 		       port_no);
@@ -5551,7 +5393,7 @@ int ata_device_add(const struct ata_prob
 			 */
 		}
 
-		if (!ap->ops->probe_reset) {
+		if (ap->ops->error_handler) {
 			int bit = fls(ATA_FLAG_LOADING) - 1;
 			unsigned long flags;
 
@@ -5993,13 +5835,10 @@ EXPORT_SYMBOL_GPL(sata_phy_resume);
 EXPORT_SYMBOL_GPL(sata_phy_reset);
 EXPORT_SYMBOL_GPL(__sata_phy_reset);
 EXPORT_SYMBOL_GPL(ata_bus_reset);
-EXPORT_SYMBOL_GPL(ata_std_probeinit);
 EXPORT_SYMBOL_GPL(ata_std_prereset);
 EXPORT_SYMBOL_GPL(ata_std_softreset);
 EXPORT_SYMBOL_GPL(sata_std_hardreset);
 EXPORT_SYMBOL_GPL(ata_std_postreset);
-EXPORT_SYMBOL_GPL(ata_std_probe_reset);
-EXPORT_SYMBOL_GPL(ata_drive_probe_reset);
 EXPORT_SYMBOL_GPL(ata_dev_revalidate);
 EXPORT_SYMBOL_GPL(ata_dev_classify);
 EXPORT_SYMBOL_GPL(ata_dev_pair);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 99a2b68..f2d55ee 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -302,7 +302,6 @@ struct ata_queued_cmd;
 
 /* typedefs */
 typedef void (*ata_qc_cb_t) (struct ata_queued_cmd *qc);
-typedef void (*ata_probeinit_fn_t)(struct ata_port *ap);
 typedef int (*ata_prereset_fn_t)(struct ata_port *ap);
 typedef int (*ata_reset_fn_t)(struct ata_port *ap, unsigned int *classes);
 typedef void (*ata_postreset_fn_t)(struct ata_port *ap, unsigned int *classes);
@@ -551,7 +550,6 @@ struct ata_port_operations {
 
 	void (*phy_reset) (struct ata_port *ap); /* obsolete */
 	void (*set_mode) (struct ata_port *ap);
-	int (*probe_reset) (struct ata_port *ap, unsigned int *classes);
 
 	void (*post_set_mode) (struct ata_port *ap);
 
@@ -626,11 +624,6 @@ extern void ata_bus_reset(struct ata_por
 extern int sata_set_spd(struct ata_port *ap);
 extern int sata_phy_debounce(struct ata_port *ap, const unsigned long *param);
 extern int sata_phy_resume(struct ata_port *ap, const unsigned long *param);
-extern int ata_drive_probe_reset(struct ata_port *ap,
-			ata_probeinit_fn_t probeinit,
-			ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
-			ata_postreset_fn_t postreset, unsigned int *classes);
-extern void ata_std_probeinit(struct ata_port *ap);
 extern int ata_std_prereset(struct ata_port *ap);
 extern int ata_std_softreset(struct ata_port *ap, unsigned int *classes);
 extern int sata_std_hardreset(struct ata_port *ap, unsigned int *class);
@@ -686,7 +679,6 @@ extern void ata_std_dev_select (struct a
 extern u8 ata_check_status(struct ata_port *ap);
 extern u8 ata_altstatus(struct ata_port *ap);
 extern void ata_exec_command(struct ata_port *ap, const struct ata_taskfile *tf);
-extern int ata_std_probe_reset(struct ata_port *ap, unsigned int *classes);
 extern int ata_port_start (struct ata_port *ap);
 extern void ata_port_stop (struct ata_port *ap);
 extern void ata_host_stop (struct ata_host_set *host_set);
-- 
1.3.2

[PATCH 13/13] libata-hp: move ata_do_reset() to libata-eh.c

[Tejun Heo]

With ops->probe_init() gone, no user is left in libata-core.c.  Move
ata_do_reset() to libata-eh.c and make it static.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   28 ----------------------------
 drivers/scsi/libata-eh.c   |   28 ++++++++++++++++++++++++++++
 drivers/scsi/libata.h      |    2 --
 3 files changed, 28 insertions(+), 30 deletions(-)

854a6e658abac125cb2dbe8d081448d9a76c166a
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index c1a10ac..2274468 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -2778,34 +2778,6 @@ void ata_std_postreset(struct ata_port *
 	DPRINTK("EXIT\n");
 }
 
-int ata_do_reset(struct ata_port *ap, ata_reset_fn_t reset,
-		 unsigned int *classes)
-{
-	int i, rc;
-
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		classes[i] = ATA_DEV_UNKNOWN;
-
-	rc = reset(ap, classes);
-	if (rc)
-		return rc;
-
-	/* If any class isn't ATA_DEV_UNKNOWN, consider classification
-	 * is complete and convert all ATA_DEV_UNKNOWN to
-	 * ATA_DEV_NONE.
-	 */
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		if (classes[i] != ATA_DEV_UNKNOWN)
-			break;
-
-	if (i < ATA_MAX_DEVICES)
-		for (i = 0; i < ATA_MAX_DEVICES; i++)
-			if (classes[i] == ATA_DEV_UNKNOWN)
-				classes[i] = ATA_DEV_NONE;
-
-	return 0;
-}
-
 /**
  *	ata_dev_same_device - Determine whether new ID matches configured device
  *	@dev: device to compare against
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 9eb73bb..26bd83d 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -1316,6 +1316,34 @@ static void ata_eh_report(struct ata_por
 	}
 }
 
+static int ata_do_reset(struct ata_port *ap, ata_reset_fn_t reset,
+			unsigned int *classes)
+{
+	int i, rc;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		classes[i] = ATA_DEV_UNKNOWN;
+
+	rc = reset(ap, classes);
+	if (rc)
+		return rc;
+
+	/* If any class isn't ATA_DEV_UNKNOWN, consider classification
+	 * is complete and convert all ATA_DEV_UNKNOWN to
+	 * ATA_DEV_NONE.
+	 */
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (classes[i] != ATA_DEV_UNKNOWN)
+			break;
+
+	if (i < ATA_MAX_DEVICES)
+		for (i = 0; i < ATA_MAX_DEVICES; i++)
+			if (classes[i] == ATA_DEV_UNKNOWN)
+				classes[i] = ATA_DEV_NONE;
+
+	return 0;
+}
+
 static int ata_eh_followup_srst_needed(int rc, int classify,
 				       const unsigned int *classes)
 {
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 2d8cc05..2320b4d 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -57,8 +57,6 @@ extern int sata_down_spd_limit(struct at
 extern int sata_set_spd_needed(struct ata_port *ap);
 extern int ata_down_xfermask_limit(struct ata_device *dev, int force_pio0);
 extern int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev);
-extern int ata_do_reset(struct ata_port *ap, ata_reset_fn_t reset,
-			unsigned int *classes);
 extern void ata_qc_free(struct ata_queued_cmd *qc);
 extern void ata_qc_issue(struct ata_queued_cmd *qc);
 extern void __ata_qc_complete(struct ata_queued_cmd *qc);
-- 
1.3.2

Re: [PATCH 13/13] libata-hp: move ata_do_reset() to libata-eh.c

[Jeff Garzik]

Tejun Heo wrote:
> With ops->probe_init() gone, no user is left in libata-core.c.  Move
> ata_do_reset() to libata-eh.c and make it static.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK 12-13

[PATCH 07/13] libata-hp: implement unload-unplug

[Tejun Heo]

Implement unload unplug - driver unloading / PCI removal via hot
unplug path.  This is done by ata_port_detach() which requests detach
of all devices, schedules EH and wait for it to complete.  EH path is
slightly modified to handle this (e.g. force zero eh_info during
unloading).  With this patch, EH and unloading are properly
synchronized and unloading should be safe under any circumstances.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ahci.c        |   10 ++-----
 drivers/scsi/libata-core.c |   59 ++++++++++++++++++++++++++++++++++++++++----
 drivers/scsi/libata-eh.c   |    7 +++--
 include/linux/libata.h     |    1 +
 4 files changed, 61 insertions(+), 16 deletions(-)

dc54139a72085d78f1fc35a721cff16cdeb38ef9
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index afb3805..60f455b 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -1389,21 +1389,17 @@ static void ahci_remove_one (struct pci_
 	struct device *dev = pci_dev_to_dev(pdev);
 	struct ata_host_set *host_set = dev_get_drvdata(dev);
 	struct ahci_host_priv *hpriv = host_set->private_data;
-	struct ata_port *ap;
 	unsigned int i;
 	int have_msi;
 
-	for (i = 0; i < host_set->n_ports; i++) {
-		ap = host_set->ports[i];
-
-		scsi_remove_host(ap->host);
-	}
+	for (i = 0; i < host_set->n_ports; i++)
+		ata_port_detach(host_set->ports[i]);
 
 	have_msi = hpriv->flags & AHCI_FLAG_MSI;
 	free_irq(host_set->irq, host_set);
 
 	for (i = 0; i < host_set->n_ports; i++) {
-		ap = host_set->ports[i];
+		struct ata_port *ap = host_set->ports[i];
 
 		ata_scsi_release(ap->host);
 		scsi_host_put(ap->host);
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index f45e04f..b622c61 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5613,6 +5613,55 @@ err_free_ret:
 }
 
 /**
+ *	ata_port_detach - Detach ATA port in prepration of device removal
+ *	@ap: ATA port to be detached
+ *
+ *	Detach all ATA devices and the associated SCSI devices of @ap;
+ *	then, remove the associated SCSI host.  @ap is guaranteed to
+ *	be quiescent on return from this function.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+void ata_port_detach(struct ata_port *ap)
+{
+	unsigned long flags;
+	int i;
+
+	if (!ap->ops->error_handler)
+		return;
+
+	/* tell EH to self-destruct */
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	ap->flags |= ATA_FLAG_UNLOADING;
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		ap->device[i].flags |= ATA_DFLAG_DETACH;
+
+	ata_port_schedule_eh(ap);
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+	/* wait for EH */
+	while (scsi_host_in_recovery(ap->host))
+		msleep(100);
+
+	/* Flush hotplug task.  The sequence is similar to
+	 * ata_port_flush_task().  ATA_FLAG_UNLOADING prevents further
+	 * queueing of hotplug_task.
+	 */
+	flush_workqueue(ata_scsi_wq);
+	cancel_delayed_work(&ap->hotplug_task);
+	flush_workqueue(ata_scsi_wq);
+
+	/* freeze the port */
+	ata_eh_freeze_port(ap);
+
+	/* remove the associated SCSI host */
+	scsi_remove_host(ap->host);
+}
+
+/**
  *	ata_host_set_remove - PCI layer callback for device removal
  *	@host_set: ATA host set that was removed
  *
@@ -5625,18 +5674,15 @@ err_free_ret:
 
 void ata_host_set_remove(struct ata_host_set *host_set)
 {
-	struct ata_port *ap;
 	unsigned int i;
 
-	for (i = 0; i < host_set->n_ports; i++) {
-		ap = host_set->ports[i];
-		scsi_remove_host(ap->host);
-	}
+	for (i = 0; i < host_set->n_ports; i++)
+		ata_port_detach(host_set->ports[i]);
 
 	free_irq(host_set->irq, host_set);
 
 	for (i = 0; i < host_set->n_ports; i++) {
-		ap = host_set->ports[i];
+		struct ata_port *ap = host_set->ports[i];
 
 		ata_scsi_release(ap->host);
 
@@ -5904,6 +5950,7 @@ EXPORT_SYMBOL_GPL(sata_deb_timing_before
 EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_device_add);
+EXPORT_SYMBOL_GPL(ata_port_detach);
 EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 7b5cb02..9eb73bb 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -234,7 +234,8 @@ void ata_scsi_error(struct Scsi_Host *ho
 		spin_lock_irqsave(hs_lock, flags);
 
 		memset(&ap->eh_context, 0, sizeof(ap->eh_context));
-		ap->eh_context.i = ap->eh_info;
+		if (!(ap->flags & ATA_FLAG_UNLOADING))
+			ap->eh_context.i = ap->eh_info;
 		memset(&ap->eh_info, 0, sizeof(ap->eh_info));
 
 		ap->flags &= ~ATA_FLAG_EH_PENDING;
@@ -290,7 +291,7 @@ void ata_scsi_error(struct Scsi_Host *ho
 		int bit = fls(ATA_FLAG_LOADING) - 1;
 		clear_bit(bit, &ap->flags);
 		wake_up_bit(&ap->flags, bit);
-	} else {
+	} else if (!(ap->flags & ATA_FLAG_UNLOADING)) {
 		if (ap->flags & ATA_FLAG_SCSI_HOTPLUG)
 			queue_work(ata_scsi_wq, &ap->hotplug_task);
 		if (ap->flags & ATA_FLAG_RECOVERED)
@@ -1769,7 +1770,7 @@ void ata_do_eh(struct ata_port *ap, ata_
 	       ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
 	       ata_postreset_fn_t postreset)
 {
-	if (!(ap->flags & ATA_FLAG_LOADING)) {
+	if (!(ap->flags & (ATA_FLAG_LOADING | ATA_FLAG_UNLOADING))) {
 		ata_eh_autopsy(ap);
 		ata_eh_report(ap);
 	}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b216400..99a2b68 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -647,6 +647,7 @@ extern int ata_pci_device_resume(struct 
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
 extern int ata_device_add(const struct ata_probe_ent *ent);
+extern void ata_port_detach(struct ata_port *ap);
 extern void ata_host_set_remove(struct ata_host_set *host_set);
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
-- 
1.3.2

Re: [PATCH 07/13] libata-hp: implement unload-unplug

[Jeff Garzik]

Tejun Heo wrote:
> Implement unload unplug - driver unloading / PCI removal via hot
> unplug path.  This is done by ata_port_detach() which requests detach
> of all devices, schedules EH and wait for it to complete.  EH path is
> slightly modified to handle this (e.g. force zero eh_info during
> unloading).  With this patch, EH and unloading are properly
> synchronized and unloading should be safe under any circumstances.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

How does this behave for controller hotplug?  For that case, there is 
nothing to EH...

	Jeff

Re: [PATCH 07/13] libata-hp: implement unload-unplug

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Implement unload unplug - driver unloading / PCI removal via hot
>> unplug path.  This is done by ata_port_detach() which requests detach
>> of all devices, schedules EH and wait for it to complete.  EH path is
>> slightly modified to handle this (e.g. force zero eh_info during
>> unloading).  With this patch, EH and unloading are properly
>> synchronized and unloading should be safe under any circumstances.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> How does this behave for controller hotplug?  For that case, there is 
> nothing to EH...

I've only tested with module unloading but when EH is invoked for 
unplugging, autopsy and report are skipped and recover is called with 
zero ehi but with all DFLAG_DETACH set.  This amounts to unconditional 
detachment of all devices without any other operation (well, of course, 
other than being synchronized to other EH invocations).  So, I guess it 
should work okay.  Can you tell me how to test such condition?  I guess 
I can yank out my sil3132 and unload sata_sil24.  Would that be enough?

-- 
tejun

Re: [PATCH 07/13] libata-hp: implement unload-unplug

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Implement unload unplug - driver unloading / PCI removal via hot
>>> unplug path.  This is done by ata_port_detach() which requests detach
>>> of all devices, schedules EH and wait for it to complete.  EH path is
>>> slightly modified to handle this (e.g. force zero eh_info during
>>> unloading).  With this patch, EH and unloading are properly
>>> synchronized and unloading should be safe under any circumstances.
>>>
>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>> How does this behave for controller hotplug?  For that case, there is 
>> nothing to EH...
> 
> I've only tested with module unloading but when EH is invoked for 
> unplugging, autopsy and report are skipped and recover is called with 
> zero ehi but with all DFLAG_DETACH set.  This amounts to unconditional 
> detachment of all devices without any other operation (well, of course, 
> other than being synchronized to other EH invocations).  So, I guess it 
> should work okay.  Can you tell me how to test such condition?  I guess 
> I can yank out my sil3132 and unload sata_sil24.  Would that be enough?

ISTR there is some sysfs trigger for PCI eject, that will allow you test 
this.

	Jeff

[PATCH 11/13] sata_sil24: convert to new probing mechanism and add hotplug support

[Tejun Heo]

Convert to new probing mechanism and add hotplug support by enabling
PORT_IRQ_PHYRDY_CHG, marking ehi for hotplug and scheduling EH on
PORT_IRQ_PHYRDY_CHG or PORT_IRQ_DEV_XCHG.

Sil3124/32 family of controllers don't have any mechanism to wait for
the first FIS34 after hotplug, so ATA_FLAG_CANT_WAIT_FIS34 is used.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/sata_sil24.c |   27 ++++++++-------------------
 1 files changed, 8 insertions(+), 19 deletions(-)

cea49da1f8d8278d477d8b82fc6ae0a4f195faae
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index d4ca6d6..97e9339 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -159,7 +159,8 @@ enum {
 	PORT_IRQ_SDB_NOTIFY	= (1 << 11), /* SDB notify received */
 
 	DEF_PORT_IRQ		= PORT_IRQ_COMPLETE | PORT_IRQ_ERROR |
-				  PORT_IRQ_DEV_XCHG | PORT_IRQ_UNK_FIS,
+				  PORT_IRQ_PHYRDY_CHG | PORT_IRQ_DEV_XCHG |
+				  PORT_IRQ_UNK_FIS,
 
 	/* bits[27:16] are unmasked (raw) */
 	PORT_IRQ_RAW_SHIFT	= 16,
@@ -228,7 +229,7 @@ enum {
 	/* host flags */
 	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  ATA_FLAG_NCQ,
+				  ATA_FLAG_NCQ | ATA_FLAG_SKIP_D2H_BSY,
 	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
 
 	IRQ_STAT_4PORTS		= 0xf,
@@ -325,7 +326,6 @@ static u8 sil24_check_status(struct ata_
 static u32 sil24_scr_read(struct ata_port *ap, unsigned sc_reg);
 static void sil24_scr_write(struct ata_port *ap, unsigned sc_reg, u32 val);
 static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
-static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void sil24_qc_prep(struct ata_queued_cmd *qc);
 static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc);
 static void sil24_irq_clear(struct ata_port *ap);
@@ -385,8 +385,6 @@ static const struct ata_port_operations 
 
 	.tf_read		= sil24_tf_read,
 
-	.probe_reset		= sil24_probe_reset,
-
 	.qc_prep		= sil24_qc_prep,
 	.qc_issue		= sil24_qc_issue,
 
@@ -635,13 +633,6 @@ static int sil24_hardreset(struct ata_po
 	return -EIO;
 }
 
-static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes)
-{
-	return ata_drive_probe_reset(ap, ata_std_probeinit,
-				     sil24_softreset, sil24_hardreset,
-				     ata_std_postreset, classes);
-}
-
 static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
 				 struct sil24_sge *sge)
 {
@@ -772,13 +763,11 @@ static void sil24_error_intr(struct ata_
 
 	ata_ehi_push_desc(ehi, "irq_stat 0x%08x", irq_stat);
 
-	if (irq_stat & PORT_IRQ_DEV_XCHG) {
-		ehi->err_mask |= AC_ERR_ATA_BUS;
-		/* sil24 doesn't recover very well from phy
-		 * disconnection with a softreset.  Force hardreset.
-		 */
-		ehi->action |= ATA_EH_HARDRESET;
-		ata_ehi_push_desc(ehi, ", device_exchanged");
+	if (irq_stat & (PORT_IRQ_PHYRDY_CHG | PORT_IRQ_DEV_XCHG)) {
+		ata_ehi_hotplugged(ehi);
+		ata_ehi_push_desc(ehi, ", %s",
+			       irq_stat & PORT_IRQ_PHYRDY_CHG ?
+			       "PHY RDY changed" : "device exchanged");
 		freeze = 1;
 	}
 
-- 
1.3.2

Re: [PATCH 11/13] sata_sil24: convert to new probing mechanism and add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> Convert to new probing mechanism and add hotplug support by enabling
> PORT_IRQ_PHYRDY_CHG, marking ehi for hotplug and scheduling EH on
> PORT_IRQ_PHYRDY_CHG or PORT_IRQ_DEV_XCHG.
> 
> Sil3124/32 family of controllers don't have any mechanism to wait for
> the first FIS34 after hotplug, so ATA_FLAG_CANT_WAIT_FIS34 is used.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

same comment as before.  "FIS34" string must die :)