USB2 patches

Andrew Thompson thompsa at FreeBSD.org
Sun Feb 1 11:08:27 PST 2009


On Sun, Feb 01, 2009 at 07:22:15PM +0100, Hans Petter Selasky wrote:
> Hi Andrew,
> 
> > > 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.
> >
> > I disagree. It hugely simplifies drivers to know all the resources are
> > allocated after attach, you dont need to check for null pointers
> > throughout the code. 
> 
> I think you misunderstand. The attach code that loads firmware, reads eeprom 
> and performs other configuration must be handed off to another thread, or in 
> your case a taskqueue, so that the USB attach thread can go on and do other 
> things.

The attach routine allocates resources and attaches the hardware. Things
like loading firmware and other configuration are done in the ifnet init
routine.

> > > 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!
> >
> > This isnt correct. Take your example of WLAN drivers, the net80211 layer
> > will use ic_curchan for its logic so the driver needs to program the
> > chip to this value at all times. You can _not_ queue channel changes as
> > these will be out of sync. 
> 
> Being a little off-sync is not the big problem. The big problem is if the 
> channel value is changing during execution of the code. For example, in the 
> beginning of the code the channel value is 5, then suddenly it becomes 6, and 
> the programming just ends up crazy!
> 
> xxx_set_channel()
> {
> 	hw_reg = array1[wlan->curchan];
> 
> 	write USB register;
> 
> 	hw_reg = array2[wlan->curchan];
> 
> 	write USB register;
> }

The code should be,

xxx_set_channel()
{
	ieee80211_channel c = wlan->curchan;

	hw_reg = array1[c];
	write USB register;
	hw_reg = array2[c];
	write USB register;
}


cheers,
Andrew


More information about the freebsd-usb mailing list