DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BBE9920827
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org C3ECE603D2
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Maya Erez <merez@codeaurora.org>
Cc:     linux-wireless@vger.kernel.org, wil6210@qti.qualcomm.com
Subject: Re: [PATCH 13/16] wil6210: ignore HALP ICR if already handled
References: <1540975944-30576-1-git-send-email-merez@codeaurora.org>
        <1540975944-30576-14-git-send-email-merez@codeaurora.org>
Date:   Tue, 06 Nov 2018 12:04:32 +0200
In-Reply-To: <1540975944-30576-14-git-send-email-merez@codeaurora.org> (Maya
        Erez's message of "Wed, 31 Oct 2018 10:52:21 +0200")
Message-ID: <875zxabjyn.fsf@purkki.adurom.net>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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