* [PATCH 1/3] drivers/pci/hotplug: Handle presence detection change properly
2017-01-11 0:50 [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
@ 2017-01-11 0:50 ` Gavin Shan
2017-02-16 5:59 ` [1/3] " Michael Ellerman
2017-01-11 0:50 ` [PATCH 2/3] drivers/pci/hotplug: Fix initial state for empty slot Gavin Shan
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Gavin Shan @ 2017-01-11 0:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, hankmax0000, williel, Gavin Shan, stable, #v4.9+
The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
driver. In the interrupt handler, pnv_php_interrupt(), we bail when
pnv_pci_get_presence_state() returns zero wrongly. It causes the
presence change event is always ignored incorrectly.
This fixes the issue by bailing on error (non-zero value) returned
from pnv_pci_get_presence_state().
Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <hankmax0000@gmail.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Tested-by: Willie Liauw <williel@supermicro.com.tw>
---
drivers/pci/hotplug/pnv_php.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 56efaf7..306728c 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -707,8 +707,12 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
} else if (sts & PCI_EXP_SLTSTA_PDC) {
ret = pnv_pci_get_presence_state(php_slot->id, &presence);
- if (!ret)
+ if (ret) {
+ dev_warn(&pdev->dev, "PCI slot [%s] error %d getting presence (0x%04x), to retry the operation.\n",
+ php_slot->name, ret, sts);
return IRQ_HANDLED;
+ }
+
added = !!(presence == OPAL_PCI_SLOT_PRESENT);
} else {
return IRQ_NONE;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [1/3] drivers/pci/hotplug: Handle presence detection change properly
2017-01-11 0:50 ` [PATCH 1/3] drivers/pci/hotplug: Handle presence detection change properly Gavin Shan
@ 2017-02-16 5:59 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-02-16 5:59 UTC (permalink / raw)
To: Gavin Shan, linuxppc-dev; +Cc: #v4.9+, williel, Gavin Shan, stable, hankmax0000
On Wed, 2017-01-11 at 00:50:06 UTC, Gavin Shan wrote:
> The surprise hotplug is driven by interrupt in PowerNV PCI hotplug
> driver. In the interrupt handler, pnv_php_interrupt(), we bail when
> pnv_pci_get_presence_state() returns zero wrongly. It causes the
> presence change event is always ignored incorrectly.
>
> This fixes the issue by bailing on error (non-zero value) returned
> from pnv_pci_get_presence_state().
>
> Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
> Cc: stable@vger.kernel.org #v4.9+
> Reported-by: Hank Chang <hankmax0000@gmail.com>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> Tested-by: Willie Liauw <williel@supermicro.com.tw>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d7d55536c6cd1f80295b6d7483ad05
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] drivers/pci/hotplug: Fix initial state for empty slot
2017-01-11 0:50 [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
2017-01-11 0:50 ` [PATCH 1/3] drivers/pci/hotplug: Handle presence detection change properly Gavin Shan
@ 2017-01-11 0:50 ` Gavin Shan
2017-01-11 0:50 ` [PATCH 3/3] drivers/pci/hotplug: Mask PDC interrupt if required Gavin Shan
2017-01-31 5:42 ` [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-11 0:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, hankmax0000, williel, Gavin Shan, stable, #v4.9+
In PowerNV PCI hotplug driver, the initial PCI slot's state is set
to PNV_PHP_STATE_POPULATED if no PCI devices are connected to the
slot. The PCI devices that are hot added to the slot won't be probed
and populated because of the check in pnv_php_enable():
/* Check if the slot has been configured */
if (php_slot->state != PNV_PHP_STATE_REGISTERED)
return 0;
This fixes the issue by leaving the slot in PNV_PHP_STATE_REGISTERED
state initially if nothing is connected to the slot.
Fixes: 360aebd85a4 ("drivers/pci/hotplug: Support surprise hotplug in powernv driver")
Cc: stable@vger.kernel.org #v4.9+
Reported-by: Hank Chang <hankmax0000@gmail.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Tested-by: Willie Liauw <williel@supermicro.com.tw>
---
drivers/pci/hotplug/pnv_php.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 306728c..41304b3 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -430,9 +430,21 @@ static int pnv_php_enable(struct pnv_php_slot *php_slot, bool rescan)
if (ret)
return ret;
- /* Proceed if there have nothing behind the slot */
- if (presence == OPAL_PCI_SLOT_EMPTY)
+ /*
+ * Proceed if there have nothing behind the slot. However,
+ * we should leave the slot in registered state at the
+ * beginning. Otherwise, the PCI devices inserted afterwards
+ * won't be probed and populated.
+ */
+ if (presence == OPAL_PCI_SLOT_EMPTY) {
+ if (!php_slot->power_state_check) {
+ php_slot->power_state_check = true;
+
+ return 0;
+ }
+
goto scan;
+ }
/*
* If the power supply to the slot is off, we can't detect
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] drivers/pci/hotplug: Mask PDC interrupt if required
2017-01-11 0:50 [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
2017-01-11 0:50 ` [PATCH 1/3] drivers/pci/hotplug: Handle presence detection change properly Gavin Shan
2017-01-11 0:50 ` [PATCH 2/3] drivers/pci/hotplug: Fix initial state for empty slot Gavin Shan
@ 2017-01-11 0:50 ` Gavin Shan
2017-01-31 5:42 ` [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-11 0:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, hankmax0000, williel, Gavin Shan
We're supporting surprise hotplug on PCI slots behind root port
or PCIe switch downstream ports, which don't claim the capability
in hardware register (offset: PCIe cap + PCI_EXP_SLTCAP). PEX8718
is one of the examples. For those PCI slots, the PDC (Presence
Detection Change) event isn't reliable and the underly (skiboot)
firmware has best judgement.
This masks the PDC event when skiboot requests by "ibm,slot-broken-pdc"
property in PCI slot's device-tree node.
Reported-by: Hank Chang <hankmax0000@gmail.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Tested-by: Willie Liauw <williel@supermicro.com.tw>
---
arch/powerpc/include/asm/pnv-pci.h | 2 ++
drivers/pci/hotplug/pnv_php.c | 27 ++++++++++++++++++++++-----
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
index 696438f..de96810 100644
--- a/arch/powerpc/include/asm/pnv-pci.h
+++ b/arch/powerpc/include/asm/pnv-pci.h
@@ -57,6 +57,8 @@ struct pnv_php_slot {
uint64_t id;
char *name;
int slot_no;
+ unsigned int flags;
+#define PNV_PHP_FLAG_BROKEN_PDC 0x1
struct kref kref;
#define PNV_PHP_STATE_INITIALIZED 0
#define PNV_PHP_STATE_REGISTERED 1
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 41304b3..63cd9f3 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -717,7 +717,8 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
if (sts & PCI_EXP_SLTSTA_DLLSC) {
pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lsts);
added = !!(lsts & PCI_EXP_LNKSTA_DLLLA);
- } else if (sts & PCI_EXP_SLTSTA_PDC) {
+ } else if (!(php_slot->flags & PNV_PHP_FLAG_BROKEN_PDC) &&
+ (sts & PCI_EXP_SLTSTA_PDC)) {
ret = pnv_pci_get_presence_state(php_slot->id, &presence);
if (ret) {
dev_warn(&pdev->dev, "PCI slot [%s] error %d getting presence (0x%04x), to retry the operation.\n",
@@ -768,6 +769,7 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
{
struct pci_dev *pdev = php_slot->pdev;
+ u32 broken_pdc = 0;
u16 sts, ctrl;
int ret;
@@ -779,9 +781,18 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
return;
}
+ /* Check PDC (Presence Detection Change) is broken or not */
+ ret = of_property_read_u32(php_slot->dn, "ibm,slot-broken-pdc",
+ &broken_pdc);
+ if (!ret && broken_pdc)
+ php_slot->flags |= PNV_PHP_FLAG_BROKEN_PDC;
+
/* Clear pending interrupts */
pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &sts);
- sts |= (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC);
+ if (php_slot->flags & PNV_PHP_FLAG_BROKEN_PDC)
+ sts |= PCI_EXP_SLTSTA_DLLSC;
+ else
+ sts |= (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC);
pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, sts);
/* Request the interrupt */
@@ -795,9 +806,15 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
/* Enable the interrupts */
pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &ctrl);
- ctrl |= (PCI_EXP_SLTCTL_HPIE |
- PCI_EXP_SLTCTL_PDCE |
- PCI_EXP_SLTCTL_DLLSCE);
+ if (php_slot->flags & PNV_PHP_FLAG_BROKEN_PDC) {
+ ctrl &= ~PCI_EXP_SLTCTL_PDCE;
+ ctrl |= (PCI_EXP_SLTCTL_HPIE |
+ PCI_EXP_SLTCTL_DLLSCE);
+ } else {
+ ctrl |= (PCI_EXP_SLTCTL_HPIE |
+ PCI_EXP_SLTCTL_PDCE |
+ PCI_EXP_SLTCTL_DLLSCE);
+ }
pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, ctrl);
/* The interrupt is initialized successfully when @irq is valid */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement
2017-01-11 0:50 [PATCH 0/3] powerpc/powernv: PCI hotplug fixes and improvement Gavin Shan
` (2 preceding siblings ...)
2017-01-11 0:50 ` [PATCH 3/3] drivers/pci/hotplug: Mask PDC interrupt if required Gavin Shan
@ 2017-01-31 5:42 ` Gavin Shan
3 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2017-01-31 5:42 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev, mpe, hankmax0000, williel
On Wed, Jan 11, 2017 at 11:50:05AM +1100, Gavin Shan wrote:
>This series fixes couple issues or improves its reliability:
>
> * The PDC event is never detected inside the IRQ handler in surprise
> hot-add path.
> * The slot's initial state should be REGISTERED state if it's emtpy.
> Otherwise, the PCI devices behind the slot won't be populated properly
> in hot-add path.
> * PDC isn't reliable for hot-add on some machines. Introduce another
> device-tree property to disable it if required by (skiboot) firmware.
>
>Gavin Shan (3):
> drivers/pci/hotplug: Handle presence detection change properly
> drivers/pci/hotplug: Fix initial state for empty slot
> drivers/pci/hotplug: Mask PDC interrupt if required
>
Michael, could you help to take a look when you have available time?
I hope those 3 patches can be merged in next merge window, which seems
to come pretty soon.
Thanks,
Gavin
> arch/powerpc/include/asm/pnv-pci.h | 2 ++
> drivers/pci/hotplug/pnv_php.c | 49 +++++++++++++++++++++++++++++++-------
> 2 files changed, 43 insertions(+), 8 deletions(-)
>
>--
>2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread