[RFC/PATCHSET] libata: new reset mechanism

[RFC/PATCHSET] libata: new reset mechanism

[Tejun Heo]

Hello, all.

Currently libata uses ->phy_reset to reset ports.  However,
->phy_reset is tightly woven into probing process (directly
manipulating device[]->class and disabling ports on failure) and thus
awkward to use for error handling or other purposes.

Another problem with the current scheme is that libata doesn't have
much control over reset process, which isn't very nice for both
initialization and error handling.

This patchset implements new reset mechanism.  The new mechanism can
peacefully live side-by-side with ->phy_reset mechanism and low level
drivers can be converted gradually.  This patchset converts sata_sil,
sata_sil24, ata_piix and ahci as a start.

This patchset is composed of the following 14 patches.

#01-03	: implement new reset mechanism and standard callbacks
#04	: preparation for low level driver conversion
#05	: convert sata_sil
#06-07	: convert sata_sil24 and add hardreset
#08-09	: convert ata_piix
#10-14	: convert ahci and add softreset

Thanks.

-- 
tejun

[PATCH 01/14] libata: modify ata_dev_try_classify

[Tejun Heo]

Make ata_dev_try_classify take @perr to store Error register value on
completion and return device class instead of directly manipulating
dev->class.  This is preparation for new reset mechanism.

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

--

Unless specifically noted, all patches are against upstream head as of
today (c6329f4df2263ab5a40601a5236639e61b682f51)

