In article <[email protected]>, Kengo NAKAHARA <[email protected]> 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
