> On Sun, Feb 10, 2019 at 11:22:25AM +0100, Lorenzo Bianconi wrote: > > > On Sat, Feb 09, 2019 at 09:29:05PM +0100, Stefan Wahren wrote: > > > > > could you please test the following series: > > > > > https://patchwork.kernel.org/cover/10764453/ > > > > > > > > yeah this fixed the probing timeout and the driver will probe successful. AFAIK the dwc2 host mode doesn't support scatter-gather yet. > > > > > > So this is either dwc2 scatter-gather problem which should be addressed in > > > this driver or mt76x0u does something wrong when configuring SG. > > > > > > Disabling SG is just workaround, which do not address actual problem. > > > > > > I think I found mt76x0u issue that could cause this USB probe error > > > (and possibly also address AMD IOMMU issue). We seems do not correctly > > > set URB transfer length smaller than sg buffer length. Attached patch > > > should correct that. > > > > Hi Stanislaw, > > > > I think 'sg[0].length' is already set in mt76u_fill_rx_sg(). > > It is, buf->len and sg[0].length are initialized to the same value for 1 > segment. But then buf->len (assigned to urb->buffer_transfer_length) change > to smaller value , but sg[0].length stay the same. What I think can be > problem for usb host driver. > > > Moreover applying this patch I got the following crash (rpi-5.0.y): > > Ok, so with patch probe fail instantly and trigger yet another bug(s) > on error path. You seems to address that already. > > > Moreover for mt76x0u SG is 'already' disabled since we use just one > > buffer so from performance point of view I do not see any difference > > of using a standard usb buffer. > > This patch has been tested in multiple scenarios and seems to fix > > reported issues (for usb2.0). > > Ok, so passing buffer via urb->transfer_buffer works. But why urb->sg > does not work for 1 segment ? Here it is a different issue respect to the AMD IOMMU one, dwc2 host driver does not implement SG I/O so probing fails. I guess it is still useful to implement a 'legacy' mode that enable mt76 on host controllers that do not implement SG I/O (rpi is a very common device so it will be cool to have mt76 working on it). Moreover we are not removing functionalities, user experience will remain the same > > > Are you concerned about increasing code complexity? > > That's one of my concerns. Another, more important one, is that > changing to urb->transfer_buffer just hide the problems. And they will > pop up when someone will start to use SG (BTW how this can be tested > for more than one fragment, IOW how multiple fragments skb's can > be generated ? ). You need: - usb 3.0 controller/device - A-MSDU capable AP > > And now I think the bugs can be in mt76 driver taking that problems > happened on different platforms (rpi and AMD IOMMU), i.e. we do not > correctly set urb->nub_seg or length or do some other thing wrong. We still need to fix it on usb3.0 with AMD cpu/motherboard :) Regards, Lorenzo > > Stanislaw
