From owner-svn-src-head@FreeBSD.ORG Mon Aug 3 18:33:12 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9AE441065674; Mon, 3 Aug 2009 18:33:12 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe04.swip.net [212.247.154.97]) by mx1.freebsd.org (Postfix) with ESMTP id 4851E8FC20; Mon, 3 Aug 2009 18:33:10 +0000 (UTC) (envelope-from hselasky@c2i.net) X-Cloudmark-Score: 0.000000 [] X-Cloudmark-Analysis: v=1.0 c=1 a=GB8WYDQo1twA:10 a=MnI1ikcADjEx7bvsp0jZvQ==:17 a=8kQB0OdkAAAA:8 a=jLZIlSE2p0-g7UWB96EA:9 a=5ocN6vfTY-oPw3kcfPYA:7 a=_xabkXgocQkS6TZn6XAcsfAHLIIA:4 a=9aOQ2cSd83gA:10 a=LDJSNgQqv9YS3UIxoDEA:9 a=vYSQaxOngYuaPbuBOYmfoldUcGEA:4 Received: from [188.126.201.140] (account mc467741@c2i.net HELO laptop.adsl.tele2.no) by mailfe04.swip.net (CommuniGate Pro SMTP 5.2.13) with ESMTPA id 1289462337; Mon, 03 Aug 2009 20:33:09 +0200 From: Hans Petter Selasky To: Bruce Evans Date: Mon, 3 Aug 2009 20:33:04 +0200 User-Agent: KMail/1.11.4 (FreeBSD/8.0-BETA2; KDE/4.2.4; i386; ; ) References: <20090802192902.GS47463@elvis.mu.org> <20090803.012206.1492586399.imp@bsdimp.com> <20090804032402.J21599@delplex.bde.org> In-Reply-To: <20090804032402.J21599@delplex.bde.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_k1ydKb8nTi1cteP" Message-Id: <200908032033.08169.hselasky@c2i.net> 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" Subject: Re: svn commit: r195960 - in head/sys/dev/usb: . controller input X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Aug 2009 18:33:13 -0000 --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--