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