On Fri, Dec 11, 2015 at 09:44:54PM +0800, fengwei.yin wrote:
> >            /* skip this frame if we can't alloc a new rx buffer */
> >            if (ret)
> >                   goto drop;
> This can't work because we need to initialize the DMA for the old skb again.
> Which is done in following
>       switch (ch->ch_type) {
> block.

Hmm, good point.  You could still move that out to a function like this:

diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c 
b/drivers/net/wireless/ath/wcn36xx/dxe.c
index f8dfa05..fd447bf 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -467,6 +467,27 @@ out_err:
 
 }
 
+/* or whatever name makes sense... */
+static void wcn36xx_restart_dma(struct wcn36xx *wcn,
+                               struct wcn36xx_dxe_ch *ch,
+                               struct wcn36xx_dxe_desc *dxe)
+{
+       switch (ch->ch_type) {
+       case WCN36XX_DXE_CH_RX_L:
+               dxe->ctrl = WCN36XX_DXE_CTRL_RX_L;
+               wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_ENCH_ADDR,
+                                          WCN36XX_DXE_INT_CH1_MASK);
+               break;
+       case WCN36XX_DXE_CH_RX_H:
+               dxe->ctrl = WCN36XX_DXE_CTRL_RX_H;
+               wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_ENCH_ADDR,
+                                          WCN36XX_DXE_INT_CH3_MASK);
+               break;
+       default:
+               wcn36xx_warn("Unknown channel\n");
+       }
+}
+
 static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,
                                     struct wcn36xx_dxe_ch *ch)
 {
@@ -478,26 +499,18 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,
        while (!(dxe->ctrl & WCN36XX_DXE_CTRL_VALID_MASK)) {
                skb = ctl->skb;
                dma_addr = dxe->dst_addr_l;
-               wcn36xx_dxe_fill_skb(wcn->dev, ctl);
+               ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl);
 
-               switch (ch->ch_type) {
-               case WCN36XX_DXE_CH_RX_L:
-                       dxe->ctrl = WCN36XX_DXE_CTRL_RX_L;
-                       wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_ENCH_ADDR,
-                                                  WCN36XX_DXE_INT_CH1_MASK);
-                       break;
-               case WCN36XX_DXE_CH_RX_H:
-                       dxe->ctrl = WCN36XX_DXE_CTRL_RX_H;
-                       wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_ENCH_ADDR,
-                                                  WCN36XX_DXE_INT_CH3_MASK);
-                       break;
-               default:
-                       wcn36xx_warn("Unknown channel\n");
-               }
+               /* skip this frame in OOM condition */
+               if (ret)
+                       goto drop;
 
                dma_unmap_single(wcn->dev, dma_addr, WCN36XX_PKT_SIZE,
                                 DMA_FROM_DEVICE);
                wcn36xx_rx_skb(wcn, skb);
+
+drop:
+               wcn36xx_restart_dma(wcn, ch, dxe);
                ctl = ctl->next;
                dxe = ctl->desc;
        }



...that said, not really sure it's worth it now that the 'goto' is only
skipping two lines.  So, I would be ok with the original patch too.

-- 
Bob Copeland %% http://bobcopeland.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to