diff mbox

pci_pcie_cap invalid on AER/EEH enabled PPC?

Message ID 4E0E1253.1000909@linux.vnet.ibm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Richard A Lary July 1, 2011, 6:30 p.m. UTC
On 7/1/2011 8:24 AM, Jon Mason wrote:
> I recently sent out a number of patches to migrate drivers calling
> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap.  This
> function takes uses a PCI-E capability offset that was determined by
> calling pci_find_capability during the PCI bus walking.  In response
> to one of the patches, James Smart posted:
>
> "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."
>
> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole post.
>
> Based on his description above pci_pcie_cap
> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
> equivalent.  If this is not safe, then the PCI bus walking code is
> most likely busted on EEH enabled PPC systems (and that is a BIG
> problem).  Can anyone confirm this is still an issue?

Jon,

I applied the following debug patch to lpfc driver in a 2.6.32 distro
kernel ( I had this one handy, I can try with mainline later today )

---
  drivers/scsi/lpfc/lpfc_init.c |   10 	10 +	0 -	0 !
  1 file changed, 10 insertions(+)


This is output upon driver load on an IBM Power 7 model 8233-E8B server.

dmesg | grep pcicap
Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version 4.3.4 
[gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1 09:31:27 PDT 2011
pcicap: is_pcie=0 pci_cap=0 pcie_type=0
pcicap: false
pcicap: is_pcie=0 pci_cap=0 pcie_type=0
pcicap: false
pcicap: is_pcie=0 pci_cap=0 pcie_type=0
pcicap: false
pcicap: is_pcie=0 pci_cap=0 pcie_type=0
pcicap: false

It would appear that the pcie information is not set in pci_dev structure for
this device at the time the driver is being initialized during boot.

-rich


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jon Mason July 1, 2011, 7:02 p.m. UTC | #1
On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary <rlary@linux.vnet.ibm.com> wrote:
> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>
>> I recently sent out a number of patches to migrate drivers calling
>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap.  This
>> function takes uses a PCI-E capability offset that was determined by
>> calling pci_find_capability during the PCI bus walking.  In response
>> to one of the patches, James Smart posted:
>>
>> "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."
>>
>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>> post.
>>
>> Based on his description above pci_pcie_cap
>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>> equivalent.  If this is not safe, then the PCI bus walking code is
>> most likely busted on EEH enabled PPC systems (and that is a BIG
>> problem).  Can anyone confirm this is still an issue?
>
> Jon,
>
> I applied the following debug patch to lpfc driver in a 2.6.32 distro
> kernel ( I had this one handy, I can try with mainline later today )
>
> ---
>  drivers/scsi/lpfc/lpfc_init.c |   10   10 +    0 -     0 !
>  1 file changed, 10 insertions(+)
>
> Index: b/drivers/scsi/lpfc/lpfc_init.c
> ===================================================================
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>        pci_try_set_mwi(pdev);
>        pci_save_state(pdev);
>
> +       printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
> +               pdev->is_pcie,
> +               pdev->pcie_cap,
> +               pdev->pcie_type);
> +
> +       if (pci_is_pcie(pdev))
> +               printk(KERN_WARNING "pcicap: true\n");
> +       else
> +               printk(KERN_WARNING "pcicap: false\n");
> +
>        /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>        if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>                pdev->needs_freset = 1;
>
> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>
> dmesg | grep pcicap
> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version 4.3.4
> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1 09:31:27
> PDT 2011
> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
> pcicap: false
> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
> pcicap: false
> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
> pcicap: false
> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
> pcicap: false
>
> It would appear that the pcie information is not set in pci_dev structure
> for
> this device at the time the driver is being initialized during boot.

Thanks for trying this.  Can you confirm that the other devices in the
system have this issue as well (or show that it is isolated to the lpr
device)?  You can add printks in set_pcie_port_type() to verify what
is being set on bus walking and to see when it is being called with
respect to when it is being populated by firmware.

Thanks,
Jon

