| Message ID | 20210113112052.17063-1-nsaenzjulienne@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: dwc2: Fixes and improvements | expand |
Hi, On Wed, Jan 13, 2021 at 3:21 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > I'm picking up this series by Guenter Roeck as he stated he has no time > for it ATM. It was found to solve some unaligned DMA access issues on > Raspberry Pi 3. You can find the original discussion here: > https://lore.kernel.org/linux-usb/20200226210414.28133-1-linux@roeck-us.net/ > > I removed the fist patch from the original series as it turned out to be > contententious and needs more in-depth testing. Following is the edited > origin series description. Note that extra testing was performed on > RPi3: > > "This series addresses the following problems: > > - Fix receive transfers with 0 byte transfer length > - Abort transactions after unknown receive errors > if the receive buffer is full > - Reduce "trimming xfer length" logging noise > > The problems fixed with this series were observed when connecting > a DM9600 Ethernet adapter to Veyron Chromebooks such as the ASUS > Chromebook C201PA. The series was tested extensively with this and > other adapters. > > The observed problems are also reported when tethering various > phones, so test coverage with such phones would be very appreciated." > > --- > > Guenter Roeck (3): > usb: dwc2: Do not update data length if it is 0 on inbound transfers > usb: dwc2: Abort transaction after errors with unknown reason > usb: dwc2: Make "trimming xfer length" a debug message > > drivers/usb/dwc2/hcd.c | 15 ++++++++------- > drivers/usb/dwc2/hcd_intr.c | 14 +++++++++++++- > 2 files changed, 21 insertions(+), 8 deletions(-) It's been long enough ago that I've forgotten where this was left off, but IIRC the 3 patches that you have here are all fine to land (and have my Reviewed-by tag). However, I think Guenter was still tracking down additional problems. Guenter: does that match your recollection? It looks like there are still bugs open for this on our public bug tracker: https://issuetracker.google.com/issues/172208170 https://issuetracker.google.com/issues/172216241 ...but, as Guenter said, I don't think there's anyone actively working on them. I'm not really doing too much with dwc2 these days either and don't currently have good HW setup for testing, so for the most part I'll leave it to you. I wanted to at least summarize what I remembered, though! :-) -Doug
On Wed, Jan 13, 2021 at 03:20:55PM -0800, Doug Anderson wrote: > Hi, > [ ... ] > > It's been long enough ago that I've forgotten where this was left off, > but IIRC the 3 patches that you have here are all fine to land (and > have my Reviewed-by tag). However, I think Guenter was still tracking > down additional problems. Guenter: does that match your recollection? > > It looks like there are still bugs open for this on our public bug tracker: > > https://issuetracker.google.com/issues/172208170 > https://issuetracker.google.com/issues/172216241 > > ...but, as Guenter said, I don't think there's anyone actively working on them. > > I'm not really doing too much with dwc2 these days either and don't > currently have good HW setup for testing, so for the most part I'll > leave it to you. I wanted to at least summarize what I remembered, > though! :-) > The patches in this series still match what I had in my latest test code, so it makes sense to move forward with them. I don't think I ever found an acceptable version of the DMA alignment code. Guenter
Hi Guenter, Doug, thanks for having a look at this. On Wed, 2021-01-13 at 19:07 -0800, Guenter Roeck wrote: > On Wed, Jan 13, 2021 at 03:20:55PM -0800, Doug Anderson wrote: > > Hi, > > > [ ... ] > > > > It's been long enough ago that I've forgotten where this was left off, > > but IIRC the 3 patches that you have here are all fine to land (and > > have my Reviewed-by tag). However, I think Guenter was still tracking > > down additional problems. Guenter: does that match your recollection? > > > > It looks like there are still bugs open for this on our public bug tracker: > > > > https://issuetracker.google.com/issues/172208170 > > https://issuetracker.google.com/issues/172216241 > > > > ...but, as Guenter said, I don't think there's anyone actively working on them. > > > > I'm not really doing too much with dwc2 these days either and don't > > currently have good HW setup for testing, so for the most part I'll > > leave it to you. I wanted to at least summarize what I remembered, > > though! :-) > > > > The patches in this series still match what I had in my latest test code, > so it makes sense to move forward with them. I don't think I ever found > an acceptable version of the DMA alignment code. As for the alignment code rework, can you recall the underlying issue that warranted it? Regards, Nicolas
On Thu, Jan 14, 2021 at 10:26:25AM +0100, Nicolas Saenz Julienne wrote: > Hi Guenter, Doug, thanks for having a look at this. > > On Wed, 2021-01-13 at 19:07 -0800, Guenter Roeck wrote: > > On Wed, Jan 13, 2021 at 03:20:55PM -0800, Doug Anderson wrote: > > > Hi, > > > > > [ ... ] > > > > > > It's been long enough ago that I've forgotten where this was left off, > > > but IIRC the 3 patches that you have here are all fine to land (and > > > have my Reviewed-by tag). However, I think Guenter was still tracking > > > down additional problems. Guenter: does that match your recollection? > > > > > > It looks like there are still bugs open for this on our public bug tracker: > > > > > > https://issuetracker.google.com/issues/172208170 > > > https://issuetracker.google.com/issues/172216241 > > > > > > ...but, as Guenter said, I don't think there's anyone actively working on them. > > > > > > I'm not really doing too much with dwc2 these days either and don't > > > currently have good HW setup for testing, so for the most part I'll > > > leave it to you. I wanted to at least summarize what I remembered, > > > though! :-) > > > > > > > The patches in this series still match what I had in my latest test code, > > so it makes sense to move forward with them. I don't think I ever found > > an acceptable version of the DMA alignment code. > > As for the alignment code rework, can you recall the underlying issue that > warranted it? > See https://patchwork.kernel.org/project/linux-usb/patch/20200226210414.28133-2-linux@roeck-us.net/ for details. It isn't up to date - it says that buffer alignment to DWC2_USB_DMA_ALIGN would be acceptable. However, it turned out in testing that buffers do have to be aligned to dma_get_cache_alignment(), at least on some mips systems. My latest work-in-progress patch describes the changes made as: To simplify the code, move the old data pointer back to the beginning of the new buffer, restoring most of the original commit. Increase buffer alignment to dma_get_cache_alignment(). Ensure that the data pointer is DMA aligned by using ____cacheline_aligned instead of realigning it after allocation. Ensure that the allocated buffer is a multiple of wMaxPacketSize to guarantee that the chip does not write beyond the end of the buffer. I can provide that version of the patch in case someone wants to pick it up, but it would need thorough testing on a variety of systems before it is applied. Guenter