Date: Mon, 3 Aug 2009 20:33:04 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Bruce Evans <brde@optusnet.com.au> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, alfred@freebsd.org, rwatson@freebsd.org, nparhar@gmail.com, svn-src-head@freebsd.org, "M. Warner Losh" <imp@bsdimp.com> Subject: Re: svn commit: r195960 - in head/sys/dev/usb: . controller input Message-ID: <200908032033.08169.hselasky@c2i.net> In-Reply-To: <20090804032402.J21599@delplex.bde.org> References: <20090802192902.GS47463@elvis.mu.org> <20090803.012206.1492586399.imp@bsdimp.com> <20090804032402.J21599@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_k1ydKb8nTi1cteP Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Monday 03 August 2009 19:46:16 Bruce Evans wrote: > On Mon, 3 Aug 2009, M. Warner Losh wrote: > > In message: <200908030827.21108.hselasky@c2i.net> > > > > : I see two solutions: > > : > > : 1) Disable the timekeeping if no keys are pressed. > > : > > : 2) Second option is to use getmicrotime. Actually what I need is just a > > : millisecond time reference so I know when to repeat the last key. > > : > > : Any opinions? DELAY() or getmicrotime() ? > > DELAY(1) is somewhet usable. I think DELAY(1) is not accurate enough. I suggest that the DELAY(1000) is only active while a key is actually pressed. See attached patch. Please test and report back. --HPS > > getmicrotime() is unusable because apart from the problems from it using > microtime() (actually binuptime()), getmicrotime() has is only updated > every 1/HZ seconds and depends on interrupts for the update. 1/HZ may > usefully be as high as 1/10 seconds, and the update would never occur > in ddb mode. > > microtime() is not a supported API in ddb or panic mode, but it works > for short delays in most cases. Short means however long it takes for > the hardware counter to wrap. See another reply for more details. > > binuptime() is better than microtime() for most purposes. Another > problem with microtime() is that it is not guaranteed to be monotonic. > > The patch using microtime() has mounds of style bugs (mainly an empty > line after almost every comment and statement). > > > I'd note the state at each poll, and if > 1ms has passed since the > > down event, I'd repeat. I wouldn't use DELAY at all to see if you > > needed to repeat: I'd let the clocking of the polling drive you here > > (eg, you know that someone else will call it a lot, so leverage that > > to avoid the delay). > > Determining whether 1 mS has elapsed is not supported in ddb or panic > mode by any API, except microtime() usually works (see above). > > For polled console input (not necessarily in ddb or panic mode). It > shouldn't be necessary for the low-level driver to spin internally, > since cngetc() spins externally. ddb mode is the only mode that > actually works almost right here: ddb disables all interrupts and stops > all other CPUs, so interrupts and other CPUs can't eat the input. > However, state changes to set up for polling, if any, should occur at > a higher level (on entry to ddb for ddb mode), not on every call to > cncheckc() or even on every call to cngetc(). The changes would be > device-specific and wouldn't depend on disabling all interrupts and > stopping all other CPUs. Then all modes would work (of fail) similarly > to ddb mode. > > Bruce --Boundary-00=_k1ydKb8nTi1cteP Content-Type: text/x-patch; charset="iso-8859-1"; name="ukbd.c.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ukbd.c.diff" --- ukbd.c 2009-08-02 16:28:40.000000000 +0200 +++ ukbd.c 2009-08-03 20:28:09.000000000 +0200 @@ -302,17 +302,27 @@ static void ukbd_do_poll(struct ukbd_softc *sc, uint8_t wait) { + uint8_t i; + uint8_t j; + DPRINTFN(2, "polling\n"); while (sc->sc_inputs == 0) { usbd_transfer_poll(sc->sc_xfer, UKBD_N_TRANSFER); - DELAY(1000); /* delay 1 ms */ + /* Delay-optimised support for repetition of keys */ - sc->sc_time_ms++; + for (j = i = 0; i < UKBD_NKEYCODE; i++) + j |= sc->sc_odata.keycode[i]; - /* support repetition of keys: */ + if (j) { + /* a key is pressed - need timekeeping */ + DELAY(1000); + + /* 1 millisecond has passed */ + sc->sc_time_ms += 1; + } ukbd_interrupt(sc); --Boundary-00=_k1ydKb8nTi1cteP--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908032033.08169.hselasky>