[0/3] usb: dwc2: Fixes and improvements

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(-)

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