On Fri, 2015-01-23 at 14:50 +1100, Gavin Shan wrote:

Messages from Brian for reference:

| The API has changed. I wrote the pci_set_pcie_reset_state API originally.
| When this API was put in place initially, it was perfectly legal to call it
| from an atomic context. Can you clarify why we have to have the delay in the
| pci_set_pcie_reset_state function? Shouldn't it be the responsibility of the
| callers to ensure a proper delay is used? This was always the case until
| recently.

So please ignore this patch and I'll send another one, which is implemented in
above approach.

I still think it's not a great idea to allow that API to be called in
atomic context.

Brian, the reset API for PCIe involves FW calls which might have to do
a bunch of stuff under the hood, including potentially significant
delays.

For example, under OPAL (and I suppose PowerVM), if doing a PERST, the
API calls will loop until the link is back up, at least when "releasing"
the reset line.

I wouldn't be surprised if on x86, similar kinds of ACPI calls are
needed which may not be the best thing to do in atomic context.

I don't see any specific performance issues with issuing resets, so I
would strongly advocate for changing the API requirements instead so
that it's called from a task context.

Cheers,
Ben.

Thanks,
Gavin

quoted

quoted

quoted

quoted

Signed-off-by: Gavin Shan <redacted>
Tested-by: Wen Xiong<redacted>
---
 arch/powerpc/platforms/pseries/eeh_pseries.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index a6c7e19..67623a3 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c

@@ -503,8 +503,7 @@ static int pseries_eeh_get_state(struct eeh_pe *pe, int *state)
  */
 static int pseries_eeh_reset(struct eeh_pe *pe, int option)
 {
-	int config_addr;
-	int ret;
+	int config_addr, delay, ret;
 
 	/* Figure out PE address */
 	config_addr = pe->config_addr;

@@ -528,9 +527,14 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option)
 	/* We need reset hold or settlement delay */
 	if (option == EEH_RESET_FUNDAMENTAL ||
 	    option == EEH_RESET_HOT)
-		msleep(EEH_PE_RST_HOLD_TIME);
+		delay = EEH_PE_RST_HOLD_TIME;
+	else
+		delay = EEH_PE_RST_SETTLE_TIME;
+
+	if (in_atomic())
+		udelay(delay * 1000);
 	else
-		msleep(EEH_PE_RST_SETTLE_TIME);
+		msleep(delay);
 
 	return ret;
 }