> +#ifdef CONFIG_PCIEAER
> +#include <linux/aer.h>
> +#endif
I don't think this ifdef is needed... <linux/aer.h> looks fine to
include even if PCIEAER isn't set.
> +#ifndef PCI_MSIX_FLAGS
> +#define PCI_MSIX_FLAGS 2
> +#endif
> +#ifndef PCI_MSIX_FLAGS_ENABLE
> +#define PCI_MSIX_FLAGS_ENABLE (1 << 15)
> +#endif
> +#ifndef PCI_MSIX_FLAGS_QSIZE
> +#define PCI_MSIX_FLAGS_QSIZE 0x7FF
> +#endif
None of this is needed ... all these constants are defined, right?
> +#ifdef CONFIG_PCIEAER
> + /* enable basic AER reporting. Perhaps more later */
> + if (pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR)) {
> + ret = pci_enable_pcie_error_reporting(pdev);
> + if (ret)
> + qib_early_err(&pdev->dev,
> + "Unable to enable pcie error reporting"
> + ": %d\n", ret);
> + } else
> + qib_dbg("AER capability not found! AER reports not enabled\n");
> +#endif
ifdef here isn't needed either, I don't think. And neither is the test
for the capability... just call pci_enable_pcie_error_reporting always
and it will return an error if the config option isn't set or the cap
isn't found.
> +#ifdef CONFIG_PCI_MSI
again, ifdef not needed, right?
> +static void qib_msix_setup(struct qib_devdata *dd, int pos, u32 *msixcnt,
> + struct msix_entry *msix_entry)
> +{
> + int ret;
> + u32 tabsize = 0;
> + u16 msix_flags;
> +
> + pci_read_config_word(dd->pcidev, pos + PCI_MSIX_FLAGS, &msix_flags);
> + tabsize = 1 + (msix_flags & PCI_MSIX_FLAGS_QSIZE);
> + if (tabsize > *msixcnt)
> + tabsize = *msixcnt;
> + ret = pci_enable_msix(dd->pcidev, msix_entry, tabsize);
all the monkeying with config space seems dubious... fallback should
handle the table size, and I don't think drivers should be peeking and
poking in config space anyway. Fix the PCI core if some generic
function is missing...
> +/**
> + * We save the msi lo and hi values, so we can restore them after
> + * chip reset (the kernel PCI infrastructure doesn't yet handle that
> + * correctly.
> + */
again... if the core doesn't do something right, fix it rather than
hacking around it in a driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
