On 30/05/16(Mon) 12:15, Phil Vachon wrote:
> On May 30, 2016 at 9:28:52 AM, Martin Pieuchot wrote:
> > On 26/05/16(Thu) 14:55, Phil Vachon wrote:
> > You should get in touch with Grant Czajkowski, he did something similar
> > during the GSoC 2015, see:
> > 
> > https://www.google-melange.com/gsoc/project/details/google/gsoc2015/coolguy1253/5649050225344512
> > 
> 
> Damn, now I wish I had looked harder before I embarked on all this. I
> will reach out to Grant and see how we could maybe work together to
> get this over the finish line to go into an upcoming release. It's
> very helpful to have! I'm guessing Grant's contact info can be found
> in the mailing list archives, so I'll have a look.

It would be *very* helpful indeed :) 

> > > Currently broken things:
> > > - The entire synchronous interface (poll-related changes)
> > 
> > That's fine as long as libusb is update accordingly. But if it doesn't
> > make sense to keep two set of ioctl(2) then, simply get rid of the old
> > ones ;)
> > 
> 
> Loud and clear. I'll see if we can keep the current interface as
> intact as possible (based on your comment below), but if there are
> 'appendices' that remain, they will be removed.

Well the current interface does not necessarily need to stay.  As long
as the following components are adapted to work with the new ABI,
there's no problem in changing it:

        usr.sbin/usbdevs
        usr.sbin/usbhidaction
        usr.sbin/usbhidctl
        lib/libusbhid

and the port devel/libusb1

I'd actually prefer to have, in the end, fewer ioctl(2), which mean
fewer different code paths and a smaller API to maintain.

> > > - Cancelling pending transfers
> > 
> > That's the hard part of the problem. Grant's work includes a working
> > solution, but modifying the HC driver for that should be avoided.
> > 
> 
> This is one thing I'm not 100% understanding -- my plan (though Grant
> has already gone through this) was to expose the pipe's abort()
> method through a similar usbi_* function. I presumed this should be
> callable safely at splusb(), since this is how a pipe goes about
> cleaning itself up? There was a change to uhci(4) that seems to insert
> a USB task for every transfer that is to be cancelled, is this the HC
> driver change you're referring to?

I'm referring to his diff.  The problem with the pipe's abort() function
is that it abort *all* the pending transfers on a given pipe and wait
until this is done.  I'm not sure that's the semantic asked by libusb.

> I am guessing that my presumption that abort() is safely callable at
> splusb() is wrong?

You don't event need to raise the SPL, usbd_abort_pipe() do it for you.

> The other thing that I was trying to avoid in cancellation was having
> complexities around identifying pending operations. Originally I
> hacked something together using the address of the source/target
> userspace buffer for identifying the operation. But the same buffer
> could be reused for multiple transactions so this does not make
> sense. The other idea I had was to provide a transaction 'handle'
> returned by the USB_ASYNC_SUBMIT ioctl(2) that would then be used to
> later reference the operation. Both approaches have lots of downsides,
> but since libusb is already doing bookkeeping on the programmer's
> behalf, the latter approach might not be too onerous.

Since you won't have a high number of in-flight transfer, iterating over
a small list to compare a cookie is perfectly ok performance wise.  So
definitively the second approach.  Once again, see how Grant solved
that.  I'm not saying that's the only or best approach, but there are
some ideas and working code.

> > > - Isochronous transfers
> > 
> > That's not important in the first place.
> > 
> 
> I didn't think so, but I was thinking of providing a kernel-managed
> queue for isochronous transfers. Then userspace would populate the
> queue with transfers to be submitted after the next isochronous
> transfer completes. Something similar could be done for interrupt
> endpoints too, which would allow for the timing 'requirements' to be
> met more readily for such things. 

I'd be happy to see a diff for that, but I believe it's bit too early
considering that there's nothing for control and bulk transfers in-tree.

> > > - Kqueue (maybe? I haven't tested it thoroughly yet)
> > 
> > Not sure if we should keep support it. If libusb's handle_event doesn't
> > need kqueue(2) support after your modifications then it can go.
> > 
> 
> It _could_ be used I suppose, since the libusb user could register to
> receive notification of all new fds, and use kqueue(2) rather than
> poll(2) to receive notification of completed operations. I was
> planning to support it, based on that.

Well you can discus that with libusb people.  They are really friendly
and I guess will be interested to offload a bit of their OpenBSD support
to somebody else ;)

Reply via email to