Re: [PATCH 2.6.17-rc1-mm2 1/1] ahci: add support for VIA VT8251

[Tejun Heo <htejun@gmail.com>]

Hello, Bastiaan.

Looks good to me.  Please see below.

Bastiaan Jacques wrote:
> Adds AHCI support for the VIA VT8251.
> 
> Includes a workaround for a hardware bug which requires a Command List
> Override before reset.
> 
> Signed-off-by: Bastiaan Jacques <b.jacques@planet.nl>
> Acked-by: Jeff Garzik <jeff@garzik.org>
> ---
[--snip--]
>  static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
>  {
> +	if ((ap->flags & AHCI_FLAG_RESET_NEEDS_CLO) &&
> +	    (ata_busy_wait(ap, ATA_BUSY, 1000) & ATA_BUSY)) {

I'm kind of uncomfortable with busy sleeping with @cnt at 1000.  That's 
 >> 10ms of busy waiting.  However, this is a minor issue and even if it 
gets changed it probably belongs to another patch.

Hmmm, ata_busy_sleep() would be a bit weird with the 'be patient' 
message and I always thought the function contained way too much busy 
waiting.  Processors are blazing fast and context switch is cheap these 
days.

Maybe we need to update ata_busy_sleep() so that it ignores tmout_pat if 
it's zero.  What do you think, Jeff?

> +		/* ATA_BUSY hasn't cleared, so send a CLO */
> +		ahci_clo(ap);
> +	}
> +

-- 
tejun