* [PATCH] PCI: dwc: Check the pme_turn_off callback return value
@ 2026-11-11 4:18 Shawn Lin
2026-11-12 3:00 ` Frank Li
0 siblings, 1 reply; 3+ messages in thread
From: Shawn Lin @ 2026-11-11 4:18 UTC (permalink / raw)
To: Bjorn Helgaas, Manivannan Sadhasivam
Cc: linux-pci, imx, linuxppc-dev, linux-rockchip, linux-arm-msm,
Shawn Lin
dw_pcie_suspend_noirq() invokes the optional host ->pme_turn_off()
callback but ignores its result, so a platform that fails to send
PME_Turn_Off still has the link brought down as if the handshake had
completed.
Turn the callback into an int returning function and propagate its
error from dw_pcie_suspend_noirq(), matching how the generic
dw_pcie_pme_turn_off() path is already handled.
All existing implementations return 0 except:
- the Layerscape PF LUT variant, which now returns -ETIMEDOUT when
the PME_Turn_Off handshake does not complete instead of only
logging it;
- the Rockchip host, which now returns an error only when
PME_Turn_Off could not be sent at all. Its PME_TO_Ack and
L23-ready wait timeouts remain non-fatal and just log, consistent
with the generic path, which does not abort the suspend when an
endpoint fails to respond: such timeouts are an endpoint behaviour
and must not make the whole system suspend fail.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
drivers/pci/controller/dwc/pci-layerscape.c | 22 +++++++++++++------
.../pci/controller/dwc/pcie-designware-host.c | 4 +++-
drivers/pci/controller/dwc/pcie-designware.h | 2 +-
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 ++++++++------
drivers/pci/controller/dwc/pcie-eswin.c | 4 +++-
drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
drivers/pci/controller/dwc/pcie-ultrarisc.c | 3 ++-
8 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 38a59795f98f..643b591c789f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1575,7 +1575,7 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
* register is reserved, so the generic DWC implementation of sending the
* PME_Turn_Off message using a dummy MMIO write cannot be used.
*/
-static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+static int imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct imx_pcie *imx_pcie = to_imx_pcie(pci);
@@ -1584,6 +1584,8 @@ static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, IMX6SX_GPR12_PCIE_PM_TURN_OFF);
usleep_range(PCIE_PME_TO_L2_TIMEOUT_US/10, PCIE_PME_TO_L2_TIMEOUT_US);
+
+ return 0;
}
static const struct dw_pcie_host_ops imx_pcie_host_ops = {
diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
index 475aea3ef58a..8bd879226971 100644
--- a/drivers/pci/controller/dwc/pci-layerscape.c
+++ b/drivers/pci/controller/dwc/pci-layerscape.c
@@ -117,7 +117,7 @@ static void ls_pcie_pf_lut_writel(struct ls_pcie *pcie, u32 off, u32 val)
iowrite32(val, pcie->pf_lut_base + off);
}
-static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
+static int ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct ls_pcie *pcie = to_ls_pcie(pci);
@@ -132,8 +132,12 @@ static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
val, !(val & PF_MCR_PTOMR),
PCIE_PME_TO_L2_TIMEOUT_US/10,
PCIE_PME_TO_L2_TIMEOUT_US);
- if (ret)
+ if (ret) {
dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
+ return ret;
+ }
+
+ return 0;
}
static int ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
@@ -179,7 +183,7 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
return 0;
}
-static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
+static int scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
{
/* Send PME_Turn_Off message */
regmap_write_bits(scfg, reg, mask, mask);
@@ -195,14 +199,17 @@ static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
* to complete the PME_Turn_Off handshake.
*/
regmap_write_bits(scfg, reg, mask, 0);
+
+ return 0;
}
-static void ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
+static int ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct ls_pcie *pcie = to_ls_pcie(pci);
- scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), PMXMTTURNOFF);
+ return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index),
+ PMXMTTURNOFF);
}
static int scfg_pcie_exit_from_l2(struct regmap *scfg, u32 reg, u32 mask)
@@ -222,12 +229,13 @@ static int ls1021a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
return scfg_pcie_exit_from_l2(pcie->scfg, SCFG_PEXSFTRSTCR, PEXSR(pcie->index));
}
-static void ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
+static int ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct ls_pcie *pcie = to_ls_pcie(pci);
- scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR, PEXPME(pcie->index));
+ return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR,
+ PEXPME(pcie->index));
}
static int ls1043a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f531c5b51b3c..eb11556aca72 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1249,7 +1249,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
return 0;
if (pci->pp.ops->pme_turn_off) {
- pci->pp.ops->pme_turn_off(&pci->pp);
+ ret = pci->pp.ops->pme_turn_off(&pci->pp);
+ if (ret)
+ return ret;
} else {
ret = dw_pcie_pme_turn_off(pci);
if (ret)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 0ece9c433229..9836ea9127bd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -440,7 +440,7 @@ struct dw_pcie_host_ops {
void (*deinit)(struct dw_pcie_rp *pp);
void (*post_init)(struct dw_pcie_rp *pp);
int (*msi_init)(struct dw_pcie_rp *pp);
- void (*pme_turn_off)(struct dw_pcie_rp *pp);
+ int (*pme_turn_off)(struct dw_pcie_rp *pp);
};
struct dw_pcie_rp {
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 525a26ee1d07..ea7ef0be1214 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -464,7 +464,7 @@ static void rockchip_pcie_host_post_init(struct dw_pcie_rp *pp)
pp->bridge->broken_l1ss_resume = true;
}
-static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+static int rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
@@ -478,18 +478,16 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
PCIE_PME_TO_L2_TIMEOUT_US);
if (ret) {
- dev_warn(dev, "Failed to send PME_Turn_Off\n");
- return;
+ dev_err(dev, "Failed to send PME_Turn_Off\n");
+ return ret;
}
/* 2. Wait for PME_TO_Ack, bit 9 will be set once received */
ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_INTR_STATUS_MSG_RX,
status, status & BIT(9), PCIE_PME_TO_L2_TIMEOUT_US / 10,
PCIE_PME_TO_L2_TIMEOUT_US);
- if (ret) {
+ if (ret)
dev_warn(dev, "Failed to receive PME_TO_Ack\n");
- return;
- }
/* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
@@ -497,8 +495,12 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
status, status & PME_READY_ENTER_L23,
PCIE_PME_TO_L2_TIMEOUT_US / 10,
PCIE_PME_TO_L2_TIMEOUT_US);
- if (ret)
+ if (ret) {
dev_err(dev, "Failed to get ready to enter L23 message\n");
+ ret = 0;
+ }
+
+ return ret;
}
static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
index 5639666d5148..ed498601312f 100644
--- a/drivers/pci/controller/dwc/pcie-eswin.c
+++ b/drivers/pci/controller/dwc/pcie-eswin.c
@@ -269,7 +269,7 @@ static void eswin_pcie_host_deinit(struct dw_pcie_rp *pp)
clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
}
-static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+static int eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct eswin_pcie *pcie = to_eswin_pcie(pci);
@@ -281,6 +281,8 @@ static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
* the skip_l23_ready has been set.
*/
pp->skip_l23_ready = pcie->data->skip_l23;
+
+ return 0;
}
static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index eba1b2cb30f0..9548c6568c82 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1594,11 +1594,13 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
pcie->cfg->ops->host_post_init(pcie);
}
-static void qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
+static int qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pci->elbi_base + ELBI_SYS_CTRL);
+
+ return 0;
}
static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
index ee844ff7ed55..493b6c68b42f 100644
--- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
+++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
@@ -77,12 +77,13 @@ static int ultrarisc_pcie_host_init(struct dw_pcie_rp *pp)
return 0;
}
-static void ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+static int ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
{
/*
* DP1000 does not support sending PME_Turn_Off from the RC.
* Keep this callback empty to skip the generic MSG TLP path.
*/
+ return 0;
}
static const struct dw_pcie_host_ops ultrarisc_pcie_host_ops = {
--
2.25.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: dwc: Check the pme_turn_off callback return value
2026-11-11 4:18 [PATCH] PCI: dwc: Check the pme_turn_off callback return value Shawn Lin
@ 2026-11-12 3:00 ` Frank Li
2026-11-12 7:09 ` Shawn Lin
0 siblings, 1 reply; 3+ messages in thread
From: Frank Li @ 2026-11-12 3:00 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, Manivannan Sadhasivam, linux-pci, imx,
linuxppc-dev, linux-rockchip, linux-arm-msm
On Wed, Nov 11, 2026 at 12:18:21PM +0800, Shawn Lin wrote:
> dw_pcie_suspend_noirq() invokes the optional host ->pme_turn_off()
> callback but ignores its result, so a platform that fails to send
> PME_Turn_Off still has the link brought down as if the handshake had
> completed.
>
> Turn the callback into an int returning function and propagate its
> error from dw_pcie_suspend_noirq(), matching how the generic
> dw_pcie_pme_turn_off() path is already handled.
>
> All existing implementations return 0 except:
> - the Layerscape PF LUT variant, which now returns -ETIMEDOUT when
> the PME_Turn_Off handshake does not complete instead of only
> logging it;
> - the Rockchip host, which now returns an error only when
> PME_Turn_Off could not be sent at all.
I remember first version it is return int.
How do you know PME_Turn_off have not sent out at all? I remember spec
require continue after timeout.
Frank
> Its PME_TO_Ack and
> L23-ready wait timeouts remain non-fatal and just log, consistent
> with the generic path, which does not abort the suspend when an
> endpoint fails to respond: such timeouts are an endpoint behaviour
> and must not make the whole system suspend fail.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
> drivers/pci/controller/dwc/pci-layerscape.c | 22 +++++++++++++------
> .../pci/controller/dwc/pcie-designware-host.c | 4 +++-
> drivers/pci/controller/dwc/pcie-designware.h | 2 +-
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 ++++++++------
> drivers/pci/controller/dwc/pcie-eswin.c | 4 +++-
> drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
> drivers/pci/controller/dwc/pcie-ultrarisc.c | 3 ++-
> 8 files changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 38a59795f98f..643b591c789f 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1575,7 +1575,7 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> * register is reserved, so the generic DWC implementation of sending the
> * PME_Turn_Off message using a dummy MMIO write cannot be used.
> */
> -static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +static int imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> @@ -1584,6 +1584,8 @@ static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, IMX6SX_GPR12_PCIE_PM_TURN_OFF);
>
> usleep_range(PCIE_PME_TO_L2_TIMEOUT_US/10, PCIE_PME_TO_L2_TIMEOUT_US);
> +
> + return 0;
> }
>
> static const struct dw_pcie_host_ops imx_pcie_host_ops = {
> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
> index 475aea3ef58a..8bd879226971 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> @@ -117,7 +117,7 @@ static void ls_pcie_pf_lut_writel(struct ls_pcie *pcie, u32 off, u32 val)
> iowrite32(val, pcie->pf_lut_base + off);
> }
>
> -static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> +static int ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct ls_pcie *pcie = to_ls_pcie(pci);
> @@ -132,8 +132,12 @@ static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> val, !(val & PF_MCR_PTOMR),
> PCIE_PME_TO_L2_TIMEOUT_US/10,
> PCIE_PME_TO_L2_TIMEOUT_US);
> - if (ret)
> + if (ret) {
> dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> @@ -179,7 +183,7 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> return 0;
> }
>
> -static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
> +static int scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
> {
> /* Send PME_Turn_Off message */
> regmap_write_bits(scfg, reg, mask, mask);
> @@ -195,14 +199,17 @@ static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
> * to complete the PME_Turn_Off handshake.
> */
> regmap_write_bits(scfg, reg, mask, 0);
> +
> + return 0;
> }
>
> -static void ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> +static int ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct ls_pcie *pcie = to_ls_pcie(pci);
>
> - scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), PMXMTTURNOFF);
> + return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index),
> + PMXMTTURNOFF);
> }
>
> static int scfg_pcie_exit_from_l2(struct regmap *scfg, u32 reg, u32 mask)
> @@ -222,12 +229,13 @@ static int ls1021a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> return scfg_pcie_exit_from_l2(pcie->scfg, SCFG_PEXSFTRSTCR, PEXSR(pcie->index));
> }
>
> -static void ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> +static int ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct ls_pcie *pcie = to_ls_pcie(pci);
>
> - scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR, PEXPME(pcie->index));
> + return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR,
> + PEXPME(pcie->index));
> }
>
> static int ls1043a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f531c5b51b3c..eb11556aca72 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1249,7 +1249,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> return 0;
>
> if (pci->pp.ops->pme_turn_off) {
> - pci->pp.ops->pme_turn_off(&pci->pp);
> + ret = pci->pp.ops->pme_turn_off(&pci->pp);
> + if (ret)
> + return ret;
> } else {
> ret = dw_pcie_pme_turn_off(pci);
> if (ret)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 0ece9c433229..9836ea9127bd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -440,7 +440,7 @@ struct dw_pcie_host_ops {
> void (*deinit)(struct dw_pcie_rp *pp);
> void (*post_init)(struct dw_pcie_rp *pp);
> int (*msi_init)(struct dw_pcie_rp *pp);
> - void (*pme_turn_off)(struct dw_pcie_rp *pp);
> + int (*pme_turn_off)(struct dw_pcie_rp *pp);
> };
>
> struct dw_pcie_rp {
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 525a26ee1d07..ea7ef0be1214 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -464,7 +464,7 @@ static void rockchip_pcie_host_post_init(struct dw_pcie_rp *pp)
> pp->bridge->broken_l1ss_resume = true;
> }
>
> -static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +static int rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> @@ -478,18 +478,16 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
> PCIE_PME_TO_L2_TIMEOUT_US);
> if (ret) {
> - dev_warn(dev, "Failed to send PME_Turn_Off\n");
> - return;
> + dev_err(dev, "Failed to send PME_Turn_Off\n");
> + return ret;
> }
>
> /* 2. Wait for PME_TO_Ack, bit 9 will be set once received */
> ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_INTR_STATUS_MSG_RX,
> status, status & BIT(9), PCIE_PME_TO_L2_TIMEOUT_US / 10,
> PCIE_PME_TO_L2_TIMEOUT_US);
> - if (ret) {
> + if (ret)
> dev_warn(dev, "Failed to receive PME_TO_Ack\n");
> - return;
> - }
>
> /* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
> rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
> @@ -497,8 +495,12 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> status, status & PME_READY_ENTER_L23,
> PCIE_PME_TO_L2_TIMEOUT_US / 10,
> PCIE_PME_TO_L2_TIMEOUT_US);
> - if (ret)
> + if (ret) {
> dev_err(dev, "Failed to get ready to enter L23 message\n");
> + ret = 0;
> + }
> +
> + return ret;
> }
>
> static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
> index 5639666d5148..ed498601312f 100644
> --- a/drivers/pci/controller/dwc/pcie-eswin.c
> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> @@ -269,7 +269,7 @@ static void eswin_pcie_host_deinit(struct dw_pcie_rp *pp)
> clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> }
>
> -static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +static int eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct eswin_pcie *pcie = to_eswin_pcie(pci);
> @@ -281,6 +281,8 @@ static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> * the skip_l23_ready has been set.
> */
> pp->skip_l23_ready = pcie->data->skip_l23;
> +
> + return 0;
> }
>
> static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index eba1b2cb30f0..9548c6568c82 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1594,11 +1594,13 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
> pcie->cfg->ops->host_post_init(pcie);
> }
>
> -static void qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
> +static int qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>
> writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pci->elbi_base + ELBI_SYS_CTRL);
> +
> + return 0;
> }
>
> static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
> diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
> index ee844ff7ed55..493b6c68b42f 100644
> --- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
> +++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
> @@ -77,12 +77,13 @@ static int ultrarisc_pcie_host_init(struct dw_pcie_rp *pp)
> return 0;
> }
>
> -static void ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +static int ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> {
> /*
> * DP1000 does not support sending PME_Turn_Off from the RC.
> * Keep this callback empty to skip the generic MSG TLP path.
> */
> + return 0;
> }
>
> static const struct dw_pcie_host_ops ultrarisc_pcie_host_ops = {
> --
> 2.25.1
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: dwc: Check the pme_turn_off callback return value
2026-11-12 3:00 ` Frank Li
@ 2026-11-12 7:09 ` Shawn Lin
0 siblings, 0 replies; 3+ messages in thread
From: Shawn Lin @ 2026-11-12 7:09 UTC (permalink / raw)
To: Frank Li
Cc: shawn.lin, Bjorn Helgaas, Manivannan Sadhasivam, linux-pci, imx,
linuxppc-dev, linux-rockchip, linux-arm-msm
Hi Frank,
在 2026/11/12 星期四 11:00, Frank Li 写道:
> On Wed, Nov 11, 2026 at 12:18:21PM +0800, Shawn Lin wrote:
>> dw_pcie_suspend_noirq() invokes the optional host ->pme_turn_off()
>> callback but ignores its result, so a platform that fails to send
>> PME_Turn_Off still has the link brought down as if the handshake had
>> completed.
>>
>> Turn the callback into an int returning function and propagate its
>> error from dw_pcie_suspend_noirq(), matching how the generic
>> dw_pcie_pme_turn_off() path is already handled.
>>
>> All existing implementations return 0 except:
>> - the Layerscape PF LUT variant, which now returns -ETIMEDOUT when
>> the PME_Turn_Off handshake does not complete instead of only
>> logging it;
>> - the Rockchip host, which now returns an error only when
>> PME_Turn_Off could not be sent at all.
>
> I remember first version it is return int.
>
> How do you know PME_Turn_off have not sent out at all? I remember spec
> require continue after timeout.
>
I don't know whether other dwc instances could check it, but there is a
dedicated register for Rockchip one to know if the PME_Turn_off is sent
successfully or not.
Right, the spec does say to continue about a short 10ms timeout if no
PME_TO_Ack was received from downstream port. So you could see rockchip
driver also ignors PME_TO_Ack timeout to follow the generic path[1].
[1]
https://elixir.bootlin.com/linux/v7.3/source/drivers/pci/controller/dwc/pcie-designware-host.c#L1262
> Frank
>
>> Its PME_TO_Ack and
>> L23-ready wait timeouts remain non-fatal and just log, consistent
>> with the generic path, which does not abort the suspend when an
>> endpoint fails to respond: such timeouts are an endpoint behaviour
>> and must not make the whole system suspend fail.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
>> drivers/pci/controller/dwc/pci-layerscape.c | 22 +++++++++++++------
>> .../pci/controller/dwc/pcie-designware-host.c | 4 +++-
>> drivers/pci/controller/dwc/pcie-designware.h | 2 +-
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 ++++++++------
>> drivers/pci/controller/dwc/pcie-eswin.c | 4 +++-
>> drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
>> drivers/pci/controller/dwc/pcie-ultrarisc.c | 3 ++-
>> 8 files changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
>> index 38a59795f98f..643b591c789f 100644
>> --- a/drivers/pci/controller/dwc/pci-imx6.c
>> +++ b/drivers/pci/controller/dwc/pci-imx6.c
>> @@ -1575,7 +1575,7 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
>> * register is reserved, so the generic DWC implementation of sending the
>> * PME_Turn_Off message using a dummy MMIO write cannot be used.
>> */
>> -static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> +static int imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct imx_pcie *imx_pcie = to_imx_pcie(pci);
>> @@ -1584,6 +1584,8 @@ static void imx_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, IMX6SX_GPR12_PCIE_PM_TURN_OFF);
>>
>> usleep_range(PCIE_PME_TO_L2_TIMEOUT_US/10, PCIE_PME_TO_L2_TIMEOUT_US);
>> +
>> + return 0;
>> }
>>
>> static const struct dw_pcie_host_ops imx_pcie_host_ops = {
>> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
>> index 475aea3ef58a..8bd879226971 100644
>> --- a/drivers/pci/controller/dwc/pci-layerscape.c
>> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
>> @@ -117,7 +117,7 @@ static void ls_pcie_pf_lut_writel(struct ls_pcie *pcie, u32 off, u32 val)
>> iowrite32(val, pcie->pf_lut_base + off);
>> }
>>
>> -static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> +static int ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct ls_pcie *pcie = to_ls_pcie(pci);
>> @@ -132,8 +132,12 @@ static void ls_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> val, !(val & PF_MCR_PTOMR),
>> PCIE_PME_TO_L2_TIMEOUT_US/10,
>> PCIE_PME_TO_L2_TIMEOUT_US);
>> - if (ret)
>> + if (ret) {
>> dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> }
>>
>> static int ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
>> @@ -179,7 +183,7 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
>> return 0;
>> }
>>
>> -static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
>> +static int scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
>> {
>> /* Send PME_Turn_Off message */
>> regmap_write_bits(scfg, reg, mask, mask);
>> @@ -195,14 +199,17 @@ static void scfg_pcie_send_turnoff_msg(struct regmap *scfg, u32 reg, u32 mask)
>> * to complete the PME_Turn_Off handshake.
>> */
>> regmap_write_bits(scfg, reg, mask, 0);
>> +
>> + return 0;
>> }
>>
>> -static void ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> +static int ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct ls_pcie *pcie = to_ls_pcie(pci);
>>
>> - scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), PMXMTTURNOFF);
>> + return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMWRCR(pcie->index),
>> + PMXMTTURNOFF);
>> }
>>
>> static int scfg_pcie_exit_from_l2(struct regmap *scfg, u32 reg, u32 mask)
>> @@ -222,12 +229,13 @@ static int ls1021a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
>> return scfg_pcie_exit_from_l2(pcie->scfg, SCFG_PEXSFTRSTCR, PEXSR(pcie->index));
>> }
>>
>> -static void ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> +static int ls1043a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct ls_pcie *pcie = to_ls_pcie(pci);
>>
>> - scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR, PEXPME(pcie->index));
>> + return scfg_pcie_send_turnoff_msg(pcie->scfg, SCFG_PEXPMECR,
>> + PEXPME(pcie->index));
>> }
>>
>> static int ls1043a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index f531c5b51b3c..eb11556aca72 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -1249,7 +1249,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>> return 0;
>>
>> if (pci->pp.ops->pme_turn_off) {
>> - pci->pp.ops->pme_turn_off(&pci->pp);
>> + ret = pci->pp.ops->pme_turn_off(&pci->pp);
>> + if (ret)
>> + return ret;
>> } else {
>> ret = dw_pcie_pme_turn_off(pci);
>> if (ret)
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 0ece9c433229..9836ea9127bd 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -440,7 +440,7 @@ struct dw_pcie_host_ops {
>> void (*deinit)(struct dw_pcie_rp *pp);
>> void (*post_init)(struct dw_pcie_rp *pp);
>> int (*msi_init)(struct dw_pcie_rp *pp);
>> - void (*pme_turn_off)(struct dw_pcie_rp *pp);
>> + int (*pme_turn_off)(struct dw_pcie_rp *pp);
>> };
>>
>> struct dw_pcie_rp {
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> index 525a26ee1d07..ea7ef0be1214 100644
>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> @@ -464,7 +464,7 @@ static void rockchip_pcie_host_post_init(struct dw_pcie_rp *pp)
>> pp->bridge->broken_l1ss_resume = true;
>> }
>>
>> -static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> +static int rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>> @@ -478,18 +478,16 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
>> PCIE_PME_TO_L2_TIMEOUT_US);
>> if (ret) {
>> - dev_warn(dev, "Failed to send PME_Turn_Off\n");
>> - return;
>> + dev_err(dev, "Failed to send PME_Turn_Off\n");
>> + return ret;
>> }
>>
>> /* 2. Wait for PME_TO_Ack, bit 9 will be set once received */
>> ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_INTR_STATUS_MSG_RX,
>> status, status & BIT(9), PCIE_PME_TO_L2_TIMEOUT_US / 10,
>> PCIE_PME_TO_L2_TIMEOUT_US);
>> - if (ret) {
>> + if (ret)
>> dev_warn(dev, "Failed to receive PME_TO_Ack\n");
>> - return;
>> - }
>>
>> /* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
>> rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
>> @@ -497,8 +495,12 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> status, status & PME_READY_ENTER_L23,
>> PCIE_PME_TO_L2_TIMEOUT_US / 10,
>> PCIE_PME_TO_L2_TIMEOUT_US);
>> - if (ret)
>> + if (ret) {
>> dev_err(dev, "Failed to get ready to enter L23 message\n");
>> + ret = 0;
>> + }
>> +
>> + return ret;
>> }
>>
>> static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
>> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
>> index 5639666d5148..ed498601312f 100644
>> --- a/drivers/pci/controller/dwc/pcie-eswin.c
>> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
>> @@ -269,7 +269,7 @@ static void eswin_pcie_host_deinit(struct dw_pcie_rp *pp)
>> clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
>> }
>>
>> -static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> +static int eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct eswin_pcie *pcie = to_eswin_pcie(pci);
>> @@ -281,6 +281,8 @@ static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> * the skip_l23_ready has been set.
>> */
>> pp->skip_l23_ready = pcie->data->skip_l23;
>> +
>> + return 0;
>> }
>>
>> static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index eba1b2cb30f0..9548c6568c82 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1594,11 +1594,13 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
>> pcie->cfg->ops->host_post_init(pcie);
>> }
>>
>> -static void qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
>> +static int qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>
>> writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pci->elbi_base + ELBI_SYS_CTRL);
>> +
>> + return 0;
>> }
>>
>> static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
>> diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
>> index ee844ff7ed55..493b6c68b42f 100644
>> --- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
>> +++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
>> @@ -77,12 +77,13 @@ static int ultrarisc_pcie_host_init(struct dw_pcie_rp *pp)
>> return 0;
>> }
>>
>> -static void ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> +static int ultrarisc_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>> {
>> /*
>> * DP1000 does not support sending PME_Turn_Off from the RC.
>> * Keep this callback empty to skip the generic MSG TLP path.
>> */
>> + return 0;
>> }
>>
>> static const struct dw_pcie_host_ops ultrarisc_pcie_host_ops = {
>> --
>> 2.25.1
>>
>>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-12 7:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11 4:18 [PATCH] PCI: dwc: Check the pme_turn_off callback return value Shawn Lin
2026-11-12 3:00 ` Frank Li
2026-11-12 7:09 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox