Re: change MSI/MSI-X APIs

[christos%astron.com@localhost (Christos Zoulas)]

In article <55A72C01.2050401%iij.ad.jp@localhost>,
Kengo NAKAHARA  <k-nakahara%iij.ad.jp@localhost> wrote:
>
>Thank you for your comment. I fix pci_intr_alloc() uses int *counts.
>I also fix missing about man installation and some wording. Here is
>new patch,
>    http://netbsd.org/~knakahara/unify-alloc-api/unify-alloc-api2.patch

+int		pci_intr_alloc(const struct pci_attach_args *pa,
+		    pci_intr_handle_t **, int *, pci_intr_type_t);

Parameters should not have names in declarations. The man wording could
be improved, but it is a complicated explanation and what is there now
is good enough.

>    http://netbsd.org/~knakahara/unify-alloc-api/unify-alloc-api-wm-examp=
>le2.patch
>
>>> Could you comment this patch?

+	/* Allocation settings */
+	counts[PCI_INTR_TYPE_MSIX] = WM_MAX_NINTR;

perhaps:

+	memset(counts, 0, sizeof(counts));

before setting the counts?

+	if (pci_intr_alloc(pa, &sc->sc_intrs, counts, PCI_INTR_TYPE_MSIX) != 0) {

That should probably be:
+	if (pci_intr_alloc(pa, &sc->sc_intrs, counts, __arraycount(counts)) != 0) {
or:
+	if (pci_intr_alloc(pa, &sc->sc_intrs, counts, PCI_INTR_TYPE_SIZE) != 0) {

I would keep:
-	if (pci_intr_type(sc->sc_intrs[0]) == PCI_INTR_TYPE_MSIX) {
+	intr_type = pci_intr_type(sc->sc_intrs[0]);
+	if (intr_type == PCI_INTR_TYPE_MSIX) {

So I wouldn't have to re-evaluate it.

LGTM, ship it.

christos