| Message ID | 20241115090321.527694-1-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] PCI: dwc: Clean up some unnecessary codes in dw_pcie_suspend_noirq() | expand |
On Fri, Nov 15, 2024 at 05:03:21PM +0800, Richard Zhu wrote: > Before sending PME_TURN_OFF, don't test the LTSSM stat. Since it's safe s/stat/state here and below > to send PME_TURN_OFF message regardless of whether the link is up or > down. So, there would be no need to test the LTSSM stat before sending > PME_TURN_OFF message. > > Only dump the message when ltssm_stat is not in DETECT and POLL. s/dump/print > In the other words, there isn't a notification when no endpoint is s/notification/error message > connected at all. > > When the endpoint is connected and after PME_TURN_OFF is issued. Just > print out one information instead of an error and exit, if the link > doesn't entry DW_PCIE_LTSSM_L2_IDLE stat. Since the recovery would be > done in the following closely dw_pcie_resume_noirq(). > How about, "Also, when the endpoint is connected and PME_TURN_OFF is sent, do not return error if the link doesn't enter L2. Just print a warning and continue with the suspend as the link will be recovered in dw_pcie_resume_noirq(). " > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > .../pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++--------- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index f7ceeb785fb0..c2053555c44b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -927,23 +927,26 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) > if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1) > return 0; > > - if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_ACT) { > - /* Only send out PME_TURN_OFF when PCIE link is up */ > - if (pci->pp.ops->pme_turn_off) > - pci->pp.ops->pme_turn_off(&pci->pp); > - else > - ret = dw_pcie_pme_turn_off(pci); > - > - if (ret) > - return ret; > + if (pci->pp.ops->pme_turn_off) > + pci->pp.ops->pme_turn_off(&pci->pp); > + else > + ret = dw_pcie_pme_turn_off(pci); > + if (ret) > + return ret; > > - ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, > - PCIE_PME_TO_L2_TIMEOUT_US/10, > - PCIE_PME_TO_L2_TIMEOUT_US, false, pci); > - if (ret) { > - dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); > - return ret; > - } > + ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, > + PCIE_PME_TO_L2_TIMEOUT_US/10, > + PCIE_PME_TO_L2_TIMEOUT_US, false, pci); > + if (ret && (val > DW_PCIE_LTSSM_DETECT_WAIT)) > + /* Only dump message when ltssm_stat isn't in DETECT and POLL */ > + dev_info(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); dev_warn() would be more appropriate. - Mani > + else > + /* > + * Refer to r6.0, sec 5.3.3.2.1, software should wait at least > + * 100ns after L2/L3 Ready before turning off refclock and > + * main power. It's harmless too when no endpoint connected. > + */ > + udelay(1); > } > > dw_pcie_stop_link(pci); > @@ -952,7 +955,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) > > pci->suspended = true; > > - return ret; > + return 0; > } > EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq); > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 347ab74ac35a..bf036e66717e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -330,6 +330,7 @@ enum dw_pcie_ltssm { > /* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */ > DW_PCIE_LTSSM_DETECT_QUIET = 0x0, > DW_PCIE_LTSSM_DETECT_ACT = 0x1, > + DW_PCIE_LTSSM_DETECT_WAIT = 0x6, > DW_PCIE_LTSSM_L0 = 0x11, > DW_PCIE_LTSSM_L2_IDLE = 0x15, > > -- > 2.37.1 > >
On Fri, Nov 15, 2024 at 05:03:21PM +0800, Richard Zhu wrote: > Before sending PME_TURN_OFF, don't test the LTSSM stat. Since it's safe > to send PME_TURN_OFF message regardless of whether the link is up or > down. So, there would be no need to test the LTSSM stat before sending > PME_TURN_OFF message. > > Only dump the message when ltssm_stat is not in DETECT and POLL. > In the other words, there isn't a notification when no endpoint is > connected at all. > > When the endpoint is connected and after PME_TURN_OFF is issued. Just > print out one information instead of an error and exit, if the link > doesn't entry DW_PCIE_LTSSM_L2_IDLE stat. Since the recovery would be > done in the following closely dw_pcie_resume_noirq(). > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > .../pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++--------- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index f7ceeb785fb0..c2053555c44b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -927,23 +927,26 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) > if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1) > return 0; > > - if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_ACT) { > - /* Only send out PME_TURN_OFF when PCIE link is up */ > - if (pci->pp.ops->pme_turn_off) > - pci->pp.ops->pme_turn_off(&pci->pp); > - else > - ret = dw_pcie_pme_turn_off(pci); > - > - if (ret) > - return ret; > + if (pci->pp.ops->pme_turn_off) > + pci->pp.ops->pme_turn_off(&pci->pp); > + else > + ret = dw_pcie_pme_turn_off(pci); > + if (ret) > + return ret; > > - ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, > - PCIE_PME_TO_L2_TIMEOUT_US/10, > - PCIE_PME_TO_L2_TIMEOUT_US, false, pci); > - if (ret) { > - dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); > - return ret; > - } > + ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, val == DW_PCIE_LTSSM_L2_IDLE || val <= DW_PCIE_LTSSM_DETECT_WAIT it will avoid wait 10ms when no link up > + PCIE_PME_TO_L2_TIMEOUT_US/10, > + PCIE_PME_TO_L2_TIMEOUT_US, false, pci); > + if (ret && (val > DW_PCIE_LTSSM_DETECT_WAIT)) > + /* Only dump message when ltssm_stat isn't in DETECT and POLL */ > + dev_info(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); > + else > + /* > + * Refer to r6.0, sec 5.3.3.2.1, software should wait at least > + * 100ns after L2/L3 Ready before turning off refclock and > + * main power. It's harmless too when no endpoint connected. > + */ > + udelay(1); > } > > dw_pcie_stop_link(pci); > @@ -952,7 +955,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) > > pci->suspended = true; > > - return ret; > + return 0; > } > EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq); > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 347ab74ac35a..bf036e66717e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -330,6 +330,7 @@ enum dw_pcie_ltssm { > /* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */ > DW_PCIE_LTSSM_DETECT_QUIET = 0x0, > DW_PCIE_LTSSM_DETECT_ACT = 0x1, > + DW_PCIE_LTSSM_DETECT_WAIT = 0x6, > DW_PCIE_LTSSM_L0 = 0x11, > DW_PCIE_LTSSM_L2_IDLE = 0x15, > > -- > 2.37.1 >
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index f7ceeb785fb0..c2053555c44b 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -927,23 +927,26 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1) return 0; - if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_ACT) { - /* Only send out PME_TURN_OFF when PCIE link is up */ - if (pci->pp.ops->pme_turn_off) - pci->pp.ops->pme_turn_off(&pci->pp); - else - ret = dw_pcie_pme_turn_off(pci); - - if (ret) - return ret; + if (pci->pp.ops->pme_turn_off) + pci->pp.ops->pme_turn_off(&pci->pp); + else + ret = dw_pcie_pme_turn_off(pci); + if (ret) + return ret; - ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, - PCIE_PME_TO_L2_TIMEOUT_US/10, - PCIE_PME_TO_L2_TIMEOUT_US, false, pci); - if (ret) { - dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); - return ret; - } + ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE, + PCIE_PME_TO_L2_TIMEOUT_US/10, + PCIE_PME_TO_L2_TIMEOUT_US, false, pci); + if (ret && (val > DW_PCIE_LTSSM_DETECT_WAIT)) + /* Only dump message when ltssm_stat isn't in DETECT and POLL */ + dev_info(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val); + else + /* + * Refer to r6.0, sec 5.3.3.2.1, software should wait at least + * 100ns after L2/L3 Ready before turning off refclock and + * main power. It's harmless too when no endpoint connected. + */ + udelay(1); } dw_pcie_stop_link(pci); @@ -952,7 +955,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci) pci->suspended = true; - return ret; + return 0; } EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 347ab74ac35a..bf036e66717e 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -330,6 +330,7 @@ enum dw_pcie_ltssm { /* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */ DW_PCIE_LTSSM_DETECT_QUIET = 0x0, DW_PCIE_LTSSM_DETECT_ACT = 0x1, + DW_PCIE_LTSSM_DETECT_WAIT = 0x6, DW_PCIE_LTSSM_L0 = 0x11, DW_PCIE_LTSSM_L2_IDLE = 0x15,
Before sending PME_TURN_OFF, don't test the LTSSM stat. Since it's safe to send PME_TURN_OFF message regardless of whether the link is up or down. So, there would be no need to test the LTSSM stat before sending PME_TURN_OFF message. Only dump the message when ltssm_stat is not in DETECT and POLL. In the other words, there isn't a notification when no endpoint is connected at all. When the endpoint is connected and after PME_TURN_OFF is issued. Just print out one information instead of an error and exit, if the link doesn't entry DW_PCIE_LTSSM_L2_IDLE stat. Since the recovery would be done in the following closely dw_pcie_resume_noirq(). Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- .../pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++--------- drivers/pci/controller/dwc/pcie-designware.h | 1 + 2 files changed, 21 insertions(+), 17 deletions(-)