On 19/02/15(Thu) 21:49, David Higgs wrote: > On Feb 13, 2015, at 7:29 AM, David Higgs <hig...@gmail.com> wrote: > > On Friday, February 13, 2015, Martin Pieuchot <mpieuc...@nolizard.org> > > wrote: > > On 13/02/15(Fri) 00:28, David Higgs wrote: > > > I guess nobody else has tried calling uhidev_get_report_async() yet. :) > > > > > > First I was getting a NULL pointer deref in the uhidev async callback. > > > Then I realized that due to USBD_NO_COPY, xfer->buffer was always > > > NULL. Next, I tried to use the DMA buffer, but I ended up in DDB in a > > > very cryptic way. I believe this is because the DMA buffer isn't > > > available when the callback is invoked. > > > > > > For the async callback to get a valid dmabuf, it needs to be invoked > > > prior to usb_freemem() in usbd_transfer_complete(). The xfer->status > > > determination would need to move up too. I'd do this myself but I > > > don't understand the logic and ordering of pipe->repeat stuff, and am > > > concerned about unintentionally breaking other devices. > > > > > > This is partially my fault, because I "tested" the original diff that > > > added the USBD_NO_COPY semantics to verify that it didn't break my > > > synchronous code paths, but hadn't yet written anything for upd(4) to > > > check the async ones. > > > > Does the diff below help? > > > > Partially but not enough. I had already figured out that I needed that to > > solve the NULL pointer dereference. See my 2nd paragraph above. > > > OK, I figured out my issue - the crazy DDB backtrace is produced when you > execute a NULL callback. > > It still doesn’t seem legal for the callback to access DMA buffer contents > after they are “freed”. I assume this won’t work in all cases (host > controllers / architectures / cache behaviors), but I don’t experience any > problems in my i386 VM. I tried reordering parts of > usbd_transfer_complete(), but DIAGNOSTIC code became very unhappy with the > results. > > Fortunately, the diff below doesn’t touch that code path and just fixes the > uhidev layer. My async upd(4) changes will be forthcoming in a different > thread.
Committed since nothing uses it at the moment. Thanks! Martin
