| Message ID | 1495593023-4256-1-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, 2017-05-24 at 12:30 +1000, Gavin Shan wrote: > There are two issues in current implementation: It should return errcode > visibile to Linux, which has prefix OPAL_*. The code isn't very obvious. > > This returns OPAL_HARDWARE when the PHB is broken. Otherwise, OPAL_SUCCESS > is always returned. In the mean while, It refactors the code to make it > obvious: OPAL_PCI_SLOT_PRESENT is returned when the presence signal (low active) > or PCIe link is active. Otherwise, OPAL_PCI_SLOT_EMPTY is returned. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > hw/phb4.c | 32 +++++++++++--------------------- > 1 file changed, 11 insertions(+), 21 deletions(-) > > diff --git a/hw/phb4.c b/hw/phb4.c > index 318d934..17685e7 100644 > --- a/hw/phb4.c > +++ b/hw/phb4.c > @@ -1844,36 +1844,26 @@ static int64_t phb4_get_presence_state(struct pci_slot *slot, uint8_t *val) > if (p->state == PHB4_STATE_BROKEN) > return OPAL_HARDWARE; > > - /* Read hotplug status */ > + /* Check hotplug status */ > hps = in_be64(p->regs + PHB_PCIE_HOTPLUG_STATUS); > - > - /* Read link status */ > - dtctl = in_be64(p->regs + PHB_PCIE_DLP_TRAIN_CTL); > - > - PHBDBG(p, "hp_status=0x%016llx, dlp_train_ctl=0x%016llx\n", > - hps, dtctl); > - > - *val = OPAL_PCI_SLOT_PRESENT; > - /* Check presence detect */ > - if (hps & PHB_PCIE_HPSTAT_PRESENCE) { > - /* If it says not present but link is up, then we assume > + if (!(hps & PHB_PCIE_HPSTAT_PRESENCE)) { > + *val = OPAL_PCI_SLOT_PRESENT; > + } else { > + /* > + * If it says not present but link is up, then we assume > * we are on a broken simulation environment and still > * return a valid presence. Otherwise, not present. > */ > + dtctl = in_be64(p->regs + PHB_PCIE_DLP_TRAIN_CTL); > if (dtctl & PHB_PCIE_DLP_TL_LINKACT) { > PHBERR(p, "Presence detect 0 but link set !\n"); > - return OPAL_SHPC_DEV_PRESENT; > + *val = OPAL_PCI_SLOT_PRESENT; > + } else { > + *val = OPAL_PCI_SLOT_EMPTY; > } > - *val = OPAL_PCI_SLOT_EMPTY; > - return OPAL_SHPC_DEV_NOT_PRESENT; > } > > - /* > - * Anything else, we assume device present, the link state > - * machine will perform an early bail out if no electrical > - * signaling is established after a second. > - */ > - return OPAL_SHPC_DEV_PRESENT; > + return OPAL_SUCCESS; > } > > static int64_t phb4_get_link_state(struct pci_slot *slot, uint8_t *val)
Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > There are two issues in current implementation: It should return errcode > visibile to Linux, which has prefix OPAL_*. The code isn't very obvious. > > This returns OPAL_HARDWARE when the PHB is broken. Otherwise, OPAL_SUCCESS > is always returned. In the mean while, It refactors the code to make it > obvious: OPAL_PCI_SLOT_PRESENT is returned when the presence signal (low active) > or PCIe link is active. Otherwise, OPAL_PCI_SLOT_EMPTY is returned. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > hw/phb4.c | 32 +++++++++++--------------------- > 1 file changed, 11 insertions(+), 21 deletions(-) Thanks! Merged to master as of d065caf3b3101400744d4afb915edf57755a0c95
diff --git a/hw/phb4.c b/hw/phb4.c index 318d934..17685e7 100644 --- a/hw/phb4.c +++ b/hw/phb4.c @@ -1844,36 +1844,26 @@ static int64_t phb4_get_presence_state(struct pci_slot *slot, uint8_t *val) if (p->state == PHB4_STATE_BROKEN) return OPAL_HARDWARE; - /* Read hotplug status */ + /* Check hotplug status */ hps = in_be64(p->regs + PHB_PCIE_HOTPLUG_STATUS); - - /* Read link status */ - dtctl = in_be64(p->regs + PHB_PCIE_DLP_TRAIN_CTL); - - PHBDBG(p, "hp_status=0x%016llx, dlp_train_ctl=0x%016llx\n", - hps, dtctl); - - *val = OPAL_PCI_SLOT_PRESENT; - /* Check presence detect */ - if (hps & PHB_PCIE_HPSTAT_PRESENCE) { - /* If it says not present but link is up, then we assume + if (!(hps & PHB_PCIE_HPSTAT_PRESENCE)) { + *val = OPAL_PCI_SLOT_PRESENT; + } else { + /* + * If it says not present but link is up, then we assume * we are on a broken simulation environment and still * return a valid presence. Otherwise, not present. */ + dtctl = in_be64(p->regs + PHB_PCIE_DLP_TRAIN_CTL); if (dtctl & PHB_PCIE_DLP_TL_LINKACT) { PHBERR(p, "Presence detect 0 but link set !\n"); - return OPAL_SHPC_DEV_PRESENT; + *val = OPAL_PCI_SLOT_PRESENT; + } else { + *val = OPAL_PCI_SLOT_EMPTY; } - *val = OPAL_PCI_SLOT_EMPTY; - return OPAL_SHPC_DEV_NOT_PRESENT; } - /* - * Anything else, we assume device present, the link state - * machine will perform an early bail out if no electrical - * signaling is established after a second. - */ - return OPAL_SHPC_DEV_PRESENT; + return OPAL_SUCCESS; } static int64_t phb4_get_link_state(struct pci_slot *slot, uint8_t *val)
There are two issues in current implementation: It should return errcode visibile to Linux, which has prefix OPAL_*. The code isn't very obvious. This returns OPAL_HARDWARE when the PHB is broken. Otherwise, OPAL_SUCCESS is always returned. In the mean while, It refactors the code to make it obvious: OPAL_PCI_SLOT_PRESENT is returned when the presence signal (low active) or PCIe link is active. Otherwise, OPAL_PCI_SLOT_EMPTY is returned. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- hw/phb4.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-)