Re: [PATCH 2/5] sata_mv clean up mv_stop_edma usage

[Tejun Heo <htejun@gmail.com>]

Hello, Mark.

Mark Lord wrote:
> -    /* now properly wait for the eDMA to stop */
> -    for (i = 1000; i > 0; i--) {
> -        reg = readl(port_mmio + EDMA_CMD_OFS);
> +    /* Wait for the chip to confirm eDMA is off. */
> +    for (i = 10000; i > 0; i--) {
> +        u32 reg = readl(port_mmio + EDMA_CMD_OFS);
>         if (!(reg & EDMA_EN))
> -            break;
> -
> -        udelay(100);
> -    }
> -
> -    if (reg & EDMA_EN) {
> -        ata_port_printk(ap, KERN_ERR, "Unable to stop eDMA\n");
> -        err = -EIO;
> +            return 0;
> +        udelay(10);

Unless the hardware calls for really short polling interval, I think 
it's generally better to limit polling with jiffies and using msleep() 
instead of delays.

Also, mv_stop_edma() skips actual operation if EDMA_EN isn't set, which 
I think is the correct way to do it in hot paths but I think it's better 
to stop the edma engine unconditionally prior to reset as that's where 
we try to bring the controller back into senses.

Thanks.

-- 
tejun