From owner-cvs-all Fri Feb 21 14:23:18 2003 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 02F4637B401; Fri, 21 Feb 2003 14:23:16 -0800 (PST) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.FreeBSD.org (Postfix) with ESMTP id 864FC43FBF; Fri, 21 Feb 2003 14:23:15 -0800 (PST) (envelope-from mux@freebsd.org) Received: by elvis.mu.org (Postfix, from userid 1920) id 4E11EAE163; Fri, 21 Feb 2003 14:23:15 -0800 (PST) Date: Fri, 21 Feb 2003 23:23:15 +0100 From: Maxime Henrion To: Nate Lawson Cc: src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/pci if_xl.c Message-ID: <20030221222315.GR60813@elvis.mu.org> References: <20030220212005.GP60813@elvis.mu.org> <20030220233651.GQ60813@elvis.mu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030220233651.GQ60813@elvis.mu.org> User-Agent: Mutt/1.4i Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Maxime Henrion wrote: > Nate Lawson wrote: > > On Thu, 20 Feb 2003, Maxime Henrion wrote: > > > Nate Lawson wrote: > > > > On Wed, 19 Feb 2003, Maxime Henrion wrote: > > > > > Modified files: > > > > > sys/pci if_xl.c > > > > > Log: > > > > > Fix panic on sparc64 introduced in my last commit. I really > > > > > wish the busdma APIs were more consistent accross architectures. > > > > > > > > > > We should probably move all the other DMA map creations in > > > > > xl_attach() where we can really handle them failing, since > > > > > xl_init() is void and shouldn't fail. > > > > > > > > Please see my patches for adding/fixing error handling for dmamap_load > > > > (posted to -current). I am going to remove the unnecessary intr alloc > > > > move but otherwise they should be fine. These patches look mostly fine (I only looked at the busdma drivers). I saw two minor nits though. You're using "if (error != 0)", I personally prefer "if (error)" (I don't know if style(9) says something for this), but what's more important is that it makes style inconsistent in those drivers where the latter is used everywhere else. Also, there's an unreachable bus_teardown_intr() call in the xl(4) diff. It should be just removed since with your patches, the interrupt is the last thing allocated in the xl_init() function and thus we never need to deallocate it there. Cheers, Maxime To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message