[PATCHSET 9/9] add hotplug support

[PATCHSET 9/9] add hotplug support

[Tejun Heo]

Hellooooo, all.

Finally, the last one.  This is the first take of add-hotplug-support
patchset.  This patchset includes 13 patches.

#01-04	implement hotplug framework
#05-07	implement warm plug (SCSI scan/delete callbacks)
#08-10	hook and activate hotplug
#11-13	add LLDD hotplug supports (PHY status change notifications)

Hotplugging is implemented as an integral part of EH.  A new EH helper
ata_eh_hotplug() is defined which should be called after all regular
EH is done.  LLDDs have to do very little to support hotplug.

Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
- adding a call to ata_eh_hotplug() at the end of ->error_handler is
enough (even this is unnecessary if it's using bmdma error_handler).
e.g. ata_piix isn't changed by hotplug patchsets at all but still
supports warm plugging (even PATA warm plugging works although it's
electronically unsafe).

To support full hot unplug/plug, all a LLDD has to do are enabling PHY
status changed interrupt and, when PHY RDY status changes, call
ata_schedule_probe() and schedule EH.  ahci, sata_sil and sata_sil24
are converted this way and hotplugging works perfectly for those
drivers.

One of the design goals was making hotplug support and the driver
resillient.  Hardware might get stuck but the driver should NEVER oops
or malfunction no matter what the user does.  As far as libata is
conerned, all are allowed - yanking and plugging at any time,
requesting warm plug or unplug under heavy IO or during EH.  So, if
you have some time, torture libata, be pervert and break it.  If you
succeed, please let me know.  I'll try to reproduce your perverseness
and fix the breakage.

Some hardware gotchas I've found...

* ICH7 in piix mode has troubles seeing new SATA devices if the port
  was not occupied on boot.

* sil3124 locks up really hard if it's requested to scan the bus
  (which triggers immediate ATA_EH_FREEZE) while commands to the port
  are in progress.  Port reset cannot resume the port.  Power cycling
  works.  Maybe host reset needed?

This patchset is against

  upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2)
  + [1] misc-reset-updates patchset (repost)
  + [2] implement-and-use-ata_wait_register patchset (repost)
  + [3] misc-ata_bus_probe-updates patchset
  + [4] fixes-errata-workaround-and-reset-updates patchset, take 2
  + [5] implement-scsi_eh_schedule patchset
  + [6] fix-scsi_kill_request-busy-count-handling patch
  + [7] new-EH-framework patchset, take 2
  + [8] new-EH-implementation patchset, take 2
  + [9] add-NCQ-support patchset, take 2
  + [10] prep-for-hotplug-support patchset

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/9495
[2] http://article.gmane.org/gmane.linux.ide/9499
[3] http://article.gmane.org/gmane.linux.ide/9506
[4] http://article.gmane.org/gmane.linux.ide/9516
[5] http://article.gmane.org/gmane.linux.ide/9290
[6] http://article.gmane.org/gmane.linux.ide/9487
[7] http://article.gmane.org/gmane.linux.ide/9524
[8] http://article.gmane.org/gmane.linux.ide/9540
[9] http://article.gmane.org/gmane.linux.ide/9555
[10] http://article.gmane.org/gmane.linux.ide/9579

[PATCH 03/15] libata-hp: implement ata_eh_scsi_hotplug()

[Tejun Heo]

This is SCSI part of hotplug support.  Devices marked for SCSI unplug
are scsi_remove'd, and, if SCSI plug is requested, all enabled but
unattached ATA devices get scanned and attached.  This patch only
implements SCSI hotplug support.  The next patch will connect ATA
hotplug events to SCSI hotplug.

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

---

 drivers/scsi/libata-eh.c   |   59 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libata-scsi.c |   41 +++++++++++++++++++++++++++++++
 drivers/scsi/libata.h      |    2 +
 3 files changed, 102 insertions(+), 0 deletions(-)

14867aadbb30dc446856a73ff519456a38e1758d
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 7dc6ea6..ee46221 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -1472,3 +1472,62 @@ void ata_eh_hotplug(struct ata_port *ap)
 	 * current EH run is complete.
 	 */
 }
+
+/**
+ *	ata_eh_scsi_hotplug - SCSI part of hot [un]plugging
+ *	@data: Pointer to ATA port to perform SCSI hot [un]plugging on
+ *
+ *	Perform SCSI part of hot [un]plugging.  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.
+ */
+void ata_eh_scsi_hotplug(void *data)
+{
+	struct ata_port *ap = data;
+	unsigned long timeout;
+	int i;
+
+	ata_clr_hotplug_flags(ap, ATA_HOTPLUG_SCSI_UNPLUG);
+
+	/* always unplug detached devices */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+
+		if (dev->flags & ATA_DFLAG_DETACH_SCSI) {
+			unsigned long flags;
+
+			spin_lock_irqsave(&ap->host_set->lock, flags);
+			dev->flags &= ~ATA_DFLAG_DETACH_SCSI;
+			spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+			ata_scsi_remove_dev(ap, dev);
+		}
+	}
+
+	/* SCSI hotplug is requested.  EH might still be running and
+	 * we wanna scan the bus after EH is complete; otherwise, SCSI
+	 * scan fails 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 at most 5 secs.
+	 *
+	 * The worst that can happen here is bus scan failure, well,
+	 * the user will have to replug the device.  No biggie.
+	 */
+	timeout = jiffies + 5 * HZ;
+	do {
+		if (!scsi_host_in_recovery(ap->host))
+			break;
+		msleep(100);
+	} while (time_before(jiffies, timeout));
+
+	if (scsi_host_in_recovery(ap->host))
+		printk(KERN_WARNING "ata%u: EH still in progress after "
+		       "5 secs, SCSI scan might fail\n", ap->id);
+
+	/* scan if plug operation is requested */
+	if (ap->hotplug_flags & ATA_HOTPLUG_SCSI_PLUG) {
+		ata_clr_hotplug_flags(ap, ATA_HOTPLUG_SCSI_PLUG);
+		ata_scsi_scan_host(ap);
+	}
+}
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 2e8fb2f..fac37cd 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2778,3 +2778,44 @@ void ata_scsi_offline_dev(struct ata_por
 	if (dev->sdev)
 		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
 }
+
+/**
+ *	ata_scsi_remove_dev - remove attached SCSI device
+ *	@ap: ATA port @dev is on
+ *	@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).
+ */
+void ata_scsi_remove_dev(struct ata_port *ap, struct ata_device *dev)
+{
+	struct scsi_device *sdev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	sdev = dev->sdev;
+	if (!sdev) {
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+		return;
+	}
+
+	/* clearing dev->sdev is protected by host_set lock */
+	dev->sdev = NULL;
+
+	/* 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);
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+	printk(KERN_INFO "ata%u: dev %u detaching (SCSI %s)\n",
+	       ap->id, dev->devno, sdev->sdev_gendev.bus_id);
+
+	scsi_remove_device(sdev);
+}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 9e8adee..88f697a 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -77,6 +77,7 @@ extern struct scsi_transport_template at
 
 extern void ata_scsi_scan_host(struct ata_port *ap);
 extern void ata_scsi_offline_dev(struct ata_port *ap, struct ata_device *dev);
+extern void ata_scsi_remove_dev(struct ata_port *ap, struct ata_device *dev);
 extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
 			       unsigned int buflen);
 
@@ -110,5 +111,6 @@ extern void ata_scsi_rbuf_fill(struct at
 extern void ata_ering_init(struct ata_ering *ering, int size);
 extern enum scsi_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd);
 extern void ata_eh_schedule_qc(struct ata_queued_cmd *qc);
+extern void ata_eh_scsi_hotplug(void *data);
 
 #endif /* __LIBATA_H__ */
-- 
1.2.4

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

[Tejun Heo]

This function is to be called from EH to disable a device and mark it
for detach.

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

---

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

7e411eeec0a5d02daa41a48debc6a73ad970921d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 610f963..8f50a99 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5483,6 +5483,7 @@ EXPORT_SYMBOL_GPL(ata_eng_timeout);
 EXPORT_SYMBOL_GPL(ata_eh_schedule_port);
 EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
 EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
+EXPORT_SYMBOL_GPL(ata_eh_detach_dev);
 EXPORT_SYMBOL_GPL(ata_eh_determine_qc);
 EXPORT_SYMBOL_GPL(ata_eh_autopsy);
 EXPORT_SYMBOL_GPL(ata_eh_report);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 1a87bce..1f55cc9 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -560,6 +560,26 @@ static unsigned int atapi_eh_request_sen
 }
 
 /**
+ *	ata_eh_detach_dev - EH helper to disable device and schedule detach
+ *	@ap: port on which device to be detached resides
+ *	@dev: device to be detached
+ *
+ *	Disable @dev and schedule it to be detached.
+ *
+ *	LOCKING:
+ *	None.
+ */
+void ata_eh_detach_dev(struct ata_port *ap, struct ata_device *dev)
+{
+	unsigned long flags;
+
+	ata_dev_disable(ap, dev);
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+	dev->flags |= ATA_DFLAG_DETACH_ATA | ATA_DFLAG_DETACH_SCSI;
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+}
+
+/**
  *	ata_read_log_page - read a specific log page
  *	@ap: port on which device we wish to probe resides
  *	@dev: target device
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5f8faca..ee86a87 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -719,6 +719,7 @@ extern void ata_eng_timeout(struct ata_p
 extern void ata_eh_schedule_port(struct ata_port *ap, unsigned int flags);
 extern void ata_eh_qc_complete(struct ata_queued_cmd *qc);
 extern void ata_eh_qc_retry(struct ata_queued_cmd *qc);
+extern void ata_eh_detach_dev(struct ata_port *ap, struct ata_device *dev);
 extern struct ata_queued_cmd * ata_eh_determine_qc(struct ata_port *ap,
 						   int use_log_10h,
 						   struct ata_taskfile *tf);
-- 
1.2.4

[PATCH 08/15] libata-hp: add hotplug hooks into regular EH

[Tejun Heo]

Add hotplug handling into ata_eh_analyze_serror() and ata_eh_revive()
such that they schedule probe or detach if it seems appropriate.

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

---

 drivers/scsi/libata-eh.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

a335b73795061654e9dee96c2fc5f161e92780c7
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 6a61c5c..443f81f 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -855,7 +855,8 @@ static unsigned int ata_eh_analyze_tf(st
  *	@p_err_mask: Resulting err_mask
  *
  *	Analyze SError if available and further determine cause of
- *	failure.
+ *	failure.  This function also schedules probing if PHY status
+ *	has changed.
  *
  *	LOCKING:
  *	None.
@@ -885,6 +886,11 @@ static unsigned int ata_eh_analyze_serro
 		*p_err_mask |= AC_ERR_SYSTEM;
 		action |= ATA_PORT_SOFTRESET;
 	}
+	if (serror & (SERR_PHYRDY_CHG | SERR_DEV_XCHG)) {
+		*p_err_mask |= AC_ERR_ATA_BUS;
+		action |= ATA_PORT_SOFTRESET;
+		ata_schedule_probe(ap);
+	}
 
 	return action;
 }
@@ -1282,8 +1288,10 @@ int ata_eh_revive(struct ata_port *ap, u
 
  fail:
 	switch (rc) {
-	case -EINVAL:
 	case -ENODEV:
+		ata_schedule_probe(ap);
+	case -EINVAL:
+		ata_eh_detach_dev(ap, dev);
 		tries[dev->devno] = 0;
 		break;
 	case -EIO:
-- 
1.2.4

[PATCH 02/15] libata-hp: implement ata_eh_hotplug()

[Tejun Heo]

This is ATA part of hotplug support.  This function is to be called
from ->error_handler after all regular EH stuff is complete.
ata_eh_hotplug() may automatically mark some devices for detach and
detaches all marked devices.  Then, if requested and makes sense,
ata_bus_probe() is invoked to probe newly attached devices.

ata_eh_hotplug() makes sure that a detached ATA device is not accessed
from its corresponding SCSI device by offlining the SCSI device before
reusing the ATA device.  The rest of SCSI hotplug will be implemented
in the next patch.

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

---

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

adf5feec346d7dea425d98a9e473a8704b2751d6
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 8f50a99..85f0600 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5489,3 +5489,4 @@ EXPORT_SYMBOL_GPL(ata_eh_autopsy);
 EXPORT_SYMBOL_GPL(ata_eh_report);
 EXPORT_SYMBOL_GPL(ata_eh_revive);
 EXPORT_SYMBOL_GPL(ata_eh_finish_qcs);
+EXPORT_SYMBOL_GPL(ata_eh_hotplug);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 1f55cc9..7dc6ea6 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -237,6 +237,9 @@ int ata_scsi_error(struct Scsi_Host *hos
 
 	scsi_eh_flush_done_q(&ap->eh_done_q);
 
+	/* bookkeeping for hotplug */
+	ata_clr_hotplug_flags(ap, ATA_HOTPLUG_PROBE | ATA_HOTPLUG_DID_PROBE);
+
 	DPRINTK("EXIT\n");
 	return 0;
 }
