Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Mar 2014 17:22:31 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Ryan Stone <rysto32@gmail.com>
Cc:        "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
Subject:   Re: [PATCH] Add MSI support to puc(9)
Message-ID:  <201403111722.31559.jhb@freebsd.org>
In-Reply-To: <CAFMmRNw-pKOczNyNwi5jhJEKW=sYeGe8DB5URGT=B-Z-prOe0w@mail.gmail.com>
References:  <CAFMmRNy9EG3byRV1b=4W5MejMiAz8yV6UnnEMjcJmY-FyMj2sg@mail.gmail.com> <201402271330.02699.jhb@freebsd.org> <CAFMmRNw-pKOczNyNwi5jhJEKW=sYeGe8DB5URGT=B-Z-prOe0w@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403111722.31559.jhb>