Re: [PATCH 2/4] ath9k: fix stopping tx dma on reset

[Felix Fietkau <nbd@openwrt.org>]

On 2011-03-10 5:21 AM, Mark Mentovai wrote:
>> --- a/drivers/net/wireless/ath/ath9k/mac.c
> [...]
>> +bool ath9k_hw_abort_tx_dma(struct ath_hw *ah)
> [...]
>> +	for (q = 0; q < AR_NUM_QCU; q++) {
>> +		for (i = 1000; i > 0; i--) {
>> +			if (!ath9k_hw_numtxpending(ah, q))
>> +				break;
>> +
>> +			udelay(5);
>> +		}
>> +	}
>> +	if (!i)
>> +		return false;
> 
> Awesome-looking patch set.
> 
> What’s the return value of this function supposed to mean? As
> written, it only returns false there’s anything left pending on the
> final queue. Notably, if the inner loop ran for all 999 iterations
> and ath9k_hw_numtxpending never returned false, the code above would
> effectively ignore that condition.
Makes sense

> I’m less concerned about the return value (because it actually seems
> that the function could be declared void, you don’t use the return
> value anywhere) than I am about the REG_CLR_BIT and REG_WRITE
> operations that follow. It seems that you might be either missing
> those in cases where they should happen, or performing them in cases
> where they shouldn’t. Did you mean to leave AR_Q_TXD set when
> something is left pending on the final hardware queue?
I'll drop the return code and make the clearing of AR_Q_TXD and the
other bits unconditional.

- Felix