Index: work/drivers/scsi/libata-core.c
===================================================================
--- work.orig/drivers/scsi/libata-core.c	2005-12-18 20:16:38.000000000 +0900
+++ work/drivers/scsi/libata-core.c	2005-12-18 20:16:57.000000000 +0900
@@ -811,6 +811,7 @@ unsigned int ata_dev_classify(const stru
  *	ata_dev_try_classify - Parse returned ATA device signature
  *	@ap: ATA channel to examine
  *	@device: Device to examine (starting at zero)
+ *	@perr: Value of error register on completion
  *
  *	After an event -- SRST, E.D.D., or SATA COMRESET -- occurs,
  *	an ATA/ATAPI-defined set of values is placed in the ATA
@@ -823,11 +824,14 @@ unsigned int ata_dev_classify(const stru
  *
  *	LOCKING:
  *	caller.
+ *
+ *	RETURNS:
+ *	Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, or %ATA_DEV_NONE.
  */
 
-static u8 ata_dev_try_classify(struct ata_port *ap, unsigned int device)
+static unsigned int
+ata_dev_try_classify(struct ata_port *ap, unsigned int device, u8 *perr)
 {
-	struct ata_device *dev = &ap->device[device];
 	struct ata_taskfile tf;
 	unsigned int class;
 	u8 err;
@@ -838,8 +842,8 @@ static u8 ata_dev_try_classify(struct at
 
 	ap->ops->tf_read(ap, &tf);
 	err = tf.feature;
-
-	dev->class = ATA_DEV_NONE;
+	if (perr)
+		*perr = err;
 
 	/* see if device passed diags */
 	if (err == 1)
@@ -847,18 +851,16 @@ static u8 ata_dev_try_classify(struct at
 	else if ((device == 0) && (err == 0x81))
 		/* do nothing */ ;
 	else
-		return err;
+		return ATA_DEV_NONE;
 
-	/* determine if device if ATA or ATAPI */
+	/* determine if device is ATA or ATAPI */
 	class = ata_dev_classify(&tf);
 	if (class == ATA_DEV_UNKNOWN)
-		return err;
-	if ((class == ATA_DEV_ATA) && (ata_chk_status(ap) == 0))
-		return err;
-
-	dev->class = class;
+		class = ATA_DEV_NONE;
+	else if ((class == ATA_DEV_ATA) && (ata_chk_status(ap) == 0))
+		class = ATA_DEV_NONE;
 
-	return err;
+	return class;
 }
 
 /**
@@ -2095,9 +2097,9 @@ void ata_bus_reset(struct ata_port *ap)
 	/*
 	 * determine by signature whether we have ATA or ATAPI devices
 	 */
-	err = ata_dev_try_classify(ap, 0);
+	ap->device[0].class = ata_dev_try_classify(ap, 0, &err);
 	if ((slave_possible) && (err != 0x81))
-		ata_dev_try_classify(ap, 1);
+		ap->device[1].class = ata_dev_try_classify(ap, 1, &err);
 
 	/* re-enable interrupts */
 	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */

[PATCH 02/14] libata: implement new reset mechanism

[Tejun Heo]

Implement new reset mechanism.  This patch adds the following ata
operations.

int (*soft_reset) (struct ata_port *ap, unsigned int *classes);
int (*hard_reset) (struct ata_port *ap, unsigned int *classes);
void (*post_reset) (struct ata_port *ap, const unsigned int *classes);

In a reset trial, ->soft_reset and ->hard_reset may be called
multiple times before succeeding and post_reset is called only once
after reset finally succeeds.

A new function ata_reset() is added.  This function intelligently
drives reset process using above operations.  All resets must be
performed by calling ata_reset() with appropriate flags.  ata_reset()
prefers ->soft_reset but can fall back to ->hard_reset and knows how
to deal with reset methods which can't report signature.

ata_reset() does not modify any libata data structure, it just
performs reset using requested methods and report completion status
and classes to the caller, making it easily useable from EH.

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

Index: work/drivers/scsi/libata-core.c
===================================================================
--- work.orig/drivers/scsi/libata-core.c	2005-12-18 17:58:18.000000000 +0900
+++ work/drivers/scsi/libata-core.c	2005-12-18 17:59:44.000000000 +0900
@@ -1424,7 +1424,24 @@ static int ata_bus_probe(struct ata_port
 {
 	unsigned int i, found = 0;
 
-	ap->ops->phy_reset(ap);
+	if (ap->ops->phy_reset)
+		ap->ops->phy_reset(ap);
+	else {
+		unsigned int classes[2];
+		int ret;
+
+		ata_port_probe(ap);
+
+		ret = ata_reset(ap, ATA_RESET_ANY|ATA_RESET_CLASSIFY, classes);
+		if (ret == 0) {
+			ap->device[0].class = classes[0];
+			ap->device[1].class = classes[1];
+		} else {
+			printk("ata%u: reset failed, disabling port\n", ap->id);
+			ata_port_disable(ap);
+		}
+	}
+
 	if (ap->flags & ATA_FLAG_PORT_DISABLED)
 		goto err_out;
 
@@ -2134,6 +2151,111 @@ err_out:
 	DPRINTK("EXIT\n");
 }
 
+/**
+ *	ata_reset - reset host port and associated ATA channel
+ *	@ap: port to reset
+ *	@flags: ATA_RESET_* flags
+ *	@r_classes: resulting classes of attached devices
+ *
+ *	This function tries to reset the given port.  Low-level
+ *	drivers may supply either one or both of hard and soft reset
+ *	methods, and the caller can tell ata_reset() which ones to use
+ *	via ATA_RESET_SOFT, HARD and ANY flags.  ata_reset() always
+ *	tries to use softreset if possible.
+ *
+ *	If ATA_RESET_CLASSIFY flag is given, ata_reset() will try
+ *	allowed reset methods until classification is done.
+ *
+ *	Reset methods may or may not classify attached devices.  For
+ *	example, it's not possible to obtain signature after hard
+ *	reset on some SATA controllers.  However, at least one reset
+ *	method should classify attached devices.
+ *
+ *	If a reset method doesn't support classification, it just
+ *	leaves @classes unmodified on return.  Also, it's allowed for
+ *	reset methods to classify only the first device.  If the first
+ *	one is classifed but the second isn't, the second device is
+ *	considered to be ATA_DEV_NONE.
+ *
+ *	This function returns 0 on success, -EINVAL if the ap doesn't
+ *	support specified reset methods, -ENODEV if ATA_RESET_CLASSIFY
+ *	is specified but classification fails, and any error code from
+ *	reset methods if reset fails.
+ */
+
+int ata_reset(struct ata_port *ap, unsigned int flags, unsigned int *r_classes)
+{
+	const unsigned int unknown[2] = { ATA_DEV_UNKNOWN, ATA_DEV_UNKNOWN };
+	unsigned int classes[2];
+	int ret = 0;
+
+	DPRINTK("ENTER\n");
+
+	if (ap->ops->soft_reset == NULL)
+		flags &= ~ATA_RESET_SOFT;
+	if (ap->ops->hard_reset == NULL)
+		flags &= ~ATA_RESET_HARD;
+
+	if (!(flags & (ATA_RESET_SOFT | ATA_RESET_HARD))) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+ retry:
+	if (flags & ATA_RESET_SOFT) {
+		memcpy(classes, unknown, sizeof(classes));
+		ret = ap->ops->soft_reset(ap, classes);
+		if (ret == 0) {
+			if (!(flags & ATA_RESET_CLASSIFY) ||
+			    classes[0] != ATA_DEV_UNKNOWN)
+				goto success;
+			/* no signature on SRST, try HRST */
+			flags &= ~ATA_RESET_SOFT;
+			ret = -ENODEV;	/* no sigature */
+		}
+		/* SRST failed, try HRST */
+	}
+
+	if (flags & ATA_RESET_HARD) {
+		memcpy(classes, unknown, sizeof(classes));
+		ret = ap->ops->hard_reset(ap, classes);
+		if (ret == 0) {
+			if (!(flags & ATA_RESET_CLASSIFY) ||
+			    classes[0] != ATA_DEV_UNKNOWN)
+				goto success;
+			/* no signature on HRST, retry SRST */
+			flags &= ~ATA_RESET_HARD;
+			ret = -ENODEV;	/* no sigature */
+			goto retry;
+		}
+		/* HRST failed, give up */
+	}
+
+	goto out;
+
+ success:
+	if (classes[0] == ATA_DEV_UNKNOWN) {
+		/* classification not performed, use old values */
+		classes[0] = ap->device[0].class;
+		classes[1] = ap->device[1].class;
+	} else if (classes[1] == ATA_DEV_UNKNOWN) {
+		/* only dev0 is classifed, assume NONE for dev1 */
+		classes[1] = ATA_DEV_NONE;
+	}
+
+	if (ap->ops->post_reset)
+		ap->ops->post_reset(ap, classes);
+
+	if (r_classes) {
+		r_classes[0] = classes[0];
+		r_classes[1] = classes[1];
+	}
+
+ out:
+	DPRINTK("EXIT, ret=%d\n", ret);
+	return ret;
+}
+
 static void ata_pr_blacklisted(const struct ata_port *ap,
 			       const struct ata_device *dev)
 {
@@ -4985,6 +5107,7 @@ EXPORT_SYMBOL_GPL(ata_port_probe);
 EXPORT_SYMBOL_GPL(sata_phy_reset);
 EXPORT_SYMBOL_GPL(__sata_phy_reset);
 EXPORT_SYMBOL_GPL(ata_bus_reset);
+EXPORT_SYMBOL_GPL(ata_reset);
 EXPORT_SYMBOL_GPL(ata_port_disable);
 EXPORT_SYMBOL_GPL(ata_ratelimit);
 EXPORT_SYMBOL_GPL(ata_scsi_ioctl);
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h	2005-12-18 17:58:10.000000000 +0900
+++ work/include/linux/libata.h	2005-12-18 18:00:31.000000000 +0900
@@ -115,9 +115,9 @@ enum {
 	ATA_FLAG_PORT_DISABLED	= (1 << 2), /* port is disabled, ignore it */
 	ATA_FLAG_SATA		= (1 << 3),
 	ATA_FLAG_NO_LEGACY	= (1 << 4), /* no legacy mode check */
-	ATA_FLAG_SRST		= (1 << 5), /* use ATA SRST, not E.D.D. */
+	ATA_FLAG_SRST		= (1 << 5), /* (obsolete) use ATA SRST, not E.D.D. */
 	ATA_FLAG_MMIO		= (1 << 6), /* use MMIO, not PIO */
-	ATA_FLAG_SATA_RESET	= (1 << 7), /* use COMRESET */
+	ATA_FLAG_SATA_RESET	= (1 << 7), /* (obsolete) use COMRESET */
 	ATA_FLAG_PIO_DMA	= (1 << 8), /* PIO cmds via DMA */
 	ATA_FLAG_NOINTR		= (1 << 9), /* FIXME: Remove this once
 					     * proper HSM is in place. */
@@ -170,6 +170,12 @@ enum {
 	/* Masks for port functions */
 	ATA_PORT_PRIMARY	= (1 << 0),
 	ATA_PORT_SECONDARY	= (1 << 1),
+
+	/* Flags for reset function */
+	ATA_RESET_SOFT		= (1 << 0),
+	ATA_RESET_HARD		= (1 << 1),
+	ATA_RESET_ANY		= ATA_RESET_SOFT | ATA_RESET_HARD,
+	ATA_RESET_CLASSIFY	= (1 << 2),
 };
 
 enum hsm_task_states {
@@ -373,7 +379,11 @@ struct ata_port_operations {
 	u8   (*check_altstatus)(struct ata_port *ap);
 	void (*dev_select)(struct ata_port *ap, unsigned int device);
 
-	void (*phy_reset) (struct ata_port *ap);
+	void (*phy_reset) (struct ata_port *ap); /* obsolete */
+
+	int (*soft_reset) (struct ata_port *ap, unsigned int *classes);
+	int (*hard_reset) (struct ata_port *ap, unsigned int *classes);
+	void (*post_reset) (struct ata_port *ap, const unsigned int *classes);
 	void (*post_set_mode) (struct ata_port *ap);
 
 	int (*check_atapi_dma) (struct ata_queued_cmd *qc);
@@ -430,6 +440,7 @@ extern void ata_port_probe(struct ata_po
 extern void __sata_phy_reset(struct ata_port *ap);
 extern void sata_phy_reset(struct ata_port *ap);
 extern void ata_bus_reset(struct ata_port *ap);
+extern int ata_reset(struct ata_port *ap, unsigned int flags, unsigned int *classes);
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI

[PATCH 03/14] libata: implement standard reset methods

[Tejun Heo]

Implement ata_std_softreset_srst, ata_std_softreset_edd,
ata_std_hardreset_sata and ata_std_postreset.  These routines are to
new reset mechanism what sata_phy_reset and ata_bus_reset are to
->phy_reset mechanism.  Most low level drivers should be able to use
these directly or with some wrapping.

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

Index: work/drivers/scsi/libata-core.c
===================================================================
--- work.orig/drivers/scsi/libata-core.c	2005-12-18 20:49:54.000000000 +0900
+++ work/drivers/scsi/libata-core.c	2005-12-18 20:50:33.000000000 +0900
@@ -2152,6 +2152,216 @@ err_out:
 }
 
 /**
+ *	ata_std_softreset_srst - reset host port via ATA SRST
+ *	@ap: port to reset
+ *	@classes: resulting classes of attached devices
+ *
+ *	Reset host port using ATA SRST.
+ */
+int ata_std_softreset_srst(struct ata_port *ap, unsigned int *classes)
+{
+	unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
+	unsigned int devmask = 0;
+	u8 err;
+
+	DPRINTK("ENTER\n");
+
+	/* determine if device 0/1 are present */
+	if (ata_devchk(ap, 0))
+		devmask |= (1 << 0);
+	if (slave_possible && ata_devchk(ap, 1))
+		devmask |= (1 << 1);
+
+	/* devchk reports device presence without actual device on
+	 * most SATA controllers.  Check SStatus and turn devmask off
+	 * if link is offline.  Note that we should continue resetting
+	 * even when it seems like there's no device.
+	 */
+	if (ap->ops->scr_read && !sata_dev_present(ap))
+		devmask = 0;
+
+	/* select device 0 again */
+	ap->ops->dev_select(ap, 0);
+
+	/* issue bus reset */
+	DPRINTK("about to softreset, devmask=%x\n", devmask);
+	if (ata_bus_softreset(ap, devmask)) {
+		DPRINTK("EXIT, softreset failed\n");
+		return -EIO;
+	}
+
+	/* determine by signature whether we have ATA or ATAPI devices */
+	classes[0] = ata_dev_try_classify(ap, 0, &err);
+	if (slave_possible && err != 0x81)
+		classes[1] = ata_dev_try_classify(ap, 1, &err);
+
+	DPRINTK("EXIT, classes[0]=%u [1]=%u\n", classes[0], classes[1]);
+	return 0;
+}
+
+/**
+ *	ata_std_softreset_edd - perform EDD and classify devices
+ *	@ap: port to perform EDD on
+ *	@classes: resulting classes of attached devices
+ *
+ *	This isn't really a softreset method.  It just performs EDD
+ *	and classify devices.  If a controller doesn't support SRST
+ *	and hardreset cannot classify, this function can be used as
+ *	softreset method for classification.
+ */
+int ata_std_softreset_edd(struct ata_port *ap, unsigned int *classes)
+{
+	struct ata_ioports *ioaddr = &ap->ioaddr;
+	unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
+	u8 err;
+
+	DPRINTK("ENTER\n");
+
+	/* select device 0 */
+	ap->ops->dev_select(ap, 0);
+
+	/* set up device control */
+	if (ap->flags & ATA_FLAG_MMIO)
+		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
+	else
+		outb(ap->ctl, ioaddr->ctl_addr);
+
+	if (ata_bus_edd(ap)) {
+		DPRINTK("EXIT, edd failed\n");
+		return -EIO;
+	}
+
+	/* determine by signature whether we have ATA or ATAPI devices */
+	classes[0] = ata_dev_try_classify(ap, 0, &err);
+	if (slave_possible && err != 0x81)
+		classes[1] = ata_dev_try_classify(ap, 1, &err);
+
+	DPRINTK("EXIT, classes[0]=%u [1]=%u\n", classes[0], classes[1]);
+	return 0;
+}
+
+/**
+ *	ata_std_hardreset_sata - reset host port via SATA phy reset
+ *	@ap: port to reset
+ *	@classes: resulting classes of attached devices
+ *
+ *	SATA phy-reset host port using DET bits of SControl register.
+ */
+int ata_std_hardreset_sata(struct ata_port *ap, unsigned int *classes)
+{
+	u32 sstatus;
+	unsigned long timeout = jiffies + (HZ * 5);
+
+	DPRINTK("ENTER\n");
+
+	/* issue phy wake/reset */
+	scr_write_flush(ap, SCR_CONTROL, 0x301);
+
+	/* Couldn't find anything in SATA I/II specs, but
+	 * AHCI-1.1 10.4.2 says at least 1 ms.
+	 */
+	msleep(1);
+
+	scr_write_flush(ap, SCR_CONTROL, 0x300);
+
+	/* wait for phy to become ready, if necessary */
+	do {
+		msleep(200);
+		sstatus = scr_read(ap, SCR_STATUS);
+		if ((sstatus & 0xf) != 1)
+			break;
+	} while (time_before(jiffies, timeout));
+
+	/* TODO: phy layer with polling, timeouts, etc. */
+	if (!sata_dev_present(ap)) {
+		classes[0] = ATA_DEV_NONE;
+		DPRINTK("EXIT, link offline\n");
+		return 0;
+	}
+
+	if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+		DPRINTK("EXIT, device not ready\n");
+		return -EIO;
+	}
+
+	classes[0] = ata_dev_try_classify(ap, 0, NULL);
+
+	DPRINTK("EXIT, class=%u\n", classes[0]);
+	return 0;
+}
+
+static void ata_sata_print_link_status(struct ata_port *ap)
+{
+	u32 sstatus, tmp;
+	const char *speed;
+
+	if (!ap->ops->scr_read)
+		return;
+
+	sstatus = scr_read(ap, SCR_STATUS);
+
+	if (sata_dev_present(ap)) {
+		tmp = (sstatus >> 4) & 0xf;
+		if (tmp & (1 << 0))
+			speed = "1.5";
+		else if (tmp & (1 << 1))
+			speed = "3.0";
+		else
+			speed = "<unknown>";
+		printk(KERN_INFO "ata%u: SATA link up %s Gbps (SStatus %X)\n",
+		       ap->id, speed, sstatus);
+	} else {
+		printk(KERN_INFO "ata%u: SATA link down (SStatus %X)\n",
+		       ap->id, sstatus);
+	}
+}
+
+/**
+ *	ata_std_postreset - standard postreset method
+ *	@ap: the target ata_port
+ *	@classes: classes of attached devices
+ *
+ *	post_reset method is invoked after a successful reset.  Note
+ *	that the device might have been reset more than once using
+ *	different reset methods before post_reset is invoked.
+ *	post_reset is reponsible for setting cable type.
+ *
+ *	ata_std_postreset() is the standard post_reset method.  It
+ *	sets cable type according to ATA_FLAG_SATA and performs other
+ *	standard choirs.
+ */
+void ata_std_postreset(struct ata_port *ap, const unsigned int *classes)
+{
+	DPRINTK("ENTER\n");
+
+	/* set cable type */
+	if (ap->cbl == ATA_CBL_NONE && ap->flags & ATA_FLAG_SATA)
+		ap->cbl = ATA_CBL_SATA;
+
+	/* print link status */
+	if (ap->cbl == ATA_CBL_SATA)
+		ata_sata_print_link_status(ap);
+
+	/* bail out if no device is present */
+	if (classes[0] == ATA_DEV_NONE && classes[1] == ATA_DEV_NONE) {
+		DPRINTK("EXIT, no device\n");
+		return;
+	}
+
+	/* is double-select really necessary? */
+	if (classes[0] != ATA_DEV_NONE)
+		ap->ops->dev_select(ap, 1);
+	if (classes[1] != ATA_DEV_NONE)
+		ap->ops->dev_select(ap, 0);
+
+	/* re-enable interrupts & set up device control */
+	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
+		ata_irq_on(ap);
+
+	DPRINTK("EXIT\n");
+}
+
+/**
  *	ata_reset - reset host port and associated ATA channel
  *	@ap: port to reset
  *	@flags: ATA_RESET_* flags
@@ -5093,6 +5303,10 @@ EXPORT_SYMBOL_GPL(ata_tf_from_fis);
 EXPORT_SYMBOL_GPL(ata_check_status);
 EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);
+EXPORT_SYMBOL_GPL(ata_std_hardreset_sata);
+EXPORT_SYMBOL_GPL(ata_std_softreset_srst);
+EXPORT_SYMBOL_GPL(ata_std_softreset_edd);
+EXPORT_SYMBOL_GPL(ata_std_postreset);
 EXPORT_SYMBOL_GPL(ata_port_start);
 EXPORT_SYMBOL_GPL(ata_port_stop);
 EXPORT_SYMBOL_GPL(ata_host_stop);
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h	2005-12-18 20:49:54.000000000 +0900
+++ work/include/linux/libata.h	2005-12-18 20:50:33.000000000 +0900
@@ -470,6 +470,10 @@ 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_softreset_srst(struct ata_port *ap, unsigned int *classes);
+extern int ata_std_softreset_edd(struct ata_port *ap, unsigned int *classes);
+extern int ata_std_hardreset_sata(struct ata_port *ap, unsigned int *classes);
+extern void ata_std_postreset(struct ata_port *ap, const 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);

[PATCH 04/14] libata: export ata_busy_sleep()

[Tejun Heo]

Some low level drivers need ata_busy_sleep while performing resets.
Export it.

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

Index: work/drivers/scsi/libata-core.c
===================================================================
--- work.orig/drivers/scsi/libata-core.c	2005-12-18 20:50:11.000000000 +0900
+++ work/drivers/scsi/libata-core.c	2005-12-18 20:50:22.000000000 +0900
@@ -61,9 +61,6 @@
 
 #include "libata.h"
 
-static unsigned int ata_busy_sleep (struct ata_port *ap,
-				    unsigned long tmout_pat,
-			    	    unsigned long tmout);
 static void ata_dev_reread_id(struct ata_port *ap, struct ata_device *dev);
 static void ata_dev_init_params(struct ata_port *ap, struct ata_device *dev);
 static void ata_set_mode(struct ata_port *ap);
@@ -1893,9 +1890,8 @@ err_out:
  *
  */
 
-static unsigned int ata_busy_sleep (struct ata_port *ap,
-				    unsigned long tmout_pat,
-			    	    unsigned long tmout)
+unsigned int ata_busy_sleep (struct ata_port *ap,
+			     unsigned long tmout_pat, unsigned long tmout)
 {
 	unsigned long timer_start, timeout;
 	u8 status;
@@ -5324,6 +5320,7 @@ EXPORT_SYMBOL_GPL(ata_bus_reset);
 EXPORT_SYMBOL_GPL(ata_reset);
 EXPORT_SYMBOL_GPL(ata_port_disable);
 EXPORT_SYMBOL_GPL(ata_ratelimit);
+EXPORT_SYMBOL_GPL(ata_busy_sleep);
 EXPORT_SYMBOL_GPL(ata_scsi_ioctl);
 EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
 EXPORT_SYMBOL_GPL(ata_scsi_error);
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h	2005-12-18 20:49:56.000000000 +0900
+++ work/include/linux/libata.h	2005-12-18 20:50:22.000000000 +0900
@@ -457,6 +457,9 @@ extern int ata_scsi_error(struct Scsi_Ho
 extern int ata_scsi_release(struct Scsi_Host *host);
 extern unsigned int ata_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc);
 extern int ata_ratelimit(void);
+extern unsigned int ata_busy_sleep(struct ata_port *ap,
+				   unsigned long timeout_pat,
+				   unsigned long timeout);
 
 /*
  * Default driver ops implementations

[PATCH 05/14] sata_sil: convert to new reset mechanism

[Tejun Heo]

Convert sata_sil to use new reset mechanism.  sata_sil is fairly
generic and can directly use all std routines.

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

Index: work/drivers/scsi/sata_sil.c
===================================================================
--- work.orig/drivers/scsi/sata_sil.c	2005-12-18 16:32:04.000000000 +0900
+++ work/drivers/scsi/sata_sil.c	2005-12-18 18:10:09.000000000 +0900
@@ -158,7 +158,9 @@ static const struct ata_port_operations 
 	.check_status		= ata_check_status,
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
-	.phy_reset		= sata_phy_reset,
+	.soft_reset		= ata_std_softreset_srst,
+	.hard_reset		= ata_std_hardreset_sata,
+	.post_reset		= ata_std_postreset,
 	.post_set_mode		= sil_post_set_mode,
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
@@ -181,7 +183,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO,
+				  ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -190,8 +192,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  SIL_FLAG_MOD15WRITE,
+				  ATA_FLAG_MMIO | SIL_FLAG_MOD15WRITE,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -200,7 +201,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO,
+				  ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */

[PATCH 06/14] sata_sil24: convert to new reset mechanism

[Tejun Heo]

Convert sata_sil24 ->phy_reset to ->soft_reset.

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

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-12-18 18:21:56.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-12-18 18:35:13.000000000 +0900
@@ -249,7 +249,7 @@ 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 void sil24_phy_reset(struct ata_port *ap);
+static int sil24_softreset(struct ata_port *ap, unsigned int *classes);
 static void sil24_qc_prep(struct ata_queued_cmd *qc);
 static int sil24_qc_issue(struct ata_queued_cmd *qc);
 static void sil24_irq_clear(struct ata_port *ap);
@@ -306,7 +306,8 @@ static const struct ata_port_operations 
 
 	.tf_read		= sil24_tf_read,
 
-	.phy_reset		= sil24_phy_reset,
+	.soft_reset		= sil24_softreset,
+	.post_reset		= ata_std_postreset,
 
 	.qc_prep		= sil24_qc_prep,
 	.qc_issue		= sil24_qc_issue,
@@ -336,8 +337,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(4),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(4),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -347,8 +348,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(2),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(2),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -358,8 +359,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(1),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(1),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -428,7 +429,7 @@ static void sil24_tf_read(struct ata_por
 	*tf = pp->tf;
 }
 
-static int sil24_issue_SRST(struct ata_port *ap)
+static int sil24_softreset(struct ata_port *ap, unsigned int *classes)
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
 	struct sil24_port_priv *pp = ap->private_data;
@@ -437,6 +438,8 @@ static int sil24_issue_SRST(struct ata_p
 	u32 irq_enable, irq_stat;
 	int cnt;
 
+	DPRINTK("ENTER\n");
+
 	/* temporarily turn off IRQs during SRST */
 	irq_enable = readl(port + PORT_IRQ_ENABLE_SET);
 	writel(irq_enable, port + PORT_IRQ_ENABLE_CLR);
@@ -466,30 +469,20 @@ static int sil24_issue_SRST(struct ata_p
 	/* restore IRQs */
 	writel(irq_enable, port + PORT_IRQ_ENABLE_SET);
 
-	if (!(irq_stat & PORT_IRQ_COMPLETE))
-		return -1;
-
-	/* update TF */
-	sil24_update_tf(ap);
-	return 0;
-}
-
-static void sil24_phy_reset(struct ata_port *ap)
-{
-	struct sil24_port_priv *pp = ap->private_data;
-
-	__sata_phy_reset(ap);
-	if (ap->flags & ATA_FLAG_PORT_DISABLED)
-		return;
+	if (sata_dev_present(ap)) {
+		if (!(irq_stat & PORT_IRQ_COMPLETE)) {
+			DPRINTK("EXIT, srst failed\n");
+			return -EIO;
+		}
 
-	if (sil24_issue_SRST(ap) < 0) {
-		printk(KERN_ERR DRV_NAME
-		       " ata%u: SRST failed, disabling port\n", ap->id);
-		ap->ops->port_disable(ap);
-		return;
+		sil24_update_tf(ap);
+		classes[0] = ata_dev_classify(&pp->tf);
 	}
+	if (classes[0] == ATA_DEV_UNKNOWN)
+		classes[0] = ATA_DEV_NONE;
 
-	ap->device->class = ata_dev_classify(&pp->tf);
+	DPRINTK("EXIT, classes[0]=%u [1]=%u\n", classes[0], classes[1]);
+	return 0;
 }
 
 static inline void sil24_fill_sg(struct ata_queued_cmd *qc,

[PATCH 07/14] sata_sil24: add hardreset

[Tejun Heo]

Now that libata is smart enough to handle both soft and hard resets,
add hardreset method.  Note that sil24 hardreset doesn't supply
signature; still, the new reset mechanism can make good use of it.

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

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-12-18 18:35:13.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-12-18 18:35:35.000000000 +0900
@@ -250,6 +250,7 @@ static u32 sil24_scr_read(struct ata_por
 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_softreset(struct ata_port *ap, unsigned int *classes);
+static int sil24_hardreset(struct ata_port *ap, unsigned int *classes);
 static void sil24_qc_prep(struct ata_queued_cmd *qc);
 static int sil24_qc_issue(struct ata_queued_cmd *qc);
 static void sil24_irq_clear(struct ata_port *ap);
@@ -307,6 +308,7 @@ static const struct ata_port_operations 
 	.tf_read		= sil24_tf_read,
 
 	.soft_reset		= sil24_softreset,
+	.hard_reset		= sil24_hardreset,
 	.post_reset		= ata_std_postreset,
 
 	.qc_prep		= sil24_qc_prep,
@@ -485,6 +487,14 @@ static int sil24_softreset(struct ata_po
 	return 0;
 }
 
+static int sil24_hardreset(struct ata_port *ap, unsigned int *classes)
+{
+	unsigned int dummy_classes[2];
+
+	/* sil24 doesn't report device signature after hard reset */
+	return ata_std_hardreset_sata(ap, dummy_classes);
+}
+
 static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
 				 struct sil24_sge *sge)
 {

[PATCH 08/14] ata_piix: convert pata to new reset mechanism

[Tejun Heo]

Convert ata_piix pata ->phy_reset to ->soft_reset.

--

NOTE: piix patches are against upstream + piix MAPVALUE fix I posted
earlier today.

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

Index: work/drivers/scsi/ata_piix.c
===================================================================
--- work.orig/drivers/scsi/ata_piix.c	2005-12-18 21:11:44.000000000 +0900
+++ work/drivers/scsi/ata_piix.c	2005-12-18 21:11:58.000000000 +0900
@@ -131,7 +131,8 @@ enum {
 static int piix_init_one (struct pci_dev *pdev,
 				    const struct pci_device_id *ent);
 
-static void piix_pata_phy_reset(struct ata_port *ap);
+static int piix_pata_softreset(struct ata_port *ap, unsigned int *classes);
+static void piix_pata_postreset(struct ata_port *ap, const unsigned int *classes);
 static void piix_sata_phy_reset(struct ata_port *ap);
 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);
@@ -201,7 +202,8 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= piix_pata_phy_reset,
+	.soft_reset		= piix_pata_softreset,
+	.post_reset		= piix_pata_postreset,
 
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
@@ -252,8 +254,7 @@ static struct ata_port_info piix_port_in
 	/* ich5_pata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
-				  PIIX_FLAG_CHECKINTR,
+		.host_flags	= ATA_FLAG_SLAVE_POSS | PIIX_FLAG_CHECKINTR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 #if 0
 		.mwdma_mask	= 0x06, /* mwdma1-2 */
@@ -278,7 +279,7 @@ static struct ata_port_info piix_port_in
 	/* piix4_pata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
+		.host_flags	= ATA_FLAG_SLAVE_POSS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 #if 0
 		.mwdma_mask	= 0x06, /* mwdma1-2 */
@@ -360,30 +361,45 @@ cbl40:
 }
 
 /**
- *	piix_pata_phy_reset - Probe specified port on PATA host controller
- *	@ap: Port to probe
+ *	piix_pata_softreset - Perform SRST softreset on PATA host controller
+ *	@ap: Port to reset
+ *	@classes: Resulting classes of attached devices
  *
- *	Probe PATA phy.
+ *	Softreset PATA phy.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
 
-static void piix_pata_phy_reset(struct ata_port *ap)
+static int piix_pata_softreset(struct ata_port *ap, unsigned int *classes)
 {
 	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_disable(ap);
 		printk(KERN_INFO "ata%u: port disabled. ignoring.\n", ap->id);
-		return;
+		classes[0] = ATA_DEV_NONE;
+		classes[1] = ATA_DEV_NONE;
+		return 0;
 	}
 
-	piix_pata_cbl_detect(ap);
+	return ata_std_softreset_srst(ap, classes);
+}
 
-	ata_port_probe(ap);
+/**
+ *	piix_pata_postreset - Postreset stuff on PATA host controller
+ *	@ap: Target port
+ *	@classes: Classes of attached devices
+ *
+ *	Postreset processing including cable detection.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
 
-	ata_bus_reset(ap);
+static void piix_pata_postreset(struct ata_port *ap, const unsigned int *classes)
+{
+	piix_pata_cbl_detect(ap);
+	ata_std_postreset(ap, classes);
 }
 
 /**

[PATCH 09/14] ata_piix: convert sata to new reset mechanism

[Tejun Heo]

Convert ata_piix sata ->phy_reset to ->soft_reset.

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

Index: work/drivers/scsi/ata_piix.c
===================================================================
--- work.orig/drivers/scsi/ata_piix.c	2005-12-18 19:06:17.000000000 +0900
+++ work/drivers/scsi/ata_piix.c	2005-12-18 19:33:56.000000000 +0900
@@ -133,7 +133,7 @@ static int piix_init_one (struct pci_dev
 
 static int piix_pata_softreset(struct ata_port *ap, unsigned int *classes);
 static void piix_pata_postreset(struct ata_port *ap, const unsigned int *classes);
-static void piix_sata_phy_reset(struct ata_port *ap);
+static int piix_sata_softreset(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);
 
@@ -231,7 +231,8 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= piix_sata_phy_reset,
+	.soft_reset		= piix_sata_softreset,
+	.post_reset		= ata_std_postreset,
 
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
@@ -268,8 +269,8 @@ static struct ata_port_info piix_port_in
 	/* ich5_sata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR,
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED |
+				  PIIX_FLAG_CHECKINTR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
@@ -293,8 +294,7 @@ static struct ata_port_info piix_port_in
 	/* ich6_sata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED_ICH6 |
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -305,8 +305,7 @@ static struct ata_port_info piix_port_in
 	/* ich6_sata_ahci */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED_ICH6 |
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS |
 				  PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
@@ -450,28 +449,26 @@ static int piix_sata_probe (struct ata_p
 }
 
 /**
- *	piix_sata_phy_reset - Probe specified port on SATA host controller
- *	@ap: Port to probe
+ *	piix_sata_softreset - Perform SRST softreset on SATA host controller
+ *	@ap: Port to reset
+ *	@classes: Resulting classes of attached devices
  *
- *	Probe SATA phy.
+ *	Softreset SATA phy.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
 
-static void piix_sata_phy_reset(struct ata_port *ap)
+static int piix_sata_softreset(struct ata_port *ap, unsigned int *classes)
 {
 	if (!piix_sata_probe(ap)) {
-		ata_port_disable(ap);
+		classes[0] = ATA_DEV_NONE;
+		classes[1] = ATA_DEV_NONE;
 		printk(KERN_INFO "ata%u: SATA port has no device.\n", ap->id);
-		return;
+		return 0;
 	}
 
-	ap->cbl = ATA_CBL_SATA;
-
-	ata_port_probe(ap);
-
-	ata_bus_reset(ap);
+	return ata_std_softreset_srst(ap, classes);
 }
 
 /**

[PATCH 10/14] ahci: separate out ahci_stop/start_engine()

[Tejun Heo]

Separate out ahci_stop/start_engine() from ahci_restart_port().  This
is preparation for conversion to new reset mechanism.

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

Index: work/drivers/scsi/ahci.c
===================================================================
--- work.orig/drivers/scsi/ahci.c	2005-12-18 16:32:04.000000000 +0900
+++ work/drivers/scsi/ahci.c	2005-12-18 19:54:50.000000000 +0900
@@ -437,6 +437,43 @@ static void ahci_scr_write (struct ata_p
 	writel(val, (void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
+static int ahci_stop_engine(struct ata_port *ap)
+{
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	int work;
+	u32 tmp;
+
+	tmp = readl(port_mmio + PORT_CMD);
+	tmp &= ~PORT_CMD_START;
+	writel(tmp, port_mmio + PORT_CMD);
+
+	/* wait for engine to stop.  TODO: this could be
+	 * as long as 500 msec
+	 */
+	work = 1000;
+	while (work-- > 0) {
+		tmp = readl(port_mmio + PORT_CMD);
+		if ((tmp & PORT_CMD_LIST_ON) == 0)
+			return 0;
+		udelay(10);
+	}
+
+	return -EIO;
+}
+
+static void ahci_start_engine(struct ata_port *ap)
+{
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	u32 tmp;
+
+	tmp = readl(port_mmio + PORT_CMD);
+	tmp |= PORT_CMD_START;
+	writel(tmp, port_mmio + PORT_CMD);
+	readl(port_mmio + PORT_CMD); /* flush */
+}
+
 static void ahci_phy_reset(struct ata_port *ap)
 {
 	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
@@ -563,7 +600,6 @@ static void ahci_restart_port(struct ata
 	void __iomem *mmio = ap->host_set->mmio_base;
 	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
 	u32 tmp;
-	int work;
 
 	if ((ap->device[0].class != ATA_DEV_ATAPI) ||
 	    ((irq_stat & PORT_IRQ_TF_ERR) == 0))
@@ -579,20 +615,7 @@ static void ahci_restart_port(struct ata
 			readl(port_mmio + PORT_SCR_ERR));
 
 	/* stop DMA */
-	tmp = readl(port_mmio + PORT_CMD);
-	tmp &= ~PORT_CMD_START;
-	writel(tmp, port_mmio + PORT_CMD);
-
-	/* wait for engine to stop.  TODO: this could be
-	 * as long as 500 msec
-	 */
-	work = 1000;
-	while (work-- > 0) {
-		tmp = readl(port_mmio + PORT_CMD);
-		if ((tmp & PORT_CMD_LIST_ON) == 0)
-			break;
-		udelay(10);
-	}
+	ahci_stop_engine(ap);
 
 	/* clear SATA phy error, if any */
 	tmp = readl(port_mmio + PORT_SCR_ERR);
@@ -611,10 +634,7 @@ static void ahci_restart_port(struct ata
 	}
 
 	/* re-start DMA */
-	tmp = readl(port_mmio + PORT_CMD);
-	tmp |= PORT_CMD_START;
-	writel(tmp, port_mmio + PORT_CMD);
-	readl(port_mmio + PORT_CMD); /* flush */
+	ahci_start_engine(ap);
 }
 
 static void ahci_eng_timeout(struct ata_port *ap)

[PATCH 11/14] ahci: convert to new reset mechanism

[Tejun Heo]

Convert ahci ->phy_reset to ->hard_reset.  The original ->phy_reset
simply called __sata_phy_reset() to perform reset; however, AHCI spec
mandates to turn off START bit during reset.  New ->hard_reset method
properly follows AHCI reset procedure.

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

Index: work/drivers/scsi/ahci.c
===================================================================
--- work.orig/drivers/scsi/ahci.c	2005-12-18 22:23:38.000000000 +0900
+++ work/drivers/scsi/ahci.c	2005-12-18 22:24:04.000000000 +0900
@@ -186,7 +186,8 @@ static void ahci_scr_write (struct ata_p
 static int ahci_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static int ahci_qc_issue(struct ata_queued_cmd *qc);
 static irqreturn_t ahci_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
-static void ahci_phy_reset(struct ata_port *ap);
+static int ahci_hardreset(struct ata_port *ap, unsigned int *classes);
+static void ahci_postreset(struct ata_port *ap, const unsigned int *classes);
 static void ahci_irq_clear(struct ata_port *ap);
 static void ahci_eng_timeout(struct ata_port *ap);
 static int ahci_port_start(struct ata_port *ap);
@@ -226,7 +227,8 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
-	.phy_reset		= ahci_phy_reset,
+	.hard_reset		= ahci_hardreset,
+	.post_reset		= ahci_postreset,
 
 	.qc_prep		= ahci_qc_prep,
 	.qc_issue		= ahci_qc_issue,
@@ -248,8 +250,7 @@ static const struct ata_port_info ahci_p
 	{
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA,
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
@@ -474,17 +475,30 @@ static void ahci_start_engine(struct ata
 	readl(port_mmio + PORT_CMD); /* flush */
 }
 
-static void ahci_phy_reset(struct ata_port *ap)
+static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
+			     unsigned long interval_msec,
+			     unsigned long timeout_msec)
 {
-	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
-	struct ata_taskfile tf;
-	struct ata_device *dev = &ap->device[0];
-	u32 new_tmp, tmp;
+	unsigned long timeout;
+	u32 tmp;
+
+	timeout = jiffies + (timeout_msec * HZ) / 1000;
+	do {
+		tmp = readl(reg);
+		if ((tmp & mask) != val)
+			return 0;
+		msleep(interval_msec);
+	} while (time_before(jiffies, timeout));
 
-	__sata_phy_reset(ap);
+	return -1;
+}
 
-	if (ap->flags & ATA_FLAG_PORT_DISABLED)
-		return;
+static unsigned int ahci_dev_classify(struct ata_port *ap)
+{
+	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+	struct ata_taskfile tf;
+	unsigned int class;
+	u32 tmp;
 
 	tmp = readl(port_mmio + PORT_SIG);
 	tf.lbah		= (tmp >> 24)	& 0xff;
@@ -492,15 +506,58 @@ static void ahci_phy_reset(struct ata_po
 	tf.lbal		= (tmp >> 8)	& 0xff;
 	tf.nsect	= (tmp)		& 0xff;
 
-	dev->class = ata_dev_classify(&tf);
-	if (!ata_dev_present(dev)) {
-		ata_port_disable(ap);
-		return;
-	}
+	class = ata_dev_classify(&tf);
+	if (class == ATA_DEV_UNKNOWN)
+		class = ATA_DEV_NONE;
+	return class;
+}
+
+static int ahci_hardreset(struct ata_port *ap, unsigned int *classes)
+{
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	u32 tmp;
+
+	DPRINTK("ENTER\n");
+
+	/* stop engine */
+	ahci_stop_engine(ap);
+
+	/* issue COMRESET */
+	writel(0x301, port_mmio + PORT_SCR_CTL);
+	readl(port_mmio + PORT_SCR_CTL); /* flush */
+	msleep(1);
+	writel(0x300, port_mmio + PORT_SCR_CTL);
+	readl(port_mmio + PORT_SCR_CTL); /* flush */
+
+	msleep(200);
+	ahci_wait_for_bit(port_mmio + PORT_SCR_STAT, 0xf, 0x1, 1, 5000);
+
+	/* clear SATA phy error, if any */
+	tmp = readl(port_mmio + PORT_SCR_ERR);
+	writel(tmp, port_mmio + PORT_SCR_ERR);
+
+	/* re-start engine */
+	ahci_start_engine(ap);
+
+	classes[0] = ATA_DEV_NONE;
+	if (sata_dev_present(ap))
+		classes[0] = ahci_dev_classify(ap);
+
+	DPRINTK("EXIT, class=%u\n", classes[0]);
+	return 0;
+}
+
+static void ahci_postreset(struct ata_port *ap, const unsigned int *classes)
+{
+	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+	u32 new_tmp, tmp;
+
+	ata_std_postreset(ap, classes);
 
 	/* Make sure port's ATAPI bit is set appropriately */
 	new_tmp = tmp = readl(port_mmio + PORT_CMD);
-	if (dev->class == ATA_DEV_ATAPI)
+	if (classes[0] == ATA_DEV_ATAPI)
 		new_tmp |= PORT_CMD_ATAPI;
 	else
 		new_tmp &= ~PORT_CMD_ATAPI;

[PATCH 12/14] ahci: separate out ahci_cmd_prep()

[Tejun Heo]

Separate out achi_cmd_prep() from ahci_qc_prep().  This is preparation
for adding softreset method.

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

Index: work/drivers/scsi/ahci.c
===================================================================
--- work.orig/drivers/scsi/ahci.c	2005-12-18 22:24:04.000000000 +0900
+++ work/drivers/scsi/ahci.c	2005-12-18 22:24:12.000000000 +0900
@@ -475,6 +475,16 @@ static void ahci_start_engine(struct ata
 	readl(port_mmio + PORT_CMD); /* flush */
 }
 
+static inline void ahci_cmd_prep(struct ata_port *ap, u32 opts)
+{
+	struct ahci_port_priv *pp = ap->private_data;
+
+	pp->cmd_slot[0].opts = cpu_to_le32(opts);
+	pp->cmd_slot[0].status = 0;
+	pp->cmd_slot[0].tbl_addr = cpu_to_le32(pp->cmd_tbl_dma & 0xffffffff);
+	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
+}
+
 static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
 			     unsigned long interval_msec,
 			     unsigned long timeout_msec)
@@ -629,10 +639,7 @@ static void ahci_qc_prep(struct ata_queu
 	if (is_atapi_taskfile(&qc->tf))
 		opts |= AHCI_CMD_ATAPI;
 
-	pp->cmd_slot[0].opts = cpu_to_le32(opts);
-	pp->cmd_slot[0].status = 0;
-	pp->cmd_slot[0].tbl_addr = cpu_to_le32(pp->cmd_tbl_dma & 0xffffffff);
-	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
+	ahci_cmd_prep(ap, opts);
 
 	/*
 	 * Fill in command table information.  First, the header,

[PATCH 13/14] ahci: add constants for SRST

[Tejun Heo]

Add constants needed to perform SRST.  This is preparation for adding
softreset method.

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

Index: work/drivers/scsi/ahci.c
===================================================================
--- work.orig/drivers/scsi/ahci.c	2005-12-18 20:11:01.000000000 +0900
+++ work/drivers/scsi/ahci.c	2005-12-18 20:12:01.000000000 +0900
@@ -66,6 +66,8 @@ enum {
 	AHCI_IRQ_ON_SG		= (1 << 31),
 	AHCI_CMD_ATAPI		= (1 << 5),
 	AHCI_CMD_WRITE		= (1 << 6),
+	AHCI_CMD_RESET		= (1 << 8),
+	AHCI_CMD_CLR_BUSY	= (1 << 10),
 
 	RX_FIS_D2H_REG		= 0x40,	/* offset of D2H Register FIS data */
 
@@ -85,6 +87,7 @@ enum {
 
 	/* HOST_CAP bits */
 	HOST_CAP_64		= (1 << 31), /* PCI DAC (64-bit DMA) support */
+	HOST_CAP_CLO		= (1 << 24), /* Command List Override support */
 
 	/* registers for each SATA port */
 	PORT_LST_ADDR		= 0x00, /* command list DMA addr */
@@ -138,6 +141,7 @@ enum {
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
 	PORT_CMD_FIS_RX		= (1 << 4), /* Enable FIS receive DMA engine */
+	PORT_CMD_CLO		= (1 << 3), /* Command list override */
 	PORT_CMD_POWER_ON	= (1 << 2), /* Power up device */
 	PORT_CMD_SPIN_UP	= (1 << 1), /* Spin up device */
 	PORT_CMD_START		= (1 << 0), /* Enable port DMA engine */

[PATCH 14/14] ahci: add softreset

[Tejun Heo]

Now that libata is smart enought to handle both soft and hard resets,
add softreset method.

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

Index: work/drivers/scsi/ahci.c
===================================================================
--- work.orig/drivers/scsi/ahci.c	2005-12-18 22:24:15.000000000 +0900
+++ work/drivers/scsi/ahci.c	2005-12-18 22:24:17.000000000 +0900
@@ -190,6 +190,7 @@ static void ahci_scr_write (struct ata_p
 static int ahci_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static 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_softreset(struct ata_port *ap, unsigned int *classes);
 static int ahci_hardreset(struct ata_port *ap, unsigned int *classes);
 static void ahci_postreset(struct ata_port *ap, const unsigned int *classes);
 static void ahci_irq_clear(struct ata_port *ap);
@@ -231,6 +232,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
+	.soft_reset		= ahci_softreset,
 	.hard_reset		= ahci_hardreset,
 	.post_reset		= ahci_postreset,
 
@@ -526,6 +528,106 @@ static unsigned int ahci_dev_classify(st
 	return class;
 }
 
+static int ahci_softreset(struct ata_port *ap, unsigned int *classes)
+{
+	struct ahci_host_priv *hpriv = ap->host_set->private_data;
+	struct ahci_port_priv *pp = ap->private_data;
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	const u32 cmd_fis_len = 5; /* five dwords */
+	struct ata_taskfile tf;
+	u8 *fis;
+	int ret;
+
+	DPRINTK("ENTER\n");
+
+	/* prepare for SRST (AHCI-1.1 10.4.1) */
+	ret = ahci_stop_engine(ap);
+	if (ret)
+		goto out_restart;
+
+	/* check BUSY/DRQ, perform Command List Override if necessary */
+	ahci_tf_read(ap, &tf);
+	if (tf.command & (ATA_BUSY | ATA_DRQ)) {
+		u32 tmp;
+
+		ret = -EIO;
+		if (!(hpriv->cap & HOST_CAP_CLO))
+			goto out_restart;
+
+		tmp = readl(port_mmio + PORT_CMD);
+		tmp |= PORT_CMD_CLO;
+		writel(tmp, port_mmio + PORT_CMD);
+		readl(port_mmio + PORT_CMD); /* flush */
+
+		ret = -EIO;
+		if (ahci_wait_for_bit(port_mmio + PORT_CMD,
+				      PORT_CMD_CLO, PORT_CMD_CLO, 1, 500))
+			goto out_restart;
+	}
+
+	/* restart engine */
+	ahci_start_engine(ap);
+
+	ata_tf_init(ap, &tf, 0);
+	fis = pp->cmd_tbl;
+
+	/* issue the first D2H Register FIS */
+	ahci_cmd_prep(ap, cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
+
+	tf.ctl |= ATA_SRST;
+	ata_tf_to_fis(&tf, fis, 0);
+	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+
+	writel(1, port_mmio + PORT_CMD_ISSUE);
+	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
+
+	if (ahci_wait_for_bit(port_mmio + PORT_CMD_ISSUE, 0x1, 0x1, 1, 500)) {
+		DPRINTK("EXIT, 1st FIS failed\n");
+		return -EIO;
+	}
+
+	/* spec says at least 5us, but be generous and sleep for 1ms */
+	msleep(1);
+
+	/* issue the second D2H Register FIS */
+	ahci_cmd_prep(ap, cmd_fis_len);
+
+	tf.ctl &= ~ATA_SRST;
+	ata_tf_to_fis(&tf, fis, 0);
+	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+
+	writel(1, port_mmio + PORT_CMD_ISSUE);
+	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
+
+	/* spec mandates ">= 2ms" before checking status.
+	 * We wait 150ms, because that was the magic delay used for
+	 * ATAPI devices in Hale Landis's ATADRVR, for the period of time
+	 * between when the ATA command register is written, and then
+	 * status is checked.  Because waiting for "a while" before
+	 * checking status is fine, post SRST, we perform this magic
+	 * delay here as well.
+	 */
+	msleep(150);
+
+	classes[0] = ATA_DEV_NONE;
+	if (sata_dev_present(ap)) {
+		if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+			DPRINTK("EXIT, device not ready\n");
+			return -EIO;
+		}
+		classes[0] = ahci_dev_classify(ap);
+	}
+
+	DPRINTK("EXIT, class=%u\n", classes[0]);
+	return 0;
+
+ out_restart:
+	DPRINTK("EXIT, error=%d\n", ret);
+	ahci_start_engine(ap);
+	return ret;
+}
+
 static int ahci_hardreset(struct ata_port *ap, unsigned int *classes)
 {
 	void __iomem *mmio = ap->host_set->mmio_base;

Re: [RFC/PATCHSET] libata: new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> Hello, all.
> 
> Currently libata uses ->phy_reset to reset ports.  However,
> ->phy_reset is tightly woven into probing process (directly
> manipulating device[]->class and disabling ports on failure) and thus
> awkward to use for error handling or other purposes.
> 
> Another problem with the current scheme is that libata doesn't have
> much control over reset process, which isn't very nice for both
> initialization and error handling.
> 
> This patchset implements new reset mechanism.  The new mechanism can
> peacefully live side-by-side with ->phy_reset mechanism and low level
> drivers can be converted gradually.  This patchset converts sata_sil,
> sata_sil24, ata_piix and ahci as a start.
> 
> This patchset is composed of the following 14 patches.
> 
> #01-03	: implement new reset mechanism and standard callbacks
> #04	: preparation for low level driver conversion
> #05	: convert sata_sil
> #06-07	: convert sata_sil24 and add hardreset
> #08-09	: convert ata_piix
> #10-14	: convert ahci and add softreset

Hard and soft reset should be implemented as

	qc = ata_qc_new_init()
	qc->tf.protocol = ATA_PROT_HARD_RESET;
	... ata_qc_issue() ...

which automatically takes advantage of the ability to specify a behavior 
using the qc_prep/qc_issue driver hooks.  The ATA passthru CDB supports 
this method of programming (hard and soft reset are specified 
protocols), and this is very similar to how the SiI 3124 behaves.  Other 
FIS-based controllers will implement qc_issue/qc_prep such that they 
send two Control FIS's.  Taskfile-based controllers use the currently 
implemented method.

On a separate note, E.D.D. support can probably be dropped.  No driver 
uses it AFAIK.  It was only used during development and in the early 
libata days.

Further, we should lean towards avoiding hard reset, and issuing SRST 
most of the time, even for drivers currently doing hard reset (COMRESET).

	Jeff

Re: [PATCH 02/14] libata: implement new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> +int ata_reset(struct ata_port *ap, unsigned int flags, unsigned int *r_classes)
> +{
> +	const unsigned int unknown[2] = { ATA_DEV_UNKNOWN, ATA_DEV_UNKNOWN };
> +	unsigned int classes[2];
> +	int ret = 0;
> +
> +	DPRINTK("ENTER\n");
> +
> +	if (ap->ops->soft_reset == NULL)
> +		flags &= ~ATA_RESET_SOFT;
> +	if (ap->ops->hard_reset == NULL)
> +		flags &= ~ATA_RESET_HARD;
> +
> +	if (!(flags & (ATA_RESET_SOFT | ATA_RESET_HARD))) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> + retry:
> +	if (flags & ATA_RESET_SOFT) {
> +		memcpy(classes, unknown, sizeof(classes));
> +		ret = ap->ops->soft_reset(ap, classes);
> +		if (ret == 0) {
> +			if (!(flags & ATA_RESET_CLASSIFY) ||
> +			    classes[0] != ATA_DEV_UNKNOWN)
> +				goto success;
> +			/* no signature on SRST, try HRST */
> +			flags &= ~ATA_RESET_SOFT;
> +			ret = -ENODEV;	/* no sigature */
> +		}
> +		/* SRST failed, try HRST */
> +	}
> +
> +	if (flags & ATA_RESET_HARD) {
> +		memcpy(classes, unknown, sizeof(classes));
> +		ret = ap->ops->hard_reset(ap, classes);
> +		if (ret == 0) {
> +			if (!(flags & ATA_RESET_CLASSIFY) ||
> +			    classes[0] != ATA_DEV_UNKNOWN)
> +				goto success;
> +			/* no signature on HRST, retry SRST */
> +			flags &= ~ATA_RESET_HARD;
> +			ret = -ENODEV;	/* no sigature */
> +			goto retry;
> +		}
> +		/* HRST failed, give up */
> +	}
> +
> +	goto out;

Fundamental problem:  you should not hard code this execution order. 
Each controller has a different way it likes to handle global and port 
resets.  Sometimes COMRESET is automatically executed for you, when you 
do a controller reset (AHCI does this).

Looking at the bigger picture of error handling, we will want to 
basically run the entire probe sequence, after we reset.  I would look 
into a few key areas:

* figuring out how to safely stop SCSI from submitting commands, while 
you are doing the reset (important!).  SCSI errors are delivered to SATA 
ports -- thus if you are doing a global reset in the EH path, only one 
port's command submission is frozen, and the SCSI layer is happily 
sending commands to the other ports.

* get libata error handlers to start using scsi_eh_flush_done_q() and 
scsi_eh_finish_cmd(), which enables retrying of commands.

* making sure ata_bus_probe() can be called again and again

* implementing srst and hard-reset as taskfile protocols, and supporting 
them via qc_prep/qc_issue

	Jeff

Re: [PATCH 10/14] ahci: separate out ahci_stop/start_engine()

[Jeff Garzik]

Tejun Heo wrote:
> Separate out ahci_stop/start_engine() from ahci_restart_port().  This
> is preparation for conversion to new reset mechanism.

this sort of patch should be sent first, since it is "easy to apply" 
even though I might disagree with earlier patches in the series.

	jeff

Re: [PATCH 11/14] ahci: convert to new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> Convert ahci ->phy_reset to ->hard_reset.  The original ->phy_reset
> simply called __sata_phy_reset() to perform reset; however, AHCI spec
> mandates to turn off START bit during reset.  New ->hard_reset method
> properly follows AHCI reset procedure.

If you feel AHCI deviates from spec, it sounds like this fix should be 
separated out into a new patch?

	Jeff

Re: [PATCH 12/14] ahci: separate out ahci_cmd_prep()

[Jeff Garzik]

Tejun Heo wrote:
> Separate out achi_cmd_prep() from ahci_qc_prep().  This is preparation
> for adding softreset method.

ditto, this should be one of the first few patches in the patch order, 
since it is "easy to apply."  Applying this sort of patch is fine for 
me, since it doesn't harm the upstream driver, and it reduces the diff 
between upstream and whatever out-of-tree stuff you are working on.

	Jeff

Re: [PATCH 13/14] ahci: add constants for SRST

[Jeff Garzik]

Tejun Heo wrote:
> Add constants needed to perform SRST.  This is preparation for adding
> softreset method.

Another easy patch that should be earlier in the series.

	Jeff

Re: [PATCH 14/14] ahci: add softreset

[Jeff Garzik]

Tejun Heo wrote:
> Now that libata is smart enought to handle both soft and hard resets,
> add softreset method.

I'm a bit skeptical that polling is what should be done here.  SATA is 
inherently event-driven...

	Jeff

Re: [RFC/PATCHSET] libata: new reset mechanism

[Tejun Heo]

Hello, Jeff.

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Hello, all.
>>
>> Currently libata uses ->phy_reset to reset ports.  However,
>> ->phy_reset is tightly woven into probing process (directly
>> manipulating device[]->class and disabling ports on failure) and thus
>> awkward to use for error handling or other purposes.
>>
>> Another problem with the current scheme is that libata doesn't have
>> much control over reset process, which isn't very nice for both
>> initialization and error handling.
>>
>> This patchset implements new reset mechanism.  The new mechanism can
>> peacefully live side-by-side with ->phy_reset mechanism and low level
>> drivers can be converted gradually.  This patchset converts sata_sil,
>> sata_sil24, ata_piix and ahci as a start.
>>
>> This patchset is composed of the following 14 patches.
>>
>> #01-03    : implement new reset mechanism and standard callbacks
>> #04    : preparation for low level driver conversion
>> #05    : convert sata_sil
>> #06-07    : convert sata_sil24 and add hardreset
>> #08-09    : convert ata_piix
>> #10-14    : convert ahci and add softreset
> 
> 
> Hard and soft reset should be implemented as
> 
>     qc = ata_qc_new_init()
>     qc->tf.protocol = ATA_PROT_HARD_RESET;
>     ... ata_qc_issue() ...
> 
> which automatically takes advantage of the ability to specify a behavior 
> using the qc_prep/qc_issue driver hooks.  The ATA passthru CDB supports 
> this method of programming (hard and soft reset are specified 
> protocols), and this is very similar to how the SiI 3124 behaves.  Other 
> FIS-based controllers will implement qc_issue/qc_prep such that they 
> send two Control FIS's.  Taskfile-based controllers use the currently 
> implemented method.

I don't really agree with you.  IMHO, resets are too different from 
standard qc execution (ie. standard ATA command execution) to be a qc 
protocol.  Also, how are we gonna represent host reset with qc?  I like 
the "everything via qc" idea but I think making resets protocols of qc 
is pushing too far.  Please consider...

* If we go with qc_prep/issue, we don't have context while executing 
resets.  Even though SATA is pretty much event-driven, many controllers 
still need quite some amount of polling during resets.  Even though a 
controller can be reset using interrupts/events, the code will be much 
more complex.  I think it's MUCH better to perform resets with context.

* If we perform resets with context, we can use ata_exec_internal() 
while performing resets.  If a controller can perform resets by issuing 
qc's or wants to do some post-reset configuration commands, it can do so 
by simply invoking ata_exec_internal().  If resets become qc protocols, 
we'll need to dance _really_ hard to do anything similar.  Please 
consider the state of IDE driver regarding resets / reconfiguration.

* Resets must be followed by a series of configuration commands.  i.e. 
we need context after performing reset anyway.  So, the context needs to 
be there whether or not resets are performed with contexts.

> On a separate note, E.D.D. support can probably be dropped.  No driver 
> uses it AFAIK.  It was only used during development and in the early 
> libata days.

Cool.

> Further, we should lean towards avoiding hard reset, and issuing SRST 
> most of the time, even for drivers currently doing hard reset (COMRESET).

That's what this patchset does.  It issues COMRESET only if SRST fails. 
  I'll talk about this in another reply.

Thanks for reviewing this.

-- 
tejun

Re: [PATCH 10/14] ahci: separate out ahci_stop/start_engine()

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Separate out ahci_stop/start_engine() from ahci_restart_port().  This
>> is preparation for conversion to new reset mechanism.
> 
> 
> this sort of patch should be sent first, since it is "easy to apply" 
> even though I might disagree with earlier patches in the series.
> 

Will do.  Just trying to place them just before they are needed.

Thanks.

-- 
tejun

Re: [PATCH 11/14] ahci: convert to new reset mechanism

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Convert ahci ->phy_reset to ->hard_reset.  The original ->phy_reset
>> simply called __sata_phy_reset() to perform reset; however, AHCI spec
>> mandates to turn off START bit during reset.  New ->hard_reset method
>> properly follows AHCI reset procedure.
> 
> 
> If you feel AHCI deviates from spec, it sounds like this fix should be 
> separated out into a new patch?
> 

Well, as I was writing a new reset method.  Maybe I can do this in two 
steps - first, fix ->phy_reset, then convert to ->hard_reset.  Hmmm... 
will send rewrite of ->phy_reset separately soon.

Thanks.

-- 
tejun

Re: [PATCH 14/14] ahci: add softreset

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Now that libata is smart enought to handle both soft and hard resets,
>> add softreset method.
> 
> 
> I'm a bit skeptical that polling is what should be done here.  SATA is 
> inherently event-driven...
> 

Well, I'm a bit skeptical the other way around.  :-)

What benefits would making softreset event-driven bring?  It would be 
much more complex and softreset is so not a hot-path.

Also, AHCI does not report events for CLO and START manipulation.  They 
need to be polled.  They can be done by dancing with timers, but....

-- 
tejun

Re: [PATCH 02/14] libata: implement new reset mechanism

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> +int ata_reset(struct ata_port *ap, unsigned int flags, unsigned int 
>> *r_classes)
>> +{
>> +    const unsigned int unknown[2] = { ATA_DEV_UNKNOWN, 
>> ATA_DEV_UNKNOWN };
>> +    unsigned int classes[2];
>> +    int ret = 0;
>> +
>> +    DPRINTK("ENTER\n");
>> +
>> +    if (ap->ops->soft_reset == NULL)
>> +        flags &= ~ATA_RESET_SOFT;
>> +    if (ap->ops->hard_reset == NULL)
>> +        flags &= ~ATA_RESET_HARD;
>> +
>> +    if (!(flags & (ATA_RESET_SOFT | ATA_RESET_HARD))) {
>> +        ret = -EINVAL;
>> +        goto out;
>> +    }
>> +
>> + retry:
>> +    if (flags & ATA_RESET_SOFT) {
>> +        memcpy(classes, unknown, sizeof(classes));
>> +        ret = ap->ops->soft_reset(ap, classes);
>> +        if (ret == 0) {
>> +            if (!(flags & ATA_RESET_CLASSIFY) ||
>> +                classes[0] != ATA_DEV_UNKNOWN)
>> +                goto success;
>> +            /* no signature on SRST, try HRST */
>> +            flags &= ~ATA_RESET_SOFT;
>> +            ret = -ENODEV;    /* no sigature */
>> +        }
>> +        /* SRST failed, try HRST */
>> +    }
>> +
>> +    if (flags & ATA_RESET_HARD) {
>> +        memcpy(classes, unknown, sizeof(classes));
>> +        ret = ap->ops->hard_reset(ap, classes);
>> +        if (ret == 0) {
>> +            if (!(flags & ATA_RESET_CLASSIFY) ||
>> +                classes[0] != ATA_DEV_UNKNOWN)
>> +                goto success;
>> +            /* no signature on HRST, retry SRST */
>> +            flags &= ~ATA_RESET_HARD;
>> +            ret = -ENODEV;    /* no sigature */
>> +            goto retry;
>> +        }
>> +        /* HRST failed, give up */
>> +    }
>> +
>> +    goto out;
> 
> 
> Fundamental problem:  you should not hard code this execution order. 
> Each controller has a different way it likes to handle global and port 
> resets.  Sometimes COMRESET is automatically executed for you, when you 
> do a controller reset (AHCI does this).

Just to clear things up.  ata_reset() handles only port resets. 
->hard_reset would be COMRESET on most SATA controllers and ->soft_reset 
SRST.  Maybe it should be renamed to ata_port_reset().

Above code executes hardreset only if softreset fails.  I thought that 
ordering was pretty generic and hardcoded it.  Even for AHCI, it will 
try SRST and if SRST fails, COMRESET.  I think we can make ata_reset() 
more flexible when need arises.  No?

> Looking at the bigger picture of error handling, we will want to 
> basically run the entire probe sequence, after we reset.  I would look 
> into a few key areas:
> 
> * figuring out how to safely stop SCSI from submitting commands, while 
> you are doing the reset (important!).  SCSI errors are delivered to SATA 
> ports -- thus if you are doing a global reset in the EH path, only one 
> port's command submission is frozen, and the SCSI layer is happily 
> sending commands to the other ports.

Port resets don't need to freeze the whole host_set.  For host resets, I 
think we can achieve synchronization by....

* invoking EH's on all ports
* after all EH's are parked, perform host reset
* release EH's.
* each EH reconfigures devices

> 
> * get libata error handlers to start using scsi_eh_flush_done_q() and 
> scsi_eh_finish_cmd(), which enables retrying of commands.

Have patches for this and other EH stuff in my repository now.  Once 
this reset thing is settled, I'll follow up with those patches.

> 
> * making sure ata_bus_probe() can be called again and again
> 

Probing and EH are similar but not identical.  I think it's better to 
factor things out from ata_bus_probe() - this patchset factors resets, 
configuration can be factored later - and drive those things from 
ata_bus_probe() or recover method properly.

> * implementing srst and hard-reset as taskfile protocols, and supporting 
> them via qc_prep/qc_issue

As I wrote in the other reply, not a big fan of this idea.  :-(

Thanks.

-- 
tejun

Re: [PATCH 14/14] ahci: add softreset

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
> 
>> Tejun Heo wrote:
>>
>>> Now that libata is smart enought to handle both soft and hard resets,
>>> add softreset method.
>>
>>
>>
>> I'm a bit skeptical that polling is what should be done here.  SATA is 
>> inherently event-driven...
>>
> 
> Well, I'm a bit skeptical the other way around.  :-)
> 
> What benefits would making softreset event-driven bring?

The underlying operations are inherently event-driven, as I stated 
above.  You send a packet, wait for the ACK.  Send another packet.  Wait 
for the ACK.

	Jeff

Re: [PATCH 14/14] ahci: add softreset

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Jeff Garzik wrote:
>>
>>> Tejun Heo wrote:
>>>
>>>> Now that libata is smart enought to handle both soft and hard resets,
>>>> add softreset method.
>>>
>>>
>>>
>>>
>>> I'm a bit skeptical that polling is what should be done here.  SATA 
>>> is inherently event-driven...
>>>
>>
>> Well, I'm a bit skeptical the other way around.  :-)
>>
>> What benefits would making softreset event-driven bring?
> 
> 
> The underlying operations are inherently event-driven, as I stated 
> above.  You send a packet, wait for the ACK.  Send another packet.  Wait 
> for the ACK.
> 

Okay.  Hmmm... I don't really see the point you're trying to make here. 
  I'll describe AHCI COMRESET sequence in detail to communicate myself 
better.

The following is mainly from section 10.4.1 of AHCI spec rev 1.1.

1. clear PxCMD.ST and wait for PxCMD.CR to clear to 0.  If PxCMD.CR 
doesn't clear within 500msec, the interface might be hung (fall back to 
hardreset).

	- no interrupt generated on CR clearing, polling required

2. if BSY or DRQ is set, they need to be cleared using PxCMD.CLO if 
supported.  CLO is performed by setting PxCMD.CLO and waiting for the 
controller to clear it.  The spec doesn't say how long it may take.

	- no interrupt generated on CLO clearing, polling required.

3. set PxCMD.ST

4. issue the first H2D Register FIS (SRST on).  This command needs to 
have CH[R] (SYNC escaping) and CH[C] (clear BSY on R_OK) set.  The CH[C] 
is needed because the device won't generate D2H Register FIS after 
receiving SRST.  The controller clears BSY when the device says it 
received the packet okay (R_OK).  This won't generate an interrupt.

	- no interrupt generated on BSY clearing, polling needed.

5. issue the second H2D Register FIS (SRST off).  The device will clear 
BSY when reporting signature with D2H FIS.  I cannot find whether the 
device is supposed to assert INTRQ on completion or not but it seems not.

	- (probably) no interrupt generated on signature reporting.


So, AFAICS, most steps need polling and some are rather long, too.  Am I 
missing something here?

Thanks.  :-)

-- 
tejun

Re: [PATCH 14/14] ahci: add softreset

[Jeff Garzik]

Tejun Heo wrote:
> So, AFAICS, most steps need polling and some are rather long, too.  Am I 
> missing something here?

OK I'm mostly convinced...  let's see how polling works out.  That 
aligns with what VIA AHCI and port multipliers want.

	Jeff

Re: [RFC/PATCHSET] libata: new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Hard and soft reset should be implemented as
>>
>>     qc = ata_qc_new_init()
>>     qc->tf.protocol = ATA_PROT_HARD_RESET;
>>     ... ata_qc_issue() ...
>>
>> which automatically takes advantage of the ability to specify a 
>> behavior using the qc_prep/qc_issue driver hooks.  The ATA passthru 
>> CDB supports this method of programming (hard and soft reset are 
>> specified protocols), and this is very similar to how the SiI 3124 
>> behaves.  Other FIS-based controllers will implement qc_issue/qc_prep 
>> such that they send two Control FIS's.  Taskfile-based controllers use 
>> the currently implemented method.
> 
> 
> I don't really agree with you.  IMHO, resets are too different from 
> standard qc execution (ie. standard ATA command execution) to be a qc 
> protocol.  Also, how are we gonna represent host reset with qc?  I like 
> the "everything via qc" idea but I think making resets protocols of qc 
> is pushing too far.  Please consider...
> 
> * If we go with qc_prep/issue, we don't have context while executing 
> resets.  Even though SATA is pretty much event-driven, many controllers 
> still need quite some amount of polling during resets.  Even though a 
> controller can be reset using interrupts/events, the code will be much 
> more complex.  I think it's MUCH better to perform resets with context.
> 
> * If we perform resets with context, we can use ata_exec_internal() 
> while performing resets.  If a controller can perform resets by issuing 
> qc's or wants to do some post-reset configuration commands, it can do so 
> by simply invoking ata_exec_internal().  If resets become qc protocols, 
> we'll need to dance _really_ hard to do anything similar.  Please 
> consider the state of IDE driver regarding resets / reconfiguration.
> 
> * Resets must be followed by a series of configuration commands.  i.e. 
> we need context after performing reset anyway.  So, the context needs to 
> be there whether or not resets are performed with contexts.

That's fine, we can do without it for now.  I definitely want to see 
that portion of the ATA passthru SCSI command supported.  The spec lists 
soft and hard reset among the protocols.  You could just hard-code a 
call to the libata reset machinery for those two protocols.  Handle it 
in the simulator.

	Jeff