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

[Guenter Roeck <linux@roeck-us.net>]

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