| Message ID | 1421257036-5382-3-git-send-email-Larry.Finger@lwfinger.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
Larry Finger <Larry.Finger@lwfinger.net> writes: > From: Troy Tan <troy_tan@realsil.com.cn> > > The hardware and firmware for the RTL8192EE utilize a FIFO list of > descriptors. There were some problems with the initial implementation. > The worst of these failed to detect that the FIFO was becoming full, > which led to the device needing to be power cycled. As this condition > is not relevant to most of the devices supported by rtlwifi, a callback > routine was added to detect this situation. This patch implements the > necessary changes in the pci handler. > > Signed-off-by: Troy Tan <troy_tan@realsil.com.cn> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> [...] > --- a/drivers/net/wireless/rtlwifi/wifi.h > +++ b/drivers/net/wireless/rtlwifi/wifi.h > @@ -2172,6 +2172,7 @@ struct rtl_hal_ops { > void (*add_wowlan_pattern)(struct ieee80211_hw *hw, > struct rtl_wow_pattern *rtl_pattern, > u8 index); > + u16 (*get_available_desc)(struct ieee80211_hw *hw, u8 q_idx); I don't see this op set anywhere within the patch. Is that correct or did I miss something?
Hi Troy, please avoid top-posting. ??? <troy_tan@realsil.com.cn> writes: > You can find get_available_desc here: > > diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/ > pci.c > index e25faac..a62170e 100644 > --- a/drivers/net/wireless/rtlwifi/pci.c > +++ b/drivers/net/wireless/rtlwifi/pci.c > @@ -578,6 +578,13 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int > prio) > else > entry = (u8 *)(&ring->desc[ring->idx]); > > + if (rtlpriv->cfg->ops->get_available_desc && > + rtlpriv->cfg->ops->get_available_desc(hw, prio) <= 1) { > + RT_TRACE(rtlpriv, (COMP_INTR | COMP_SEND), DBG_DMESG, > + "no available desc!\n"); > + return; > + } I don't see rtlpriv->cfg->ops->get_available_desc set here, only being called?
On 01/15/2015 06:00 AM, Kalle Valo wrote: > Hi Troy, > > please avoid top-posting. > > ??? <troy_tan@realsil.com.cn> writes: > >> You can find get_available_desc here: >> >> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/ >> pci.c >> index e25faac..a62170e 100644 >> --- a/drivers/net/wireless/rtlwifi/pci.c >> +++ b/drivers/net/wireless/rtlwifi/pci.c >> @@ -578,6 +578,13 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int >> prio) >> else >> entry = (u8 *)(&ring->desc[ring->idx]); >> >> + if (rtlpriv->cfg->ops->get_available_desc && >> + rtlpriv->cfg->ops->get_available_desc(hw, prio) <= 1) { >> + RT_TRACE(rtlpriv, (COMP_INTR | COMP_SEND), DBG_DMESG, >> + "no available desc!\n"); >> + return; >> + } > > I don't see rtlpriv->cfg->ops->get_available_desc set here, only being > called? Only one of the drivers (rtl8192ee) needs to implement that routine, which is the reason it is checked for non-NULL before it is called. The implementation is in patch 3 in file rtl8192ee/sw.c where it says: @@ -241,6 +239,7 @@ static struct rtl_hal_ops rtl8192ee_hal_ops = { .set_desc = rtl92ee_set_desc, .get_desc = rtl92ee_get_desc, .is_tx_desc_closed = rtl92ee_is_tx_desc_closed, + .get_available_desc = rtl92ee_get_available_desc, .tx_polling = rtl92ee_tx_polling, .enable_hw_sec = rtl92ee_enable_hw_security_config, .init_sw_leds = rtl92ee_init_sw_leds, Larry -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/15/2015 06:00 AM, Kalle Valo wrote: > Hi Troy, > > please avoid top-posting. > > ??? <troy_tan@realsil.com.cn> writes: > >> You can find get_available_desc here: >> >> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/ >> pci.c >> index e25faac..a62170e 100644 >> --- a/drivers/net/wireless/rtlwifi/pci.c >> +++ b/drivers/net/wireless/rtlwifi/pci.c >> @@ -578,6 +578,13 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int >> prio) >> else >> entry = (u8 *)(&ring->desc[ring->idx]); >> >> + if (rtlpriv->cfg->ops->get_available_desc && >> + rtlpriv->cfg->ops->get_available_desc(hw, prio) <= 1) { >> + RT_TRACE(rtlpriv, (COMP_INTR | COMP_SEND), DBG_DMESG, >> + "no available desc!\n"); >> + return; >> + } > > I don't see rtlpriv->cfg->ops->get_available_desc set here, only being > called? Kalle, Troy and I will try to prepare a patch that only fixes the bugs, and we will submit the cleanup for -next. Sorry for the noise, Larry -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Larry Finger <Larry.Finger@lwfinger.net> writes: > Troy and I will try to prepare a patch that only fixes the bugs, and > we will submit the cleanup for -next. That's great, thank you.
diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c index e25faac..a62170e 100644 --- a/drivers/net/wireless/rtlwifi/pci.c +++ b/drivers/net/wireless/rtlwifi/pci.c @@ -578,6 +578,13 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio) else entry = (u8 *)(&ring->desc[ring->idx]); + if (rtlpriv->cfg->ops->get_available_desc && + rtlpriv->cfg->ops->get_available_desc(hw, prio) <= 1) { + RT_TRACE(rtlpriv, (COMP_INTR | COMP_SEND), DBG_DMESG, + "no available desc!\n"); + return; + } + if (!rtlpriv->cfg->ops->is_tx_desc_closed(hw, prio, ring->idx)) return; ring->idx = (ring->idx + 1) % ring->entries; @@ -641,10 +648,9 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio) ieee80211_tx_status_irqsafe(hw, skb); - if ((ring->entries - skb_queue_len(&ring->queue)) - == 2) { + if ((ring->entries - skb_queue_len(&ring->queue)) <= 4) { - RT_TRACE(rtlpriv, COMP_ERR, DBG_LOUD, + RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG, "more desc left, wake skb_queue@%d, ring->idx = %d, skb_queue_len = 0x%x\n", prio, ring->idx, skb_queue_len(&ring->queue)); @@ -793,7 +799,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw) rx_remained_cnt = rtlpriv->cfg->ops->rx_desc_buff_remained_cnt(hw, hw_queue); - if (rx_remained_cnt < 1) + if (rx_remained_cnt == 0) return; } else { /* rx descriptor */ @@ -845,18 +851,18 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw) else skb_reserve(skb, stats.rx_drvinfo_size + stats.rx_bufshift); - } else { RT_TRACE(rtlpriv, COMP_ERR, DBG_WARNING, "skb->end - skb->tail = %d, len is %d\n", skb->end - skb->tail, len); - break; + dev_kfree_skb_any(skb); + goto new_trx_end; } /* handle command packet here */ if (rtlpriv->cfg->ops->rx_command_packet && rtlpriv->cfg->ops->rx_command_packet(hw, stats, skb)) { dev_kfree_skb_any(skb); - goto end; + goto new_trx_end; } /* @@ -906,6 +912,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw) } else { dev_kfree_skb_any(skb); } +new_trx_end: if (rtlpriv->use_new_trx_flow) { rtlpci->rx_ring[hw_queue].next_rx_rp += 1; rtlpci->rx_ring[hw_queue].next_rx_rp %= @@ -921,7 +928,6 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw) rtlpriv->enter_ps = false; schedule_work(&rtlpriv->works.lps_change_work); } -end: skb = new_skb; no_new: if (rtlpriv->use_new_trx_flow) { @@ -1685,6 +1691,15 @@ static int rtl_pci_tx(struct ieee80211_hw *hw, } } + if (rtlpriv->cfg->ops->get_available_desc && + rtlpriv->cfg->ops->get_available_desc(hw, hw_queue) == 0) { + RT_TRACE(rtlpriv, COMP_ERR, DBG_WARNING, + "get_available_desc fail\n"); + spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, + flags); + return skb->len; + } + if (ieee80211_is_data_qos(fc)) { tid = rtl_get_tid(skb); if (sta) { diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h index 3b3453a..413c2ab 100644 --- a/drivers/net/wireless/rtlwifi/wifi.h +++ b/drivers/net/wireless/rtlwifi/wifi.h @@ -2172,6 +2172,7 @@ struct rtl_hal_ops { void (*add_wowlan_pattern)(struct ieee80211_hw *hw, struct rtl_wow_pattern *rtl_pattern, u8 index); + u16 (*get_available_desc)(struct ieee80211_hw *hw, u8 q_idx); }; struct rtl_intf_ops {