>
> -rich
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard A Lary July 1, 2011, 8 p.m. UTC | #2
On 7/1/2011 12:02 PM, Jon Mason wrote:
> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com>  wrote:
>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>
>>> I recently sent out a number of patches to migrate drivers calling
>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap.  This
>>> function takes uses a PCI-E capability offset that was determined by
>>> calling pci_find_capability during the PCI bus walking.  In response
>>> to one of the patches, James Smart posted:
>>>
>>> "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."
>>>
>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>> post.
>>>
>>> Based on his description above pci_pcie_cap
>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>> equivalent.  If this is not safe, then the PCI bus walking code is
>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>> problem).  Can anyone confirm this is still an issue?
>>
>> Jon,
>>
>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>> kernel ( I had this one handy, I can try with mainline later today )
>>
>> ---
>>   drivers/scsi/lpfc/lpfc_init.c |   10   10 +    0 -     0 !
>>   1 file changed, 10 insertions(+)
>>
>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>> ===================================================================
>> --- a/drivers/scsi/lpfc/lpfc_init.c
>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>         pci_try_set_mwi(pdev);
>>         pci_save_state(pdev);
>>
>> +       printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>> +               pdev->is_pcie,
>> +               pdev->pcie_cap,
>> +               pdev->pcie_type);
>> +
>> +       if (pci_is_pcie(pdev))
>> +               printk(KERN_WARNING "pcicap: true\n");
>> +       else
>> +               printk(KERN_WARNING "pcicap: false\n");
>> +
>>         /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>         if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>                 pdev->needs_freset = 1;
>>
>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>
>> dmesg | grep pcicap
>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version 4.3.4
>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1 09:31:27
>> PDT 2011
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>>
>> It would appear that the pcie information is not set in pci_dev structure
>> for
>> this device at the time the driver is being initialized during boot.
>
> Thanks for trying this.  Can you confirm that the other devices in the
> system have this issue as well (or show that it is isolated to the lpr
> device)?  You can add printks in set_pcie_port_type() to verify what
> is being set on bus walking and to see when it is being called with
> respect to when it is being populated by firmware.

Jon,

I will give this suggestion a try and post results

-rich

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard A Lary July 5, 2011, 3:41 p.m. UTC | #3
On 7/1/2011 1:00 PM, Richard A Lary wrote:
> On 7/1/2011 12:02 PM, Jon Mason wrote:
>> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com> wrote:
>>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>>
>>>> I recently sent out a number of patches to migrate drivers calling
>>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap. This
>>>> function takes uses a PCI-E capability offset that was determined by
>>>> calling pci_find_capability during the PCI bus walking. In response
>>>> to one of the patches, James Smart posted:
>>>>
>>>> "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."
>>>>
>>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>>> post.
>>>>
>>>> Based on his description above pci_pcie_cap
>>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>>> equivalent. If this is not safe, then the PCI bus walking code is
>>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>>> problem). Can anyone confirm this is still an issue?
>>>
>>> Jon,
>>>
>>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>>> kernel ( I had this one handy, I can try with mainline later today )
>>>
>>> ---
>>> drivers/scsi/lpfc/lpfc_init.c | 10 10 + 0 - 0 !
>>> 1 file changed, 10 insertions(+)
>>>
>>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>>> ===================================================================
>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>> pci_try_set_mwi(pdev);
>>> pci_save_state(pdev);
>>>
>>> + printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>>> + pdev->is_pcie,
>>> + pdev->pcie_cap,
>>> + pdev->pcie_type);
>>> +
>>> + if (pci_is_pcie(pdev))
>>> + printk(KERN_WARNING "pcicap: true\n");
>>> + else
>>> + printk(KERN_WARNING "pcicap: false\n");
>>> +
>>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>> if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>> pdev->needs_freset = 1;
>>>
>>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>>
>>> dmesg | grep pcicap
>>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version 4.3.4
>>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1 09:31:27
>>> PDT 2011
>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>> pcicap: false
>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>> pcicap: false
>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>> pcicap: false
>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>> pcicap: false
>>>
>>> It would appear that the pcie information is not set in pci_dev structure
>>> for
>>> this device at the time the driver is being initialized during boot.
>>
>> Thanks for trying this. Can you confirm that the other devices in the
>> system have this issue as well (or show that it is isolated to the lpr
>> device)? You can add printks in set_pcie_port_type() to verify what
>> is being set on bus walking and to see when it is being called with
>> respect to when it is being populated by firmware.
>
> Jon,
>
> I will give this suggestion a try and post results

