[PATCH v12 0/3] dwc general suspend/resume functionality

[PATCH v12 0/3] dwc general suspend/resume functionality

[Frank Li]

Change log
 - Change from v11 to v12
   Move exit_l2() to layerscape platform

 - Change from v10 to v11
   Fixed two missed dev_err message change base on Mani's feedback

 - Change from v9 to v10
   min change according to Mani's feedback
   Add Mani's ACK tag

 - Change from v8 to v9
   Reported-by: kernel test robot <lkp@intel.com>
   Closes: https://lore.kernel.org/oe-kbuild-all/202308042251.yGAFqeDw-lkp@intel.com/

 - Change from v7 to v8
   Add new patch to add common PCIE_PME_TO_L2_TIMEOUT_US define.
   timeout value using PCIE_PME_TO_L2_TIMEOUT_US.

 - Chnage from v6 to v7
   Remove local varible struct dw_pcie *pci = pcie->pci
   Change according to Manivannan's feedback
     remove unused lut_off and lut_base
     fixed  100 to 1000(for 1ms)
     using dev_err for timeout
     refine commit message
     fix sleep value 100 (should be 1000 for 1ms).
     use dev_err when timeout

 - Change from v5 to v6
   change to NOIRQ_SYSTEM_SLEEP_PM_OPS to remove #ifdef PM_CONFIG
   refine commit message
   change according to Manivannan's comments.
     remove reduncate step dw_pcie_set_dstate()
     return 0 when .pme_turn_off is zero
     call host_deinit() in suspend
     check .host_deinit and .host_init point before call.

 - Change from v4 to v5
   Closes: https://lore.kernel.org/oe-kbuild-all/202307211904.zExw4Q8H-lkp@intel.com/

 - Change from v3 to v4
   change according to Manivannan's comments.

 - change at v2 to v3
   Basic rewrite whole patch according rob herry suggestion.
   put common function into dwc, so more soc can share the same logic.

Frank Li (2):
  PCI: Add macro PCIE_PME_TO_L2_TIMEOUT_US
  PCI: dwc: Implement general suspend/resume functionality for L2/L3
    transitions

Hou Zhiqiang (1):
  PCI: layerscape: Add power management support for ls1028a

 drivers/pci/controller/dwc/pci-layerscape.c   | 135 ++++++++++++++++--
 .../pci/controller/dwc/pcie-designware-host.c |  71 +++++++++
 drivers/pci/controller/dwc/pcie-designware.h  |  28 ++++
 drivers/pci/pci.h                             |   6 +
 4 files changed, 231 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v12 1/3] PCI: Add macro PCIE_PME_TO_L2_TIMEOUT_US

[Frank Li]

Introduce the PCIE_PME_TO_L2_TIMEOUT_US macro to facilitate checking the
L2 ready timeout in the PCI subsystem.

Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/pci.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a4c397434057..da8156663c82 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -13,6 +13,12 @@
 
 #define PCIE_LINK_RETRAIN_TIMEOUT_MS	1000
 
+/*
+ * PCIe r6.0, sec 5.3.3.2.1 <PME Synchronization>
+ * Recommends 1ms to 10ms timeout to check L2 ready.
+ */
+#define PCIE_PME_TO_L2_TIMEOUT_US	10000
+
 extern const unsigned char pcie_link_speed[];
 extern bool pci_early_dump;
 
-- 
2.34.1

[PATCH v12 2/3] PCI: dwc: Implement general suspend/resume functionality for L2/L3 transitions

[Frank Li]

Introduce helper function dw_pcie_get_ltssm() to retrieve SMLH_LTSS_STATE.

Add callback .pme_turn_off for platform specific PME handling.

Add common dw_pcie_suspend(resume)_noirq() API to avoid duplicated code
in dwc pci host controller platform driver.

Typical L2 entry workflow/dw_pcie_suspend_noirq()

1. Transmit PME turn off signal to PCI devices and wait for PME_To_Ack.
2. Await link entering L2_IDLE state.

Typical L2 exit workflow/dw_pcie_resume_noirq()

1. Reinitialize PCI host.
2. Wait for link to become active.

Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 71 +++++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h  | 28 ++++++++
 2 files changed, 99 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cf61733bf78d..5c8cbc3afae4 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -8,6 +8,7 @@
  * Author: Jingoo Han <jg1.han@samsung.com>
  */
 
+#include <linux/iopoll.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
@@ -16,6 +17,7 @@
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
 
+#include "../../pci.h"
 #include "pcie-designware.h"
 
 static struct pci_ops dw_pcie_ops;
@@ -812,3 +814,72 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(dw_pcie_setup_rc);
+
+int dw_pcie_suspend_noirq(struct dw_pcie *pci)
+{
+	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u32 val;
+	int ret;
+
+	/*
+	 * If L1SS is supported, then do not put the link into L2 as some
+	 * devices such as NVMe expect low resume latency.
+	 */
+	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)
+		return 0;
+
+	if (!pci->pp.ops->pme_turn_off)
+		return 0;
+
+	pci->pp.ops->pme_turn_off(&pci->pp);
+
+	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;
+	}
+
+	if (pci->pp.ops->host_deinit)
+		pci->pp.ops->host_deinit(&pci->pp);
+
+	pci->suspended = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq);
+
+int dw_pcie_resume_noirq(struct dw_pcie *pci)
+{
+	int ret;
+
+	if (!pci->suspended)
+		return 0;
+
+	pci->suspended = false;
+
+	if (pci->pp.ops->host_init) {
+		ret = pci->pp.ops->host_init(&pci->pp);
+		if (ret) {
+			dev_err(pci->dev, "Host init failed: %d\n", ret);
+			return ret;
+		}
+	}
+
+	dw_pcie_setup_rc(&pci->pp);
+
+	ret = dw_pcie_start_link(pci);
+	if (ret)
+		return ret;
+
+	ret = dw_pcie_wait_for_link(pci);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dw_pcie_resume_noirq);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 615660640801..91d13f9b21b1 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -288,10 +288,21 @@ enum dw_pcie_core_rst {
 	DW_PCIE_NUM_CORE_RSTS
 };
 
+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_L0 = 0x11,
+	DW_PCIE_LTSSM_L2_IDLE = 0x15,
+
+	DW_PCIE_LTSSM_UNKNOWN = 0xFFFFFFFF,
+};
+
 struct dw_pcie_host_ops {
 	int (*host_init)(struct dw_pcie_rp *pp);
 	void (*host_deinit)(struct dw_pcie_rp *pp);
 	int (*msi_host_init)(struct dw_pcie_rp *pp);
+	void (*pme_turn_off)(struct dw_pcie_rp *pp);
 };
 
 struct dw_pcie_rp {
@@ -364,6 +375,7 @@ struct dw_pcie_ops {
 	void    (*write_dbi2)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
 			      size_t size, u32 val);
 	int	(*link_up)(struct dw_pcie *pcie);
+	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
 	int	(*start_link)(struct dw_pcie *pcie);
 	void	(*stop_link)(struct dw_pcie *pcie);
 };
