miibus + USB = problem

[Pyun YongHyeon]

On Wed, Aug 02, 2006 at 02:37:55PM +0200, Hans Petter Selasky wrote:
 > Hi,
 > 
 > I am currently in the progress of porting "if_aue.c" to my new USB API, and 
 > have hit a problem that needs to be solved. The problem is that the USB 
 > system sleeps under "miibus_xxx". Questions are:
 > 
 > Is this allowed?
 > 
 > What locks are held when these functions are called ?
 > 
 > Reference:
 > 
 >  /* MII interface */
 >  DEVMETHOD(miibus_readreg, aue_miibus_readreg),
 >  DEVMETHOD(miibus_writereg, aue_miibus_writereg),
 >  DEVMETHOD(miibus_statchg, aue_miibus_statchg),
 > 

AFAIK there is no locks held when MII layer calls above methods but
it _is_ called with a driver lock from its ioctl handler.
It seems that aue(4) needs to access its register space whilst
serving above MII methods. This also means it needs a recursive mutex
if it have to serve MII request in the context. So I think the driver
should be prepared with protecting the MII methods with its own lock.
I've not tried but if you can use MTX_DEF mutex instead of MTX_RECURSE
mutex in aue(4) you may be able to sleep on the MII methods. But I
think sleeping is not a good way in the MII methods as it would
confuse MII layers. See below.

Btw, it seems that aue_csr_{read,write} checks sc->aue_dying so it
wouldn't block on these functions. But checking sc->aue_dying without
a lock held is questionable and setting sc->aue_dying before
aue_stop() may result in inconsistent state as all register operations
would be nop if sc->aue_dying == 1.

 > The problem with USB devices, is that the read-register process is very slow. 
 > It can take up to several milliseconds. And if the device is suddenly 
 > detached one has to think about adding exit code everywhere.
 > 
 > The solution I see with USB devices is something like this:
 > 
 > if (sc->device_gone) {
 > 
 >   exit mutexes ;
 > 
 >   kthread_exit(0);
 > } 
 > 
 > Of course I cannot "kthread_exit()" when the call comes from read/write/ioctl, 
 > because there is a stack, that expects a returning call. If the kernel code 
 > was objective C, then maybe one could throw an exception or do something 
 > alike so that the processor gets out of the USB-read procedure.
 > 
 > 
 > Solutions:
 > 
 > 1) use USB hardware polling, not releasing any mutexes, simply using DELAY(), 
 > until read/writes complete.
 > 

I think you can immediately return from register read/write routines
without DELAY() if you know the hardware was gone.

 > 2) pre-read all read registers regularly. How can I do this with "miibus"?
 > 

Because you have a aue_tick() which is called every hz you can cache
several registers regularly. Your MII methods can copy the value
without accessing registers with proper locks. However, this may
confuse MII layers because it needs successive register accesses to
read/write media related settings and that defeats use of cached
contents of the registers.

 > 
 > Anyone have any comments?
 > 
 > --HPS
-- 
Regards,
Pyun YongHyeon