powerpc: Add powerpc PCI-E reset API implementation
Subsystems (2)
PATCH 1/1 powerpc: Add powerpc PCI-E reset API implementation
Adds the pSeries platform implementation for a new PCI API which can be used to issue various types of PCI-E reset, including PCI-E warm reset and PCI-E hot reset. This is needed for an ipr PCI-E adapter which does not properly implement BIST. Running BIST on this adapter results in PCI-E errors. The only reliable reset mechanism that exists on this hardware is PCI Fundamental reset (warm reset).
---
Paul,
This patch requires a generic PCI layer patch, which is in Greg's queue for 2.6.22, so do NOT apply this yet. Once the PCI change goes in, I will resend this patch.
Thanks,
Brian
---
linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c | 30 +++++++++++++++++ 1 files changed, 30 insertions(+)
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +30 -0
@@ -561,6 +561,36 @@ rtas_pci_slot_reset(struct pci_dn *pdn, }/** +* pcibios_set_pcie_slot_reset - Set PCI-E reset state +* @dev: pci device struct +* @state: reset state to enter +* +* Return value: +* 0 if success +**/ +int pcibios_set_pcie_reset_state (struct pci_dev *dev,enum pcie_reset_state state) +{ +struct device_node *dn =pci_device_to_OF_node (dev); +struct pci_dn *pdn =PCI_DN (dn); + +switch (state) { +case pci_reset_normal: +rtas_pci_slot_reset (pdn,0 ); +break ; +case pci_reset_pcie_hot_reset: +rtas_pci_slot_reset (pdn,1 ); +break ; +case pci_reset_pcie_warm_reset: +rtas_pci_slot_reset (pdn,3 ); +break ; +default : +return -EINVAL ; + }; + +return 0 ; +} + +/** * rtas_set_slot_reset -- assert the pci #RST line for 1/4 second * @pdn: pci device node to be reset. *
_
Quoted 3 lines · Brian King
This patch requires a generic PCI layer patch, which is in Greg's queue for 2.6.22,
I managed to miss seeing that patch, even though I thought I was subscribed to the mailing list. Was there any discusion of it?
Quoted 1 line
+int
pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
I think you'll need an EXPORT_SMBOL_GPL if you plan to call this from a module.
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 578
Quoted 3 lines
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +3
-0+switch (state) { +case pci_reset_normal: +rtas_pci_slot_reset (pdn,0 );
I find this naming confusing. rtas_pci_slot_reset(pdn, 0), for PCI ad PCI-X means "deassert the reset"; it does not mean "do a normal reset".
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 584
Quoted 6 lines
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +6
-0+break ; +case pci_reset_pcie_hot_reset: +rtas_pci_slot_reset (pdn,1 ); +break ; +case pci_reset_pcie_warm_reset: +rtas_pci_slot_reset (pdn,3 );
For PCI and PCI-X, rtas_pci_slot_reset(pdn, 1) means "hold the reset line high, until such time that its de-asserted."
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 590
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +1
-0+return 0 ;
I notice that you do no error checking. I recently wrapped rtas_set_slot_reset() to wait for slot status to settle down before reporting success or failure of the reset.
Although the PAPR maps 1 to hot reset, and 3 to #PERST, I always had the impression that they managed to reverse meaing of these two (i.e. its a poor match to what the PCI-E spec says), and I never understood why. I am still thinking that the correct reset seqeunce on linux is to try "3" first, if its supported, and then try a "1". I've not taken steps to do this, though.
I could wrap rtas_set_slot_reset() to try a "3" first, for PCI-E slots, and do 1 "1" only if that fails. Would this solve the problem that you are having?
--linas
Quoted 6 lines · Linas Vepstas
Quoted 2 lines · Brian King
This patch requires a generic PCI layer patch, which is in Greg's queue for 2.6.22,
I managed to miss seeing that patch, even though I thought I was subscribed to the mailing list. Was there any discusion of it?
http://marc.info/?t=117035109100003&r=1&w=2
Quoted 4 lines
Quoted 1 line
+int
pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)I think you'll need an
EXPORT_SMBOL_GPLif you plan to call this from a module.
It is only to be called from the generic PCI layer. Anyone else should be calling pci_set_pcie_reset_state, which is the PCI layer wrapper for it.
Quoted 7 lines
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 578
Quoted 3 lines
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +3
-0+switch (state) { +case pci_reset_normal: +rtas_pci_slot_reset (pdn,0 );I find this naming confusing.
rtas_pci_slot_reset(pdn, 0), for PCI ad PCI-X means "deassert the reset"; it does not mean "do a normal reset".
Normal refers to the reset state of the slot, not the type of reset. I'm happy to change it if that would be less confusing.
Quoted 9 lines
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 584
Quoted 6 lines
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +6
-0+break ; +case pci_reset_pcie_hot_reset: +rtas_pci_slot_reset (pdn,1 ); +break ; +case pci_reset_pcie_warm_reset: +rtas_pci_slot_reset (pdn,3 );For PCI and PCI-X,
rtas_pci_slot_reset(pdn, 1)means "hold the reset line high, until such time that its de-asserted."
Correct. This is a PCI-E only API. I don't think we want to be supporting an API that allows asserting reset on a potentially shared PCI bus.
Quoted 5 lines
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 590
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +1
-0+return 0 ;I notice that you do no error checking. I recently wrapped
rtas_set_slot_reset()to wait for slot status to settle down before reporting success or failure of the reset.
I didn't do any error checking because there was no error checking in __rtas_set_slot_reset either. I can add error checking if needed, but I'm not sure I would do anything with it in ipr.
Quoted 6 lines
Although the PAPR maps 1 to hot reset, and 3 to #PERST, I always had the impression that they managed to reverse meaing of these two (i.e. its a poor match to what the PCI-E spec says), and I never understood why. I am still thinking that the correct reset seqeunce on linux is to try "3" first, if its supported, and then try a "1". I've not taken steps to do this, though.
Possibly. Its not clear to me how you can tell if a 3 is supported, however. You can check if the platform supports it, but there is no guarantee that the adapter itself supports it. It may ignore it, or it may do bad things.
Quoted 3 lines
I could wrap
rtas_set_slot_reset()to try a "3" first, for PCI-E slots, and do 1 "1" only if that fails. Would this solve the problem that you are having?
rtas_set_slot_reset can sleep, which means the amount of code and complexity in the ipr driver to use this API would grow quite a bit.
Additionally, as I already mentioned, how would you know that 3 failed? The device may respond just fine to config cycles after a failed "3", but still not actually be in a healthy state. As a corollary, with this particular ipr adapter a hot reset appears to work from a pci config access perspective, but the adapter firmware ends up in a bad state since the adapter's processor does not get reset. Its not clear would don't have adapters that have issues in the other direction.
Brian
Signature · 3 lines
Brian King eServer Storage I/O IBM Linux Technology Center
Quoted 11 lines · Brian King
Quoted 9 lines · Linas Vepstas
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 578
Quoted 3 lines · Brian King
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +3
-0+switch (state) { +case pci_reset_normal: +rtas_pci_slot_reset (pdn,0 );I find this naming confusing.
rtas_pci_slot_reset(pdn, 0), for PCI ad PCI-X means "deassert the reset"; it does not mean "do a normal reset".
Quoted 2 lines
Normal refers to the reset state of the slot, not the type of reset. I'm happy to change it if that would be less confusing.
Hmm. Well, then, can I get you to change it to "pcie_deassert_reset"?
Quoted 1 line
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 580
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +1
-0+case pci_reset_pcie_hot_reset:
and while we're at it, shorten this to "pcie_hot_reset"
Quoted 1 line
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 583
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +1
-0+case pci_reset_pcie_warm_reset:
"pcie_warm_reset"
Quoted 1 line
Correct. This is a PCI-E only API.
which is why I suggest the shortened names.
Quoted 3 lines
I don't think we want to be
supporting an API that allows asserting reset on a potentially shared PCI bus.
Actually, we very nearly do so already. The "pci error recovery" API does define a way of coordinating a pending reset to be delivered to multiple PCI functions or multiple PCI cards. What's missing is an external, public API for triggering the thing; right now, its only triggered by the hardware and handled by static, private routines.
Quoted 8 lines
Quoted 5 lines
arch/powerpc/platforms/pseries/eeh.c rtas_pci_slot_reset(struct pci_dn *pdn, · line 590
Quoted 1 line
arch/powerpc/platforms/pseries/eeh.c renamed from arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 +1
-0+return 0 ;I notice that you do no error checking. I recently wrapped
rtas_set_slot_reset()to wait for slot status to settle down before reporting success or failure of the reset.I didn't do any error checking because there was no error checking in
__rtas_set_slot_reseteither.
Yeah, its a new function: "eeh_wait_for_slot_status", its public, but cleary very arch dependent. Just FYI, I don't think you'd want to use it.
Quoted 2 lines
Quoted 2 lines
Although the PAPR maps 1 to hot reset, and 3 to #PERST, I always had the impression that they managed to reverse meaing of these two
[...]
Quoted 6 lines
The device may respond just fine to config cycles after a failed "3", but still not actually be in a healthy state. As a corollary, with this particular ipr adapter a hot reset appears to work from a pci config access perspective, but the adapter firmware ends up in a bad state since the adapter's processor does not get reset. Its not clear would don't have adapters that have issues in the other direction.
Yes... we had this conversation once before, didn't we? The ugliness of it all made me scurry away to some other task.
--linas