@@ -393,6 +405,7 @@ struct dw_pcie {
 	struct reset_control_bulk_data	app_rsts[DW_PCIE_NUM_APP_RSTS];
 	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
 	struct gpio_desc		*pe_rst;
+	bool			suspended;
 };
 
 #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
@@ -431,6 +444,9 @@ int dw_pcie_edma_detect(struct dw_pcie *pci);
 void dw_pcie_edma_remove(struct dw_pcie *pci);
 void dw_pcie_print_link_status(struct dw_pcie *pci);
 
+int dw_pcie_suspend_noirq(struct dw_pcie *pci);
+int dw_pcie_resume_noirq(struct dw_pcie *pci);
+
 static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
 {
 	dw_pcie_write_dbi(pci, reg, 0x4, val);
@@ -502,6 +518,18 @@ static inline void dw_pcie_stop_link(struct dw_pcie *pci)
 		pci->ops->stop_link(pci);
 }
 
+static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
+{
+	u32 val;
+
+	if (pci->ops && pci->ops->get_ltssm)
+		return pci->ops->get_ltssm(pci);
+
+	val = dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0);
+
+	return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
+}
+
 #ifdef CONFIG_PCIE_DW_HOST
 irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
 int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
-- 
2.34.1

[PATCH v12 3/3] PCI: layerscape: Add power management support for ls1028a

[Frank Li]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

Add PME_Turn_off/PME_TO_Ack handshake sequence for ls1028a platform. Call
common dwc dw_pcie_suspend(resume)_noirq() function when system enter/exit
suspend state.

Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-layerscape.c | 135 ++++++++++++++++++--
 1 file changed, 126 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
index ed5fb492fe08..97b8d3329df7 100644
--- a/drivers/pci/controller/dwc/pci-layerscape.c
+++ b/drivers/pci/controller/dwc/pci-layerscape.c
@@ -8,9 +8,11 @@
  * Author: Minghuan Lian <Minghuan.Lian@freescale.com>
  */
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/interrupt.h>
 #include <linux/init.h>
+#include <linux/iopoll.h>
 #include <linux/of_pci.h>
 #include <linux/of_platform.h>
 #include <linux/of_address.h>
@@ -20,6 +22,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 
+#include "../../pci.h"
 #include "pcie-designware.h"
 
 /* PEX Internal Configuration Registers */
@@ -27,12 +30,26 @@
 #define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
 #define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
 
+/* PF Message Command Register */
+#define LS_PCIE_PF_MCR		0x2c
+#define PF_MCR_PTOMR		BIT(0)
+#define PF_MCR_EXL2S		BIT(1)
+
 #define PCIE_IATU_NUM		6
 
+struct ls_pcie_drvdata {
+	const u32 pf_off;
+	bool pm_support;
+};
+
 struct ls_pcie {
 	struct dw_pcie *pci;
+	const struct ls_pcie_drvdata *drvdata;
+	void __iomem *pf_base;
+	bool big_endian;
 };
 
+#define ls_pcie_pf_readl_addr(addr)	ls_pcie_pf_readl(pcie, addr)
 #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
 
 static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
@@ -73,6 +90,64 @@ static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
 	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
 }
 
+static u32 ls_pcie_pf_readl(struct ls_pcie *pcie, u32 off)
+{
+	if (pcie->big_endian)
+		return ioread32be(pcie->pf_base + off);
+
+	return ioread32(pcie->pf_base + off);
+}
+
+static void ls_pcie_pf_writel(struct ls_pcie *pcie, u32 off, u32 val)
+{
+	if (pcie->big_endian)
+		iowrite32be(val, pcie->pf_base + off);
+	else
+		iowrite32(val, pcie->pf_base + off);
+}
+
+static void 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);
+	u32 val;
+	int ret;
+
+	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
+	val |= PF_MCR_PTOMR;
+	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
+
+	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
+				 val, !(val & PF_MCR_PTOMR),
+				 PCIE_PME_TO_L2_TIMEOUT_US/10,
+				 PCIE_PME_TO_L2_TIMEOUT_US);
+	if (ret)
+		dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
+}
+
+static void ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct ls_pcie *pcie = to_ls_pcie(pci);
+	u32 val;
+	int ret;
+
+	/*
+	 * Set PF_MCR_EXL2S bit in LS_PCIE_PF_MCR register for the link
+	 * to exit L2 state.
+	 */
+	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
+	val |= PF_MCR_EXL2S;
+	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
+
+	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
+				 val, !(val & PF_MCR_EXL2S),
+				 1000,
+				 10000);
+	if (ret)
+		dev_err(pcie->pci->dev, "L2 exit timeout\n");
+}
+
 static int ls_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -91,18 +166,27 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
 
 static const struct dw_pcie_host_ops ls_pcie_host_ops = {
 	.host_init = ls_pcie_host_init,
+	.pme_turn_off = ls_pcie_send_turnoff_msg,
+};
+
+static const struct ls_pcie_drvdata ls1021a_drvdata = {
+};
+
+static const struct ls_pcie_drvdata layerscape_drvdata = {
+	.pf_off = 0xc0000,
+	.pm_support = true,
 };
 
 static const struct of_device_id ls_pcie_of_match[] = {
-	{ .compatible = "fsl,ls1012a-pcie", },
-	{ .compatible = "fsl,ls1021a-pcie", },
-	{ .compatible = "fsl,ls1028a-pcie", },
-	{ .compatible = "fsl,ls1043a-pcie", },
-	{ .compatible = "fsl,ls1046a-pcie", },
-	{ .compatible = "fsl,ls2080a-pcie", },
-	{ .compatible = "fsl,ls2085a-pcie", },
-	{ .compatible = "fsl,ls2088a-pcie", },
-	{ .compatible = "fsl,ls1088a-pcie", },
+	{ .compatible = "fsl,ls1012a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021a_drvdata },
+	{ .compatible = "fsl,ls1028a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1021a_drvdata },
+	{ .compatible = "fsl,ls1046a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls2080a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls2085a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls2088a-pcie", .data = &layerscape_drvdata },
+	{ .compatible = "fsl,ls1088a-pcie", .data = &layerscape_drvdata },
 	{ },
 };
 
@@ -121,6 +205,8 @@ static int ls_pcie_probe(struct platform_device *pdev)
 	if (!pci)
 		return -ENOMEM;
 
+	pcie->drvdata = of_device_get_match_data(dev);
+
 	pci->dev = dev;
 	pci->pp.ops = &ls_pcie_host_ops;
 
@@ -131,6 +217,10 @@ static int ls_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
 
+	pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
+
+	pcie->pf_base = pci->dbi_base + pcie->drvdata->pf_off;
+
 	if (!ls_pcie_is_bridge(pcie))
 		return -ENODEV;
 
@@ -139,12 +229,39 @@ static int ls_pcie_probe(struct platform_device *pdev)
 	return dw_pcie_host_init(&pci->pp);
 }
 
+static int ls_pcie_suspend_noirq(struct device *dev)
+{
+	struct ls_pcie *pcie = dev_get_drvdata(dev);
+
+	if (!pcie->drvdata->pm_support)
+		return 0;
+
+	return dw_pcie_suspend_noirq(pcie->pci);
+}
+
+static int ls_pcie_resume_noirq(struct device *dev)
+{
+	struct ls_pcie *pcie = dev_get_drvdata(dev);
+
+	if (!pcie->drvdata->pm_support)
+		return 0;
+
+	ls_pcie_exit_from_l2(&pcie->pci->pp);
+
+	return dw_pcie_resume_noirq(pcie->pci);
+}
+
+static const struct dev_pm_ops ls_pcie_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(ls_pcie_suspend_noirq, ls_pcie_resume_noirq)
+};
+
 static struct platform_driver ls_pcie_driver = {
 	.probe = ls_pcie_probe,
 	.driver = {
 		.name = "layerscape-pcie",
 		.of_match_table = ls_pcie_of_match,
 		.suppress_bind_attrs = true,
+		.pm = &ls_pcie_pm_ops,
 	},
 };
 builtin_platform_driver(ls_pcie_driver);
