Re: [PATCH 05/10] libata: implement new EH action ATA_EH_SPINUP

[Jeff Garzik <jgarzik@pobox.com>]

Tejun Heo wrote:
> Implement new EH action ATA_EH_SPINUP.  This will be used by new PM
> implementation.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

This patch further reinforces something I've been thinking about, as I 
watch the EH grow:  these EH actions really should be separated into 
small, discrete operations.  Then the EH "driver" will push discrete 
operations onto an execution stack.

It's always getting tough to follow ata_eh_recover() and ata_eh_reset() 
's code through multiple iterations of recovery.  This patch 
(ata_eh_spinup) also illustrates how difficult it would be if the 
ordering of these operations ever needed to change.

So, I'm not NAK'ing the patch, just throwing out a 'caution' sign.  :)

I've often thought something along the lines of a list of tasks 
("eh_op_list"), for when a port is frozen and doing EH:
	ata_eh_freeze()
	ata_eh_push(ap, EH_OP_RESET_BUS, ...)
	ata_eh_push(ap, EH_OP_SPINUP, ...)
	ata_eh_push(ap, EH_OP_CONFIGURE, ...)
	ata_eh_push(ap, EH_OP_SET_MODE, ...)
	ata_eh_run()
	ata_eh_thaw()

would be nice.  A discrete operation would handle its own discrete 
operation, prepend and append other operations to eh_op_list, or 
optionally cause the engine to fail, and return immediately.

	Jeff