From owner-p4-projects@FreeBSD.ORG Sat Jun 2 18:53:39 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 78A3A16A46C; Sat, 2 Jun 2007 18:53:39 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 20E1816A46B for ; Sat, 2 Jun 2007 18:53:39 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from ug-out-1314.google.com (ug-out-1314.google.com [66.249.92.172]) by mx1.freebsd.org (Postfix) with ESMTP id 7524C13C447 for ; Sat, 2 Jun 2007 18:53:38 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by ug-out-1314.google.com with SMTP id u2so506076uge for ; Sat, 02 Jun 2007 11:53:37 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=nTVIjEcJ7J6oyDMVFpY8gAqJSCKl2eAGrpwspFJHrH7u2Wu7aHLETzD789hBt63dQp7P6OG0cwv44xhNx5/ZBrOUIB+J8w1s5h0ixKkPcRNtbzzYlAFH9ZZCmpb2raKwEd4FoL+Iy1UBAV/heUsvds/LemYO1lzEDQS5VufvQqs= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=qmX5buozqsPdfxAJAZmwQESQ5H0cKc1+OX0wyDcrRtLNFCn7dRvp47MK0oJXRDbTd8+cPIhSP6IreKUGYKIwtFwrJ6Rjx4t4X0v591k3w0WUs/cTEoz4gJrgYGzG5IoHA4rHjDUyFOLIAK8VZaFkFDGdfVIqmgJCz47CeA5NnNQ= Received: by 10.67.99.1 with SMTP id b1mr1746040ugm.1180810417357; Sat, 02 Jun 2007 11:53:37 -0700 (PDT) Received: from ?151.75.242.86? ( [151.75.242.86]) by mx.google.com with ESMTP id q55sm424033ugq.2007.06.02.11.53.35; Sat, 02 Jun 2007 11:53:36 -0700 (PDT) Message-ID: <4661BC9E.9070603@FreeBSD.org> Date: Sat, 02 Jun 2007 20:53:18 +0200 From: Attilio Rao User-Agent: Thunderbird 1.5 (X11/20060526) MIME-Version: 1.0 To: Rui Paulo References: <200706021756.l52Huq9A049371@repoman.freebsd.org> In-Reply-To: <200706021756.l52Huq9A049371@repoman.freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: Attilio Rao Cc: Perforce Change Reviews Subject: Re: PERFORCE change 120788 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Jun 2007 18:53:39 -0000 Rui Paulo wrote: > http://perforce.freebsd.org/chv.cgi?CH=120788 > > Change 120788 by rpaulo@rpaulo_epsilon on 2007/06/02 17:55:58 > > Add locking. > > Affected files ... > > .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 edit > .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 edit > > Differences ... > > ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 (text+ko) ==== > > @@ -23,7 +23,7 @@ > * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > * POSSIBILITY OF SUCH DAMAGE. > * > - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#7 $ > + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 $ > * > */ > > @@ -46,7 +46,8 @@ > #include > #include > #include > -#include > +#include > +#include > > #include You should mantain includes sorted by name (where possible). > @@ -239,6 +240,8 @@ > > model = asmc_match(dev); > > + mtx_init(&sc->sc_mtx, "asmc_mtx", NULL, MTX_SPIN); > + > asmc_init(dev); You don't need to write again 'mtx' in the description. Just add something descriptive for the mutexes (since you alredy know it is a spin mutexes). > @@ -740,6 +756,9 @@ > { > uint8_t type; > device_t dev = (device_t) arg; > + struct asmc_softc *sc = device_get_softc(dev); > + > + mtx_lock_spin(&sc->sc_mtx); > > type = inb(ASMC_INTPORT); > > @@ -757,6 +776,8 @@ > device_printf(dev, "%s unknown interrupt\n", __func__); > } > > + mtx_unlock_spin(&sc->sc_mtx); I'm not sure you really need to maintain the lock for so long. Probabilly, you just need for the duration of inb(). > ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 (text+ko) ==== > > @@ -23,7 +23,7 @@ > * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > * POSSIBILITY OF SUCH DAMAGE. > * > - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#3 $ > + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 $ > * > */ > > @@ -49,6 +49,8 @@ > int sc_rid; > struct resource *sc_res; > void *sc_cookie; > + > + struct mtx sc_mtx; > }; The extra white-line is not needed and since mutexes are not so light, it would be better to put it on the top of the structure. Attilio