USB2 patches

[Hans Petter Selasky]

Hi Andrew,

We need to go through your patches together, to work out some issues.

1) A nit.

-	if (usb2_proc_setup(&ssc->sc_config_td, p_mtx, USB_PRI_MED)) {
-		usb2_com_units_free(root_unit, sub_units);
-		return (ENOMEM);
-	}
+
+	error = usb2_proc_create(&ssc->sc_tq, USB_PRI_MED, "ucom");
+	if (error)

< missing "usb2_com_units_free()" >

+		return (error);
+

2) Please explain why you think that "usb2_proc_is_gone()" can be removed.

-	if (usb2_proc_is_gone(&ssc->sc_config_td)) {
-		DPRINTF("proc is gone\n");
-		return;		/* nothing to do */
-	}

If the taskqueue system does not provide a teardown mechanism so that we 
inside the callback can know if the taskqueue is being drained, then the 
taskqueue cannot replace the original "usb2_proc_xxx()" ! Maybe this means we 
need to extend the taskqueue system?

3)

In general avoid more than a few synchronous USB transfer inside 
attach/detach. Always defer! Else there are corner cases where the device can 
hang waiting for the transfer timeout 1-5 seconds for every USB transfer, and 
that is unacceptable.

With regard to the ethernet/wlan drivers, it is Ok to create the "if" instance 
in attach, but not to start programming the chip. This needs to be 
synchronised otherwise. I suggets you make a task called: 
zyd_first_time_configure() and kick this task before allocating ifnet 
instances. Then when everything is ready you queue another task to set the 
LINK flag, so that we don't start transmitting data during early chip 
configuration! In case of failure you just drain the taskqueue.

4)

int onoff = USB_TASK_ARG(task);

This won't work! You need to be able to handle N-phase here, where N goes to 
+infinity! If the DTR pin was toggled we should also see a toggle in the 
programming of the DTR pin, not just re-program the last state! If there is a 
USB_TASK_ARG() it must be constant and that is where I start seeing problems, 
or features missing in the taskqueue system.

It is also very important that you somehow freeze the configuration at the 
moment a command is issued. For example in the WLAN drivers. If the command 
is queued when the channel is 5 then we should program channel 5 and not 
fetch the latest channel value from the softc! IMPORTANT!

I'm not saying USB cannot use the taskqueue, but we have special requirements 
which I cannot see that the taskqueue is a full drop in replacement for!

5) Many of your other changes are good!

--HPS

On Sunday 01 February 2009, Alfred Perlstein wrote:
> I'll defer to Hans if he feels confident or not about this.
>
> For what it's worth, we're about to switch GENERIC to use
> usb4bsd.
>
> -Alfred
>
> * Andrew Thompson <thompsa at FreeBSD.org> [090131 15:20] wrote:
> > Hi,
> >
> >
> > I have several patches in my svn user branch that I would like to see
> > committed to HEAD. Some of these change the usb2 core code so I am
> > interested in feedback or shootdowns. I am right behind the change to
> > USB2 and this is an effort to help.
> >
> > The patch can be found here,
> >  http://people.freebsd.org/~thompsa/usb_head1.diff
> >  73 files changed, 9229 insertions(+), 13261 deletions(-)
> >
> > but its rather large so it may be easier to look at the various changes
> > via the svn web interface.
> >
> > http://svn.freebsd.org/viewvc/base/user/thompsa/usb/
> >
> >
> > r187750
> >   http://svn.freebsd.org/viewvc/base?view=revision&revision=187750
> >
> >   Change over to using taskqueue(9) instead of hand rolled threads and
> >   the config_td system. This removes the config_td code and makes the
> >   API much simpler to use.
> >
> > r187751, r187752, r187753, r187754, r187755, r187756
> >   http://svn.freebsd.org/viewvc/base?view=revision&revision=187751
> >
> >   Change over to usb2_proc w/ taskqueues for the usb2/ethernet,
> >   usb2/serial and usb2/wlan code.
> >
> > r187965
> >   http://svn.freebsd.org/viewvc/base?view=revision&revision=187965
> >
> >   Move most of the ifnet logic into the usb2_ethernet module, this
> > includes, - make all usb ethernet interfaces named ue%d
> >    - handle all threading in usb2_ethernet
> >    - provide default ioctl handler
> >    - handle mbuf rx
> >    - provide locked callbacks for init,start,stop,etc
> >
> >   The drivers are not much more than data pushers now.
> >
> >
> > regards,
> > Andrew