On Power PC platforms, set_pcie_port_type() is not called.  On Power PC,
pci_dev structure is initialized by of_create_pci_dev().  However, the
structure member pcie_cap is NOT computed nor set in this function.

The information used to populate pci_dev comes from the Power PC
device_tree passed to the OS by Open Firmware.

Based upon standing Power PC design, we cannot support patches
which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
pci_is_pcie(pdev) on Power PC platforms.

-rich

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Mason July 5, 2011, 4:18 p.m. UTC | #4
On Tue, Jul 5, 2011 at 10:41 AM, Richard A Lary
<rlary@linux.vnet.ibm.com> wrote:
> On 7/1/2011 1:00 PM, Richard A Lary wrote:
>>
>> On 7/1/2011 12:02 PM, Jon Mason wrote:
>>>
>>> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com>
>>> wrote:
>>>>
>>>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>>>
>>>>> I recently sent out a number of patches to migrate drivers calling
>>>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap. This
>>>>> function takes uses a PCI-E capability offset that was determined by
>>>>> calling pci_find_capability during the PCI bus walking. In response
>>>>> to one of the patches, James Smart posted:
>>>>>
>>>>> "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."
>>>>>
>>>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>>>> post.
>>>>>
>>>>> Based on his description above pci_pcie_cap
>>>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>>>> equivalent. If this is not safe, then the PCI bus walking code is
>>>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>>>> problem). Can anyone confirm this is still an issue?
>>>>
>>>> Jon,
>>>>
>>>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>>>> kernel ( I had this one handy, I can try with mainline later today )
>>>>
>>>> ---
>>>> drivers/scsi/lpfc/lpfc_init.c | 10 10 + 0 - 0 !
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>>>> ===================================================================
>>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>>> pci_try_set_mwi(pdev);
>>>> pci_save_state(pdev);
>>>>
>>>> + printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>>>> + pdev->is_pcie,
>>>> + pdev->pcie_cap,
>>>> + pdev->pcie_type);
>>>> +
>>>> + if (pci_is_pcie(pdev))
>>>> + printk(KERN_WARNING "pcicap: true\n");
>>>> + else
>>>> + printk(KERN_WARNING "pcicap: false\n");
>>>> +
>>>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>>> if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>>> pdev->needs_freset = 1;
>>>>
>>>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>>>
>>>> dmesg | grep pcicap
>>>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version
>>>> 4.3.4
>>>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1
>>>> 09:31:27
>>>> PDT 2011
>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>> pcicap: false
>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>> pcicap: false
>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>> pcicap: false
>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>> pcicap: false
>>>>
>>>> It would appear that the pcie information is not set in pci_dev
>>>> structure
>>>> for
>>>> this device at the time the driver is being initialized during boot.
>>>
>>> Thanks for trying this. Can you confirm that the other devices in the
>>> system have this issue as well (or show that it is isolated to the lpr
>>> device)? You can add printks in set_pcie_port_type() to verify what
>>> is being set on bus walking and to see when it is being called with
>>> respect to when it is being populated by firmware.
>>
>> Jon,
>>
>> I will give this suggestion a try and post results
>
> On Power PC platforms, set_pcie_port_type() is not called.  On Power PC,
> pci_dev structure is initialized by of_create_pci_dev().  However, the
> structure member pcie_cap is NOT computed nor set in this function.

Yes, it is.  of_create_pci_dev() calls set_pcie_port_type()
http://lxr.linux.no/linux+v2.6.39/arch/powerpc/kernel/pci_of_scan.c#L144

That function sets pdev->pcie_cap
http://lxr.linux.no/linux+v2.6.39/drivers/pci/probe.c#L896

So, it should be set.  It looks like there is a bug in
of_create_pci_dev, as set_pcie_port_type is being called BEFORE the
BARs are setup.  If you move set_pcie_port_type prior to
pci_device_add (perhaps even after), then I bet the issue is resolved.

Thanks,
Jon

