mt76x0u: Probing issues on Raspberry Pi 3 B+

BUG Stanislaw Gruszka 1 participant 1 message Feb 20, 2019 USB

Share & tools

Stanislaw Gruszka

author

2019-02-20 16:14 UTC #0 lore raw

Quoted 32 lines · Lorenzo Bianconi

Quoted 32 lines · Felix Fietkau
Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
The way I see it, we have two choices.
1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
2. Rely on urb->transfer_buffer and keep urb->sg NULL
I agree, if this is only needed for dwc2. Though I would investigate if this is not a bug on other platforms as well.
From what I can see, using Lorenzo's patches seems to be the better
Quoted 8 lines
Quoted 3 lines
solution, since they avoid these corner cases in dwc2 (and maybe other drivers as well). I will apply them and then we'll see if we need to do any further improvements later on.
They work on rpi dwc2, but they do not address root of the problem. There is clearly something wrong how mt76usb handle SG, what is not fixed. And adding disable_usb_sg module parameter for hcd's supporting SG should be red flag.
Quoted 5 lines
I think we're simply dealing with multiple issues here, only some of which are fixed by Lorenzo's patches.
I'm pretty sure it's still wrong for mt76 to try to align its buffers, since the Linux USB API supports non-aligned transfer buffers and it should be up to the controller driver to deal with that.
Agree.
Quoted 2 lines
dwc2 tries to do that, but that has limitations which I already pointed out and which are properly dealt with by Lorenzo's patches.
I planed to just accept current solution, but I started to work on patch that remove len, sglen arguments from mt76u_buf_alloc() and use q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how related code is now tangled.

Quoted 7 lines

Quoted 3 lines
Would be ok to send this patch with proper changelog as fix for RPI against wireless-drivers and cc:stable (assuming it works and really fix things on RPI) and revert Lorenzo's patches in -next ?
Hi Stanislaw,
what is the advantage of doing so?

To have small fix proper for -stable to fix the problem in 4.20 and 4.19, and have simpler code.

Quoted 2 lines

You have duplicated most of the fields that are
already in the urb data structure and you use transfer_buffer (no SG I/O).

URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern this can be optimized by packing num_sgs, len, done into fields variable.

Quoted 3 lines

Moreover I have ready a series that removes 99% of the dual allocation code and maintain it in the control path (instead of the datapath one).
I need just to rebase it ontop of your series. I will post it soon.

So I would ask what is the point to adding bunch of code and remove it in very next patch?

Stanislaw