RE: [PATCH] [IB/QIB] Fix failure to load driver if PCI error reporting doesn't enable

2010-10-22 Thread Ralph Campbell
pci_enable_pcie_error_reporting() is optional.

From: linux-rdma-ow...@vger.kernel.org [linux-rdma-ow...@vger.kernel.org] On 
Behalf Of Jason Gunthorpe [jguntho...@obsidianresearch.com]
Sent: Friday, October 22, 2010 2:36 PM
To: Ralph Campbell
Cc: RDMA list
Subject: Re: [PATCH] [IB/QIB] Fix failure to load driver if PCI error   
reporting doesn't enable

On Fri, Oct 22, 2010 at 02:28:54PM -0700, Ralph Campbell wrote:

> I'm not sure this is the right change. I agree there are a lot of
> clean up on error bugs. pci_set_consistent_dma_mask() isn't optional
> because the chip doesn't support 32-bit PCIe addressing.

Do you think pci_enable_pcie_error_reporting is optional? My test
machine doesn't have a chipset with AER so that will always fail (or
something), the driver hasn't exploded yet ... :)

> I'll look it over and try to come up with something better.
>
> I'll submit a patch for the CONFIG_PCI_MSI not set case.

The resource leaks suck too, since you have to reboot to try again.

Jason
--
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

--
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] [IB/QIB] Fix failure to load driver if PCI error reporting doesn't enable

2010-10-22 Thread Jason Gunthorpe
On Fri, Oct 22, 2010 at 02:28:54PM -0700, Ralph Campbell wrote:

> I'm not sure this is the right change. I agree there are a lot of
> clean up on error bugs. pci_set_consistent_dma_mask() isn't optional
> because the chip doesn't support 32-bit PCIe addressing.

Do you think pci_enable_pcie_error_reporting is optional? My test
machine doesn't have a chipset with AER so that will always fail (or
something), the driver hasn't exploded yet ... :)

> I'll look it over and try to come up with something better.
>
> I'll submit a patch for the CONFIG_PCI_MSI not set case.

The resource leaks suck too, since you have to reboot to try again.

Jason
--
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] [IB/QIB] Fix failure to load driver if PCI error reporting doesn't enable

2010-10-22 Thread Ralph Campbell
I'm not sure this is the right change. I agree there are a lot of clean up on 
error
bugs. pci_set_consistent_dma_mask() isn't optional because the chip doesn't
support 32-bit PCIe addressing.
I'll look it over and try to come up with something better.

I'll submit a patch for the CONFIG_PCI_MSI not set case.

From: linux-rdma-ow...@vger.kernel.org [linux-rdma-ow...@vger.kernel.org] On 
Behalf Of Jason Gunthorpe [jguntho...@obsidianresearch.com]
Sent: Friday, October 22, 2010 1:42 PM
To: Ralph Campbell; RDMA list
Subject: [PATCH] [IB/QIB] Fix failure to load driver if PCI error reporting 
doesn't enable

This seems to be the intention of the code, since the jump to bail
is missing. PCI-E advanced error reporting seems optional, but
I wonder if pci_set_consistent_dma_mask is also optional?

This also fixes one case where the PCI region is leaked during
device startup. qib_init_one assumes that qib_pcie_init cleans up if
it fails.

Note: There appear to be several other leaks of the PCI region in
qib_init_one between qib_pcie_init and qib_init that I did not attempt
to fix, and a null pointer de-reference if CONFIG_PCI_MSI is not set
for 6120.

Signed-off-by: Jason Gunthorpe 
---
 drivers/infiniband/hw/qib/qib_pcie.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c 
b/drivers/infiniband/hw/qib/qib_pcie.c
index 7fa6e55..16ce9e7 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -113,6 +113,7 @@ int qib_pcie_init(struct pci_dev *pdev, const struct 
pci_device_id *ent)
qib_early_err(&pdev->dev,
  "Unable to enable pcie error reporting: %d\n",
  ret);
+   ret = 0;
goto done;

 bail:
--
1.6.0.4


--
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

--
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