> The information used to populate pci_dev comes from the Power PC
> device_tree passed to the OS by Open Firmware.
>
> Based upon standing Power PC design, we cannot support patches
> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
> pci_is_pcie(pdev) on Power PC platforms.
>
> -rich
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard A Lary July 5, 2011, 5:22 p.m. UTC | #5
On 7/5/2011 9:18 AM, Jon Mason wrote:
> On Tue, Jul 5, 2011 at 10:41 AM, Richard A Lary
> <rlary@linux.vnet.ibm.com>  wrote:
>> On 7/1/2011 1:00 PM, Richard A Lary wrote:
>>>
>>> On 7/1/2011 12:02 PM, Jon Mason wrote:
>>>>
>>>> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com>
>>>> wrote:
>>>>>
>>>>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>>>>
>>>>>> I recently sent out a number of patches to migrate drivers calling
>>>>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap. This
>>>>>> function takes uses a PCI-E capability offset that was determined by
>>>>>> calling pci_find_capability during the PCI bus walking. In response
>>>>>> to one of the patches, James Smart posted:
>>>>>>
>>>>>> "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."
>>>>>>
>>>>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>>>>> post.
>>>>>>
>>>>>> Based on his description above pci_pcie_cap
>>>>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>>>>> equivalent. If this is not safe, then the PCI bus walking code is
>>>>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>>>>> problem). Can anyone confirm this is still an issue?
>>>>>
>>>>> Jon,
>>>>>
>>>>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>>>>> kernel ( I had this one handy, I can try with mainline later today )
>>>>>
>>>>> ---
>>>>> drivers/scsi/lpfc/lpfc_init.c | 10 10 + 0 - 0 !
>>>>> 1 file changed, 10 insertions(+)
>>>>>
>>>>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>>>>> ===================================================================
>>>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>>>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>>>> pci_try_set_mwi(pdev);
>>>>> pci_save_state(pdev);
>>>>>
>>>>> + printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>>>>> + pdev->is_pcie,
>>>>> + pdev->pcie_cap,
>>>>> + pdev->pcie_type);
>>>>> +
>>>>> + if (pci_is_pcie(pdev))
>>>>> + printk(KERN_WARNING "pcicap: true\n");
>>>>> + else
>>>>> + printk(KERN_WARNING "pcicap: false\n");
>>>>> +
>>>>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>>>> if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>>>> pdev->needs_freset = 1;
>>>>>
>>>>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>>>>
>>>>> dmesg | grep pcicap
>>>>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version
>>>>> 4.3.4
>>>>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1
>>>>> 09:31:27
>>>>> PDT 2011
>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>> pcicap: false
>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>> pcicap: false
>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>> pcicap: false
>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>> pcicap: false
>>>>>
>>>>> It would appear that the pcie information is not set in pci_dev
>>>>> structure
>>>>> for
>>>>> this device at the time the driver is being initialized during boot.
>>>>
>>>> Thanks for trying this. Can you confirm that the other devices in the
>>>> system have this issue as well (or show that it is isolated to the lpr
>>>> device)? You can add printks in set_pcie_port_type() to verify what
>>>> is being set on bus walking and to see when it is being called with
>>>> respect to when it is being populated by firmware.
>>>
>>> Jon,
>>>
>>> I will give this suggestion a try and post results
>>
>> On Power PC platforms, set_pcie_port_type() is not called.  On Power PC,
>> pci_dev structure is initialized by of_create_pci_dev().  However, the
>> structure member pcie_cap is NOT computed nor set in this function.
>
> Yes, it is.  of_create_pci_dev() calls set_pcie_port_type()
> http://lxr.linux.no/linux+v2.6.39/arch/powerpc/kernel/pci_of_scan.c#L144
>
> That function sets pdev->pcie_cap
> http://lxr.linux.no/linux+v2.6.39/drivers/pci/probe.c#L896
>
> So, it should be set.  It looks like there is a bug in
> of_create_pci_dev, as set_pcie_port_type is being called BEFORE the
> BARs are setup.  If you move set_pcie_port_type prior to
> pci_device_add (perhaps even after), then I bet the issue is resolved.
>

The claim above was based upon observation that with this patch applied
to a 2.6.32 kernel, the printk output did not appear in dmesg upon boot.

  static void set_pcie_hotplug_bridge(struct pci_dev *pdev)