-- 
2.34.1

Re: [PATCH v12 3/3] PCI: layerscape: Add power management support for ls1028a

[Lorenzo Pieralisi]

On Mon, Aug 21, 2023 at 02:48:15PM -0400, Frank Li wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> Add PME_Turn_off/PME_TO_Ack handshake sequence for ls1028a platform. Call
> common dwc dw_pcie_suspend(resume)_noirq() function when system enter/exit
> suspend state.
> 
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-layerscape.c | 135 ++++++++++++++++++--
>  1 file changed, 126 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
> index ed5fb492fe08..97b8d3329df7 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> @@ -8,9 +8,11 @@
>   * Author: Minghuan Lian <Minghuan.Lian@freescale.com>
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/interrupt.h>
>  #include <linux/init.h>
> +#include <linux/iopoll.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_address.h>
> @@ -20,6 +22,7 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
>  
> +#include "../../pci.h"
>  #include "pcie-designware.h"
>  
>  /* PEX Internal Configuration Registers */
> @@ -27,12 +30,26 @@
>  #define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
>  #define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
>  
> +/* PF Message Command Register */
> +#define LS_PCIE_PF_MCR		0x2c
> +#define PF_MCR_PTOMR		BIT(0)
> +#define PF_MCR_EXL2S		BIT(1)
> +
>  #define PCIE_IATU_NUM		6
>  
> +struct ls_pcie_drvdata {
> +	const u32 pf_off;
> +	bool pm_support;
> +};
> +
>  struct ls_pcie {
>  	struct dw_pcie *pci;
> +	const struct ls_pcie_drvdata *drvdata;
> +	void __iomem *pf_base;
> +	bool big_endian;
>  };
>  
> +#define ls_pcie_pf_readl_addr(addr)	ls_pcie_pf_readl(pcie, addr)
>  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
>  
>  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
> @@ -73,6 +90,64 @@ static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
>  	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
>  }
>  
> +static u32 ls_pcie_pf_readl(struct ls_pcie *pcie, u32 off)
> +{
> +	if (pcie->big_endian)
> +		return ioread32be(pcie->pf_base + off);
> +
> +	return ioread32(pcie->pf_base + off);
> +}
> +
> +static void ls_pcie_pf_writel(struct ls_pcie *pcie, u32 off, u32 val)
> +{
> +	if (pcie->big_endian)
> +		iowrite32be(val, pcie->pf_base + off);
> +	else
> +		iowrite32(val, pcie->pf_base + off);
> +}
> +
> +static void 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);
> +	u32 val;
> +	int ret;
> +
> +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> +	val |= PF_MCR_PTOMR;
> +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> +
> +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> +				 val, !(val & PF_MCR_PTOMR),
> +				 PCIE_PME_TO_L2_TIMEOUT_US/10,
> +				 PCIE_PME_TO_L2_TIMEOUT_US);
> +	if (ret)
> +		dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
> +}
> +
> +static void ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct ls_pcie *pcie = to_ls_pcie(pci);
> +	u32 val;
> +	int ret;
> +
> +	/*
> +	 * Set PF_MCR_EXL2S bit in LS_PCIE_PF_MCR register for the link
> +	 * to exit L2 state.
> +	 */
> +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> +	val |= PF_MCR_EXL2S;
> +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> +
> +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> +				 val, !(val & PF_MCR_EXL2S),
> +				 1000,
> +				 10000);

I can add a comment myself - please explain how this delay was chosen,
any piece of information could be useful for a future developer, let
me know and I will add it.

> +	if (ret)
> +		dev_err(pcie->pci->dev, "L2 exit timeout\n");
> +}
> +
>  static int ls_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -91,18 +166,27 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  static const struct dw_pcie_host_ops ls_pcie_host_ops = {
>  	.host_init = ls_pcie_host_init,
> +	.pme_turn_off = ls_pcie_send_turnoff_msg,
> +};
> +
> +static const struct ls_pcie_drvdata ls1021a_drvdata = {

I suggest adding .pm_support = false explicitly here, I can
do it myself.

Thanks,
Lorenzo

> +};
> +
> +static const struct ls_pcie_drvdata layerscape_drvdata = {
> +	.pf_off = 0xc0000,
> +	.pm_support = true,
>  };
>  
>  static const struct of_device_id ls_pcie_of_match[] = {
> -	{ .compatible = "fsl,ls1012a-pcie", },
> -	{ .compatible = "fsl,ls1021a-pcie", },
> -	{ .compatible = "fsl,ls1028a-pcie", },
> -	{ .compatible = "fsl,ls1043a-pcie", },
> -	{ .compatible = "fsl,ls1046a-pcie", },
> -	{ .compatible = "fsl,ls2080a-pcie", },
> -	{ .compatible = "fsl,ls2085a-pcie", },
> -	{ .compatible = "fsl,ls2088a-pcie", },
> -	{ .compatible = "fsl,ls1088a-pcie", },
> +	{ .compatible = "fsl,ls1012a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021a_drvdata },
> +	{ .compatible = "fsl,ls1028a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1021a_drvdata },
> +	{ .compatible = "fsl,ls1046a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls2080a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls2085a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls2088a-pcie", .data = &layerscape_drvdata },
> +	{ .compatible = "fsl,ls1088a-pcie", .data = &layerscape_drvdata },
>  	{ },
>  };
>  
> @@ -121,6 +205,8 @@ static int ls_pcie_probe(struct platform_device *pdev)
>  	if (!pci)
>  		return -ENOMEM;
>  
> +	pcie->drvdata = of_device_get_match_data(dev);
> +
>  	pci->dev = dev;
>  	pci->pp.ops = &ls_pcie_host_ops;
>  
> @@ -131,6 +217,10 @@ static int ls_pcie_probe(struct platform_device *pdev)
>  	if (IS_ERR(pci->dbi_base))
>  		return PTR_ERR(pci->dbi_base);
>  
> +	pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
> +
> +	pcie->pf_base = pci->dbi_base + pcie->drvdata->pf_off;
> +
>  	if (!ls_pcie_is_bridge(pcie))
>  		return -ENODEV;
>  
> @@ -139,12 +229,39 @@ static int ls_pcie_probe(struct platform_device *pdev)
>  	return dw_pcie_host_init(&pci->pp);
>  }
>  
> +static int ls_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> +
> +	if (!pcie->drvdata->pm_support)
> +		return 0;
> +
> +	return dw_pcie_suspend_noirq(pcie->pci);
> +}
> +
> +static int ls_pcie_resume_noirq(struct device *dev)
> +{
> +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> +
> +	if (!pcie->drvdata->pm_support)
> +		return 0;
> +
> +	ls_pcie_exit_from_l2(&pcie->pci->pp);
> +
> +	return dw_pcie_resume_noirq(pcie->pci);
> +}
> +
> +static const struct dev_pm_ops ls_pcie_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(ls_pcie_suspend_noirq, ls_pcie_resume_noirq)
> +};
> +
>  static struct platform_driver ls_pcie_driver = {
>  	.probe = ls_pcie_probe,
>  	.driver = {
>  		.name = "layerscape-pcie",
>  		.of_match_table = ls_pcie_of_match,
>  		.suppress_bind_attrs = true,
> +		.pm = &ls_pcie_pm_ops,
>  	},
>  };
>  builtin_platform_driver(ls_pcie_driver);
> -- 
> 2.34.1
> 

