Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Daniel Drake <drake@endlessm.com>
To:     yhchuang@realtek.com
Cc:     briannorris@chromium.org, gojun077@gmail.com,
        jano.vesely@gmail.com, kvalo@codeaurora.org,
        linux-wireless@vger.kernel.org, linux@endlessm.com
Subject: Re: [PATCH v2] rtw88: pci: enable MSI interrupt
Date:   Fri, 23 Aug 2019 14:37:28 +0800
Message-Id: <20190823063728.14117-1-drake@endlessm.com>
In-Reply-To: <1565166487-22048-1-git-send-email-yhchuang@realtek.com>
References: <1565166487-22048-1-git-send-email-yhchuang@realtek.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

> +	rtw_pci_disable_interrupt(rtwdev, rtwpci);

I checked the discussion on the v1 patch thread but I still don't follow
this.

You're worried about the case where we're inside the interrupt handler and:
 1. We read the interrupt status to note what needs to be done
 2. <another interrupt arrives here, requiring other work to be done>
 3. We clear the interrupt status bits
 4. We proceed to handle the interrupt but missing any work requested by
    the interrupt in step 2.

Is that right?

I'm not an expert here, but I don't think this is something that drivers
have to worry about. Surely the interrupt controller can be expected to
have a mechanism to "queue up" any interrupt that arrives while an
interrupt is being handled? Otherwise handling of all types of
edge-triggered interrupts (not just MSI) would be overly painful across the
board.

See e.g. https://patchwork.kernel.org/patch/3333681/ as a reference for
what correct interrupt controller behaviour should look like.

> +		ret = pci_enable_msi(pdev);

pci_enable_msi() is "deprecated, don't use"

Thanks
Daniel