@@ -774,6 +780,8 @@ int pci_setup_device(struct pci_dev *dev
         u8 hdr_type;
         struct pci_slot *slot;

+       printk(KERN_WARNING "pcicap: setup_device %p\n", dev);
+
         if (pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type))
                 return -EIO;

I can make no claim about my understanding of pci device initialization
on Power PC, so I have added Anton Blanchard to the cc list.

Perhaps Anton can explain why pcie_cap is always 0 on Power PC.

-rich

>
>> The information used to populate pci_dev comes from the Power PC
>> device_tree passed to the OS by Open Firmware.
>>
>> Based upon standing Power PC design, we cannot support patches
>> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
>> pci_is_pcie(pdev) on Power PC platforms.
>>
>> -rich
>>
>>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard A Lary July 5, 2011, 8:34 p.m. UTC | #6
On 7/5/2011 10:22 AM, Richard A Lary wrote:
> On 7/5/2011 9:18 AM, Jon Mason wrote:
>> On Tue, Jul 5, 2011 at 10:41 AM, Richard A Lary
>> <rlary@linux.vnet.ibm.com> wrote:
>>> On 7/1/2011 1:00 PM, Richard A Lary wrote:
>>>>
>>>> On 7/1/2011 12:02 PM, Jon Mason wrote:
>>>>>
>>>>> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com>
>>>>> wrote:
>>>>>>
>>>>>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>>>>>
>>>>>>> I recently sent out a number of patches to migrate drivers calling
>>>>>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap. This
>>>>>>> function takes uses a PCI-E capability offset that was determined by
>>>>>>> calling pci_find_capability during the PCI bus walking. In response
>>>>>>> to one of the patches, James Smart posted:
>>>>>>>
>>>>>>> "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."
>>>>>>>
>>>>>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>>>>>> post.
>>>>>>>
>>>>>>> Based on his description above pci_pcie_cap
>>>>>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>>>>>> equivalent. If this is not safe, then the PCI bus walking code is
>>>>>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>>>>>> problem). Can anyone confirm this is still an issue?
>>>>>>
>>>>>> Jon,
>>>>>>
>>>>>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>>>>>> kernel ( I had this one handy, I can try with mainline later today )
>>>>>>
>>>>>> ---
>>>>>> drivers/scsi/lpfc/lpfc_init.c | 10 10 + 0 - 0 !
>>>>>> 1 file changed, 10 insertions(+)
>>>>>>
>>>>>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>>>>>> ===================================================================
>>>>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>>>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>>>>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>>>>> pci_try_set_mwi(pdev);
>>>>>> pci_save_state(pdev);
>>>>>>
>>>>>> + printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>>>>>> + pdev->is_pcie,
>>>>>> + pdev->pcie_cap,
>>>>>> + pdev->pcie_type);
>>>>>> +
>>>>>> + if (pci_is_pcie(pdev))
>>>>>> + printk(KERN_WARNING "pcicap: true\n");
>>>>>> + else
>>>>>> + printk(KERN_WARNING "pcicap: false\n");
>>>>>> +
>>>>>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>>>>> if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>>>>> pdev->needs_freset = 1;
>>>>>>
>>>>>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>>>>>
>>>>>> dmesg | grep pcicap
>>>>>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version
>>>>>> 4.3.4
>>>>>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1
>>>>>> 09:31:27
>>>>>> PDT 2011
>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>> pcicap: false
>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>> pcicap: false
>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>> pcicap: false
>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>> pcicap: false
>>>>>>
>>>>>> It would appear that the pcie information is not set in pci_dev
>>>>>> structure
>>>>>> for
>>>>>> this device at the time the driver is being initialized during boot.
>>>>>
>>>>> Thanks for trying this. Can you confirm that the other devices in the
>>>>> system have this issue as well (or show that it is isolated to the lpr
>>>>> device)? You can add printks in set_pcie_port_type() to verify what
>>>>> is being set on bus walking and to see when it is being called with
>>>>> respect to when it is being populated by firmware.
>>>>
>>>> Jon,
>>>>
>>>> I will give this suggestion a try and post results
>>>
>>> On Power PC platforms, set_pcie_port_type() is not called. On Power PC,
>>> pci_dev structure is initialized by of_create_pci_dev(). However, the
>>> structure member pcie_cap is NOT computed nor set in this function.
>>
>> Yes, it is. of_create_pci_dev() calls set_pcie_port_type()
>> http://lxr.linux.no/linux+v2.6.39/arch/powerpc/kernel/pci_of_scan.c#L144
>>
>> That function sets pdev->pcie_cap
>> http://lxr.linux.no/linux+v2.6.39/drivers/pci/probe.c#L896
>>
>> So, it should be set. It looks like there is a bug in
>> of_create_pci_dev, as set_pcie_port_type is being called BEFORE the
>> BARs are setup. If you move set_pcie_port_type prior to
>> pci_device_add (perhaps even after), then I bet the issue is resolved.
>>
>
> The claim above was based upon observation that with this patch applied
> to a 2.6.32 kernel, the printk output did not appear in dmesg upon boot.
>
> static void set_pcie_hotplug_bridge(struct pci_dev *pdev)
> @@ -774,6 +780,8 @@ int pci_setup_device(struct pci_dev *dev
> u8 hdr_type;
> struct pci_slot *slot;
>
> + printk(KERN_WARNING "pcicap: setup_device %p\n", dev);
> +
> if (pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type))
> return -EIO;
>
> I can make no claim about my understanding of pci device initialization
> on Power PC, so I have added Anton Blanchard to the cc list.
>
> Perhaps Anton can explain why pcie_cap is always 0 on Power PC.