Re: [PATCH v12 3/3] PCI: layerscape: Add power management support for ls1028a

[Frank Li]

On Tue, Aug 22, 2023 at 11:02:11AM +0200, Lorenzo Pieralisi wrote:
> On Mon, Aug 21, 2023 at 02:48:15PM -0400, Frank Li wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > 
> > Add PME_Turn_off/PME_TO_Ack handshake sequence for ls1028a platform. Call
> > common dwc dw_pcie_suspend(resume)_noirq() function when system enter/exit
> > suspend state.
> > 
> > Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-layerscape.c | 135 ++++++++++++++++++--
> >  1 file changed, 126 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
> > index ed5fb492fe08..97b8d3329df7 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > @@ -8,9 +8,11 @@
> >   * Author: Minghuan Lian <Minghuan.Lian@freescale.com>
> >   */
> >  
> > +#include <linux/delay.h>
> >  #include <linux/kernel.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/init.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/of_pci.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/of_address.h>
> > @@ -20,6 +22,7 @@
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/regmap.h>
> >  
> > +#include "../../pci.h"
> >  #include "pcie-designware.h"
> >  
> >  /* PEX Internal Configuration Registers */
> > @@ -27,12 +30,26 @@
> >  #define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> >  #define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
> >  
> > +/* PF Message Command Register */
> > +#define LS_PCIE_PF_MCR		0x2c
> > +#define PF_MCR_PTOMR		BIT(0)
> > +#define PF_MCR_EXL2S		BIT(1)
> > +
> >  #define PCIE_IATU_NUM		6
> >  
> > +struct ls_pcie_drvdata {
> > +	const u32 pf_off;
> > +	bool pm_support;
> > +};
> > +
> >  struct ls_pcie {
> >  	struct dw_pcie *pci;
> > +	const struct ls_pcie_drvdata *drvdata;
> > +	void __iomem *pf_base;
> > +	bool big_endian;
> >  };
> >  
> > +#define ls_pcie_pf_readl_addr(addr)	ls_pcie_pf_readl(pcie, addr)
> >  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
> >  
> >  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
> > @@ -73,6 +90,64 @@ static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> >  	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> >  }
> >  
> > +static u32 ls_pcie_pf_readl(struct ls_pcie *pcie, u32 off)
> > +{
> > +	if (pcie->big_endian)
> > +		return ioread32be(pcie->pf_base + off);
> > +
> > +	return ioread32(pcie->pf_base + off);
> > +}
> > +
> > +static void ls_pcie_pf_writel(struct ls_pcie *pcie, u32 off, u32 val)
> > +{
> > +	if (pcie->big_endian)
> > +		iowrite32be(val, pcie->pf_base + off);
> > +	else
> > +		iowrite32(val, pcie->pf_base + off);
> > +}
> > +
> > +static void 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);
> > +	u32 val;
> > +	int ret;
> > +
> > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > +	val |= PF_MCR_PTOMR;
> > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > +
> > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > +				 val, !(val & PF_MCR_PTOMR),
> > +				 PCIE_PME_TO_L2_TIMEOUT_US/10,
> > +				 PCIE_PME_TO_L2_TIMEOUT_US);
> > +	if (ret)
> > +		dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
> > +}
> > +
> > +static void ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct ls_pcie *pcie = to_ls_pcie(pci);
> > +	u32 val;
> > +	int ret;
> > +
> > +	/*
> > +	 * Set PF_MCR_EXL2S bit in LS_PCIE_PF_MCR register for the link
> > +	 * to exit L2 state.
> > +	 */
> > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > +	val |= PF_MCR_EXL2S;
> > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > +
> > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > +				 val, !(val & PF_MCR_EXL2S),
> > +				 1000,
> > +				 10000);
> 
> I can add a comment myself - please explain how this delay was chosen,
> any piece of information could be useful for a future developer, let
> me know and I will add it.

(This value is coming from exited downstream code. It is really hard to
find original owner and designer to find the reason).

How about just add below comments?

"No specific specification defines this timeout value. The timemout of 10ms
was chosen by the engineer as a sufficient amount of time to allow the
hardware to complete its actions."

