Re: [PATCH 13/16] wil6210: ignore HALP ICR if already handled

[Kalle Valo <kvalo@codeaurora.org>]

Maya Erez <merez@codeaurora.org> writes:

> HALP ICR is set as long as the FW should stay awake.
> To prevent its multiple handling the driver masks this IRQ bit.
> However, if there is a different MISC ICR before the driver clears
> this bit, there is a risk of race condition between HALP mask and
> unmask. This race leads to HALP timeout, in case it is mistakenly
> masked.
> Add an atomic flag to indicate if HALP ICR should be handled.
>
> Signed-off-by: Maya Erez <merez@codeaurora.org>

[...]

> --- a/drivers/net/wireless/ath/wil6210/interrupt.c
> +++ b/drivers/net/wireless/ath/wil6210/interrupt.c
> @@ -575,10 +575,14 @@ static irqreturn_t wil6210_irq_misc(int irq, void *cookie)
>  	}
>  
>  	if (isr & BIT_DMA_EP_MISC_ICR_HALP) {
> -		wil_dbg_irq(wil, "irq_misc: HALP IRQ invoked\n");
> -		wil6210_mask_halp(wil);
>  		isr &= ~BIT_DMA_EP_MISC_ICR_HALP;
> -		complete(&wil->halp.comp);
> +		if (atomic_read(&wil->halp.handle_icr)) {
> +			/* no need to handle HALP ICRs until next vote */
> +			atomic_set(&wil->halp.handle_icr, 0);

atomic_read() followed by atomic_set() is IMHO not really atomic :) I
would assume there's a function to reset the variable really in atomic
way.

-- 
Kalle Valo