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

[Tejun Heo <htejun@gmail.com>]

Mark Lord wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> Tejun Heo wrote:
>>>> 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.
>>> ..
>>>
>>> Oh, absolutely.  I was just leaving Jeff's (?) original udelay() in 
>>> there
>>> for now, to avoid another possible failure while testing the new stuff.
>>>
>>> But if we can *guarantee* that .qc_issue and .port_stop are
>>> always invoked only from thread context, then.. no problemo.
>>
>> qc_issue is inside spin_lock_irqsave()
> ..
> 
> Okay, let's leave it alone for now.
> Perhaps later it can be reworked again to find
> a way to busy wait without messing up other stuff.
> 
> Nearly all of the time it exits rapidly anyway.

Aikk... right, it's called from the issue path too.  In that case, 
udelay() is the only way to go.

Thanks.

-- 
tejun