I pulled down 3.0-rc6 kernel, I will build and test the pci_is_pcie(pdev)
patch in lpfc driver now that set_pcie_port_type() is called from
of_create_pci_dev().

-rich
>>
>>> The information used to populate pci_dev comes from the Power PC
>>> device_tree passed to the OS by Open Firmware.
>>>
>>> Based upon standing Power PC design, we cannot support patches
>>> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
>>> pci_is_pcie(pdev) on Power PC platforms.
>>>
>>> -rich
>>>
>>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard A Lary July 6, 2011, 12:14 a.m. UTC | #7
On 7/5/2011 1:34 PM, Richard A Lary wrote:
> On 7/5/2011 10:22 AM, Richard A Lary wrote:
>> On 7/5/2011 9:18 AM, Jon Mason wrote:
>>> On Tue, Jul 5, 2011 at 10:41 AM, Richard A Lary
>>> <rlary@linux.vnet.ibm.com> wrote:
>>>> On 7/1/2011 1:00 PM, Richard A Lary wrote:
>>>>>
>>>>> On 7/1/2011 12:02 PM, Jon Mason wrote:
>>>>>>
>>>>>> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary@linux.vnet.ibm.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>>>>>>
>>>>>>>> I recently sent out a number of patches to migrate drivers calling
>>>>>>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap. This
>>>>>>>> function takes uses a PCI-E capability offset that was determined by
>>>>>>>> calling pci_find_capability during the PCI bus walking. In response
>>>>>>>> to one of the patches, James Smart posted:
>>>>>>>>
>>>>>>>> "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."
>>>>>>>>
>>>>>>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>>>>>>> post.
>>>>>>>>
>>>>>>>> Based on his description above pci_pcie_cap
>>>>>>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>>>>>>> equivalent. If this is not safe, then the PCI bus walking code is
>>>>>>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>>>>>>> problem). Can anyone confirm this is still an issue?
>>>>>>>
>>>>>>> Jon,
>>>>>>>
>>>>>>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>>>>>>> kernel ( I had this one handy, I can try with mainline later today )
>>>>>>>
>>>>>>> ---
>>>>>>> drivers/scsi/lpfc/lpfc_init.c | 10 10 + 0 - 0 !
>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>>>>>>> ===================================================================
>>>>>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>>>>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>>>>>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>>>>>> pci_try_set_mwi(pdev);
>>>>>>> pci_save_state(pdev);
>>>>>>>
>>>>>>> + printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>>>>>>> + pdev->is_pcie,
>>>>>>> + pdev->pcie_cap,
>>>>>>> + pdev->pcie_type);
>>>>>>> +
>>>>>>> + if (pci_is_pcie(pdev))
>>>>>>> + printk(KERN_WARNING "pcicap: true\n");
>>>>>>> + else
>>>>>>> + printk(KERN_WARNING "pcicap: false\n");
>>>>>>> +
>>>>>>> /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>>>>>> if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>>>>>> pdev->needs_freset = 1;
>>>>>>>
>>>>>>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>>>>>>
>>>>>>> dmesg | grep pcicap
>>>>>>> Linux version 2.6.32.42-pcicap-ppc64 (geeko@buildhost) (gcc version
>>>>>>> 4.3.4
>>>>>>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1
>>>>>>> 09:31:27
>>>>>>> PDT 2011
>>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>>> pcicap: false
>>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>>> pcicap: false
>>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>>> pcicap: false
>>>>>>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>>>>>>> pcicap: false
>>>>>>>
>>>>>>> It would appear that the pcie information is not set in pci_dev
>>>>>>> structure
>>>>>>> for
>>>>>>> this device at the time the driver is being initialized during boot.
>>>>>>
>>>>>> Thanks for trying this. Can you confirm that the other devices in the
>>>>>> system have this issue as well (or show that it is isolated to the lpr
>>>>>> device)? You can add printks in set_pcie_port_type() to verify what
>>>>>> is being set on bus walking and to see when it is being called with
>>>>>> respect to when it is being populated by firmware.
>>>>>
>>>>> Jon,
>>>>>
>>>>> I will give this suggestion a try and post results
>>>>
>>>> On Power PC platforms, set_pcie_port_type() is not called. On Power PC,
>>>> pci_dev structure is initialized by of_create_pci_dev(). However, the
>>>> structure member pcie_cap is NOT computed nor set in this function.
>>>
>>> Yes, it is. of_create_pci_dev() calls set_pcie_port_type()
>>> http://lxr.linux.no/linux+v2.6.39/arch/powerpc/kernel/pci_of_scan.c#L144
>>>
>>> That function sets pdev->pcie_cap
>>> http://lxr.linux.no/linux+v2.6.39/drivers/pci/probe.c#L896
>>>
>>> So, it should be set. It looks like there is a bug in
>>> of_create_pci_dev, as set_pcie_port_type is being called BEFORE the
>>> BARs are setup. If you move set_pcie_port_type prior to
>>> pci_device_add (perhaps even after), then I bet the issue is resolved.
>>>
>>
>> The claim above was based upon observation that with this patch applied
>> to a 2.6.32 kernel, the printk output did not appear in dmesg upon boot.
>>
>> static void set_pcie_hotplug_bridge(struct pci_dev *pdev)
>> @@ -774,6 +780,8 @@ int pci_setup_device(struct pci_dev *dev
>> u8 hdr_type;
>> struct pci_slot *slot;
>>
>> + printk(KERN_WARNING "pcicap: setup_device %p\n", dev);
>> +
>> if (pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type))
>> return -EIO;
>>
>> I can make no claim about my understanding of pci device initialization
>> on Power PC, so I have added Anton Blanchard to the cc list.
>>
>> Perhaps Anton can explain why pcie_cap is always 0 on Power PC.
>
> I pulled down 3.0-rc6 kernel, I will build and test the pci_is_pcie(pdev)
> patch in lpfc driver now that set_pcie_port_type() is called from
> of_create_pci_dev().

Jon,

I applied the debug patches mentioned above along with the lpfc patch
http://marc.info/?l=linux-scsi&m=130919648513685&w=2
to linux 3.0-rc6 kernel.

The debug patches show that pci_dev members "is_pcie, pci_cap and pcie_type" are 
now all being set to correct values now that 'of_create_pci_dev()' calls 
'set_pcie_port_type()'.  I was not able to determine when this patch went in.

My tests show that lpfc driver now recovers from injected PCIe bus errors
using test the 'if (pci_is_pcie(pdev))' for PCIe adapter type.

I did not apply the test patch which changed the location of 
set_pcie_port_type(dev) in of_create_pci_dev().  I can apply and test
this change if you think it is necessary?

Based upon these results, I will ACK the change to the lpfc driver for
"[PATCH 03/19] lpfc: remove unnecessary read of PCI_CAP_ID_EXP".
I suspect that other drivers which are modified to use 'if (pci_is_pcie(pdev))'
will work on Power PC as well, but I did not test any drivers other than lpfc.


-rich


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt July 6, 2011, 2:42 a.m. UTC | #8
> On Power PC platforms, set_pcie_port_type() is not called.  On Power PC,
> pci_dev structure is initialized by of_create_pci_dev().  However, the
> structure member pcie_cap is NOT computed nor set in this function.

Just a quick correction here, please don't say "On Power PC platforms"
such generic statements that only apply to IBM pSeries platforms running
under pHyp :-)