> 
> > +	if (ret)
> > +		dev_err(pcie->pci->dev, "L2 exit timeout\n");
> > +}
> > +
> >  static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -91,18 +166,27 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> >  
> >  static const struct dw_pcie_host_ops ls_pcie_host_ops = {
> >  	.host_init = ls_pcie_host_init,
> > +	.pme_turn_off = ls_pcie_send_turnoff_msg,
> > +};
> > +
> > +static const struct ls_pcie_drvdata ls1021a_drvdata = {
> 
> I suggest adding .pm_support = false explicitly here, I can
> do it myself.
> 
> Thanks,
> Lorenzo

Thanks!

> 
> > +};
> > +
> > +static const struct ls_pcie_drvdata layerscape_drvdata = {
> > +	.pf_off = 0xc0000,
> > +	.pm_support = true,
> >  };
> >  
> >  static const struct of_device_id ls_pcie_of_match[] = {
> > -	{ .compatible = "fsl,ls1012a-pcie", },
> > -	{ .compatible = "fsl,ls1021a-pcie", },
> > -	{ .compatible = "fsl,ls1028a-pcie", },
> > -	{ .compatible = "fsl,ls1043a-pcie", },
> > -	{ .compatible = "fsl,ls1046a-pcie", },
> > -	{ .compatible = "fsl,ls2080a-pcie", },
> > -	{ .compatible = "fsl,ls2085a-pcie", },
> > -	{ .compatible = "fsl,ls2088a-pcie", },
> > -	{ .compatible = "fsl,ls1088a-pcie", },
> > +	{ .compatible = "fsl,ls1012a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021a_drvdata },
> > +	{ .compatible = "fsl,ls1028a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1021a_drvdata },
> > +	{ .compatible = "fsl,ls1046a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls2080a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls2085a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls2088a-pcie", .data = &layerscape_drvdata },
> > +	{ .compatible = "fsl,ls1088a-pcie", .data = &layerscape_drvdata },
> >  	{ },
> >  };
> >  
> > @@ -121,6 +205,8 @@ static int ls_pcie_probe(struct platform_device *pdev)
> >  	if (!pci)
> >  		return -ENOMEM;
> >  
> > +	pcie->drvdata = of_device_get_match_data(dev);
> > +
> >  	pci->dev = dev;
> >  	pci->pp.ops = &ls_pcie_host_ops;
> >  
> > @@ -131,6 +217,10 @@ static int ls_pcie_probe(struct platform_device *pdev)
> >  	if (IS_ERR(pci->dbi_base))
> >  		return PTR_ERR(pci->dbi_base);
> >  
> > +	pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
> > +
> > +	pcie->pf_base = pci->dbi_base + pcie->drvdata->pf_off;
> > +
> >  	if (!ls_pcie_is_bridge(pcie))
> >  		return -ENODEV;
> >  
> > @@ -139,12 +229,39 @@ static int ls_pcie_probe(struct platform_device *pdev)
> >  	return dw_pcie_host_init(&pci->pp);
> >  }
> >  
> > +static int ls_pcie_suspend_noirq(struct device *dev)
> > +{
> > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > +
> > +	if (!pcie->drvdata->pm_support)
> > +		return 0;
> > +
> > +	return dw_pcie_suspend_noirq(pcie->pci);
> > +}
> > +
> > +static int ls_pcie_resume_noirq(struct device *dev)
> > +{
> > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > +
> > +	if (!pcie->drvdata->pm_support)
> > +		return 0;
> > +
> > +	ls_pcie_exit_from_l2(&pcie->pci->pp);
> > +
> > +	return dw_pcie_resume_noirq(pcie->pci);
> > +}
> > +
> > +static const struct dev_pm_ops ls_pcie_pm_ops = {
> > +	NOIRQ_SYSTEM_SLEEP_PM_OPS(ls_pcie_suspend_noirq, ls_pcie_resume_noirq)
> > +};
> > +
> >  static struct platform_driver ls_pcie_driver = {
> >  	.probe = ls_pcie_probe,
> >  	.driver = {
> >  		.name = "layerscape-pcie",
> >  		.of_match_table = ls_pcie_of_match,
> >  		.suppress_bind_attrs = true,
> > +		.pm = &ls_pcie_pm_ops,
> >  	},
> >  };
> >  builtin_platform_driver(ls_pcie_driver);
> > -- 
> > 2.34.1
> > 

Re: [PATCH v12 3/3] PCI: layerscape: Add power management support for ls1028a

[Manivannan Sadhasivam]

On Tue, Aug 22, 2023 at 10:30:08AM -0400, Frank Li wrote:
> On Tue, Aug 22, 2023 at 11:02:11AM +0200, Lorenzo Pieralisi wrote:
> > On Mon, Aug 21, 2023 at 02:48:15PM -0400, Frank Li wrote:
> > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > > 
> > > Add PME_Turn_off/PME_TO_Ack handshake sequence for ls1028a platform. Call
> > > common dwc dw_pcie_suspend(resume)_noirq() function when system enter/exit
> > > suspend state.
> > > 
> > > Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-layerscape.c | 135 ++++++++++++++++++--
> > >  1 file changed, 126 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
> > > index ed5fb492fe08..97b8d3329df7 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > > @@ -8,9 +8,11 @@
> > >   * Author: Minghuan Lian <Minghuan.Lian@freescale.com>
> > >   */
> > >  
> > > +#include <linux/delay.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/interrupt.h>
> > >  #include <linux/init.h>
> > > +#include <linux/iopoll.h>
> > >  #include <linux/of_pci.h>
> > >  #include <linux/of_platform.h>
> > >  #include <linux/of_address.h>
> > > @@ -20,6 +22,7 @@
> > >  #include <linux/mfd/syscon.h>
> > >  #include <linux/regmap.h>
> > >  
> > > +#include "../../pci.h"
> > >  #include "pcie-designware.h"
> > >  
> > >  /* PEX Internal Configuration Registers */
> > > @@ -27,12 +30,26 @@
> > >  #define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> > >  #define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
> > >  
> > > +/* PF Message Command Register */
> > > +#define LS_PCIE_PF_MCR		0x2c
> > > +#define PF_MCR_PTOMR		BIT(0)
> > > +#define PF_MCR_EXL2S		BIT(1)
> > > +
> > >  #define PCIE_IATU_NUM		6
> > >  
> > > +struct ls_pcie_drvdata {
> > > +	const u32 pf_off;
> > > +	bool pm_support;
> > > +};
> > > +
> > >  struct ls_pcie {
> > >  	struct dw_pcie *pci;
> > > +	const struct ls_pcie_drvdata *drvdata;
> > > +	void __iomem *pf_base;
> > > +	bool big_endian;
> > >  };
> > >  
> > > +#define ls_pcie_pf_readl_addr(addr)	ls_pcie_pf_readl(pcie, addr)
> > >  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
> > >  
> > >  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
> > > @@ -73,6 +90,64 @@ static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> > >  	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> > >  }
> > >  
> > > +static u32 ls_pcie_pf_readl(struct ls_pcie *pcie, u32 off)
> > > +{
> > > +	if (pcie->big_endian)
> > > +		return ioread32be(pcie->pf_base + off);
> > > +
> > > +	return ioread32(pcie->pf_base + off);
> > > +}
> > > +
> > > +static void ls_pcie_pf_writel(struct ls_pcie *pcie, u32 off, u32 val)
> > > +{
> > > +	if (pcie->big_endian)
> > > +		iowrite32be(val, pcie->pf_base + off);
> > > +	else
> > > +		iowrite32(val, pcie->pf_base + off);
> > > +}
> > > +
> > > +static void 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);
> > > +	u32 val;
> > > +	int ret;
> > > +
> > > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > > +	val |= PF_MCR_PTOMR;
> > > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > > +
> > > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > > +				 val, !(val & PF_MCR_PTOMR),
> > > +				 PCIE_PME_TO_L2_TIMEOUT_US/10,
> > > +				 PCIE_PME_TO_L2_TIMEOUT_US);
> > > +	if (ret)
> > > +		dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
> > > +}
> > > +
> > > +static void ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> > > +{
> > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > +	struct ls_pcie *pcie = to_ls_pcie(pci);
> > > +	u32 val;
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Set PF_MCR_EXL2S bit in LS_PCIE_PF_MCR register for the link
> > > +	 * to exit L2 state.
> > > +	 */
> > > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > > +	val |= PF_MCR_EXL2S;
> > > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > > +
> > > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > > +				 val, !(val & PF_MCR_EXL2S),
> > > +				 1000,
> > > +				 10000);
> > 
> > I can add a comment myself - please explain how this delay was chosen,
> > any piece of information could be useful for a future developer, let
> > me know and I will add it.
> 
> (This value is coming from exited downstream code. It is really hard to
> find original owner and designer to find the reason).
> 
> How about just add below comments?
> 
> "No specific specification defines this timeout value. The timemout of 10ms
> was chosen by the engineer as a sufficient amount of time to allow the
> hardware to complete its actions."
> 

