> -----Original Message----- > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > > Hi, > > One more comment for now, below: > > On Wed, Jan 30, 2019 at 12:02:10PM +0800, yhchuang@xxxxxxxxxxx wrote: > > From: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx> > > > > hci files for Realtek 802.11ac wireless network chips > > > > For now there is only PCI bus supported by rtwlan, in the future it > > will also have USB/SDIO > > > > Signed-off-by: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx> > > --- > > drivers/net/wireless/realtek/rtw88/hci.h | 211 ++++++ > > drivers/net/wireless/realtek/rtw88/pci.c | 1210 > ++++++++++++++++++++++++++++++ > > drivers/net/wireless/realtek/rtw88/pci.h | 229 ++++++ > > 3 files changed, 1650 insertions(+) > > create mode 100644 drivers/net/wireless/realtek/rtw88/hci.h > > create mode 100644 drivers/net/wireless/realtek/rtw88/pci.c > > create mode 100644 drivers/net/wireless/realtek/rtw88/pci.h > > ... > > > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c > b/drivers/net/wireless/realtek/rtw88/pci.c > > new file mode 100644 > > index 0000000..ef3c9bb > > --- /dev/null > > +++ b/drivers/net/wireless/realtek/rtw88/pci.c > > @@ -0,0 +1,1210 @@ > > > +static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci, > > + u8 hw_queue) > > +{ > > + struct rtw_chip_info *chip = rtwdev->chip; > > + struct rtw_pci_rx_ring *ring; > > + struct rtw_rx_pkt_stat pkt_stat; > > + struct ieee80211_rx_status rx_status; > > + struct sk_buff *skb, *new; > > + u32 cur_wp, cur_rp, tmp; > > + u32 count; > > + u32 pkt_offset; > > + u32 pkt_desc_sz = chip->rx_pkt_desc_sz; > > + u32 buf_desc_sz = chip->rx_buf_desc_sz; > > + u8 *rx_desc; > > + dma_addr_t dma; > > + > > + ring = &rtwpci->rx_rings[RTW_RX_QUEUE_MPDU]; > > + > > + tmp = rtw_read32(rtwdev, RTK_PCI_RXBD_IDX_MPDUQ); > > + cur_wp = tmp >> 16; > > + cur_wp &= 0xfff; > > + if (cur_wp >= ring->r.wp) > > + count = cur_wp - ring->r.wp; > > + else > > + count = ring->r.len - (ring->r.wp - cur_wp); > > + > > + cur_rp = ring->r.rp; > > + while (count--) { > > + rtw_pci_dma_check(rtwdev, ring, cur_rp); > > + skb = ring->buf[cur_rp]; > > + dma = *((dma_addr_t *)skb->cb); > > + pci_unmap_single(rtwpci->pdev, dma, RTK_PCI_RX_BUF_SIZE, > > + PCI_DMA_FROMDEVICE); > > + rx_desc = skb->data; > > + chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status); > > + > > + /* offset from rx_desc to payload */ > > + pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz + > > + pkt_stat.shift; > > + > > + if (pkt_stat.is_c2h) { > > + /* keep rx_desc, halmac needs it */ > > + skb_put(skb, pkt_stat.pkt_len + pkt_offset); > > + > > + /* pass offset for further operation */ > > + *((u32 *)skb->cb) = pkt_offset; > > + skb_queue_tail(&rtwdev->c2h_queue, skb); > > + ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work); > > + } else { > > + /* remove rx_desc, maybe use skb_pull? */ > > + skb_put(skb, pkt_stat.pkt_len); > > + skb_reserve(skb, pkt_offset); > > + > > + /* alloc a smaller skb to mac80211 */ > > + new = dev_alloc_skb(pkt_stat.pkt_len); > > + if (!new) { > > + new = skb; > > + } else { > > + skb_put_data(new, skb->data, skb->len); > > + dev_kfree_skb_any(skb); > > + } > > + /* TODO: merge into rx.c */ > > + rtw_rx_stats(rtwdev, pkt_stat.vif, skb); > > + memcpy(new->cb, &rx_status, sizeof(rx_status)); > > + ieee80211_rx_irqsafe(rtwdev->hw, new); > > + } > > + > > + /* skb delivered to mac80211, alloc a new one in rx ring */ > > + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE); > > + if (WARN(!new, "rx routine starvation\n")) > > + return; > > + > > + ring->buf[cur_rp] = new; > > + rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz); > > You aren't handling failures from this function. It's not quite clear to > me whether that will just leak the SKB, or if it will end in an > unbalanced unmap later. Unfortunately I think it will end up in an unbalanced unmap. But I have no idea now about how to deal with the dma_map failure. Since the rx dma runs ring-based, if the dma_map failed, seems the only way is to retry to dma_map again until it has a mapped dma address. Otherwise the dma engine will dma contents of a packet to an unknown dma address (previous skb in this case, also will corrupt the memory) after it ran over a cycle and hit this desc. Hence I think here we should put an err or WARN here to explicitly know there has something went wrong. But I do not know how to deal with the dma failure :( Need to re-construct them in my brain and consider the rx path. Yan-Hsuan