@@ -1369,3 +1372,103 @@ void ata_eh_finish_qcs(struct ata_port *
 		}
 	}
 }
+
+/**
+ *	ata_eh_hotplug - ATA part of hot [un]plugging
+ *	@ap: ATA port to perform ATA hot [un]plugging on
+ *
+ *	Perform ATA port of hot [un]plugging.  Devices to be removed
+ *	are offlined and marked gone.  If probing is requested,
+ *	ata_bus_probe() is called.  This function only handles ATA
+ *	part of hot [un]plugging.  After ATA part is complete SCSI
+ *	operation is requested using SCSI_HOT[UN]PLUG host flags.
+ *	ata_eh_scsi_hotplug() performs the SCSI part.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+void ata_eh_hotplug(struct ata_port *ap)
+{
+	int scr_valid = ap->cbl == ATA_CBL_SATA && ap->ops->scr_read;
+	int i;
+
+	/* Turn off probing request and mark disabled device gone if
+	 * SStatus says no device is attached.
+	 */
+	if (scr_valid && !sata_dev_present(ap)) {
+		struct ata_device *dev = &ap->device[0];
+
+		ata_clr_hotplug_flags(ap, ATA_HOTPLUG_PROBE);
+		if (ata_dev_disabled(dev))
+			ata_eh_detach_dev(ap, dev);
+	}
+
+	/* probe only once per EH run to prevent EH->probe->EH->probe cycle */
+	if (ap->hotplug_flags & ATA_HOTPLUG_DID_PROBE)
+		ata_clr_hotplug_flags(ap, ATA_HOTPLUG_PROBE);
+	ata_set_hotplug_flags(ap, ATA_HOTPLUG_DID_PROBE);
+
+	/* detach all !enabled devices if probing is requested */
+	if (ap->hotplug_flags & ATA_HOTPLUG_PROBE) {
+		for (i = 0; i < ATA_MAX_DEVICES; i++) {
+			struct ata_device *dev = &ap->device[i];
+
+			if (!ata_dev_enabled(dev))
+				ata_eh_detach_dev(ap, dev);
+		}
+	}
+
+	/* offline and detach devices marked for ATA detach */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+		unsigned long flags;
+
+		if (!(dev->flags & ATA_DFLAG_DETACH_ATA))
+			continue;
+
+		/* probably already disabled, but make sure */
+		ata_dev_disable(ap, dev);
+
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		dev->class = ATA_DEV_UNKNOWN;
+		dev->flags &= ~ATA_DFLAG_DETACH_ATA;
+		ata_scsi_offline_dev(ap, dev);
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	}
+
+	/* probe requested? */
+	if (!(ap->hotplug_flags & ATA_HOTPLUG_PROBE))
+		return;
+
+	/* do we have any unoccupied slot? */
+	if (ap->device[0].class == ATA_DEV_UNKNOWN ||
+	    ((ap->flags & ATA_FLAG_SLAVE_POSS) &&
+	     ap->device[1].class == ATA_DEV_UNKNOWN)) {
+		unsigned long flags;
+
+		printk("ata%u: probing bus for new devices\n", ap->id);
+
+		/* make sure the port is frozen */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ata_port_freeze(ap);
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+		ata_eh_wait_before_reset(ap);
+
+		/* probe */
+		if (ata_bus_probe(ap) == 1) {
+			/* An existing device has failed, we need to
+			 * repeat EH.  Freeze the port forcing EH
+			 * framework to reinvoke EH.
+			 */
+			ata_eh_schedule_port(ap, ATA_EH_FREEZE);
+		}
+	}
+
+	/* Probing is allowed only once per EH invocation and probing
+	 * request isn't maintained acress multiple EH runs.  To
+	 * implement these rules, HOTPLUG_PROBE and HOTPLUG_DID_PROBE
+	 * are cleared at the tail of ata_scsi_error() after the
+	 * current EH run is complete.
+	 */
+}
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 1793474..2e8fb2f 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2760,3 +2760,21 @@ void ata_scsi_scan_host(struct ata_port 
 	}
 }
 
+/**
+ *	ata_scsi_offline_dev - offline attached SCSI device
+ *	@ap: ATA port @dev is on
+ *	@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)
+ */
+void ata_scsi_offline_dev(struct ata_port *ap, struct ata_device *dev)
+{
+	if (dev->sdev)
+		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
+}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 09fddf6..9e8adee 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -76,6 +76,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 void ata_scsi_offline_dev(struct ata_port *ap, struct ata_device *dev);
 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 ee86a87..b9dd793 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -734,6 +734,7 @@ extern int ata_eh_revive(struct ata_port
 			 ata_postreset_fn_t postreset);
 extern void ata_eh_finish_qcs(struct ata_port *ap, struct ata_queued_cmd *qc,
 			      struct ata_taskfile *tf);
+extern void ata_eh_hotplug(struct ata_port *ap);
 
 
 static inline int
-- 
1.2.4

[PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH

[Tejun Heo]

Now all the required pieces for hotplug support are in place.  Update
all ->error_handler's to call ata_eh_hotplug() after regular EH is
finished.  This enables hotplug support for all drivers which use new
EH.  With this patch, all new EH drivers can do warm unplug and plug.

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

---

 drivers/scsi/ahci.c         |    1 +
 drivers/scsi/libata-bmdma.c |    1 +
 drivers/scsi/sata_sil24.c   |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index a8b699a..7f478c4 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -864,6 +864,7 @@ static void ahci_error_handler(struct at
 	ata_eh_revive(ap, action,
 		      ahci_softreset, ahci_hardreset, ahci_postreset);
 	ata_eh_finish_qcs(ap, qc, &tf);
+	ata_eh_hotplug(ap);
 }
 
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 3c3bc71..a87efae 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port 
 	ata_eh_report(ap, qc, &tf, serror, action, NULL);
 	ata_eh_revive(ap, action, softreset, hardreset, postreset);
 	ata_eh_finish_qcs(ap, qc, &tf);
+	ata_eh_hotplug(ap);
 }
 
 /**
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 9acf7a4..1ca76f7 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -852,6 +852,7 @@ static void sil24_error_handler(struct a
 	ata_eh_revive(ap, action,
 		      sil24_softreset, sil24_hardreset, sil24_postreset);
 	ata_eh_finish_qcs(ap, qc, &tf);
+	ata_eh_hotplug(ap);
 }
 
 static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)
-- 
1.2.4

[PATCH 06/15] libata-hp: use ata_scsi_slave_destroy() in low level drivers

[Tejun Heo]

Use ata_scsi_slave_destroy() in all libata low level drivers.

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

---

 drivers/scsi/ahci.c         |    1 +
 drivers/scsi/ata_piix.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 +
 15 files changed, 15 insertions(+), 0 deletions(-)

ad8e69e7f7a668b6d60583cb4bfeb1f690331ca7
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 3f1fd7c..a8b699a 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -226,6 +226,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 382ea51..d8ea3f2 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/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 1364b1d..603460e 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -153,6 +153,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 e9152f8..ffb86f4 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -388,6 +388,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 c92fbd0..ad030d9 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -211,6 +211,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 000844a..63503ed 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 0b5446a..b1876c6 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -143,6 +143,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 4b9b2d1..3853c46 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -159,6 +159,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 6a7ea97..9acf7a4 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -371,6 +371,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 eb3cf5b..b66a320 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -97,6 +97,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 a20bc81..6fc5b73 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -300,6 +300,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 801ac37..3600420 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -192,6 +192,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 513dd78..682747a 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -91,6 +91,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 de4e0eb..023aa75 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -104,6 +104,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 cecc1f7..10cd202 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -273,6 +273,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.2.4

[PATCH 07/15] libata-hp: implement transportt->user_scan

[Tejun Heo]

Implement ata_scsi_user_scan() which is used for
ata_scsi_transport_template->user_scan.  This function schedules ATA
hotplug probe on user request.

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

---

 drivers/scsi/libata-scsi.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

18519ac3e6800187945e6d81bd1574c091eb0eaf
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 5e68305..b05cd8a 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -55,6 +55,8 @@ typedef unsigned int (*ata_xlat_func_t)(
 
 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
 #define RW_RECOVERY_MPAGE_LEN 12
@@ -101,6 +103,7 @@ static const u8 def_control_mpage[CONTRO
  */
 struct scsi_transport_template ata_scsi_transport_template = {
 	.eh_timed_out		= ata_scsi_timed_out,
+	.user_scan		= ata_scsi_user_scan,
 };
 
 
