From owner-freebsd-hackers@FreeBSD.ORG  Tue Mar 11 21:22:45 2014
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Delivered-To: freebsd-hackers@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id 3BFBC4D1
 for <freebsd-hackers@freebsd.org>; Tue, 11 Mar 2014 21:22:45 +0000 (UTC)
Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1])
 (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.freebsd.org (Postfix) with ESMTPS id 1370C387
 for <freebsd-hackers@freebsd.org>; Tue, 11 Mar 2014 21:22:45 +0000 (UTC)
Received: from jhbbsd.localnet (unknown [209.249.190.124])
 by bigwig.baldwin.cx (Postfix) with ESMTPSA id E9292B926;
 Tue, 11 Mar 2014 17:22:42 -0400 (EDT)
From: John Baldwin <jhb@freebsd.org>
To: Ryan Stone <rysto32@gmail.com>
Subject: Re: [PATCH] Add MSI support to puc(9)
Date: Tue, 11 Mar 2014 17:22:31 -0400
User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20130906; KDE/4.5.5; amd64; ; )
References: <CAFMmRNy9EG3byRV1b=4W5MejMiAz8yV6UnnEMjcJmY-FyMj2sg@mail.gmail.com>
 <201402271330.02699.jhb@freebsd.org>
 <CAFMmRNw-pKOczNyNwi5jhJEKW=sYeGe8DB5URGT=B-Z-prOe0w@mail.gmail.com>
In-Reply-To: <CAFMmRNw-pKOczNyNwi5jhJEKW=sYeGe8DB5URGT=B-Z-prOe0w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Message-Id: <201403111722.31559.jhb@freebsd.org>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7
 (bigwig.baldwin.cx); Tue, 11 Mar 2014 17:22:43 -0400 (EDT)
Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
X-BeenThere: freebsd-hackers@freebsd.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Technical Discussions relating to FreeBSD
 <freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-hackers>, 
 <mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers/>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
 <mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Mar 2014 21:22:45 -0000

On Tuesday, March 11, 2014 5:12:09 pm Ryan Stone wrote:
> On Thu, Feb 27, 2014 at 1:30 PM, John Baldwin <jhb@freebsd.org> wrote:
> > I would suggest reworking this so that you try MSI for all PCI devices.
> > I would do this by removing the 'sc_irid = 0' from puc_bfe_attach() so
> > that it can be set by callers.  You could then add attach/detach routines in
> > puc_pci.c that use pci_alloc_msi() and set sc_irid to 1 if MSI works.  The
> > sc_irid value would also work as a flag for knowing if detach needs to call
> > pci_release_msi() (or puc_pci_attach() handling failure in puc_bfe_attach())
> > (though I wouldn't be opposed to keeping sc_msi as a separate flag).
> 
> Thanks for the review.  I have reworked the patch as requested.  The
> new version can be found at the same path:
> http://people.freebsd.org/~rstone/patches/puc_msi.diff

This generally looks good, but I don't really like abusing sc_irid as the
count parameter.  I would use a standalone count and only set sc_irid to 1 if
it works:

	count = 1;
	if (pci_alloc_msi(dev, &count) == 0) {
		sc->sc_msi = 1;
		sc->sc_irid = 1;
	}


-- 
John Baldwin