From owner-freebsd-embedded@FreeBSD.ORG Sun Dec 18 10:29:41 2011 Return-Path: Delivered-To: freebsd-embedded@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8C057106566B; Sun, 18 Dec 2011 10:29:41 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-vw0-f54.google.com (mail-vw0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 2DC598FC1B; Sun, 18 Dec 2011 10:29:41 +0000 (UTC) Received: by vbbfr13 with SMTP id fr13so5929747vbb.13 for ; Sun, 18 Dec 2011 02:29:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=KtAoOXB6Yn9UudWa+Y1ZWjK8Wydd+d2RBtmGV0f+O+M=; b=A48WUEIV/RNHgubAQlhgnJeymxn+1BQEcV1j93++sZmnI0A3J7g4T0/whLwdVFRN3e olRrl6gXhx+p8LNbTQwOPi3aJnF1r2/LYDGLkCOiF2kpMKjUFj2bSRaKlCqR6YTVhXLk Sca88N42Wj5BCmL4AVXO3yt28Ltpi5YjMHlg0= MIME-Version: 1.0 Received: by 10.52.20.165 with SMTP id o5mr9443186vde.79.1324204180622; Sun, 18 Dec 2011 02:29:40 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.52.26.50 with HTTP; Sun, 18 Dec 2011 02:29:40 -0800 (PST) In-Reply-To: <18CABB46-9B9A-41CB-8742-6723C5FF4D67@lassitu.de> References: <0F6CC18F-6973-42A2-AC03-F01BF59458AE@lassitu.de> <1100F70E-9DA9-4163-AC9A-423ECE5AA9A3@lassitu.de> <18CABB46-9B9A-41CB-8742-6723C5FF4D67@lassitu.de> Date: Sun, 18 Dec 2011 02:29:40 -0800 X-Google-Sender-Auth: QFFXWjflRfx4fP9SvggBTV7uig0 Message-ID: From: Adrian Chadd To: Stefan Bethke Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Oleksandr Tymoshenko , "freebsd-embedded@freebsd.org" Subject: Re: Updated switch/glue patch? X-BeenThere: freebsd-embedded@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Dedicated and Embedded Systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Dec 2011 10:29:41 -0000 On 18 December 2011 02:17, Stefan Bethke wrote: >> Erm, surely that's a bit ridiculous.. surely the locking doesn't need >> to be that fine grained _and_ multi-levelled. There has to be a better >> way to do this. :) > > Exactly. > > I've reimplemented iicbb.c to be slightly more protocol compliant, and to= be able to tune transfer speeds to the actual hardware capabilities. =A0I'= ve timed a single GETSCL (with WITNESS) to 8.7 microseconds, clearly that w= on't do. > > I think I'll look at gpio next, as you have, and see if the overhead can = be reduced. I'm just hacking up gpio and gpioiic at the moment. gpiobus already _has_ locks for access to the bus, so as long as gpioiic actually grabs the gpio bus for the duration of the transfer, the individual GPIO operations shouldn't require locks. Same deal with gpioiic and the gpioiic locks being used for each sda/scl operation. The bus should already be acquired. With those locks removed (and I haven't committed that to my git tree, I have no idea what the implications are), things certainly look like they're behaving better. There are still the occasional spike in CPU time: this is a vmstat 0.1: # vmstat 0.1 procs memory page disks faults cp= u r b w avm fre flt re pi po fr sr fl0 md0 in sy cs us sy id 0 0 0 62832k 7536k 212 1 2 0 137 0 0 0 0 182 265 2 21 77 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 640 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 430 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 350 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 410 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 440 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 350 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 410 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 450 0 58 42 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 460 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 400 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 370 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 370 0 8 92 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 400 0 0 100 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 470 8 0 92 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 460 0 8 92 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 460 0 8 92 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 510 0 14 86 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 450 0 61 39 0 0 0 62832k 7536k 0 0 0 0 0 0 0 0 0 1070 500 0 8 92 .. but notice that it uses a lot of CPU in a short burst, rather than actually tying up the CPU for almost a second. I still don't like it (it's only doing a couple hundred register accesses a second over the gpio bus, surely it shouldn't take that much CPU..) but at least now it's behaving correctly. So now - why are the gpioiic and ar71xx_gpio locks even required? If all consumers of the gpio bus code actually use the gpiobus lock/unlock device methods correctly, surely that should be enough? I may actually add a couple of debug device methods to allow a driver to effectively call BLAH_LOCK_ASSERT() and BLAH_UNLOCK_ASSERT(). That way the GPIO driver code can ensure that the gpio bus is actually locked whenever an operation is done. Similarly, the i2c bus code can ensure that the gpio bus it's hanging off is also locked. That should capture any stupid crap from occuring. G'night, and thanks very much for looking into this. :) Adrian