From owner-cvs-all@FreeBSD.ORG Sun Jun 8 04:57:17 2008 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0E6EE1065672; Sun, 8 Jun 2008 04:57:17 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 810FE8FC14; Sun, 8 Jun 2008 04:57:16 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (zion.baldwin.cx [IPv6:2001:470:1f11:75:2a0:d2ff:fe18:8b38]) (authenticated bits=0) by server.baldwin.cx (8.14.2/8.14.2) with ESMTP id m584v9lf031972; Sun, 8 Jun 2008 00:57:10 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Marcel Moolenaar Date: Sun, 8 Jun 2008 00:52:24 -0400 User-Agent: KMail/1.9.7 References: <200806072259.m57Mxmdo083847@repoman.freebsd.org> In-Reply-To: <200806072259.m57Mxmdo083847@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200806080052.25362.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [IPv6:2001:470:1f11:75::1]); Sun, 08 Jun 2008 00:57:10 -0400 (EDT) X-Virus-Scanned: ClamAV 0.91.2/7401/Sat Jun 7 22:49:18 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/conf NOTES files.powerpc src/sys/dev/bm if_bm.c if_bmreg.h if_bmvar.h src/sys/dev/mii lxtphy.c src/sys/modules Makefile src/sys/modules/bm Makefile src/sys/powerpc/conf GENERIC X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Jun 2008 04:57:17 -0000 On Saturday 07 June 2008 06:58:32 pm Marcel Moolenaar wrote: > marcel 2008-06-07 22:58:32 UTC > > FreeBSD src repository > > Modified files: > sys/conf NOTES files.powerpc > sys/dev/mii lxtphy.c > sys/modules Makefile > sys/powerpc/conf GENERIC > Added files: > sys/dev/bm if_bm.c if_bmreg.h if_bmvar.h > sys/modules/bm Makefile > Log: > SVN rev 179645 on 2008-06-07 22:58:32Z by marcel > > Add support for the Apple Big Mac (BMAC) Ethernet controller, > found on various Apple G3 models. > > Submitted by: Nathan Whitehorn A few notes from a quick perusal: You should not need locking in the mii read/write routines as the lock is already held at the higher levels via the ifmedia handlers and the timer routine. Once that is done you can remove MTX_RECURSE from the mutex. New drivers should probably just use bus_foo() (e.g. bus_read_1()) rather than bus_space_foo(). This would mean not having the bus space tag/handle in the softc anymore. ether_ifattach() already sets if_data.ifi_hdrlen to the correct size for ethernet so no need to do that in the attach routine. In detach you should only hold the lock over the call to bm_stop(). Right now it is subjec to deadlocks by holding it over bus_teardown_intr(), for example. The callout_drain() must be moved after the bm_stop(). It is also missing a call to ether_ifdetach(). The first part of detach shound look like: { BM_LOCK(sc); bm_stop(sc); BM_UNLOCK(sc); callout_drain(...) ether_ifdetach(...) bus_teardown_intr(...) Then you can free DMA memory and I/O resources and finally destroy the mutex. The detach routine is also missing a call to if_free() (near the bottom usually, but anytime after the above code sequence is fine). bm_shutdown() is missing locking. bm_stop() should do a callout_stop() of the timer. There isn't a need to check for link status or tx timeouts when the device is marked down. You really don't need to have the EJUSTRETURN magic in the watchdog routine. The callout code doesn't mind if you schedule the callout twice. -- John Baldwin