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
[-- Attachment #1 --]
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
[-- Attachment #2 --]
--- 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908032033.08169.hselasky>