Hi > On Behalf Of Tony Chuang > Subject: RE: [PATCH v4 03/13] rtw88: hci files > > > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > > > > Hi, > > > > A few scattered comments: > > > > > +static void rtw_pci_dma_check(struct rtw_dev *rtwdev, > > > + struct rtw_pci_rx_ring *rx_ring, > > > + u32 idx) > > > +{ > > > + struct rtw_chip_info *chip = rtwdev->chip; > > > + struct rtw_pci_rx_buffer_desc *buf_desc; > > > + u32 desc_sz = chip->rx_buf_desc_sz; > > > + u16 total_pkt_size; > > > + int i; > > > + > > > + buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head + > > > + idx * desc_sz); > > > + for (i = 0; i < 20; i++) { > > > + total_pkt_size = le16_to_cpu(buf_desc->total_pkt_size); > > > + if (total_pkt_size) > > > + return; > > > + } > > > > > > Umm, what are you trying to do here? This is a non-sensical loop. I > > *imagine* you're trying to do some kind of timeout loop here, but since > > there's nothing telling the compiler that this is anything but normal > > memory, this loop gets flattened by the compiler into a single check of > > ->total_pkt_size (I checked; my compiler gets rid of the loop). > > > > So, at a minimum, you should just remove the loop. But I'm not sure if > > this "check" function has any value at all... > > > > > + > > > + if (i >= 20) > > > + rtw_warn(rtwdev, "pci bus timeout, drop packet\n"); > > > > ...BTW, I'm seeing this get triggered quite a bit. > > It's quite strange though... triggered on my laptop as well. > I am looking for the reason that cause it. I found that 8822BE does not > trigger this. And it's like even if I added a volatile before it to hint the > compiler not to optimize, it still happens. Now trying to figure out why > the bus continues to timeout. > > After this problem is resolved I'll either remove the loop or add proper > hint for the compiler to check the value. But it seems to take many days > to debug, so I think for this patch set I can just remain it here. > > > > > Do you have some kind of memory mapping/ordering issue or something? I > > wouldn't think you should expect to just drop packets on the floor so > > often like this. > > > > Looks like the cause is that the DMA might not have done when the interrupt > arrived. Need to do more test to figure it out. > I tested and checked again. I found that I didn't enable DMA sync for the TRX path. After some work it can be resolved if I turn DMA sync on. So I will include this in the PATCH v5. > > > Yan-Hsuan > Yan-Hsuan
