Re: [PATCH 27/53] IB/qib: Add qib_pcie.c
On Thu, 19 Nov 2009, Roland Dreier wrote:
| > +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...
It's needed on device resets. The core kernel code (at least up through
2.6.27, we'll have to reverify on 2.6.32), doesn't handle that
sufficiently well, with the result that no further interrupts are
delivered.
These are driver-synchronous resets, so I guess we could add core code
to save and restore these, but I'd be surprised if very many (any?) other
drivers would ever use them.
We can also try to develop patches to the core pci code for the actual device
reset (as opposed to helper functions), but that would be potentially
risky, and I don't know that many pcie drivers could or would use that
functionality.
| > +/**
| > + * 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.
Same answer, I guess.
If it's seen as a big enough obstacle, we can drop this from the
upstream driver, and maintain it only in the qlogic and ofed-shipped
drivers, but I'd prefer not to do that, of course.
Dave Olson
dave.ol...@qlogic.com
--
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
Re: [PATCH 27/53] IB/qib: Add qib_pcie.c
> +#ifdef CONFIG_PCIEAER
> +#include
> +#endif
I don't think this ifdef is needed... 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
[PATCH 27/53] IB/qib: Add qib_pcie.c
creates the qib_pcie.c file.
Signed-off-by: Ralph Campbell
---
drivers/infiniband/hw/qib/qib_pcie.c | 850 ++
1 files changed, 850 insertions(+), 0 deletions(-)
create mode 100644 drivers/infiniband/hw/qib/qib_pcie.c
diff --git a/drivers/infiniband/hw/qib/qib_pcie.c
b/drivers/infiniband/hw/qib/qib_pcie.c
new file mode 100644
index 000..e518665
--- /dev/null
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -0,0 +1,850 @@
+/*
+ * Copyright (c) 2008, 2009 QLogic Corporation. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other materials
+ *provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#ifdef CONFIG_PCIEAER
+#include
+#endif
+
+#include "qib.h"
+
+/*
+ * This file contains PCIe utility routines that are common to the
+ * various QLogic InfiniPath adapters
+ */
+
+#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
+
+/*
+ * Code to adjust PCIe capabilities.
+ * To minimize the change footprint, we call it
+ * from qib_pcie_params, which every chip-specific
+ * file calls, even though this violates some
+ * expectations of harmlessness.
+ */
+static int qib_tune_pcie_caps(struct qib_devdata *);
+static int qib_tune_pcie_coalesce(struct qib_devdata *);
+
+/*
+ * Do all the common PCIe setup and initialization.
+ * devdata is not yet allocated, and is not allocated until after this
+ * routine returns success. Therefore qib_dev_err() can't be used for error
+ * printing.
+ */
+int qib_pcie_init(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+ int ret;
+
+ ret = pci_enable_device(pdev);
+ if (ret) {
+ /*
+* This can happen (in theory) iff:
+* We did a chip reset, and then failed to reprogram the
+* BAR, or the chip reset due to an internal error. We then
+* unloaded the driver and reloaded it.
+*
+* Both reset cases set the BAR back to initial state. For
+* the latter case, the AER sticky error bit at offset 0x718
+* should be set, but the Linux kernel doesn't yet know
+* about that, it appears. If the original BAR was retained
+* in the kernel data structures, this may be OK.
+*/
+ qib_early_err(&pdev->dev, "pci enable failed: error %d\n",
+ -ret);
+ goto done;
+ }
+
+ ret = pci_request_regions(pdev, QIB_DRV_NAME);
+ if (ret) {
+ qib_devinfo(pdev, "pci_request_regions fails: err %d\n",
+-ret);
+ goto bail;
+ }
+
+ ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (ret) {
+ /*
+* If the 64 bit setup fails, try 32 bit. Some systems
+* do not setup 64 bit maps on systems with 2GB or less
+* memory installed.
+*/
+ ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (ret) {
+ qib_devinfo(pdev, "Unable to set DMA mask: %d\n", ret);
+ goto bail;
+ }
+ qib_dbg("No 64bit DMA mask, used 32 bit mask\n");
+ ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+ } else
+ ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (ret)
+ qib_early_err(&pdev->dev,
+ "Unable
