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

[Jeff Garzik <jgarzik@pobox.com>]

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