There's plenty of other platforms supported in arch/powerpc that have
very different characteristics and use more "generic" code to build
their PCI layout.

> The information used to populate pci_dev comes from the Power PC
> device_tree passed to the OS by Open Firmware.
> 
> Based upon standing Power PC design, we cannot support patches
> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
> pci_is_pcie(pdev) on Power PC platforms.

No, that isn't correct. We can (and should) fix our arch code to do the
right thing. There is no reason why those two wouldn't be equivalent.

If we are missing a call to set_pcie_port_type() then we should add it,
however if I look at our upstream code, pci_of_scan.c seems to be
calling that, so it could be a missing backport ?

Can you try with an upstream kernel ?

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt July 6, 2011, 2:47 a.m. UTC | #9
On Tue, 2011-07-05 at 17:14 -0700, Richard A Lary wrote:

> I applied the debug patches mentioned above along with the lpfc patch
> http://marc.info/?l=linux-scsi&m=130919648513685&w=2
> to linux 3.0-rc6 kernel.
> 
> The debug patches show that pci_dev members "is_pcie, pci_cap and pcie_type" are 
> now all being set to correct values now that 'of_create_pci_dev()' calls 
> 'set_pcie_port_type()'.  I was not able to determine when this patch went in.

git is good for that :-)

bb209c8287d2d55ec4a67e3933346e0a3ee0da76:

Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>  2010-01-27 04:10:03
Committer: Benjamin Herrenschmidt <benh@kernel.crashing.org>  2010-01-29 16:51:10
Parent: 4406c56d0a4da7a37b9180abeaece6cd00bcc874 (Merge branch 'linux-next' of git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci-2.6)
Child:  26b4a0ca46985ae9586c194f7859f3838b1230f8 (powerpc/pci: Add missing hookup to pci_slot)
Branches: many (38)
Follows: v2.6.33-rc5
Precedes: v2.6.33-rc7

    powerpc/pci: Add calls to set_pcie_port_type() and set_pcie_hotplug_bridge()
    
    We are missing these when building the pci_dev from scratch off
    the Open Firmware device-tree
    
    Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

