* [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
@ 2007-05-07 22:04 Brian King
0 siblings, 0 replies; 7+ messages in thread
From: Brian King @ 2007-05-07 22:04 UTC (permalink / raw)
To: paulus; +Cc: brking, linuxppc-dev, thlin
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).
Acked-by: Linas Vepstas <linas@austin.ibm.com>
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
Paul,
The generic PCI layer patch that this patch depends on is now
in. This patch can now be merged. Hopefully it can still make
2.6.22.
Thanks,
Brian
---
linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c | 30 +++++++++++++++++
1 file changed, 30 insertions(+)
diff -puN arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 arch/powerpc/platforms/pseries/eeh.c
--- linux-2.6/arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 2007-05-07 16:52:33.000000000 -0500
+++ linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c 2007-05-07 16:52:33.000000000 -0500
@@ -580,6 +580,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 pcie_deassert_reset:
+ rtas_pci_slot_reset(pdn, 0);
+ break;
+ case pcie_hot_reset:
+ rtas_pci_slot_reset(pdn, 1);
+ break;
+ case 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.
*
_
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
@ 2007-04-06 21:40 Brian King
2007-04-09 21:22 ` Linas Vepstas
0 siblings, 1 reply; 7+ messages in thread
From: Brian King @ 2007-04-06 21:40 UTC (permalink / raw)
To: paulus; +Cc: brking, linuxppc-dev, thlin
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).
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
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.
This is an update in response to the comments Linas Vepstas had.
Thanks,
Brian
---
linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c | 30 +++++++++++++++++
1 files changed, 30 insertions(+)
diff -puN arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 arch/powerpc/platforms/pseries/eeh.c
--- linux-2.6/arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 2007-04-06 15:39:12.000000000 -0500
+++ linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c 2007-04-06 15:40:29.000000000 -0500
@@ -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 pcie_deassert_reset:
+ rtas_pci_slot_reset(pdn, 0);
+ break;
+ case pcie_hot_reset:
+ rtas_pci_slot_reset(pdn, 1);
+ break;
+ case 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.
*
_
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
2007-04-06 21:40 Brian King
@ 2007-04-09 21:22 ` Linas Vepstas
0 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-04-09 21:22 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, paulus, thlin
On Fri, Apr 06, 2007 at 04:40:40PM -0500, Brian King wrote:
>
> 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).
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Acked-by: Linas Vepstas <linas@austin.ibm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
@ 2007-04-06 14:07 Brian King
2007-04-06 18:52 ` Linas Vepstas
0 siblings, 1 reply; 7+ messages in thread
From: Brian King @ 2007-04-06 14:07 UTC (permalink / raw)
To: paulus; +Cc: brking, linuxppc-dev, thlin
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).
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
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(+)
diff -puN arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 arch/powerpc/platforms/pseries/eeh.c
--- linux-2.6/arch/powerpc/platforms/pseries/eeh.c~powerpc_slot_reset_api3 2007-04-06 08:38:35.000000000 -0500
+++ linux-2.6-bjking1/arch/powerpc/platforms/pseries/eeh.c 2007-04-06 08:38:35.000000000 -0500
@@ -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.
*
_
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
2007-04-06 14:07 Brian King
@ 2007-04-06 18:52 ` Linas Vepstas
2007-04-06 19:19 ` Brian King
0 siblings, 1 reply; 7+ messages in thread
From: Linas Vepstas @ 2007-04-06 18:52 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, linux-pci, paulus, thlin
On Fri, Apr 06, 2007 at 09:07:04AM -0500, Brian King wrote:
>
> 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?
> +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.
> + 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".
> + 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."
> + 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
2007-04-06 18:52 ` Linas Vepstas
@ 2007-04-06 19:19 ` Brian King
2007-04-06 20:30 ` Linas Vepstas
0 siblings, 1 reply; 7+ messages in thread
From: Brian King @ 2007-04-06 19:19 UTC (permalink / raw)
To: Linas Vepstas; +Cc: linuxppc-dev, linux-pci, paulus, thlin
Linas Vepstas wrote:
> On Fri, Apr 06, 2007 at 09:07:04AM -0500, Brian King wrote:
>> 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
>> +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.
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.
>> + 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.
>> + 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.
>> + 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.
> 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.
> 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
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation
2007-04-06 19:19 ` Brian King
@ 2007-04-06 20:30 ` Linas Vepstas
0 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-04-06 20:30 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, linux-pci, paulus, thlin
On Fri, Apr 06, 2007 at 02:19:04PM -0500, Brian King wrote:
> Linas Vepstas wrote:
> > On Fri, Apr 06, 2007 at 09:07:04AM -0500, Brian King wrote:
>
> >> + 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.
Hmm. Well, then, can I get you to change it to "pcie_deassert_reset"?
> >> + case pci_reset_pcie_hot_reset:
and while we're at it, shorten this to "pcie_hot_reset"
> >> + case pci_reset_pcie_warm_reset:
"pcie_warm_reset"
> Correct. This is a PCI-E only API.
which is why I suggest the shortened names.
> 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.
> >> + 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.
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.
> > 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
[...]
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-05-07 22:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-07 22:04 [PATCH 1/1] powerpc: Add powerpc PCI-E reset API implementation Brian King
-- strict thread matches above, loose matches on Subject: below --
2007-04-06 21:40 Brian King
2007-04-09 21:22 ` Linas Vepstas
2007-04-06 14:07 Brian King
2007-04-06 18:52 ` Linas Vepstas
2007-04-06 19:19 ` Brian King
2007-04-06 20:30 ` Linas Vepstas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).