How about, "L1 exit timeout of 10ms is not defined in the spec, but rather
chosen based on the practical observations."

- Mani

> > 
> > > +	if (ret)
> > > +		dev_err(pcie->pci->dev, "L2 exit timeout\n");
> > > +}
> > > +
> > >  static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> > >  {
> > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > @@ -91,18 +166,27 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> > >  
> > >  static const struct dw_pcie_host_ops ls_pcie_host_ops = {
> > >  	.host_init = ls_pcie_host_init,
> > > +	.pme_turn_off = ls_pcie_send_turnoff_msg,
> > > +};
> > > +
> > > +static const struct ls_pcie_drvdata ls1021a_drvdata = {
> > 
> > I suggest adding .pm_support = false explicitly here, I can
> > do it myself.
> > 
> > Thanks,
> > Lorenzo
> 
> Thanks!
> 
> > 
> > > +};
> > > +
> > > +static const struct ls_pcie_drvdata layerscape_drvdata = {
> > > +	.pf_off = 0xc0000,
> > > +	.pm_support = true,
> > >  };
> > >  
> > >  static const struct of_device_id ls_pcie_of_match[] = {
> > > -	{ .compatible = "fsl,ls1012a-pcie", },
> > > -	{ .compatible = "fsl,ls1021a-pcie", },
> > > -	{ .compatible = "fsl,ls1028a-pcie", },
> > > -	{ .compatible = "fsl,ls1043a-pcie", },
> > > -	{ .compatible = "fsl,ls1046a-pcie", },
> > > -	{ .compatible = "fsl,ls2080a-pcie", },
> > > -	{ .compatible = "fsl,ls2085a-pcie", },
> > > -	{ .compatible = "fsl,ls2088a-pcie", },
> > > -	{ .compatible = "fsl,ls1088a-pcie", },
> > > +	{ .compatible = "fsl,ls1012a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021a_drvdata },
> > > +	{ .compatible = "fsl,ls1028a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1021a_drvdata },
> > > +	{ .compatible = "fsl,ls1046a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls2080a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls2085a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls2088a-pcie", .data = &layerscape_drvdata },
> > > +	{ .compatible = "fsl,ls1088a-pcie", .data = &layerscape_drvdata },
> > >  	{ },
> > >  };
> > >  
> > > @@ -121,6 +205,8 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > >  	if (!pci)
> > >  		return -ENOMEM;
> > >  
> > > +	pcie->drvdata = of_device_get_match_data(dev);
> > > +
> > >  	pci->dev = dev;
> > >  	pci->pp.ops = &ls_pcie_host_ops;
> > >  
> > > @@ -131,6 +217,10 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(pci->dbi_base))
> > >  		return PTR_ERR(pci->dbi_base);
> > >  
> > > +	pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
> > > +
> > > +	pcie->pf_base = pci->dbi_base + pcie->drvdata->pf_off;
> > > +
> > >  	if (!ls_pcie_is_bridge(pcie))
> > >  		return -ENODEV;
> > >  
> > > @@ -139,12 +229,39 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > >  	return dw_pcie_host_init(&pci->pp);
> > >  }
> > >  
> > > +static int ls_pcie_suspend_noirq(struct device *dev)
> > > +{
> > > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > > +
> > > +	if (!pcie->drvdata->pm_support)
> > > +		return 0;
> > > +
> > > +	return dw_pcie_suspend_noirq(pcie->pci);
> > > +}
> > > +
> > > +static int ls_pcie_resume_noirq(struct device *dev)
> > > +{
> > > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > > +
> > > +	if (!pcie->drvdata->pm_support)
> > > +		return 0;
> > > +
> > > +	ls_pcie_exit_from_l2(&pcie->pci->pp);
> > > +
> > > +	return dw_pcie_resume_noirq(pcie->pci);
> > > +}
> > > +
> > > +static const struct dev_pm_ops ls_pcie_pm_ops = {
> > > +	NOIRQ_SYSTEM_SLEEP_PM_OPS(ls_pcie_suspend_noirq, ls_pcie_resume_noirq)
> > > +};
> > > +
> > >  static struct platform_driver ls_pcie_driver = {
> > >  	.probe = ls_pcie_probe,
> > >  	.driver = {
> > >  		.name = "layerscape-pcie",
> > >  		.of_match_table = ls_pcie_of_match,
> > >  		.suppress_bind_attrs = true,
> > > +		.pm = &ls_pcie_pm_ops,
> > >  	},
> > >  };
> > >  builtin_platform_driver(ls_pcie_driver);
> > > -- 
> > > 2.34.1
> > > 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v12 3/3] PCI: layerscape: Add power management support for ls1028a

[Frank Li]

