Hi Mauro, On Thursday 30 Mar 2017 07:28:00 Mauro Carvalho Chehab wrote: > Em Thu, 30 Mar 2017 12:34:32 +0300 Laurent Pinchart escreveu: > > On Thursday 30 Mar 2017 10:11:31 Oliver Neukum wrote: > >> Am Donnerstag, den 30.03.2017, 01:15 +0300 schrieb Laurent Pinchart: > >>>> + may also override PAD bytes at the end of the > >>>> ``transfer_buffer``, up to the > >>>> + size of the CPU word. > >>> > >>> "May" is quite weak here. If some host controller drivers require > >>> buffers to be aligned, then it's an API requirement, and all buffers > >>> must be aligned. I'm not even sure I would mention that some host > >>> drivers require it, I think we should just state that the API requires > >>> buffers to be aligned. > >> > >> That effectively changes the API. Many network drivers are written with > >> the assumption that any contiguous buffer is valid. In fact you could > >> argue that those drivers are buggy and must use bounce buffers in those > >> cases. > > Blaming the dwc2 driver was my first approach, but such patch got nacked ;) > > Btw, the dwc2 driver has a routine that creates a temporary buffer if the > buffer pointer is not DWORD aligned. My first approach were to add > a logic there to also use the temporary buffer if the buffer size is > not DWORD aligned: > https://patchwork.linuxtv.org/patch/40093/ > > While debugging this issue, I saw *a lot* of network-generated URB > traffic from RPi3 Ethernet port drivers that were using non-aligned > buffers and were subject to the temporary buffer conversion. > > My understanding here is that having a temporary bounce buffer sucks, > as the performance and latency are affected. So, I see the value of > adding this constraint to the API, pushing the task of getting > aligned buffers to the USB drivers, This could however degrade performances when the HCD can handle unaligned buffers. > but you're right: that means a lot of work, as all USB drivers should be > reviewed. If we decide in the end to push the constraint on the USB device driver side, then the dwc2 HCD driver should return an error when the buffer isn't correctly aligned. > Btw, I'm a lot more concerned about USB storage drivers. When I was > discussing about this issue at the #raspberrypi-devel IRC channel, > someone complained that, after switching from the RPi downstream Kernel > to upstream, his USB data storage got corrupted. Well, if the USB > storage drivers also assume that the buffer can be continuous, > that can corrupt data. > > That's why I think that being verbose here is a good idea. > > I'll rework on this patch to put more emphasis about this issue. > > >> So we need to include the full story here. > > > > I personally don't care much about whose side is responsible for handling > > the alignment constraints, but I want it to be documented before "fixing" > > any USB driver. -- Regards, Laurent Pinchart