@@ -2852,3 +2855,37 @@ void ata_scsi_remove_dev(struct ata_port
 
 	scsi_remove_device(sdev);
 }
+
+/**
+ *	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.  libata scans the
+ *	whole bus regardless of channel, id and lun arguments.
+ *
+ *	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 = (struct ata_port *)&shost->hostdata[0];
+	unsigned long flags;
+
+	if (!ap->ops->error_handler)
+		return -EOPNOTSUPP;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+	ata_set_hotplug_flags(ap, ATA_HOTPLUG_PROBE);
+	ata_eh_schedule_port(ap, ATA_EH_FREEZE);
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+	return 0;
+}
-- 
1.2.4

[PATCH 04/15] libata-hp: connect ATA hotplug events to SCSI hotplug

[Tejun Heo]

Schedule SCSI plug/unplug operations from EH with appropriate hotplug
flags and run scsi hotplug from ata_hotplug_wq on EH completion.  On
host initialization, SCSI hotplug starts disabled until
ATA_HOTPLUG_RUNNING flag is set on boot probing completion.  This is
to avoid running multiple SCSI plug/unplug operations simultaneously.

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

---

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

dce4c2009a3abc3fa370b639feda83e09c96dc9c
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 85f0600..91a01ca 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4896,6 +4896,7 @@ static void ata_host_init(struct ata_por
 	ap->last_ctl = 0xFF;
 
 	INIT_WORK(&ap->port_task, NULL, NULL);
+	INIT_WORK(&ap->hotplug_task, ata_eh_scsi_hotplug, ap);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
@@ -5088,6 +5089,8 @@ int ata_device_add(const struct ata_prob
 		struct ata_port *ap = host_set->ports[i];
 
 		ata_scsi_scan_host(ap);
+		/* boot probe done, allow hotplugging */
+		ata_set_hotplug_flags(ap, ATA_HOTPLUG_RUNNING);
 	}
 
 	dev_set_drvdata(dev, host_set);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index ee46221..64edf48 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -240,6 +240,12 @@ int ata_scsi_error(struct Scsi_Host *hos
 	/* bookkeeping for hotplug */
 	ata_clr_hotplug_flags(ap, ATA_HOTPLUG_PROBE | ATA_HOTPLUG_DID_PROBE);
 
+	/* schedule SCSI hotplug if requested */
+	if (ap->hotplug_flags & ATA_HOTPLUG_RUNNING &&
+	    ap->hotplug_flags & (ATA_HOTPLUG_SCSI_PLUG |
+				 ATA_HOTPLUG_SCSI_UNPLUG))
+		queue_work(ata_hotplug_wq, &ap->hotplug_task);
+
 	DPRINTK("EXIT\n");
 	return 0;
 }
@@ -1434,6 +1440,9 @@ void ata_eh_hotplug(struct ata_port *ap)
 		dev->flags &= ~ATA_DFLAG_DETACH_ATA;
 		ata_scsi_offline_dev(ap, dev);
 		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+		/* schedule SCSI unplug */
+		ata_set_hotplug_flags(ap, ATA_HOTPLUG_SCSI_UNPLUG);
 	}
 
 	/* probe requested? */
@@ -1530,4 +1539,7 @@ void ata_eh_scsi_hotplug(void *data)
 		ata_clr_hotplug_flags(ap, ATA_HOTPLUG_SCSI_PLUG);
 		ata_scsi_scan_host(ap);
 	}
+
+	/* schedule SCSI plug */
+	ata_set_hotplug_flags(ap, ATA_HOTPLUG_SCSI_PLUG);
 }
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b9dd793..f145f0e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -482,6 +482,7 @@ struct ata_port {
 	struct list_head	eh_done_q;
 
 	unsigned long		hotplug_flags;
+	struct work_struct	hotplug_task;
 
 	void			*private_data;
 };
-- 
1.2.4

[PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()

[Tejun Heo]

This function is called during sdev removal by SCSI midlayer.  For
user-initiated removals, this is the only indication we get from SCSI
midlayer that the sdev is going away.  In such cases, schedule ATA
detach and invoke EH.  For libata-initiated removal, nothing needs to
be done.

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

---

 drivers/scsi/libata-core.c |    1 +
 drivers/scsi/libata-eh.c   |   15 ++++++++++++---
 drivers/scsi/libata-scsi.c |   33 +++++++++++++++++++++++++++++++++
 include/linux/libata.h     |    1 +
 4 files changed, 47 insertions(+), 3 deletions(-)

50661abaf2563e89f2277c7e39c817123459779f
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 91a01ca..87a6579 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5452,6 +5452,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-eh.c b/drivers/scsi/libata-eh.c
index 64edf48..6a61c5c 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -175,7 +175,7 @@ int ata_scsi_error(struct Scsi_Host *hos
 {
 	struct ata_port *ap = (struct ata_port *)&host->hostdata[0];
 	spinlock_t *hs_lock = &ap->host_set->lock;
-	int repeat_cnt = ATA_EH_MAX_REPEAT;
+	int i, repeat_cnt = ATA_EH_MAX_REPEAT;
 	unsigned long flags;
 
 	DPRINTK("ENTER\n");
@@ -185,10 +185,19 @@ int ata_scsi_error(struct Scsi_Host *hos
 	spin_unlock_wait(hs_lock);
 	ata_port_flush_task(ap);
 
+	/* Scheduled to be detached but not disabled status happens
+	 * only for devices user requested to be detached via
+	 * slave_destroy.  Make sure they are disabled.
+	 */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+
+		if (dev->flags & ATA_DFLAG_DETACH_ATA)
+			ata_dev_disable(ap, dev);
+	}
+
 	/* invoke error handler */
 	if (ap->ops->error_handler) {
-		int i;
-
 		/* for new EH, all the qc's are ours now */
 		for (i = 0; i < ATA_MAX_QUEUE; i++) {
 			struct ata_queued_cmd *qc;
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index fac37cd..5e68305 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -723,6 +723,39 @@ 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 = (struct ata_port *)&sdev->host->hostdata[0];
+	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) {
+		dev->sdev = NULL;
+		dev->flags |= ATA_DFLAG_DETACH_ATA;
+		ata_eh_schedule_port(ap, ATA_EH_FREEZE);
+	}
+	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
diff --git a/include/linux/libata.h b/include/linux/libata.h
index f145f0e..84162b2 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -663,6 +663,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_port *ap, 
-- 
1.2.4

[PATCH 13/15] sata_sil: add hotplug support

[Tejun Heo]

Add hotplug support by enabling SATA IRQ for SError.N and scheduling
EH if the interrupt occurs.  Standard EH will do the right thing and
schedule probe after analyzing SError.

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

---

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

2f423e8392edda125a3cec0269e4cd7ceb7afa46
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 0b72585..49a5098 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -346,6 +346,9 @@ static void sil_postreset(struct ata_por
 
 	ata_std_postreset(ap, classes);
 
+	/* turn on SATA IRQ */
+	writel(SIL_SIEN_N, mmio_base + sil_port[ap->port_no].sien);
+
 	/* everything is back to normal, turn on IRQ */
 	tmp = readl(mmio_base + SIL_SYSCFG);
 	tmp &= ~(SIL_MASK_IDE0_INT << ap->port_no);
@@ -364,7 +367,7 @@ 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(!qc || qc->tf.ctl & ATA_NIEN))
+	if (unlikely(bmdma2 & SIL_DMA_SATA_IRQ || !qc || qc->tf.ctl & ATA_NIEN))
 		goto freeze;
 
 	switch (qc->tf.protocol) {
@@ -425,7 +428,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);
@@ -442,6 +445,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;
@@ -622,11 +628,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.2.4

[PATCH 15/15] sata_sil24: add hotplug support

[Tejun Heo]

Add hotplug support by enabling PORT_IRQ_PHYRDY_CHG and scheduling
probe when either of PORT_IRQ_PHYRDY_CHG or PORT_IRQ_DEV_XCHG happens.
Explicit ata_schedule_probe() is necessary because sil24's SError
doesn't always reflect DEV_XCHG or PHYRDY_CHG conditions properly.

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

---

 drivers/scsi/sata_sil24.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

dc756727b3be5cf5bff4a327bc13604235419fc6
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 1ca76f7..06c0ab0 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -156,7 +156,8 @@ enum {
 	PORT_IRQ_HANDSHAKE	= (1 << 10), /* handshake error threshold */
 	PORT_IRQ_SDB_NOTIFY	= (1 << 11), /* SDB notify received */
 
-	PORT_IRQ_FREEZE		= PORT_IRQ_DEV_XCHG | PORT_IRQ_UNK_FIS,
+	PORT_IRQ_FREEZE		= PORT_IRQ_PHYRDY_CHG | PORT_IRQ_DEV_XCHG |
+				  PORT_IRQ_UNK_FIS,
 	DEF_PORT_IRQ		= PORT_IRQ_FREEZE |
 				  PORT_IRQ_COMPLETE | PORT_IRQ_ERROR,
 
@@ -770,15 +771,20 @@ static unsigned int sil24_eh_autopsy(str
 	desc += rc;
 	desc_sz -= rc;
 
-	if (irq_stat & PORT_IRQ_DEV_XCHG) {
+	if (irq_stat & (PORT_IRQ_PHYRDY_CHG | PORT_IRQ_DEV_XCHG)) {
 		err_mask |= AC_ERR_ATA_BUS;
 		/* sil24 doesn't recover very well from phy
 		 * disconnection with a softreset.  Force hardreset.
 		 */
 		action |= ATA_PORT_HARDRESET;
-		rc = scnprintf(desc, desc_sz, ", device exchanged");
+		rc = scnprintf(desc, desc_sz, ", %s",
+			       irq_stat & PORT_IRQ_PHYRDY_CHG ?
+			       "PHY RDY changed" : "device exchanged");
 		desc += rc;
 		desc_sz -= rc;
+
+		/* request probing */
+		ata_schedule_probe(ap);
 	}
 
 	if (irq_stat & PORT_IRQ_UNK_FIS) {
-- 
1.2.4

[PATCH 11/15] sata_sil: add new constants in preparation for new interrupt handler

[Tejun Heo]

sata_sil is about to get brand new interrupt handler.  Add relevant
constants.  While at it, add a new line for readability.

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

---

 drivers/scsi/sata_sil.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

7e4271599752e0c0ba1742e98c26094502638a0e
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 3853c46..e46c044 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -54,6 +54,7 @@ enum {
 	 */
 	SIL_FLAG_RERR_ON_DMA_ACT = (1 << 29),
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
+
 	SIL_DFL_HOST_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO,
 
@@ -84,6 +85,20 @@ enum {
 	/* BMDMA/BMDMA2 */
 	SIL_INTR_STEERING	= (1 << 1),
 
+	SIL_DMA_ENABLE		= (1 << 0),  /* DMA run switch */
+	SIL_DMA_RDWR		= (1 << 3),  /* DMA Rd-Wr */
+	SIL_DMA_SATA_IRQ	= (1 << 4),  /* OR of all SATA IRQs */
+	SIL_DMA_ACTIVE		= (1 << 16), /* DMA running */
+	SIL_DMA_ERROR		= (1 << 17), /* PCI bus error */
+	SIL_DMA_COMPLETE	= (1 << 18), /* cmd complete / IRQ pending */
+	SIL_DMA_N_SATA_IRQ	= (1 << 6),  /* SATA_IRQ for the next channel */
+	SIL_DMA_N_ACTIVE	= (1 << 24), /* ACTIVE for the next channel */
+	SIL_DMA_N_ERROR		= (1 << 25), /* ERROR for the next channel */
+	SIL_DMA_N_COMPLETE	= (1 << 26), /* COMPLETE for the next channel */
+
+	/* SIEN */
+	SIL_SIEN_N		= (1 << 16), /* triggered by SError.N */
+
 	/*
 	 * Others
 	 */
@@ -227,6 +242,7 @@ static const struct {
 	unsigned long tf;	/* ATA taskfile register block */
 	unsigned long ctl;	/* ATA control/altstatus register block */
 	unsigned long bmdma;	/* DMA register block */
+	unsigned long bmdma2;	/* DMA register block #2 */
 	unsigned long fifo_cfg;	/* FIFO Valid Byte Count and Control */
 	unsigned long scr;	/* SATA control register block */
 	unsigned long sien;	/* SATA Interrupt Enable register */
@@ -234,10 +250,10 @@ static const struct {
 	unsigned long sfis_cfg;	/* SATA FIS reception config register */
 } sil_port[] = {
 	/* port 0 ... */
-	{ 0x80, 0x8A, 0x00, 0x40, 0x100, 0x148, 0xb4, 0x14c },
-	{ 0xC0, 0xCA, 0x08, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
-	{ 0x280, 0x28A, 0x200, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
-	{ 0x2C0, 0x2CA, 0x208, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
+	{ 0x80, 0x8A, 0x00, 0x10, 0x40, 0x100, 0x148, 0xb4, 0x14c },
+	{ 0xC0, 0xCA, 0x08, 0x18, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
+	{ 0x280, 0x28A, 0x200, 0x210, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
+	{ 0x2C0, 0x2CA, 0x208, 0x218, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
 	/* ... port 3 */
 };
 
-- 
1.2.4

[PATCH 12/15] sata_sil: new interrupt handler

[Tejun Heo]

The DMA complete bit of these controllers reflects ATA IRQ status
while no DMA command is in progress.  So, we can tell whether the
controller is raising an interrupt or not in deterministic manner.
This patch gives sata_sil its own interrupt handler which behaves much
better than the original one in terms of error detection and handling.
This change is also necessary for later hotplug support.

Further improvements are possible, in both 2 and 4 ports versions, we
can get all status with only one readl and using custom bmdma
operations can further cut down register accesses.

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

---

 drivers/scsi/sata_sil.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 81 insertions(+), 1 deletions(-)

cae31a3e780bd2cef46e299e6bf681af88d7f51d
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index e46c044..0b72585 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -112,6 +112,8 @@ static u32 sil_scr_read (struct ata_port
 static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static int sil_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void sil_post_set_mode (struct ata_port *ap);
+static irqreturn_t sil_interrupt(int irq, void *dev_instance,
+				 struct pt_regs *regs);
 static void sil_freeze(struct ata_port *ap);
 static void sil_error_handler(struct ata_port *ap);
 
@@ -197,7 +199,7 @@ static const struct ata_port_operations 
 	.freeze			= sil_freeze,
 	.error_handler		= sil_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
-	.irq_handler		= ata_interrupt,
+	.irq_handler		= sil_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sil_scr_read,
 	.scr_write		= sil_scr_write,
@@ -357,6 +359,84 @@ static int sil_probe_reset(struct ata_po
 				     sil_postreset, classes);
 }
 
+static void sil_host_intr(struct ata_port *ap, u32 bmdma2)
+{
+	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
+	u8 status;
+
+	if (unlikely(!qc || qc->tf.ctl & ATA_NIEN))
+		goto freeze;
+
+	switch (qc->tf.protocol) {
+	case ATA_PROT_DMA:
+	case ATA_PROT_ATAPI_DMA:
+	case ATA_PROT_ATAPI:
+		if (bmdma2 & SIL_DMA_ERROR) {
+			qc->err_mask |= AC_ERR_HOST_BUS;
+			goto freeze;
+		}
+
+		/* clear DMA-Start bit */
+		ap->ops->bmdma_stop(qc);
+
+		/* fall through */
+	case ATA_PROT_ATAPI_NODATA:
+	case ATA_PROT_NODATA:
+		/* check main status, clearing INTRQ */
+		status = ata_chk_status(ap);
+		if (unlikely(status & ATA_BUSY)) {
+			qc->err_mask |= AC_ERR_HSM;
+			goto freeze;
+		}
+
+		/* ack bmdma irq events */
+		ap->ops->irq_clear(ap);
+
+		/* complete taskfile transaction */
+		qc->err_mask |= ac_err_mask(status);
+		ata_qc_complete(qc);
+		break;
+
+	default:
+		goto freeze;
+	}
+	return;
+
+ freeze:
+	ata_eh_schedule_port(ap, ATA_EH_FREEZE);
+}
+
+static irqreturn_t sil_interrupt(int irq, void *dev_instance,
+				 struct pt_regs *regs)
+{
+	struct ata_host_set *host_set = dev_instance;
+	void __iomem *mmio_base = host_set->mmio_base;
+	int handled = 0;
+	unsigned long flags;
+	int i;
+
+	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
+	spin_lock_irqsave(&host_set->lock, flags);
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+		u32 bmdma2 = readl(mmio_base + sil_port[ap->port_no].bmdma2);
+
+		if (unlikely(!ap || ap->flags & ATA_FLAG_DISABLED))
+			continue;
+
+		if (!(bmdma2 & SIL_DMA_COMPLETE))
+			continue;
+
+		sil_host_intr(ap, bmdma2);
+		handled = 1;
+	}
+
+	spin_unlock_irqrestore(&host_set->lock, flags);
+
+	return IRQ_RETVAL(handled);
+}
+
 static void sil_freeze(struct ata_port *ap)
 {
 	void __iomem *mmio_base = ap->host_set->mmio_base;
-- 
1.2.4

[PATCH 10/15] libata-hp: skip EH reset if no device to recover and hotplug pending

[Tejun Heo]

Now that hotplug support is online, there is no reason to reset frozen
empty port with pending hotplug probe (it will do the same thing).
Skip EH reset in such cases.

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

---

 drivers/scsi/libata-eh.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

0d1308467ab2bc82faee607440c00b9a14b0af71
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 443f81f..d54ebb1 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -1208,8 +1208,12 @@ int ata_eh_revive(struct ata_port *ap, u
 	}
 	action &= ~ATA_PORT_REVALIDATE;
 
-	/* Skip reset if possible. */
-	if (!nr_enabled && !(ap->flags & ATA_FLAG_FROZEN))
+	/* Skip reset if possible.  Note that ata_eh_hotplug() might
+	 * determine not to perform reset and EH can finish with a
+	 * frozen port.  EH framework will take care of such cases.
+	 */
+	if (!nr_enabled && (!(ap->flags & ATA_FLAG_FROZEN) ||
+			    ap->hotplug_flags & ATA_HOTPLUG_PROBE))
 		goto out;
 
 	/* give devices some time to breath */
-- 
1.2.4

[PATCH 14/15] ahci: add hotplug support

[Tejun Heo]

Add hotplug support by enabling PORT_IRQ_PHYRDY.  Standard EH will do
the right thing and schedule probe after analyzing SError.

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

---

 drivers/scsi/ahci.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

ded75cede39c99d38376691e952327cff47bb4f6
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 7f478c4..975618e 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -135,6 +135,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 |
@@ -801,10 +802,12 @@ static unsigned int ahci_eh_autopsy(stru
 		desc_sz -= rc;
 	}
 
-	if (irq_stat & PORT_IRQ_CONNECT) {
+	if (irq_stat & (PORT_IRQ_CONNECT | PORT_IRQ_PHYRDY)) {
 		err_mask |= AC_ERR_ATA_BUS;
 		action |= ATA_PORT_SOFTRESET;
-		rc = scnprintf(desc, desc_sz, ", connection status changed");
+		rc = scnprintf(desc, desc_sz, ", %s",
+			       irq_stat & PORT_IRQ_CONNECT ?
+			       "connection status changed" : "PHY RDY changed");
 		desc += rc;
 		desc_sz -= rc;
 	}
-- 
1.2.4

Re: [PATCHSET 9/9] add hotplug support

[Tejun Heo]

Tejun Heo wrote:
> Hellooooo, all.
> 
> Finally, the last one.  This is the first take of add-hotplug-support
> patchset.  This patchset includes 13 patches.

And, of course, 15 patches.

> #01-04	implement hotplug framework
> #05-07	implement warm plug (SCSI scan/delete callbacks)
> #08-10	hook and activate hotplug
#11-12		prep sata_sil for hotplug support
#13-15		add LLDD hotplug supports (PHY status change notifications)

-- 
tejun

Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()

[Tejun Heo]

On Tue, Apr 11, 2006 at 11:14:07PM +0900, Tejun Heo wrote:
> This function is called during sdev removal by SCSI midlayer.  For
> user-initiated removals, this is the only indication we get from SCSI
> midlayer that the sdev is going away.  In such cases, schedule ATA
> detach and invoke EH.  For libata-initiated removal, nothing needs to
> be done.
> 

User initiated warm unplugging has a race condition.  If user
initiated warm unplug kicks int after ata_scsi_remove_dev() fetched
dev->sdev but before it actually removes the device, the sdev will go
away while ata_scsi_remove_dev() is still trying to remove it.  This
can be solved by doing scsi_device_get() in ata_scsi_remove_dev()
after fetching dev->sdev.

I'll post the fixed version in the next round.  The #upstream change
breaks all EH/NCQ/hotplug patchsets anyway.  My repo currently
contains the following changes from the posted version.

* above mentioned sdev removal race fix
* scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler
  relocation
* duplicate .can_queue initialization removed from sil24-NCQ patch
  removed (why doesn't compiler complian about this?)
* updated to use ata_shost_to_port()

I think above changes shouldn't hinder reviewing process too much.
Though, if you want me to post the updated version, just let me know.

Thanks.

-- 
tejun

Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()

[Jeff Garzik]

Tejun Heo wrote:
> On Tue, Apr 11, 2006 at 11:14:07PM +0900, Tejun Heo wrote:
>> This function is called during sdev removal by SCSI midlayer.  For
>> user-initiated removals, this is the only indication we get from SCSI
>> midlayer that the sdev is going away.  In such cases, schedule ATA
>> detach and invoke EH.  For libata-initiated removal, nothing needs to
>> be done.
>>
> 
> User initiated warm unplugging has a race condition.  If user
> initiated warm unplug kicks int after ata_scsi_remove_dev() fetched
> dev->sdev but before it actually removes the device, the sdev will go
> away while ata_scsi_remove_dev() is still trying to remove it.  This
> can be solved by doing scsi_device_get() in ata_scsi_remove_dev()
> after fetching dev->sdev.
> 
> I'll post the fixed version in the next round.  The #upstream change
> breaks all EH/NCQ/hotplug patchsets anyway.  My repo currently
> contains the following changes from the posted version.
> 
> * above mentioned sdev removal race fix
> * scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler
>   relocation
> * duplicate .can_queue initialization removed from sil24-NCQ patch
>   removed (why doesn't compiler complian about this?)
> * updated to use ata_shost_to_port()
> 
> I think above changes shouldn't hinder reviewing process too much.
> Though, if you want me to post the updated version, just let me know.

Actually, you have so many patches now, it would make the process go 
faster to post them in cascading git branches somewhere.

tejun.git#p5	== libata-dev.git#upstream + new EH framework
tejun.git#p6	== #p5 + new EH implementation
tejun.git#p7	== #p6 + add new NCQ support
etc.

That way, if I approve through patchset #7, I can just pull 
tejun.git#p7, and get all patchsets through patchset 7.  If some patches 
need to be revised, a script should be able to reconstitute these sets 
of branches.

git is recommended for high volume submittors :)

	Jeff

Re: [PATCH 05/15] libata-hp: implement ata_scsi_slave_destroy()

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> On Tue, Apr 11, 2006 at 11:14:07PM +0900, Tejun Heo wrote:
>>> This function is called during sdev removal by SCSI midlayer.  For
>>> user-initiated removals, this is the only indication we get from SCSI
>>> midlayer that the sdev is going away.  In such cases, schedule ATA
>>> detach and invoke EH.  For libata-initiated removal, nothing needs to
>>> be done.
>>>
>>
>> User initiated warm unplugging has a race condition.  If user
>> initiated warm unplug kicks int after ata_scsi_remove_dev() fetched
>> dev->sdev but before it actually removes the device, the sdev will go
>> away while ata_scsi_remove_dev() is still trying to remove it.  This
>> can be solved by doing scsi_device_get() in ata_scsi_remove_dev()
>> after fetching dev->sdev.
>>
>> I'll post the fixed version in the next round.  The #upstream change
>> breaks all EH/NCQ/hotplug patchsets anyway.  My repo currently
>> contains the following changes from the posted version.
>>
>> * above mentioned sdev removal race fix
>> * scsi_eh_schedule_* are updated to reflect ->eh_strategy_handler
>>   relocation
>> * duplicate .can_queue initialization removed from sil24-NCQ patch
>>   removed (why doesn't compiler complian about this?)
>> * updated to use ata_shost_to_port()
>>
>> I think above changes shouldn't hinder reviewing process too much.
>> Though, if you want me to post the updated version, just let me know.
> 
> Actually, you have so many patches now, it would make the process go 
> faster to post them in cascading git branches somewhere.
> 
> tejun.git#p5    == libata-dev.git#upstream + new EH framework
> tejun.git#p6    == #p5 + new EH implementation
> tejun.git#p7    == #p6 + add new NCQ support
> etc.
> 
> That way, if I approve through patchset #7, I can just pull 
> tejun.git#p7, and get all patchsets through patchset 7.  If some patches 
> need to be revised, a script should be able to reconstitute these sets 
> of branches.
> 

Yeap, will do.

Thanks for reviewing all these. :)

-- 
tejun

Re: [PATCHSET 9/9] add hotplug support

[zhao, forrest]

On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:

> Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
> - adding a call to ata_eh_hotplug() at the end of ->error_handler is
> enough (even this is unnecessary if it's using bmdma error_handler).
> e.g. ata_piix isn't changed by hotplug patchsets at all but still
> supports warm plugging (even PATA warm plugging works although it's
> electronically unsafe).
> 
> To support full hot unplug/plug, all a LLDD has to do are enabling PHY
> status changed interrupt and, when PHY RDY status changes, call
> ata_schedule_probe() and schedule EH.  ahci, sata_sil and sata_sil24
> are converted this way and hotplugging works perfectly for those
> drivers.
> 

In order to make ata_eh_hotplug() called, LLDD have to schedule EH
in its interrupt handler(i.e. invoke scsi_eh_schedule_host() directly
or indirectly), but I didn't find such code in your patch set.
Did I miss anything?

Thanks,
Forrest

Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH

[zhao, forrest]

On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
> Now all the required pieces for hotplug support are in place.  Update
> all ->error_handler's to call ata_eh_hotplug() after regular EH is
> finished.  This enables hotplug support for all drivers which use new
> EH.  With this patch, all new EH drivers can do warm unplug and plug.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/ahci.c         |    1 +
>  drivers/scsi/libata-bmdma.c |    1 +
>  drivers/scsi/sata_sil24.c   |    1 +
>  3 files changed, 3 insertions(+), 0 deletions(-)
> 
> 5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
> index a8b699a..7f478c4 100644
> --- a/drivers/scsi/ahci.c
> +++ b/drivers/scsi/ahci.c
> @@ -864,6 +864,7 @@ static void ahci_error_handler(struct at
>  	ata_eh_revive(ap, action,
>  		      ahci_softreset, ahci_hardreset, ahci_postreset);
>  	ata_eh_finish_qcs(ap, qc, &tf);
> +	ata_eh_hotplug(ap);
>  }
>  
>  static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
> index 3c3bc71..a87efae 100644
> --- a/drivers/scsi/libata-bmdma.c
> +++ b/drivers/scsi/libata-bmdma.c
> @@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port 
>  	ata_eh_report(ap, qc, &tf, serror, action, NULL);
>  	ata_eh_revive(ap, action, softreset, hardreset, postreset);
>  	ata_eh_finish_qcs(ap, qc, &tf);
> +	ata_eh_hotplug(ap);
>  }
>  
>  /**
> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
> index 9acf7a4..1ca76f7 100644
> --- a/drivers/scsi/sata_sil24.c
> +++ b/drivers/scsi/sata_sil24.c
> @@ -852,6 +852,7 @@ static void sil24_error_handler(struct a
>  	ata_eh_revive(ap, action,
>  		      sil24_softreset, sil24_hardreset, sil24_postreset);
>  	ata_eh_finish_qcs(ap, qc, &tf);
> +	ata_eh_hotplug(ap);
>  }
>  

Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error()
instead of invoking ata_eh_hotplug() in each LLDD in order to
make code more general?

Thanks,
Forrest

Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH

[Tejun Heo]

zhao, forrest wrote:
> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
>> Now all the required pieces for hotplug support are in place.  Update
>> all ->error_handler's to call ata_eh_hotplug() after regular EH is
>> finished.  This enables hotplug support for all drivers which use new
>> EH.  With this patch, all new EH drivers can do warm unplug and plug.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>> ---
>>
>>  drivers/scsi/ahci.c         |    1 +
>>  drivers/scsi/libata-bmdma.c |    1 +
>>  drivers/scsi/sata_sil24.c   |    1 +
>>  3 files changed, 3 insertions(+), 0 deletions(-)
>>
>> 5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
>> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
>> index a8b699a..7f478c4 100644
>> --- a/drivers/scsi/ahci.c
>> +++ b/drivers/scsi/ahci.c
>> @@ -864,6 +864,7 @@ static void ahci_error_handler(struct at
>>  	ata_eh_revive(ap, action,
>>  		      ahci_softreset, ahci_hardreset, ahci_postreset);
>>  	ata_eh_finish_qcs(ap, qc, &tf);
>> +	ata_eh_hotplug(ap);
>>  }
>>  
>>  static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
>> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
>> index 3c3bc71..a87efae 100644
>> --- a/drivers/scsi/libata-bmdma.c
>> +++ b/drivers/scsi/libata-bmdma.c
>> @@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port 
>>  	ata_eh_report(ap, qc, &tf, serror, action, NULL);
>>  	ata_eh_revive(ap, action, softreset, hardreset, postreset);
>>  	ata_eh_finish_qcs(ap, qc, &tf);
>> +	ata_eh_hotplug(ap);
>>  }
>>  
>>  /**
>> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
>> index 9acf7a4..1ca76f7 100644
>> --- a/drivers/scsi/sata_sil24.c
>> +++ b/drivers/scsi/sata_sil24.c
>> @@ -852,6 +852,7 @@ static void sil24_error_handler(struct a
>>  	ata_eh_revive(ap, action,
>>  		      sil24_softreset, sil24_hardreset, sil24_postreset);
>>  	ata_eh_finish_qcs(ap, qc, &tf);
>> +	ata_eh_hotplug(ap);
>>  }
>>  
> 
> Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error()
> instead of invoking ata_eh_hotplug() in each LLDD in order to
> make code more general?
> 

No, not really.  To do that we'll need another port operation
->hotplug(), which, I think, has no real benefits.

-- 
tejun

Re: [PATCHSET 9/9] add hotplug support

[Tejun Heo]

zhao, forrest wrote:
> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
> 
>> Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
>> - adding a call to ata_eh_hotplug() at the end of ->error_handler is
>> enough (even this is unnecessary if it's using bmdma error_handler).
>> e.g. ata_piix isn't changed by hotplug patchsets at all but still
>> supports warm plugging (even PATA warm plugging works although it's
>> electronically unsafe).
>>
>> To support full hot unplug/plug, all a LLDD has to do are enabling PHY
>> status changed interrupt and, when PHY RDY status changes, call
>> ata_schedule_probe() and schedule EH.  ahci, sata_sil and sata_sil24
>> are converted this way and hotplugging works perfectly for those
>> drivers.
>>
> 
> In order to make ata_eh_hotplug() called, LLDD have to schedule EH
> in its interrupt handler(i.e. invoke scsi_eh_schedule_host() directly
> or indirectly), but I didn't find such code in your patch set.
> Did I miss anything?
> 

The last three patches do that.  All hotplug interrupts end up invoking
EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE).

-- 
tejun

Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH

[zhao, forrest]

On Thu, 2006-04-13 at 17:45 +0900, Tejun Heo wrote:
> zhao, forrest wrote:
> > On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
> >> Now all the required pieces for hotplug support are in place.  Update
> >> all ->error_handler's to call ata_eh_hotplug() after regular EH is
> >> finished.  This enables hotplug support for all drivers which use new
> >> EH.  With this patch, all new EH drivers can do warm unplug and plug.
> >>
> >> Signed-off-by: Tejun Heo <htejun@gmail.com>
> >>
> >> ---
> >>
> >>  drivers/scsi/ahci.c         |    1 +
> >>  drivers/scsi/libata-bmdma.c |    1 +
> >>  drivers/scsi/sata_sil24.c   |    1 +
> >>  3 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> 5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
> >> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
> >> index a8b699a..7f478c4 100644
> >> --- a/drivers/scsi/ahci.c
> >> +++ b/drivers/scsi/ahci.c
> >> @@ -864,6 +864,7 @@ static void ahci_error_handler(struct at
> >>  	ata_eh_revive(ap, action,
> >>  		      ahci_softreset, ahci_hardreset, ahci_postreset);
> >>  	ata_eh_finish_qcs(ap, qc, &tf);
> >> +	ata_eh_hotplug(ap);
> >>  }
> >>  
> >>  static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
> >> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
> >> index 3c3bc71..a87efae 100644
> >> --- a/drivers/scsi/libata-bmdma.c
> >> +++ b/drivers/scsi/libata-bmdma.c
> >> @@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port 
> >>  	ata_eh_report(ap, qc, &tf, serror, action, NULL);
> >>  	ata_eh_revive(ap, action, softreset, hardreset, postreset);
> >>  	ata_eh_finish_qcs(ap, qc, &tf);
> >> +	ata_eh_hotplug(ap);
> >>  }
> >>  
> >>  /**
> >> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
> >> index 9acf7a4..1ca76f7 100644
> >> --- a/drivers/scsi/sata_sil24.c
> >> +++ b/drivers/scsi/sata_sil24.c
> >> @@ -852,6 +852,7 @@ static void sil24_error_handler(struct a
> >>  	ata_eh_revive(ap, action,
> >>  		      sil24_softreset, sil24_hardreset, sil24_postreset);
> >>  	ata_eh_finish_qcs(ap, qc, &tf);
> >> +	ata_eh_hotplug(ap);
> >>  }
> >>  
> > 
> > Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error()
> > instead of invoking ata_eh_hotplug() in each LLDD in order to
> > make code more general?
> > 
> 
> No, not really.  To do that we'll need another port operation
> ->hotplug(), which, I think, has no real benefits.

>From my understanding, in ata_scsi_error invoking
ata_eh_hotplug(ap); after ap->ops->error_handler(ap);
will do. No need to add ->hotplug() for port operation.
This way we avoid to invoke ata_eh_hotplug() in each LLDD.

Thanks,
Forrest

Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH

[Tejun Heo]

zhao, forrest wrote:
> On Thu, 2006-04-13 at 17:45 +0900, Tejun Heo wrote:
>> zhao, forrest wrote:
>>> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
>>>> Now all the required pieces for hotplug support are in place.  Update
>>>> all ->error_handler's to call ata_eh_hotplug() after regular EH is
>>>> finished.  This enables hotplug support for all drivers which use new
>>>> EH.  With this patch, all new EH drivers can do warm unplug and plug.
>>>>
>>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>>>
>>>> ---
>>>>
>>>>  drivers/scsi/ahci.c         |    1 +
>>>>  drivers/scsi/libata-bmdma.c |    1 +
>>>>  drivers/scsi/sata_sil24.c   |    1 +
>>>>  3 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> 5a9a286e07ca0d216be1b8f81ab6c6e7048ae963
>>>> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
>>>> index a8b699a..7f478c4 100644
>>>> --- a/drivers/scsi/ahci.c
>>>> +++ b/drivers/scsi/ahci.c
>>>> @@ -864,6 +864,7 @@ static void ahci_error_handler(struct at
>>>>  	ata_eh_revive(ap, action,
>>>>  		      ahci_softreset, ahci_hardreset, ahci_postreset);
>>>>  	ata_eh_finish_qcs(ap, qc, &tf);
>>>> +	ata_eh_hotplug(ap);
>>>>  }
>>>>  
>>>>  static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
>>>> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
>>>> index 3c3bc71..a87efae 100644
>>>> --- a/drivers/scsi/libata-bmdma.c
>>>> +++ b/drivers/scsi/libata-bmdma.c
>>>> @@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port 
>>>>  	ata_eh_report(ap, qc, &tf, serror, action, NULL);
>>>>  	ata_eh_revive(ap, action, softreset, hardreset, postreset);
>>>>  	ata_eh_finish_qcs(ap, qc, &tf);
>>>> +	ata_eh_hotplug(ap);
>>>>  }
>>>>  
>>>>  /**
>>>> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
>>>> index 9acf7a4..1ca76f7 100644
>>>> --- a/drivers/scsi/sata_sil24.c
>>>> +++ b/drivers/scsi/sata_sil24.c
>>>> @@ -852,6 +852,7 @@ static void sil24_error_handler(struct a
>>>>  	ata_eh_revive(ap, action,
>>>>  		      sil24_softreset, sil24_hardreset, sil24_postreset);
>>>>  	ata_eh_finish_qcs(ap, qc, &tf);
>>>> +	ata_eh_hotplug(ap);
>>>>  }
>>>>  
>>> Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error()
>>> instead of invoking ata_eh_hotplug() in each LLDD in order to
>>> make code more general?
>>>
>> No, not really.  To do that we'll need another port operation
>> ->hotplug(), which, I think, has no real benefits.
> 
>>From my understanding, in ata_scsi_error invoking
> ata_eh_hotplug(ap); after ap->ops->error_handler(ap);
> will do. No need to add ->hotplug() for port operation.
> This way we avoid to invoke ata_eh_hotplug() in each LLDD.
> 

Hmmmm... maybe you're right and we don't need to allow LLDDs to override
hotplug operation.  After all, LLDDs cannot override ata_bus_probe().
But, then again, it's just nice and consistent to allow overriding of
the operation.  And, ata_eh_hotplug() fits nicely with other EH
operations, IMHO.  And calling one more operation isn't really a big deal.

Maybe, we need to come up with the function which packages standard
calls to eh helpers for drivers like sata_sil24 and ahci, but I think
the current level of code duplication is well within acceptable range.

Do you have any other reason than duplicated calls to ata_eh_hotplug()?

-- 
tejun

Re: [PATCHSET 9/9] add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> zhao, forrest wrote:
>> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
>>
>>> Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
>>> - adding a call to ata_eh_hotplug() at the end of ->error_handler is
>>> enough (even this is unnecessary if it's using bmdma error_handler).
>>> e.g. ata_piix isn't changed by hotplug patchsets at all but still
>>> supports warm plugging (even PATA warm plugging works although it's
>>> electronically unsafe).
>>>
>>> To support full hot unplug/plug, all a LLDD has to do are enabling PHY
>>> status changed interrupt and, when PHY RDY status changes, call
>>> ata_schedule_probe() and schedule EH.  ahci, sata_sil and sata_sil24
>>> are converted this way and hotplugging works perfectly for those
>>> drivers.
>>>
>> In order to make ata_eh_hotplug() called, LLDD have to schedule EH
>> in its interrupt handler(i.e. invoke scsi_eh_schedule_host() directly
>> or indirectly), but I didn't find such code in your patch set.
>> Did I miss anything?
>>
> 
> The last three patches do that.  All hotplug interrupts end up invoking
> EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE).

It's certainly nicer and more readable, if the LLDD calls a function
like ata_eh_hotplug().

	Jeff

Re: [PATCHSET 9/9] add hotplug support

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> zhao, forrest wrote:
>>> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote:
>>>
>>>> Warm unplug/plug comes at almost no cost for LLDDs implementing new EH
>>>> - adding a call to ata_eh_hotplug() at the end of ->error_handler is
>>>> enough (even this is unnecessary if it's using bmdma error_handler).
>>>> e.g. ata_piix isn't changed by hotplug patchsets at all but still
>>>> supports warm plugging (even PATA warm plugging works although it's
>>>> electronically unsafe).
>>>>
>>>> To support full hot unplug/plug, all a LLDD has to do are enabling PHY
>>>> status changed interrupt and, when PHY RDY status changes, call
>>>> ata_schedule_probe() and schedule EH.  ahci, sata_sil and sata_sil24
>>>> are converted this way and hotplugging works perfectly for those
>>>> drivers.
>>>>
>>> In order to make ata_eh_hotplug() called, LLDD have to schedule EH
>>> in its interrupt handler(i.e. invoke scsi_eh_schedule_host() directly
>>> or indirectly), but I didn't find such code in your patch set.
>>> Did I miss anything?
>>>
>> The last three patches do that.  All hotplug interrupts end up invoking
>> EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE).
> 
> It's certainly nicer and more readable, if the LLDD calls a function
> like ata_eh_hotplug().
> 

Hi, Jeff.

Currently, a LLDD has to do the following two to schedule hotplug.

* call ata_schedule_probe(ap);
* invoke EH by calling ata_eh_schedule_port(ap, ATA_EH_FREEZE);

Calling ata_schedule_probe() can be done from interrupt handler or
inside EH.  It just has to be done before actual hotplug operation begins.

ata_schedule_probe() can be changed to include a call to
ata_eh_schedule_port() but, as can be seen from three converted drivers,
it isn't really necessary.  Scheduling hotplug is usually done by the
standard ata_eh_autopsy() function in EH after looking at the SError
register.  So, actually, all an interrupt handler has to do is just to
call in to EH if something looks weird.  Standard EH will do all the
jobs for it including scheduling hotplug operation.

Only sata_sil24 calls ata_schedule_probe() explicitly because its SError
is sort of flimsy.  And even that is not done from the interrupt
handler.  It's done by sil24_autopsy() function which is the first thing
sata_sil24 EH does on entry.

So, there really isn't much point in making ata_schedule_port() and
ata_eh_schedule_port() done in one step.  For most controllers, the
latter doesn't need to be called at all and even, when it does, there's
a good chance that it will be done from EH making scheduling EH kind of
silly, still safe though.

Hmmmm... but maybe ata_schedule_hotplug() is a better name than
ata_schedule_probe().  Or ata_eh_schedule_hotplug()?  ata_eh_hotplug()
is already taken by the function which does actual hotplug.

The reason why I chose ata_schedule_probe() was the word 'hotplug' is
currently used to mean both plugging and unplugging operations, and warm
operations too.  It means the general facility to attach and detach
devices dynamically.  So, calling it ata_schedule_hotplug() or something
similar is sort of confusing as some hotplug operations happen whether
the function is called or not.

The terms I've used are...

* hotplug	: as said above, general facility and operations to
		  dynamically add and remove devices, warm or hot.
* attach	: the action of adding a new device
* detach	: the action of removing a device
* probe		: an operation to discover new devices on a port and
		  attach them

Thanks.

-- 
tejun

Re: [PATCHSET 9/9] add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> Hellooooo, all.
> 
> Finally, the last one.  This is the first take of add-hotplug-support
> patchset.  This patchset includes 13 patches.
> 
> #01-04	implement hotplug framework
> #05-07	implement warm plug (SCSI scan/delete callbacks)
> #08-10	hook and activate hotplug
> #11-13	add LLDD hotplug supports (PHY status change notifications)
> 
> Hotplugging is implemented as an integral part of EH.  A new EH helper
> ata_eh_hotplug() is defined which should be called after all regular
> EH is done.  LLDDs have to do very little to support hotplug.

I'm going to leave the substantive review of this patchset for the next 
time it gets resent.

Overall,

* my quick review didn't turn up any obvious problems, but that was just 
looking at each patch, not looking at the end result (big picture) at all.

* hotplug design I want to see is (also mentioned in another email):

	1a) user requests bus scan
		or
	1b) hotplug interrupt
	2) wait a bit, swallowing any 1a- or 1b-type events that
	   appear during the wait (debounce).
	3) rescan bus, revalidate device(s)

I'm careful to use "revalidate", because that covers all cases:

	- existing device goes away
	- new device appears
	- existing device "blipped", but its still there, so
	  we can keep talking to it.

Re: [PATCHSET 9/9] add hotplug support

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Hellooooo, all.
>>
>> Finally, the last one.  This is the first take of add-hotplug-support
>> patchset.  This patchset includes 13 patches.
>>
>> #01-04    implement hotplug framework
>> #05-07    implement warm plug (SCSI scan/delete callbacks)
>> #08-10    hook and activate hotplug
>> #11-13    add LLDD hotplug supports (PHY status change notifications)
>>
>> Hotplugging is implemented as an integral part of EH.  A new EH helper
>> ata_eh_hotplug() is defined which should be called after all regular
>> EH is done.  LLDDs have to do very little to support hotplug.
> 
> I'm going to leave the substantive review of this patchset for the next 
> time it gets resent.

Yeap, please don't spend too much time on it.  Hotplug has been changed 
quite a bit in my current repo.

> Overall,
> 
> * my quick review didn't turn up any obvious problems, but that was just 
> looking at each patch, not looking at the end result (big picture) at all.
> 
> * hotplug design I want to see is (also mentioned in another email):
> 
>     1a) user requests bus scan
>         or
>     1b) hotplug interrupt
>     2) wait a bit, swallowing any 1a- or 1b-type events that
>        appear during the wait (debounce).
>     3) rescan bus, revalidate device(s)

In my working repo, hardware debouncing is done by invoking the 
following function in ->prereset() with the port frozen.  I'm not very 
sure whether debouncing user request is necessary though.

/**
  *	sata_debounce - debounce SATA phy status
  *	@link: ATA link to debounce SATA phy status for
  *	@interval_msec: polling interval in millisecs
  *	@duration_msec: debounce duration in millisecs
  *	@timeout_msec: timeout in millisecs
  *
  *	Make sure SStatus of @link reaches stable state, determined by
  *	holding the same value where DET is not 1 for @duration_msec
  *	polled every @interval_msec, before @timeout_msec.  Note the
  *	timeout constraints the beginning of the stable state.
  *
  *	LOCKING:
  *	Kernel thread context (may sleep)
  *
  *	RETURNS:
  *	0 on success, -errno on failure.
  */
int sata_debounce(struct ata_link *link, unsigned long interval_msec,
		  unsigned long duration_msec, unsigned long timeout_msec)
{
	unsigned long duration = duration_msec * HZ / 1000;
	unsigned long timeout = jiffies + timeout_msec * HZ / 1000;
	unsigned long last_jiffies;
	u32 last, cur;
	int rc;

	if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
		return rc;
	cur &= 0xf;

	last = cur;
	last_jiffies = jiffies;

	while (1) {
		msleep(interval_msec);
		if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
			return rc;
		cur &= 0xf;

		/* DET stable? */
		if (cur == last && cur != 1) {
			if (time_after(jiffies, last_jiffies + duration))
				return 0;
			continue;
		}

		/* unstable, start over */
		last = cur;
		last_jiffies = jiffies;

		/* check timeout */
		if (time_after(jiffies, timeout))
			return -EBUSY;
	}
}

> I'm careful to use "revalidate", because that covers all cases:
> 
>     - existing device goes away
>     - new device appears
>     - existing device "blipped", but its still there, so
>       we can keep talking to it.
> 

Yeap, all bases covered.

I'm currently finishing up PM support.  It took a lot longer than I 
though but it's shaping up pretty good.  Everything is handled nicely, 
hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands 
are outstanding to more than three devices for sil24...) are all handled 
in generic and unified way.  Adding PM support necessitated quite a bit 
of changes to EH and hotplug.  Currently, major changes in my repo are...

- boot scan, hotplug, EH all rolled up into single EH revive operation.
- simpler EH/irq synchronization.  EH now works on its own copy of EH 
info created on entry to EH.
- much tighter event handling (almost no EH/hotplug event/info loss 
except for pathological cases)
- fine-grained user scan request (user can request scan of specific device)
- ata_link abstraction for PM
- PM support with the same level of EH/NCQ/hotplug support as host ports 
(sil24 and working on AHCI)

Above list is what comes to my mind ATM.  I probably have forgotten a 
lot.  I'll make a full list when I post the next round of patches.

Jeff, until when are you available?  I think I can post the next round 
in a few days (I'm pretty sure this time :).  I'm thinking of setting up 
a git repo and merge irq-pio there too in the order you requested.  If 
schedule isn't too tight, it would be nice to push this thing to some 
branch in libata-dev.

Thanks.

-- 
tejun

Re: [PATCHSET 9/9] add hotplug support

[Jeff Garzik]

Tejun Heo wrote:
> In my working repo, hardware debouncing is done by invoking the 
> following function in ->prereset() with the port frozen.  I'm not very 
> sure whether debouncing user request is necessary though.

Agreed, I'm not sure either.


> /**
>  *    sata_debounce - debounce SATA phy status
>  *    @link: ATA link to debounce SATA phy status for
>  *    @interval_msec: polling interval in millisecs
>  *    @duration_msec: debounce duration in millisecs
>  *    @timeout_msec: timeout in millisecs
>  *
>  *    Make sure SStatus of @link reaches stable state, determined by
>  *    holding the same value where DET is not 1 for @duration_msec
>  *    polled every @interval_msec, before @timeout_msec.  Note the
>  *    timeout constraints the beginning of the stable state.
>  *
>  *    LOCKING:
>  *    Kernel thread context (may sleep)
>  *
>  *    RETURNS:
>  *    0 on success, -errno on failure.
>  */
> int sata_debounce(struct ata_link *link, unsigned long interval_msec,
>           unsigned long duration_msec, unsigned long timeout_msec)
> {
>     unsigned long duration = duration_msec * HZ / 1000;
>     unsigned long timeout = jiffies + timeout_msec * HZ / 1000;
>     unsigned long last_jiffies;
>     u32 last, cur;
>     int rc;
> 
>     if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
>         return rc;
>     cur &= 0xf;
> 
>     last = cur;
>     last_jiffies = jiffies;
> 
>     while (1) {
>         msleep(interval_msec);
>         if ((rc = ata_scr_read(link, SCR_STATUS, &cur)))
>             return rc;
>         cur &= 0xf;
> 
>         /* DET stable? */
>         if (cur == last && cur != 1) {
>             if (time_after(jiffies, last_jiffies + duration))
>                 return 0;
>             continue;
>         }
> 
>         /* unstable, start over */
>         last = cur;
>         last_jiffies = jiffies;
> 
>         /* check timeout */
>         if (time_after(jiffies, timeout))
>             return -EBUSY;
>     }
> }

hmmm, I would think something more along the lines of

	get HP irq
	ack HP irq
	ata_I_got_hotplug_event()
		if test_and_clear_bit(got_hotplug)
			start 1-second timer

	timer fires...
	clear got_hotplug
	handle hotplug, revalidate port

There's not much point in polling, the reason for the debounce period is 
to throw away spurious hotplug/unplug/hotplug events the hardware throws 
while it is figuring shit out.

Should just need a pause, following by port recovery/revalidate.


>> I'm careful to use "revalidate", because that covers all cases:
>>
>>     - existing device goes away
>>     - new device appears
>>     - existing device "blipped", but its still there, so
>>       we can keep talking to it.
>>
> 
> Yeap, all bases covered.
> 
> I'm currently finishing up PM support.  It took a lot longer than I 
> though but it's shaping up pretty good.  Everything is handled nicely, 
> hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands 
> are outstanding to more than three devices for sil24...) are all handled 
> in generic and unified way.  Adding PM support necessitated quite a bit 
> of changes to EH and hotplug.  Currently, major changes in my repo are...
> 
> - boot scan, hotplug, EH all rolled up into single EH revive operation.
> - simpler EH/irq synchronization.  EH now works on its own copy of EH 
> info created on entry to EH.
> - much tighter event handling (almost no EH/hotplug event/info loss 
> except for pathological cases)
> - fine-grained user scan request (user can request scan of specific device)
> - ata_link abstraction for PM

that's nice


> - PM support with the same level of EH/NCQ/hotplug support as host ports 
> (sil24 and working on AHCI)

what kind of PM are you testing on?


> Above list is what comes to my mind ATM.  I probably have forgotten a 
> lot.  I'll make a full list when I post the next round of patches.
> 
> Jeff, until when are you available?  I think I can post the next round 
> in a few days (I'm pretty sure this time :).  I'm thinking of setting up 
> a git repo and merge irq-pio there too in the order you requested.  If 
> schedule isn't too tight, it would be nice to push this thing to some 
> branch in libata-dev.

I leave May 3rd.  So sometime between now and then.  The goal should be 
to get #irq-pio and whatever other work you want into #upstream before I 
leave, so that people have a nice long period for testing in -mm. 
irq-pio will definitely want some testing, as will your work.  Its a lot 
to throw at people all at once.

	Jeff

Re: [PATCHSET 9/9] add hotplug support

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> In my working repo, hardware debouncing is done by invoking the 
>> following function in ->prereset() with the port frozen.  I'm not very 
>> sure whether debouncing user request is necessary though.
> 
> Agreed, I'm not sure either.
> 
> 
>> /**
>>  *    sata_debounce - debounce SATA phy status
[--snip--]
>>     }
>> }
> 
> hmmm, I would think something more along the lines of
> 
>     get HP irq
>     ack HP irq
>     ata_I_got_hotplug_event()
>         if test_and_clear_bit(got_hotplug)
>             start 1-second timer
> 
>     timer fires...
>     clear got_hotplug
>     handle hotplug, revalidate port
> 
> There's not much point in polling, the reason for the debounce period is 
> to throw away spurious hotplug/unplug/hotplug events the hardware throws 
> while it is figuring shit out.
> 
> Should just need a pause, following by port recovery/revalidate.
> 

The thing is that on detection of hotplug event, all the active qcs 
should be aborted and EH should be entered anyway.  The current working 
tree is much nicer to hardware compared to the previous posting.  It 
freezes the port only when really necessary.

I think I need to explain more about the whole freezing/polling thing.

I once participated in a static discharge / power fluctation test for 
SATA.  Although the serial link is convenient, pretty and definitely the 
way to go, it gets easily interfered by electromagnetic fluctations. 
Also, because the communication is packet based necessitating relatively 
complex state machine inside the link layer, controllers implementing 
such interfaces seem to exhibit various convoluted behaviors when put 
under stress.

I could see several different types of screaming interrupt lockups in 
the period of a few hours.  And we all know screaming interrupt is a 
scary thing.  This is the reason behind the whole 'freezing' stuff.

What I tried to achieve is to isolate the controller if things start to 
go south so that the machine as a whole isn't affected.  Fortunately, 
most such conditions are recoverable by some form of resetting and we 
can determine if the controller is acting sanely by watching how the 
reset goes.  If it seems okay, we turn it back on.  If it fails to come 
back after several retries, we leave the controller frozen such that the 
rest of the machine can function.

phy status change is a dangerous event.  I'm pretty sure most of 
electromagnetic interferences would trigger the event too.  Also, it's 
not like we can do anything other than resetting and recovering the 
device after such an event.  So, in new EH, phy status change is a 
freezing event.

As soon as such an event is detected, libata assumes the controller is 
lost and freezes it.  EH kicks in immediately and performs reset and, by 
doing so, it makes sure that the controller isn't trying to eat the 
machine alive.  Only after EH is sure that the controller is acting 
sanely, it thaws the port and revalidates the attached device.

So, in the above control flow, it's natural and even necessary to 
perform debouncing by polling.  So, the implementation.  It also has the 
advantage of being generic.  As most controllers generate phy related 
interrupts from SCR updates, they can use generic stat_debounce() 
instead of writing its own irq debouncing routine.

One more thing to note is that such debouncing is needed before and 
during several stages of resets anyway.  The current upstream code does 
this from sata_phy_resume() by waiting until DET assumes some other 
value than 1.  Unfortunately, this doesn't work for some controllers 
(sil24) as DET dances together with other bits.  So, we need better 
debouncing routine anyway, and as now all resets are probing resets, we 
need to debounce prior to every reset.  And, debouncing in ->prereset() 
can satisfy all the requirements.

Oh.. and this has been on my mind for some time now.  It would be nice 
if we can set up a project to certify controllers and drivers which pass 
certain set of standard static discharge / power fluctation tests.  With 
new EH, we have the framework but I'm pretty sure a lot of drivers would 
need some special case code to cope with such tests.  Equipments to 
perform such tests are not cheap, but it would be very helpful to a lot 
of people, especially server crowd and people trying to use Linux on 
consumer products.  Those static discharges and power fluctations are 
facts of life.  They do occur.  And, ATM, we're not dealing with it very 
well.  I hope we can persuade some companies to sponsor such a project.

> 
>>> I'm careful to use "revalidate", because that covers all cases:
>>>
>>>     - existing device goes away
>>>     - new device appears
>>>     - existing device "blipped", but its still there, so
>>>       we can keep talking to it.
>>>
>>
>> Yeap, all bases covered.
>>
>> I'm currently finishing up PM support.  It took a lot longer than I 
>> though but it's shaping up pretty good.  Everything is handled nicely, 
>> hotplug, EH, qc deferring (e.g. not issuing ATAPI command if commands 
>> are outstanding to more than three devices for sil24...) are all 
>> handled in generic and unified way.  Adding PM support necessitated 
>> quite a bit of changes to EH and hotplug.  Currently, major changes in 
>> my repo are...
>>
>> - boot scan, hotplug, EH all rolled up into single EH revive operation.
>> - simpler EH/irq synchronization.  EH now works on its own copy of EH 
>> info created on entry to EH.
>> - much tighter event handling (almost no EH/hotplug event/info loss 
>> except for pathological cases)
>> - fine-grained user scan request (user can request scan of specific 
>> device)
>> - ata_link abstraction for PM
> 
> that's nice
> 
> 
>> - PM support with the same level of EH/NCQ/hotplug support as host 
>> ports (sil24 and working on AHCI)
> 
> what kind of PM are you testing on?
> 

sil4726.  Silicon Image was kind enough to send a sample board to me.  :)

> 
>> Above list is what comes to my mind ATM.  I probably have forgotten a 
>> lot.  I'll make a full list when I post the next round of patches.
>>
>> Jeff, until when are you available?  I think I can post the next round 
>> in a few days (I'm pretty sure this time :).  I'm thinking of setting 
>> up a git repo and merge irq-pio there too in the order you requested.  
>> If schedule isn't too tight, it would be nice to push this thing to 
>> some branch in libata-dev.
> 
> I leave May 3rd.  So sometime between now and then.  The goal should be 
> to get #irq-pio and whatever other work you want into #upstream before I 
> leave, so that people have a nice long period for testing in -mm. 
> irq-pio will definitely want some testing, as will your work.  Its a lot 
> to throw at people all at once.
>

I think/hope I can pull something off in that time frame.  And, yeah, 
it's a LOT to throw at people and definitely needs a lot of testing.

Thanks.

-- 
tejun