On 19/02/20(Wed) 14:13, Vitaliy Makkoveev wrote:
> On Wed, Jan 17, 2018 at 11:40:24AM +0100, Martin Pieuchot wrote:
> > Hello Sebastien,
> > 
> > On 17/01/18(Wed) 10:19, Sebastien Marie wrote:
> > > [...] 
> > > kernel modification is desirable in some cases, at least for disabling
> > > ulpt(4) when using cups with USB printer.
> > 
> > Sorry to hijack your thread, but if somebody wants to fix this ulpt(4)
> > problem permanently here's the plan:
> > 
> >  - Add the USBD_EXCLUSIVE_USE to usbd_open_pipe() in ulptopen().
> >    Actually this flag should be the default everywhere.  This should
> >    prevent open(2) on /dev/ulpt? to work if a userland driver is using
> >    your printer.
> > 
> >  - Do some plumbing between libusb/ugen(4)/usb(4) to make it possible
> >    to submit bulk transfer via /dev/usb?.  The logic in ugenopen()
> >    should also have the USBD_EXCLUSIVE_USE flag such that it will fail
> >    if the corresponding /dev/ultp? has already been opened.
> > 
> > That should be enough to have CUPS work with GENERIC kernels without
> > having to disable anything.  I'm here to help/review diffs but since
> > I don't have a printer myself, I can't do the work.
> >
> 
> If driver exists for USB device, generic driver for this device will
> never be attached. So, if ulpt(4) is enabled and attached, corresponding
> ugen(4) is missing and CUPS/libusb can't work with this printer. I
> suppose, coexisting with ugen(4) should be allowed.

Attaching two drivers to the same piece of hardware might confuse the
stack and the hardware below it.  I don't see anything in your diff
taking care of the ownership of, at least, interfaces.

I'd also be afraid that if userland tries to change the configuration of
the device via the ugen(4) interface that might crash the kernel.  Current
drivers (or the stack) is written for that.

> Index: sys/dev/usb/ugen.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 ugen.c
> --- sys/dev/usb/ugen.c        4 Jan 2020 11:37:33 -0000       1.101
> +++ sys/dev/usb/ugen.c        19 Feb 2020 11:07:01 -0000
> @@ -222,7 +222,8 @@ ugen_set_config(struct ugen_softc *sc, i
>       memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
>       for (ifaceno = 0; ifaceno < cdesc->bNumInterface; ifaceno++) {
>               DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
> -             if (usbd_iface_claimed(sc->sc_udev, ifaceno)) {
> +             if (!sc->sc_secondary &&
> +                 usbd_iface_claimed(sc->sc_udev, ifaceno)) {
>                       DPRINTF(("%s: iface %d not available\n", __func__,
>                           ifaceno));
>                       continue;

This allows two pieces of code trying to use this interface at the same
time, what can go wrong?

Reply via email to