From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Smart Subject: Re: Warning: qla4xxx/qla2xxx/lpfc : removal of PCI_CAP_ID_EXP Date: Wed, 6 Jul 2011 10:38:10 -0400 Message-ID: <4E147352.3070407@emulex.com> References: <1309196346-15749-1-git-send-email-jdmason@kudzu.us> <4E0CDF8B.3020505@emulex.com> <4E0DDE55.30006@emulex.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from exht1.emulex.com ([138.239.113.183]:33067 "EHLO exht1.ad.emulex.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752585Ab1GFOic (ORCPT ); Wed, 6 Jul 2011 10:38:32 -0400 In-Reply-To: <4E0DDE55.30006@emulex.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: "linux-scsi@vger.kernel.org" Cc: Jon Mason , Richard Lary , Ravi Anand , "vikas.chaudhary@qlogic.com" , "iscsi-driver@qlogic.com" , Andrew Vasquez FYI: after discussion of this error on linux-pci and linuxppc-dev, the error was corrected in a patch checked into 2.6.33-rc6/3.0-rc6 (http://marc.info/?l=linux-pci&m=130992047232053&w=2) -- james s On 7/1/2011 10:48 AM, James Smart wrote: > All, > > I wanted to communicate a potential warning to those drivers that had a patch > submitted to replace config space searches of PCI_CAP_ID_EXP with shorthand > options such as is_pcie and pci_is_pcie(). > > Testing with the lpfc driver and AER/EEH identified cases where the short-hand > search options would fail on PPC platforms. The only successful option in all > cases was the explicit search via PCI_CAP_ID_EXP. Therefore, I recommend > that this change not be accepted until the platform level issue can be > identified and corrected. > > -- james s > > > > On 6/30/2011 4:41 PM, James Smart wrote: >> Jon, >> >> I must NACK this patch to the lpfc driver and recommend that all other patches >> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with >> "pci_is_pcie(pdev)" are NACK'd as well. >> >> The reason is due to an issue on PPC platforms whereby use of "pdev->is_pcie" >> and pci_is_pcie() will erroneously fail under some conditions, but explicit >> search for the capability struct via pci_find_capability() is always >> successful. I expect this to be due a shadowing of pci config space in the >> hal/platform that isn't sufficiently built up. We detected this issue while >> testing AER/EEH, and are functional only if the pci_find_capability() option >> is used. >> >> -- james s >> >> >> >> On 6/27/2011 1:39 PM, Jon Mason wrote: >>> The PCIE capability offset is saved during PCI bus walking. It will >>> remove an unnecessary search in the PCI configuration space if this >>> value is referenced instead of reacquiring it. Also, pci_is_pcie is a >>> better way of determining if the device is PCIE or not (as it uses the >>> same saved PCIE capability offset). >>> >>> Signed-off-by: Jon Mason >>> --- >>> drivers/scsi/lpfc/lpfc_init.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c >>> index 148b98d..9000ad0 100644 >>> --- a/drivers/scsi/lpfc/lpfc_init.c >>> +++ b/drivers/scsi/lpfc/lpfc_init.c >>> @@ -3970,7 +3970,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba) >>> pci_save_state(pdev); >>> >>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */ >>> - if (pci_find_capability(pdev, PCI_CAP_ID_EXP)) >>> + if (pci_is_pcie(pdev)) >>> pdev->needs_freset = 1; >>> >>> return 0; > >