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:

quoted

On Wed, Jan 13, 2021 at 03:20:55PM -0800, Doug Anderson wrote:

quoted

Hi,

[ ... ]

quoted

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