Re: [PATCH #upstream-fixes] sata_promise: request follow-up SRST

[Jeff Garzik <jeff@garzik.org>]

Tejun Heo wrote:
> sata_promise hardreset doesn't seem to be able to acquire the initial
> D2H Reg FIS after hardreset leading to hardreset timeouts.  Request
> follow-up SRST.
> 
>  http://article.gmane.org/gmane.linux.ide/36186
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> Peter, can you please test this one too?  It's essentially the same
> code just slightly prettier.  Mikael, what do you think about this?
> 
> Jeff, please commit only after Peter's verification and Mikael's ACK.
> 
> Thanks.
> 
>  drivers/ata/sata_promise.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
> index ba9a257..917038a 100644
> --- a/drivers/ata/sata_promise.c
> +++ b/drivers/ata/sata_promise.c
> @@ -710,7 +710,12 @@ static int pdc_sata_hardreset(struct ata_link *link, unsigned int *class,
>  			      unsigned long deadline)
>  {
>  	pdc_reset_port(link->ap);
> -	return sata_sff_hardreset(link, class, deadline);
> +
> +	/* sata_promise can't reliably acquire the first D2H Reg FIS
> +	 * after hardreset.  Do non-waiting hardreset and request
> +	 * follow-up SRST.
> +	 */
> +	return sata_std_hardreset(link, class, deadline);

hrm....   at this point we have deviated massively from the standard 
Promise method of hard reset...

* set PMP port
* make hotplug irqs
* reset port# in PDC_GLOBAL_CTL
* pdc_reset_port()
* reset FPDMA -- probably a good idea even if not doing FPDMA
* clear SATA Error register (0xffffffff)
* clear errors-reported-from-link-layer register
* wait standard length of time for SATA connection
* clear hotplug bits
* set PMP port

The PDC_GLOBAL_CTL bitbang is the most notable missing element in the 
hard reset path, though we also miss clearing an apparently-important 
error register as well.  FPDMA reset would be a good idea I think, even 
if not in use, mainly for paranoia's sake and because that's what 
Promise's driver does.

	Jeff