On Wed, Aug 23, 2023 at 01:23:05PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Aug 22, 2023 at 10:30:08AM -0400, Frank Li wrote:
> > On Tue, Aug 22, 2023 at 11:02:11AM +0200, Lorenzo Pieralisi wrote:
> > > On Mon, Aug 21, 2023 at 02:48:15PM -0400, Frank Li wrote:
> > > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > > > 
> > > > Add PME_Turn_off/PME_TO_Ack handshake sequence for ls1028a platform. Call
> > > > common dwc dw_pcie_suspend(resume)_noirq() function when system enter/exit
> > > > suspend state.
> > > > 
> > > > Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pci-layerscape.c | 135 ++++++++++++++++++--
> > > >  1 file changed, 126 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
> > > > index ed5fb492fe08..97b8d3329df7 100644
> > > > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > > > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > > > @@ -8,9 +8,11 @@
> > > >   * Author: Minghuan Lian <Minghuan.Lian@freescale.com>
> > > >   */
> > > >  
> > > > +#include <linux/delay.h>
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/interrupt.h>
> > > >  #include <linux/init.h>
> > > > +#include <linux/iopoll.h>
> > > >  #include <linux/of_pci.h>
> > > >  #include <linux/of_platform.h>
> > > >  #include <linux/of_address.h>
> > > > @@ -20,6 +22,7 @@
> > > >  #include <linux/mfd/syscon.h>
> > > >  #include <linux/regmap.h>
> > > >  
> > > > +#include "../../pci.h"
> > > >  #include "pcie-designware.h"
> > > >  
> > > >  /* PEX Internal Configuration Registers */
> > > > @@ -27,12 +30,26 @@
> > > >  #define PCIE_ABSERR		0x8d0 /* Bridge Slave Error Response Register */
> > > >  #define PCIE_ABSERR_SETTING	0x9401 /* Forward error of non-posted request */
> > > >  
> > > > +/* PF Message Command Register */
> > > > +#define LS_PCIE_PF_MCR		0x2c
> > > > +#define PF_MCR_PTOMR		BIT(0)
> > > > +#define PF_MCR_EXL2S		BIT(1)
> > > > +
> > > >  #define PCIE_IATU_NUM		6
> > > >  
> > > > +struct ls_pcie_drvdata {
> > > > +	const u32 pf_off;
> > > > +	bool pm_support;
> > > > +};
> > > > +
> > > >  struct ls_pcie {
> > > >  	struct dw_pcie *pci;
> > > > +	const struct ls_pcie_drvdata *drvdata;
> > > > +	void __iomem *pf_base;
> > > > +	bool big_endian;
> > > >  };
> > > >  
> > > > +#define ls_pcie_pf_readl_addr(addr)	ls_pcie_pf_readl(pcie, addr)
> > > >  #define to_ls_pcie(x)	dev_get_drvdata((x)->dev)
> > > >  
> > > >  static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
> > > > @@ -73,6 +90,64 @@ static void ls_pcie_fix_error_response(struct ls_pcie *pcie)
> > > >  	iowrite32(PCIE_ABSERR_SETTING, pci->dbi_base + PCIE_ABSERR);
> > > >  }
> > > >  
> > > > +static u32 ls_pcie_pf_readl(struct ls_pcie *pcie, u32 off)
> > > > +{
> > > > +	if (pcie->big_endian)
> > > > +		return ioread32be(pcie->pf_base + off);
> > > > +
> > > > +	return ioread32(pcie->pf_base + off);
> > > > +}
> > > > +
> > > > +static void ls_pcie_pf_writel(struct ls_pcie *pcie, u32 off, u32 val)
> > > > +{
> > > > +	if (pcie->big_endian)
> > > > +		iowrite32be(val, pcie->pf_base + off);
> > > > +	else
> > > > +		iowrite32(val, pcie->pf_base + off);
> > > > +}
> > > > +
> > > > +static void 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);
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > > > +	val |= PF_MCR_PTOMR;
> > > > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > > > +
> > > > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > > > +				 val, !(val & PF_MCR_PTOMR),
> > > > +				 PCIE_PME_TO_L2_TIMEOUT_US/10,
> > > > +				 PCIE_PME_TO_L2_TIMEOUT_US);
> > > > +	if (ret)
> > > > +		dev_err(pcie->pci->dev, "PME_Turn_off timeout\n");
> > > > +}
> > > > +
> > > > +static void ls_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> > > > +{
> > > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > +	struct ls_pcie *pcie = to_ls_pcie(pci);
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	/*
> > > > +	 * Set PF_MCR_EXL2S bit in LS_PCIE_PF_MCR register for the link
> > > > +	 * to exit L2 state.
> > > > +	 */
> > > > +	val = ls_pcie_pf_readl(pcie, LS_PCIE_PF_MCR);
> > > > +	val |= PF_MCR_EXL2S;
> > > > +	ls_pcie_pf_writel(pcie, LS_PCIE_PF_MCR, val);
> > > > +
> > > > +	ret = readx_poll_timeout(ls_pcie_pf_readl_addr, LS_PCIE_PF_MCR,
> > > > +				 val, !(val & PF_MCR_EXL2S),
> > > > +				 1000,
> > > > +				 10000);
> > > 
> > > I can add a comment myself - please explain how this delay was chosen,
> > > any piece of information could be useful for a future developer, let
> > > me know and I will add it.
> > 
> > (This value is coming from exited downstream code. It is really hard to
> > find original owner and designer to find the reason).
> > 
> > How about just add below comments?
> > 
> > "No specific specification defines this timeout value. The timemout of 10ms
> > was chosen by the engineer as a sufficient amount of time to allow the
> > hardware to complete its actions."
> > 
> 
> How about, "L1 exit timeout of 10ms is not defined in the spec, but rather
> chosen based on the practical observations."

Great!

Frank

> 
> - Mani
> 
> > > 
> > > > +	if (ret)
> > > > +		dev_err(pcie->pci->dev, "L2 exit timeout\n");
> > > > +}
> > > > +
> > > >  static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> > > >  {
> > > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > @@ -91,18 +166,27 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> > > >  
> > > >  static const struct dw_pcie_host_ops ls_pcie_host_ops = {
> > > >  	.host_init = ls_pcie_host_init,
> > > > +	.pme_turn_off = ls_pcie_send_turnoff_msg,
> > > > +};
> > > > +
> > > > +static const struct ls_pcie_drvdata ls1021a_drvdata = {
> > > 
> > > I suggest adding .pm_support = false explicitly here, I can
> > > do it myself.
> > > 
> > > Thanks,
> > > Lorenzo
> > 
> > Thanks!
> > 
> > > 
> > > > +};
> > > > +
> > > > +static const struct ls_pcie_drvdata layerscape_drvdata = {
> > > > +	.pf_off = 0xc0000,
> > > > +	.pm_support = true,
> > > >  };
> > > >  
> > > >  static const struct of_device_id ls_pcie_of_match[] = {
> > > > -	{ .compatible = "fsl,ls1012a-pcie", },
> > > > -	{ .compatible = "fsl,ls1021a-pcie", },
> > > > -	{ .compatible = "fsl,ls1028a-pcie", },
> > > > -	{ .compatible = "fsl,ls1043a-pcie", },
> > > > -	{ .compatible = "fsl,ls1046a-pcie", },
> > > > -	{ .compatible = "fsl,ls2080a-pcie", },
> > > > -	{ .compatible = "fsl,ls2085a-pcie", },
> > > > -	{ .compatible = "fsl,ls2088a-pcie", },
> > > > -	{ .compatible = "fsl,ls1088a-pcie", },
> > > > +	{ .compatible = "fsl,ls1012a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021a_drvdata },
> > > > +	{ .compatible = "fsl,ls1028a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1021a_drvdata },
> > > > +	{ .compatible = "fsl,ls1046a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls2080a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls2085a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls2088a-pcie", .data = &layerscape_drvdata },
> > > > +	{ .compatible = "fsl,ls1088a-pcie", .data = &layerscape_drvdata },
> > > >  	{ },
> > > >  };
> > > >  
> > > > @@ -121,6 +205,8 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > > >  	if (!pci)
> > > >  		return -ENOMEM;
> > > >  
> > > > +	pcie->drvdata = of_device_get_match_data(dev);
> > > > +
> > > >  	pci->dev = dev;
> > > >  	pci->pp.ops = &ls_pcie_host_ops;
> > > >  
> > > > @@ -131,6 +217,10 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > > >  	if (IS_ERR(pci->dbi_base))
> > > >  		return PTR_ERR(pci->dbi_base);
> > > >  
> > > > +	pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
> > > > +
> > > > +	pcie->pf_base = pci->dbi_base + pcie->drvdata->pf_off;
> > > > +
> > > >  	if (!ls_pcie_is_bridge(pcie))
> > > >  		return -ENODEV;
> > > >  
> > > > @@ -139,12 +229,39 @@ static int ls_pcie_probe(struct platform_device *pdev)
> > > >  	return dw_pcie_host_init(&pci->pp);
> > > >  }
> > > >  
> > > > +static int ls_pcie_suspend_noirq(struct device *dev)
> > > > +{
> > > > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > > > +
> > > > +	if (!pcie->drvdata->pm_support)
> > > > +		return 0;
> > > > +
> > > > +	return dw_pcie_suspend_noirq(pcie->pci);
> > > > +}
> > > > +
> > > > +static int ls_pcie_resume_noirq(struct device *dev)
> > > > +{
> > > > +	struct ls_pcie *pcie = dev_get_drvdata(dev);
> > > > +
> > > > +	if (!pcie->drvdata->pm_support)
> > > > +		return 0;
> > > > +
> > > > +	ls_pcie_exit_from_l2(&pcie->pci->pp);
> > > > +
> > > > +	return dw_pcie_resume_noirq(pcie->pci);
> > > > +}
> > > > +
> > > > +static const struct dev_pm_ops ls_pcie_pm_ops = {
> > > > +	NOIRQ_SYSTEM_SLEEP_PM_OPS(ls_pcie_suspend_noirq, ls_pcie_resume_noirq)
> > > > +};
> > > > +
> > > >  static struct platform_driver ls_pcie_driver = {
> > > >  	.probe = ls_pcie_probe,
> > > >  	.driver = {
> > > >  		.name = "layerscape-pcie",
> > > >  		.of_match_table = ls_pcie_of_match,
> > > >  		.suppress_bind_attrs = true,
> > > > +		.pm = &ls_pcie_pm_ops,
> > > >  	},
> > > >  };
> > > >  builtin_platform_driver(ls_pcie_driver);
> > > > -- 
> > > > 2.34.1
> > > > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v12 0/3] dwc general suspend/resume functionality

[Lorenzo Pieralisi]

On Mon, 21 Aug 2023 14:48:12 -0400, Frank Li wrote:
> Change log
>  - Change from v11 to v12
>    Move exit_l2() to layerscape platform
> 
>  - Change from v10 to v11
>    Fixed two missed dev_err message change base on Mani's feedback
> 
> [...]

Applied to controller/dwc, thanks!

[1/3] PCI: Add macro PCIE_PME_TO_L2_TIMEOUT_US
      https://git.kernel.org/pci/pci/c/e78bd50b4078
[2/3] PCI: dwc: Implement general suspend/resume functionality for L2/L3 transitions
      https://git.kernel.org/pci/pci/c/4774faf854f5
[3/3] PCI: layerscape: Add power management support for ls1028a
      https://git.kernel.org/pci/pci/c/9fda4d09905d

Thanks,
Lorenzo

Re: [PATCH v12 2/3] PCI: dwc: Implement general suspend/resume functionality for L2/L3 transitions

[Bjorn Helgaas]

On Mon, Aug 21, 2023 at 02:48:14PM -0400, Frank Li wrote:
> Introduce helper function dw_pcie_get_ltssm() to retrieve SMLH_LTSS_STATE.
> ...

> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 615660640801..91d13f9b21b1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h

> @@ -364,6 +375,7 @@ struct dw_pcie_ops {
>  	void    (*write_dbi2)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
>  			      size_t size, u32 val);
>  	int	(*link_up)(struct dw_pcie *pcie);
> +	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);