We should probably backport it to .32-stable. Any volunteer ?

You might also want to consider for backport:
26b4a0ca46985ae9586c194f7859f3838b1230f8 and
94afc008e1e6fbadfac0b75fcf193b6d7074b2f1

> My tests show that lpfc driver now recovers from injected PCIe bus errors
> using test the 'if (pci_is_pcie(pdev))' for PCIe adapter type.
> 
> I did not apply the test patch which changed the location of 
> set_pcie_port_type(dev) in of_create_pci_dev().  I can apply and test
> this change if you think it is necessary?

No I think we call it in the right place, which mirrors
pci_setup_device(), that is before the early quirk.

> Based upon these results, I will ACK the change to the lpfc driver for
> "[PATCH 03/19] lpfc: remove unnecessary read of PCI_CAP_ID_EXP".
> I suspect that other drivers which are modified to use 'if (pci_is_pcie(pdev))'
> will work on Power PC as well, but I did not test any drivers other than lpfc.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: b/drivers/scsi/lpfc/lpfc_init.c
===================================================================
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -3958,6 +3958,16 @@  lpfc_enable_pci_dev(struct lpfc_hba *phb
  	pci_try_set_mwi(pdev);
  	pci_save_state(pdev);

+	printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
+		pdev->is_pcie,
+		pdev->pcie_cap,
+		pdev->pcie_type);
+
+	if (pci_is_pcie(pdev))
+		printk(KERN_WARNING "pcicap: true\n");
+	else
+		printk(KERN_WARNING "pcicap: false\n");
+
  	/* PCIe EEH recovery on powerpc platforms needs fundamental reset */
  	if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
  		pdev->needs_freset = 1;