Re: [PATCH] rtlwifi: add MSI interrupts support
From: Larry Finger <hidden>
Date: 2014-03-24 15:14:10
Also in:
linux-wireless
On 03/24/2014 06:34 AM, Adam Lee wrote:
Add MSI interrupts support, enable it when msi_support flag is true, also could fallback to pin-based interrupts mode if MSI mode fails. Signed-off-by: Adam Lee <redacted>
Your first patch turns on MSI support unconditionally. That implies that there are no harmful effects for attempting to use MSI on a box where it is not needed, and/or not implemented. If that is the case, then the msi_support bool should be eliminated, your first patch dropped, and this one revised appropriately. Driver rtl8723be added this bool, but never uses it. Once that driver reaches mainline, a patch can be prepared to remove the variable. I think this patch should be applied to stable. Applying it back to 3.10+ should be far enough. That will pick up the initial submission of the RTL8188EE driver. Boxes that need MSI interrupts are not likely to use any of the older chips. There are some additional in-line comments below.
quoted hunk ↗ jump to hunk
--- drivers/net/wireless/rtlwifi/pci.c | 63 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c index f26f4ff..dd8497a 100644 --- a/drivers/net/wireless/rtlwifi/pci.c +++ b/drivers/net/wireless/rtlwifi/pci.c@@ -1853,6 +1853,63 @@ static bool _rtl_pci_find_adapter(struct pci_dev *pdev, return true; } +static int rtl_pci_intr_mode_msi(struct ieee80211_hw *hw) +{ + struct rtl_priv *rtlpriv = rtl_priv(hw); + struct rtl_pci_priv *pcipriv = rtl_pcipriv(hw); + struct rtl_pci *rtlpci = rtl_pcidev(pcipriv); + int ret;
I like a blank line between the declarations and the code.
+ ret = pci_enable_msi(rtlpci->pdev);
+ if (ret < 0)
+ return ret;
+
+ ret = request_irq(rtlpci->pdev->irq, &_rtl_pci_interrupt,
+ IRQF_SHARED, KBUILD_MODNAME, hw);
+ if (ret < 0) {
+ pci_disable_msi(rtlpci->pdev);
+ return ret;
+ }
+
+ rtlpci->using_msi = true;
+
+ RT_TRACE(rtlpriv, COMP_INIT|COMP_INTR, DBG_DMESG,
+ ("MSI Interrupt Mode!\n"));Doesn't checkpatch.pl complain about the alignment here?
+ return 0;
+}
+
+static int rtl_pci_intr_mode_legacy(struct ieee80211_hw *hw)
+{
+ struct rtl_priv *rtlpriv = rtl_priv(hw);
+ struct rtl_pci_priv *pcipriv = rtl_pcipriv(hw);
+ struct rtl_pci *rtlpci = rtl_pcidev(pcipriv);
+ int ret;
+
+ ret = request_irq(rtlpci->pdev->irq, &_rtl_pci_interrupt,
+ IRQF_SHARED, KBUILD_MODNAME, hw);
+ if (ret < 0)
+ return ret;
+
+ rtlpci->using_msi = false;
+ RT_TRACE(rtlpriv, COMP_INIT|COMP_INTR, DBG_DMESG,
+ ("Pin-based Interrupt Mode!\n"));
+ return 0;
+}
+
+static int rtl_pci_intr_mode_decide(struct ieee80211_hw *hw)
+{
+ struct rtl_pci_priv *pcipriv = rtl_pcipriv(hw);
+ struct rtl_pci *rtlpci = rtl_pcidev(pcipriv);
+ int ret;
+ if (rtlpci->msi_support == true) {As discussed above, this test is not needed. If it were, the current version of checkpatch.pl complains about the "== true" part.
quoted hunk ↗ jump to hunk
+ ret = rtl_pci_intr_mode_msi(hw); + if (ret < 0) + ret = rtl_pci_intr_mode_legacy(hw); + } else { + ret = rtl_pci_intr_mode_legacy(hw); + } + return ret; +} + int rtl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) {@@ -1995,8 +2052,7 @@ int rtl_pci_probe(struct pci_dev *pdev, } rtlpci = rtl_pcidev(pcipriv); - err = request_irq(rtlpci->pdev->irq, &_rtl_pci_interrupt, - IRQF_SHARED, KBUILD_MODNAME, hw); + err = rtl_pci_intr_mode_decide(hw); if (err) { RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG, "%s: failed to register IRQ handler\n",@@ -2064,6 +2120,9 @@ void rtl_pci_disconnect(struct pci_dev *pdev) rtlpci->irq_alloc = 0; } + if (rtlpci->using_msi == true)
Again "if (rtlpci->using_msi)" is preferred.
+ pci_disable_msi(rtlpci->pdev);
+
list_del(&rtlpriv->list);
if (rtlpriv->io.pci_mem_start != 0) {
pci_iounmap(pdev, (void __iomem *)rtlpriv->io.pci_mem_start);When you resubmit, use "[PATCH V2]" in the subject, and use the proper CC stable with [3.10+] following the address. Larry