On 2015/12/11 22:08, Bob Copeland wrote:
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.
I don't want to introduce "goto". But I really like your choice to create wcn36xx_restart_dma. I will keep some original patch to avoid "goto" and adopt the function wcn36xx_restart_dma. Will send the patch out. Regards Yin, Fengwei -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