This has already been applied as
https://git.kernel.org/linus/4774faf854f5 ("PCI: dwc: Implement
generic suspend/resume functionality"), but this .get_ltssm() pointer
doesn't seem to be used anywhere.  Should we remove it until we need
it?

> +static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	if (pci->ops && pci->ops->get_ltssm)
> +		return pci->ops->get_ltssm(pci);
> +
> +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0);
> +
> +	return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> +}

Re: [PATCH v12 2/3] PCI: dwc: Implement general suspend/resume functionality for L2/L3 transitions

[Frank Li]

On Thu, Feb 01, 2024 at 10:50:25AM -0600, Bjorn Helgaas wrote:
> On Mon, Aug 21, 2023 at 02:48:14PM -0400, Frank Li wrote:
> > Introduce helper function dw_pcie_get_ltssm() to retrieve SMLH_LTSS_STATE.
> > ...
> 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 615660640801..91d13f9b21b1 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> 
> > @@ -364,6 +375,7 @@ struct dw_pcie_ops {
> >  	void    (*write_dbi2)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
> >  			      size_t size, u32 val);
> >  	int	(*link_up)(struct dw_pcie *pcie);
> > +	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
> 
> This has already been applied as
> https://git.kernel.org/linus/4774faf854f5 ("PCI: dwc: Implement
> generic suspend/resume functionality"), but this .get_ltssm() pointer
> doesn't seem to be used anywhere.  Should we remove it until we need
> it?

Could you place hold on for a while? I am working on imx PCIe. One old
imx6 may need it!

If not, I will submit patch to clean it. My new patches depend on

https://lore.kernel.org/imx/ZbJ+tFPn3aOYHCwf@lizhi-Precision-Tower-5810/T/#t

All already reviewed, could you please pick up these, so I can continue
my futher work.

Another
https://lore.kernel.org/imx/20240201-pme_msg-v2-0-6767052fe6a4@nxp.com/T/#t
was under review.

After these, suspend/resume will become simple and common for all dwc
platform.

Frank.

> 
> > +static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > +{
> > +	u32 val;
> > +
> > +	if (pci->ops && pci->ops->get_ltssm)
> > +		return pci->ops->get_ltssm(pci);
> > +
> > +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0);
> > +
> > +	return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> > +}

Re: [PATCH v12 2/3] PCI: dwc: Implement general suspend/resume functionality for L2/L3 transitions

[Bjorn Helgaas]

On Thu, Feb 01, 2024 at 12:08:23PM -0500, Frank Li wrote:
> On Thu, Feb 01, 2024 at 10:50:25AM -0600, Bjorn Helgaas wrote:
> > On Mon, Aug 21, 2023 at 02:48:14PM -0400, Frank Li wrote:
> > > Introduce helper function dw_pcie_get_ltssm() to retrieve SMLH_LTSS_STATE.
> > > ...
> > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 615660640801..91d13f9b21b1 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > 
> > > @@ -364,6 +375,7 @@ struct dw_pcie_ops {
> > >  	void    (*write_dbi2)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
> > >  			      size_t size, u32 val);
> > >  	int	(*link_up)(struct dw_pcie *pcie);
> > > +	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
> > 
> > This has already been applied as
> > https://git.kernel.org/linus/4774faf854f5 ("PCI: dwc: Implement
> > generic suspend/resume functionality"), but this .get_ltssm() pointer
> > doesn't seem to be used anywhere.  Should we remove it until we need
> > it?
> 
> Could you place hold on for a while? I am working on imx PCIe. One old
> imx6 may need it!
> 
> If not, I will submit patch to clean it. My new patches depend on
> 
> https://lore.kernel.org/imx/ZbJ+tFPn3aOYHCwf@lizhi-Precision-Tower-5810/T/#t
> 
> All already reviewed, could you please pick up these, so I can continue
> my futher work.
> 
> Another
> https://lore.kernel.org/imx/20240201-pme_msg-v2-0-6767052fe6a4@nxp.com/T/#t
> was under review.

OK.  I don't see .get_ltssm() used in that series either.  But if you
will need it soon, no point in removing it right now.

Bjorn