[PATCH v3 0/6] Add some enhancements for i.MX95 PCIe

[PATCH v3 0/6] Add some enhancements for i.MX95 PCIe

[Richard Zhu]

v3 changes:
- Correct the typo error in first patch, and update the commit message of
  #1 and #6 pathes.
- Use a quirk flag to specify the errata workaround contained in post_init.

v2 changes:
- Correct typo error, and update commit message.
- Replace regmap_update_bits() by regmap_set_bits/regmap_clear_bits.
- Use post_init callback of dw_pcie_host_ops.
- Add one more PLL lock check patch.
- Reformat LUT save and restore functions.

[PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not
[PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe
[PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
[PATCH v3 4/6] PCI: imx6: Let i.MX95 PCIe compliance with 8GT/s
[PATCH v3 5/6] PCI: imx6: Add PLL clock lock check for i.MX95 PCIe
[PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for i.MX95

drivers/pci/controller/dwc/pci-imx6.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 175 insertions(+), 22 deletions(-)

[PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not required

[Richard Zhu]

The current link setup procedure is one workaround to detect the device
behind PCIe switches on some i.MX6 platforms.

To describe more accurately, change the flag name from
IMX_PCIE_FLAG_IMX_SPEED_CHANGE to IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND.

Start PCIe link directly when this flag is not set on i.MX7 or later
platforms to simple and speed up link training.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 34 +++++++++++----------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c1f7904e3600..57aa777231ae 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -91,7 +91,7 @@ enum imx_pcie_variants {
 };
 
 #define IMX_PCIE_FLAG_IMX_PHY			BIT(0)
-#define IMX_PCIE_FLAG_IMX_SPEED_CHANGE		BIT(1)
+#define IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND	BIT(1)
 #define IMX_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
 #define IMX_PCIE_FLAG_HAS_PHYDRV		BIT(3)
 #define IMX_PCIE_FLAG_HAS_APP_RESET		BIT(4)
@@ -860,6 +860,12 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 	u32 tmp;
 	int ret;
 
+	if (!(imx_pcie->drvdata->flags &
+	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
+		imx_pcie_ltssm_enable(dev);
+		return 0;
+	}
+
 	/*
 	 * Force Gen1 operation when starting the link.  In case the link is
 	 * started in Gen2 mode, there is a possibility the devices on the
@@ -896,22 +902,10 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
 		dw_pcie_dbi_ro_wr_dis(pci);
 
-		if (imx_pcie->drvdata->flags &
-		    IMX_PCIE_FLAG_IMX_SPEED_CHANGE) {
-
-			/*
-			 * On i.MX7, DIRECT_SPEED_CHANGE behaves differently
-			 * from i.MX6 family when no link speed transition
-			 * occurs and we go Gen1 -> yep, Gen1. The difference
-			 * is that, in such case, it will not be cleared by HW
-			 * which will cause the following code to report false
-			 * failure.
-			 */
-			ret = imx_pcie_wait_for_speed_change(imx_pcie);
-			if (ret) {
-				dev_err(dev, "Failed to bring link up!\n");
-				goto err_reset_phy;
-			}
+		ret = imx_pcie_wait_for_speed_change(imx_pcie);
+		if (ret) {
+			dev_err(dev, "Failed to bring link up!\n");
+			goto err_reset_phy;
 		}
 
 		/* Make sure link training is finished as well! */
@@ -1665,7 +1659,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
 			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
 			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
@@ -1681,7 +1675,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6SX] = {
 		.variant = IMX6SX,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
 			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.ltssm_off = IOMUXC_GPR12,
@@ -1696,7 +1690,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6QP] = {
 		.variant = IMX6QP,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
 			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
-- 
2.37.1

[PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe

[Richard Zhu]

Add the cold reset toggle for i.MX95 PCIe to align PHY's power up sequency.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 42 +++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 57aa777231ae..6051b3b5928f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -71,6 +71,9 @@
 #define IMX95_SID_MASK				GENMASK(5, 0)
 #define IMX95_MAX_LUT				32
 
+#define IMX95_PCIE_RST_CTRL			0x3010
+#define IMX95_PCIE_COLD_RST			BIT(0)
+
 #define to_imx_pcie(x)	dev_get_drvdata((x)->dev)
 
 enum imx_pcie_variants {
@@ -773,6 +776,43 @@ static int imx7d_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
 	return 0;
 }
 
+static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
+{
+	u32 val;
+
+	if (assert) {
+		/*
+		 * From i.MX95 PCIe PHY perspective, the COLD reset toggle
+		 * should be complete after power-up by the following sequence.
+		 *                 > 10us(at power-up)
+		 *                 > 10ns(warm reset)
+		 *               |<------------>|
+		 *                ______________
+		 * phy_reset ____/              \________________
+		 *                                   ____________
+		 * ref_clk_en_______________________/
+		 * Toggle COLD reset aligned with this sequence for i.MX95 PCIe.
+		 */
+		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
+				IMX95_PCIE_COLD_RST);
+		/*
+		 * To make sure delay enough time, do regmap_read_bypassed
+		 * before udelay(). Since udelay() might not use MMIO, and cause
+		 * delay time less than setting value.
+		 */
+		regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
+				     &val);
+		udelay(15);
+		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
+				  IMX95_PCIE_COLD_RST);
+		regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
+				     &val);
+		udelay(10);
+	}
+
+	return 0;
+}
+
 static void imx_pcie_assert_core_reset(struct imx_pcie *imx_pcie)
 {
 	reset_control_assert(imx_pcie->pciephy_reset);
@@ -1762,6 +1802,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
 		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
 		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
+		.core_reset = imx95_pcie_core_reset,
 		.init_phy = imx95_pcie_init_phy,
 	},
 	[IMX8MQ_EP] = {
@@ -1815,6 +1856,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
 		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
 		.init_phy = imx95_pcie_init_phy,
+		.core_reset = imx95_pcie_core_reset,
 		.epc_features = &imx95_pcie_epc_features,
 		.mode = DW_PCIE_EP_TYPE,
 	},
-- 
2.37.1

[PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Richard Zhu]

ERR051624: The Controller Without Vaux Cannot Exit L23 Ready Through Beacon
or PERST# De-assertion

When the auxiliary power is not available, the controller cannot exit from
L23 Ready with beacon or PERST# de-assertion when main power is not
removed.

Workaround: Set SS_RW_REG_1[SYS_AUX_PWR_DET] to 1.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 6051b3b5928f..82402e52eff2 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -48,6 +48,8 @@
 #define IMX95_PCIE_SS_RW_REG_0			0xf0
 #define IMX95_PCIE_REF_CLKEN			BIT(23)
 #define IMX95_PCIE_PHY_CR_PARA_SEL		BIT(9)
+#define IMX95_PCIE_SS_RW_REG_1			0xf4
+#define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
 
 #define IMX95_PE0_GEN_CTRL_1			0x1050
 #define IMX95_PCIE_DEVICE_TYPE			GENMASK(3, 0)
@@ -227,6 +229,19 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
 
 static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 {
+	/*
+	 * ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
+	 * Through Beacon or PERST# De-assertion
+	 *
+	 * When the auxiliary power is not available, the controller
+	 * cannot exit from L23 Ready with beacon or PERST# de-assertion
+	 * when main power is not removed.
+	 *
+	 * Workaround: Set SS_RW_REG_1[SYS_AUX_PWR_DET] to 1.
+	 */
+	regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
+			IMX95_PCIE_SYS_AUX_PWR_DET);
+
 	regmap_update_bits(imx_pcie->iomuxc_gpr,
 			IMX95_PCIE_SS_RW_REG_0,
 			IMX95_PCIE_PHY_CR_PARA_SEL,
-- 
2.37.1

[PATCH v3 4/6] PCI: imx6: Let i.MX95 PCIe compliance with 8GT/s Receiver Impedance ECN

[Richard Zhu]

ERR051586: Compliance with 8GT/s Receiver Impedance ECN.

The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] is 1 which
makes receiver non-compliant with the ZRX-DC parameter for 2.5 GT/s when
operating at 8 GT/s or higher. It causes unnecessary timeout in L1.

Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] to 0.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 31 +++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 82402e52eff2..35194b543551 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -110,6 +110,7 @@ enum imx_pcie_variants {
  */
 #define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
 #define IMX_PCIE_FLAG_HAS_LUT			BIT(10)
+#define IMX_PCIE_FLAG_8GT_ECN_ERR051586		BIT(11)
 
 #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
 
@@ -1263,6 +1264,32 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 		regulator_disable(imx_pcie->vpcie);
 }
 
+static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
+	u32 val;
+
+	if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_8GT_ECN_ERR051586) {
+		/*
+		 * ERR051586: Compliance with 8GT/s Receiver Impedance ECN
+		 *
+		 * The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
+		 * is 1 which makes receiver non-compliant with the ZRX-DC
+		 * parameter for 2.5 GT/s when operating at 8 GT/s or higher.
+		 * It causes unnecessary timeout in L1.
+		 *
+		 * Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
+		 * to 0.
+		 */
+		dw_pcie_dbi_ro_wr_en(pci);
+		val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
+		val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
+		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
+		dw_pcie_dbi_ro_wr_dis(pci);
+	}
+}
+
 static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
 {
 	struct imx_pcie *imx_pcie = to_imx_pcie(pcie);
@@ -1304,6 +1331,7 @@ static const struct dw_pcie_host_ops imx_pcie_host_ops = {
 static const struct dw_pcie_host_ops imx_pcie_host_dw_pme_ops = {
 	.init = imx_pcie_host_init,
 	.deinit = imx_pcie_host_exit,
+	.post_init = imx_pcie_host_post_init,
 };
 
 static const struct dw_pcie_ops dw_pcie_ops = {
@@ -1403,6 +1431,7 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
 	struct device *dev = pci->dev;
 
 	imx_pcie_host_init(pp);
+	imx_pcie_host_post_init(pp);
 	ep = &pci->ep;
 	ep->ops = &pcie_ep_ops;
 
@@ -1812,6 +1841,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.variant = IMX95,
 		.flags = IMX_PCIE_FLAG_HAS_SERDES |
 			 IMX_PCIE_FLAG_HAS_LUT |
+			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
 			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
 		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
@@ -1865,6 +1895,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX95_EP] = {
 		.variant = IMX95_EP,
 		.flags = IMX_PCIE_FLAG_HAS_SERDES |
+			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
 			 IMX_PCIE_FLAG_SUPPORT_64BIT,
 		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
 		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
-- 
2.37.1

[PATCH v3 5/6] PCI: imx6: Add PLL clock lock check for i.MX95 PCIe

[Richard Zhu]

Add PLL clock lock check for i.MX95 PCIe.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 28 +++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 35194b543551..40eeb02ffb5d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -45,6 +45,9 @@
 #define IMX95_PCIE_PHY_GEN_CTRL			0x0
 #define IMX95_PCIE_REF_USE_PAD			BIT(17)
 
+#define IMX95_PCIE_PHY_MPLLA_CTRL		0x10
+#define IMX95_PCIE_PHY_MPLL_STATE		BIT(30)
+
 #define IMX95_PCIE_SS_RW_REG_0			0xf0
 #define IMX95_PCIE_REF_CLKEN			BIT(23)
 #define IMX95_PCIE_PHY_CR_PARA_SEL		BIT(9)
@@ -479,6 +482,23 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie)
 		dev_err(dev, "PCIe PLL lock timeout\n");
 }
 
+static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie)
+{
+	u32 val;
+	struct device *dev = imx_pcie->pci->dev;
+
+	if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr,
+				     IMX95_PCIE_PHY_MPLLA_CTRL, val,
+				     val & IMX95_PCIE_PHY_MPLL_STATE,
+				     PHY_PLL_LOCK_WAIT_USLEEP_MAX,
+				     PHY_PLL_LOCK_WAIT_TIMEOUT)) {
+		dev_err(dev, "PCIe PLL lock timeout\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int imx_setup_phy_mpll(struct imx_pcie *imx_pcie)
 {
 	unsigned long phy_rate = 0;
@@ -824,6 +844,8 @@ static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
 		regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
 				     &val);
 		udelay(10);
+	} else {
+		return imx95_pcie_wait_for_phy_pll_lock(imx_pcie);
 	}
 
 	return 0;
@@ -843,11 +865,13 @@ static void imx_pcie_assert_core_reset(struct imx_pcie *imx_pcie)
 
 static int imx_pcie_deassert_core_reset(struct imx_pcie *imx_pcie)
 {
+	int ret = 0;
+
 	reset_control_deassert(imx_pcie->pciephy_reset);
 	reset_control_deassert(imx_pcie->apps_reset);
 
 	if (imx_pcie->drvdata->core_reset)
-		imx_pcie->drvdata->core_reset(imx_pcie, false);
+		ret = imx_pcie->drvdata->core_reset(imx_pcie, false);
 
 	/* Some boards don't have PCIe reset GPIO. */
 	if (imx_pcie->reset_gpiod) {
@@ -857,7 +881,7 @@ static int imx_pcie_deassert_core_reset(struct imx_pcie *imx_pcie)
 		msleep(100);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int imx_pcie_wait_for_speed_change(struct imx_pcie *imx_pcie)
-- 
2.37.1

[PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for i.MX95 PCIe

[Richard Zhu]

The look up table(LUT) setting would be lost during PCIe suspend on i.MX95.

To ensure proper functionality after resume, save and restore the LUT
setting in suspend and resume operations.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 40eeb02ffb5d..d8f4608eb7da 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -138,6 +138,11 @@ struct imx_pcie_drvdata {
 	const struct dw_pcie_host_ops *ops;
 };
 
+struct imx_lut_data {
+	u32 data1;
+	u32 data2;
+};
+
 struct imx_pcie {
 	struct dw_pcie		*pci;
 	struct gpio_desc	*reset_gpiod;
@@ -157,6 +162,8 @@ struct imx_pcie {
 	struct regulator	*vph;
 	void __iomem		*phy_base;
 
+	/* LUT data for pcie */
+	struct imx_lut_data	luts[IMX95_MAX_LUT];
 	/* power domain for pcie */
 	struct device		*pd_pcie;
 	/* power domain for pcie phy */
@@ -1505,6 +1512,42 @@ static void imx_pcie_msi_save_restore(struct imx_pcie *imx_pcie, bool save)
 	}
 }
 
+static void imx_pcie_lut_save(struct imx_pcie *imx_pcie)
+{
+	u32 data1, data2;
+	int i;
+
+	for (i = 0; i < IMX95_MAX_LUT; i++) {
+		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL,
+			     IMX95_PEO_LUT_RWA | i);
+		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1);
+		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, &data2);
+		if (data1 & IMX95_PE0_LUT_VLD) {
+			imx_pcie->luts[i].data1 = data1;
+			imx_pcie->luts[i].data2 = data2;
+		} else {
+			imx_pcie->luts[i].data1 = 0;
+			imx_pcie->luts[i].data2 = 0;
+		}
+	}
+}
+
+static void imx_pcie_lut_restore(struct imx_pcie *imx_pcie)
+{
+	int i;
+
+	for (i = 0; i < IMX95_MAX_LUT; i++) {
+		if ((imx_pcie->luts[i].data1 & IMX95_PE0_LUT_VLD) == 0)
+			continue;
+
+		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1,
+			     imx_pcie->luts[i].data1);
+		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2,
+			     imx_pcie->luts[i].data2);
+		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL, i);
+	}
+}
+
 static int imx_pcie_suspend_noirq(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
@@ -1513,6 +1556,8 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 		return 0;
 
 	imx_pcie_msi_save_restore(imx_pcie, true);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+		imx_pcie_lut_save(imx_pcie);
 	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
 		/*
 		 * The minimum for a workaround would be to set PERST# and to
@@ -1557,6 +1602,8 @@ static int imx_pcie_resume_noirq(struct device *dev)
 		if (ret)
 			return ret;
 	}
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+		imx_pcie_lut_restore(imx_pcie);
 	imx_pcie_msi_save_restore(imx_pcie, false);
 
 	return 0;
-- 
2.37.1

Re: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not required

[Frank Li]

On Fri, Mar 28, 2025 at 11:02:08AM +0800, Richard Zhu wrote:
> The current link setup procedure is one workaround to detect the device
> behind PCIe switches on some i.MX6 platforms.
>
> To describe more accurately, change the flag name from
> IMX_PCIE_FLAG_IMX_SPEED_CHANGE to IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND.
>
> Start PCIe link directly when this flag is not set on i.MX7 or later
> platforms to simple and speed up link training.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/pci/controller/dwc/pci-imx6.c | 34 +++++++++++----------------
>  1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index c1f7904e3600..57aa777231ae 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -91,7 +91,7 @@ enum imx_pcie_variants {
>  };
>
>  #define IMX_PCIE_FLAG_IMX_PHY			BIT(0)
> -#define IMX_PCIE_FLAG_IMX_SPEED_CHANGE		BIT(1)
> +#define IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND	BIT(1)
>  #define IMX_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
>  #define IMX_PCIE_FLAG_HAS_PHYDRV		BIT(3)
>  #define IMX_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> @@ -860,6 +860,12 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>  	u32 tmp;
>  	int ret;
>
> +	if (!(imx_pcie->drvdata->flags &
> +	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
> +		imx_pcie_ltssm_enable(dev);
> +		return 0;
> +	}
> +
>  	/*
>  	 * Force Gen1 operation when starting the link.  In case the link is
>  	 * started in Gen2 mode, there is a possibility the devices on the
> @@ -896,22 +902,10 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
>  		dw_pcie_dbi_ro_wr_dis(pci);
>
> -		if (imx_pcie->drvdata->flags &
> -		    IMX_PCIE_FLAG_IMX_SPEED_CHANGE) {
> -
> -			/*
> -			 * On i.MX7, DIRECT_SPEED_CHANGE behaves differently
> -			 * from i.MX6 family when no link speed transition
> -			 * occurs and we go Gen1 -> yep, Gen1. The difference
> -			 * is that, in such case, it will not be cleared by HW
> -			 * which will cause the following code to report false
> -			 * failure.
> -			 */
> -			ret = imx_pcie_wait_for_speed_change(imx_pcie);
> -			if (ret) {
> -				dev_err(dev, "Failed to bring link up!\n");
> -				goto err_reset_phy;
> -			}
> +		ret = imx_pcie_wait_for_speed_change(imx_pcie);
> +		if (ret) {
> +			dev_err(dev, "Failed to bring link up!\n");
> +			goto err_reset_phy;
>  		}
>
>  		/* Make sure link training is finished as well! */
> @@ -1665,7 +1659,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  	[IMX6Q] = {
>  		.variant = IMX6Q,
>  		.flags = IMX_PCIE_FLAG_IMX_PHY |
> -			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
> +			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
>  			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
>  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.dbi_length = 0x200,
> @@ -1681,7 +1675,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  	[IMX6SX] = {
>  		.variant = IMX6SX,
>  		.flags = IMX_PCIE_FLAG_IMX_PHY |
> -			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
> +			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
>  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
>  		.ltssm_off = IOMUXC_GPR12,
> @@ -1696,7 +1690,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  	[IMX6QP] = {
>  		.variant = IMX6QP,
>  		.flags = IMX_PCIE_FLAG_IMX_PHY |
> -			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
> +			 IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND |
>  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.dbi_length = 0x200,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
> --
> 2.37.1
>

Re: [PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe

[Frank Li]

On Fri, Mar 28, 2025 at 11:02:09AM +0800, Richard Zhu wrote:
> Add the cold reset toggle for i.MX95 PCIe to align PHY's power up sequency.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 42 +++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 57aa777231ae..6051b3b5928f 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -71,6 +71,9 @@
>  #define IMX95_SID_MASK				GENMASK(5, 0)
>  #define IMX95_MAX_LUT				32
>
> +#define IMX95_PCIE_RST_CTRL			0x3010
> +#define IMX95_PCIE_COLD_RST			BIT(0)
> +
>  #define to_imx_pcie(x)	dev_get_drvdata((x)->dev)
>
>  enum imx_pcie_variants {
> @@ -773,6 +776,43 @@ static int imx7d_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
>  	return 0;
>  }
>
> +static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
> +{
> +	u32 val;
> +
> +	if (assert) {
> +		/*
> +		 * From i.MX95 PCIe PHY perspective, the COLD reset toggle
> +		 * should be complete after power-up by the following sequence.
> +		 *                 > 10us(at power-up)
> +		 *                 > 10ns(warm reset)
> +		 *               |<------------>|
> +		 *                ______________
> +		 * phy_reset ____/              \________________
> +		 *                                   ____________
> +		 * ref_clk_en_______________________/
> +		 * Toggle COLD reset aligned with this sequence for i.MX95 PCIe.
> +		 */
> +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> +				IMX95_PCIE_COLD_RST);
> +		/*
> +		 * To make sure delay enough time, do regmap_read_bypassed
> +		 * before udelay(). Since udelay() might not use MMIO, and cause
> +		 * delay time less than setting value.
> +		 */
> +		regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> +				     &val);
> +		udelay(15);
> +		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> +				  IMX95_PCIE_COLD_RST);
> +		regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> +				     &val);
> +		udelay(10);
> +	}
> +
> +	return 0;
> +}
> +
>  static void imx_pcie_assert_core_reset(struct imx_pcie *imx_pcie)
>  {
>  	reset_control_assert(imx_pcie->pciephy_reset);
> @@ -1762,6 +1802,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
>  		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
>  		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
> +		.core_reset = imx95_pcie_core_reset,
>  		.init_phy = imx95_pcie_init_phy,
>  	},
>  	[IMX8MQ_EP] = {
> @@ -1815,6 +1856,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
>  		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
>  		.init_phy = imx95_pcie_init_phy,
> +		.core_reset = imx95_pcie_core_reset,
>  		.epc_features = &imx95_pcie_epc_features,
>  		.mode = DW_PCIE_EP_TYPE,
>  	},
> --
> 2.37.1
>

Re: [PATCH v3 4/6] PCI: imx6: Let i.MX95 PCIe compliance with 8GT/s Receiver Impedance ECN

[Frank Li]

On Fri, Mar 28, 2025 at 11:02:11AM +0800, Richard Zhu wrote:
> ERR051586: Compliance with 8GT/s Receiver Impedance ECN.
>
> The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] is 1 which
> makes receiver non-compliant with the ZRX-DC parameter for 2.5 GT/s when
> operating at 8 GT/s or higher. It causes unnecessary timeout in L1.
>
> Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] to 0.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/pci/controller/dwc/pci-imx6.c | 31 +++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 82402e52eff2..35194b543551 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -110,6 +110,7 @@ enum imx_pcie_variants {
>   */
>  #define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
>  #define IMX_PCIE_FLAG_HAS_LUT			BIT(10)
> +#define IMX_PCIE_FLAG_8GT_ECN_ERR051586		BIT(11)
>
>  #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
>
> @@ -1263,6 +1264,32 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  		regulator_disable(imx_pcie->vpcie);
>  }
>
> +static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> +	u32 val;
> +
> +	if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_8GT_ECN_ERR051586) {
> +		/*
> +		 * ERR051586: Compliance with 8GT/s Receiver Impedance ECN
> +		 *
> +		 * The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
> +		 * is 1 which makes receiver non-compliant with the ZRX-DC
> +		 * parameter for 2.5 GT/s when operating at 8 GT/s or higher.
> +		 * It causes unnecessary timeout in L1.
> +		 *
> +		 * Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
> +		 * to 0.
> +		 */
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> +		val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
> +		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +}
> +
>  static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
>  {
>  	struct imx_pcie *imx_pcie = to_imx_pcie(pcie);
> @@ -1304,6 +1331,7 @@ static const struct dw_pcie_host_ops imx_pcie_host_ops = {
>  static const struct dw_pcie_host_ops imx_pcie_host_dw_pme_ops = {
>  	.init = imx_pcie_host_init,
>  	.deinit = imx_pcie_host_exit,
> +	.post_init = imx_pcie_host_post_init,
>  };
>
>  static const struct dw_pcie_ops dw_pcie_ops = {
> @@ -1403,6 +1431,7 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
>  	struct device *dev = pci->dev;
>
>  	imx_pcie_host_init(pp);
> +	imx_pcie_host_post_init(pp);
>  	ep = &pci->ep;
>  	ep->ops = &pcie_ep_ops;
>
> @@ -1812,6 +1841,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.variant = IMX95,
>  		.flags = IMX_PCIE_FLAG_HAS_SERDES |
>  			 IMX_PCIE_FLAG_HAS_LUT |
> +			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
>  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
>  		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
> @@ -1865,6 +1895,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  	[IMX95_EP] = {
>  		.variant = IMX95_EP,
>  		.flags = IMX_PCIE_FLAG_HAS_SERDES |
> +			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
>  			 IMX_PCIE_FLAG_SUPPORT_64BIT,
>  		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
>  		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
> --
> 2.37.1
>

Re: [PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for i.MX95 PCIe

[Frank Li]

On Fri, Mar 28, 2025 at 11:02:13AM +0800, Richard Zhu wrote:
> The look up table(LUT) setting would be lost during PCIe suspend on i.MX95.
>
> To ensure proper functionality after resume, save and restore the LUT
> setting in suspend and resume operations.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 40eeb02ffb5d..d8f4608eb7da 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -138,6 +138,11 @@ struct imx_pcie_drvdata {
>  	const struct dw_pcie_host_ops *ops;
>  };
>
> +struct imx_lut_data {
> +	u32 data1;
> +	u32 data2;
> +};
> +
>  struct imx_pcie {
>  	struct dw_pcie		*pci;
>  	struct gpio_desc	*reset_gpiod;
> @@ -157,6 +162,8 @@ struct imx_pcie {
>  	struct regulator	*vph;
>  	void __iomem		*phy_base;
>
> +	/* LUT data for pcie */
> +	struct imx_lut_data	luts[IMX95_MAX_LUT];
>  	/* power domain for pcie */
>  	struct device		*pd_pcie;
>  	/* power domain for pcie phy */
> @@ -1505,6 +1512,42 @@ static void imx_pcie_msi_save_restore(struct imx_pcie *imx_pcie, bool save)
>  	}
>  }
>
> +static void imx_pcie_lut_save(struct imx_pcie *imx_pcie)
> +{
> +	u32 data1, data2;
> +	int i;
> +
> +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL,
> +			     IMX95_PEO_LUT_RWA | i);
> +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1);
> +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, &data2);
> +		if (data1 & IMX95_PE0_LUT_VLD) {
> +			imx_pcie->luts[i].data1 = data1;
> +			imx_pcie->luts[i].data2 = data2;
> +		} else {
> +			imx_pcie->luts[i].data1 = 0;
> +			imx_pcie->luts[i].data2 = 0;
> +		}
> +	}
> +}
> +
> +static void imx_pcie_lut_restore(struct imx_pcie *imx_pcie)
> +{
> +	int i;
> +
> +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> +		if ((imx_pcie->luts[i].data1 & IMX95_PE0_LUT_VLD) == 0)
> +			continue;
> +
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1,
> +			     imx_pcie->luts[i].data1);
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2,
> +			     imx_pcie->luts[i].data2);
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL, i);
> +	}
> +}
> +
>  static int imx_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
> @@ -1513,6 +1556,8 @@ static int imx_pcie_suspend_noirq(struct device *dev)
>  		return 0;
>
>  	imx_pcie_msi_save_restore(imx_pcie, true);
> +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> +		imx_pcie_lut_save(imx_pcie);
>  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
>  		/*
>  		 * The minimum for a workaround would be to set PERST# and to
> @@ -1557,6 +1602,8 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  		if (ret)
>  			return ret;
>  	}
> +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> +		imx_pcie_lut_restore(imx_pcie);
>  	imx_pcie_msi_save_restore(imx_pcie, false);
>
>  	return 0;
> --
> 2.37.1
>

Re: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not required

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:08AM +0800, Richard Zhu wrote:
> The current link setup procedure is one workaround to detect the device
> behind PCIe switches on some i.MX6 platforms.
> 
> To describe more accurately, change the flag name from
> IMX_PCIE_FLAG_IMX_SPEED_CHANGE to IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND.
> 
> Start PCIe link directly when this flag is not set on i.MX7 or later
> platforms to simple and speed up link training.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

One observation below (not related to this change).

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 34 +++++++++++----------------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index c1f7904e3600..57aa777231ae 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -91,7 +91,7 @@ enum imx_pcie_variants {
>  };
>  
>  #define IMX_PCIE_FLAG_IMX_PHY			BIT(0)
> -#define IMX_PCIE_FLAG_IMX_SPEED_CHANGE		BIT(1)
> +#define IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND	BIT(1)
>  #define IMX_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
>  #define IMX_PCIE_FLAG_HAS_PHYDRV		BIT(3)
>  #define IMX_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> @@ -860,6 +860,12 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>  	u32 tmp;
>  	int ret;
>  
> +	if (!(imx_pcie->drvdata->flags &
> +	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
> +		imx_pcie_ltssm_enable(dev);
> +		return 0;

While looking into the code, I realized that we could skip waiting for link
during the workaround in atleast one instance:

```
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 5f267dd261b5..9dbfbd9de2da 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -875,11 +875,11 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
        /* Start LTSSM. */
        imx_pcie_ltssm_enable(dev);
 
-       ret = dw_pcie_wait_for_link(pci);
-       if (ret)
-               goto err_reset_phy;
-
        if (pci->max_link_speed > 1) {
+               ret = dw_pcie_wait_for_link(pci);
+               if (ret)
+                       goto err_reset_phy;
+
                /* Allow faster modes after the link is up */
                dw_pcie_dbi_ro_wr_en(pci);
                tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
@@ -913,17 +913,10 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
                                goto err_reset_phy;
                        }
                }
-
-               /* Make sure link training is finished as well! */
-               ret = dw_pcie_wait_for_link(pci);
-               if (ret)
-                       goto err_reset_phy;
        } else {
                dev_info(dev, "Link: Only Gen1 is enabled\n");
        }
 
-       tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
-       dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
        return 0;
 
 err_reset_phy:
```

dw_pcie_wait_for_link() in DWC core should serve the purpose.

- Mani

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

Re: [PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:09AM +0800, Richard Zhu wrote:
> Add the cold reset toggle for i.MX95 PCIe to align PHY's power up sequency.

What do you mean by 'cold' reset? Is it 'core' reset? I see both terminologies
used in the code.

> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 42 +++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 57aa777231ae..6051b3b5928f 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -71,6 +71,9 @@
>  #define IMX95_SID_MASK				GENMASK(5, 0)
>  #define IMX95_MAX_LUT				32
>  
> +#define IMX95_PCIE_RST_CTRL			0x3010
> +#define IMX95_PCIE_COLD_RST			BIT(0)
> +
>  #define to_imx_pcie(x)	dev_get_drvdata((x)->dev)
>  
>  enum imx_pcie_variants {
> @@ -773,6 +776,43 @@ static int imx7d_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
>  	return 0;
>  }
>  
> +static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
> +{
> +	u32 val;
> +
> +	if (assert) {
> +		/*
> +		 * From i.MX95 PCIe PHY perspective, the COLD reset toggle
> +		 * should be complete after power-up by the following sequence.
> +		 *                 > 10us(at power-up)
> +		 *                 > 10ns(warm reset)
> +		 *               |<------------>|
> +		 *                ______________
> +		 * phy_reset ____/              \________________
> +		 *                                   ____________
> +		 * ref_clk_en_______________________/
> +		 * Toggle COLD reset aligned with this sequence for i.MX95 PCIe.
> +		 */
> +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> +				IMX95_PCIE_COLD_RST);

Is this really COLD reset? Or CORE reset?

> +		/*
> +		 * To make sure delay enough time, do regmap_read_bypassed
> +		 * before udelay(). Since udelay() might not use MMIO, and cause
> +		 * delay time less than setting value.
> +		 */

This comment could be rephrased:

		/*
		 * Make sure the write to IMX95_PCIE_RST_CTRL is flushed to the
		 * hardware by doing a read. Otherwise, there is no guarantee
		 * that the write has reached the hardware before udelay().
		 */

- Mani

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

Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> ERR051624: The Controller Without Vaux Cannot Exit L23 Ready Through Beacon
> or PERST# De-assertion

Is it possible to share the link to the erratum?

> 
> When the auxiliary power is not available, the controller cannot exit from
> L23 Ready with beacon or PERST# de-assertion when main power is not
> removed.
> 

I don't understand how the presence of Vaux affects the controller. Same goes
for PERST# deassertion. How does that relate to Vaux? Is this erratum for
a specific endpoint behavior?

- Mani

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

Re: [PATCH v3 4/6] PCI: imx6: Let i.MX95 PCIe compliance with 8GT/s Receiver Impedance ECN

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:11AM +0800, Richard Zhu wrote:
> ERR051586: Compliance with 8GT/s Receiver Impedance ECN.
> 
> The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] is 1 which
> makes receiver non-compliant with the ZRX-DC parameter for 2.5 GT/s when
> operating at 8 GT/s or higher. It causes unnecessary timeout in L1.
> 
> Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL] to 0.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 31 +++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 82402e52eff2..35194b543551 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -110,6 +110,7 @@ enum imx_pcie_variants {
>   */
>  #define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
>  #define IMX_PCIE_FLAG_HAS_LUT			BIT(10)
> +#define IMX_PCIE_FLAG_8GT_ECN_ERR051586		BIT(11)
>  
>  #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
>  
> @@ -1263,6 +1264,32 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  		regulator_disable(imx_pcie->vpcie);
>  }
>  
> +static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> +	u32 val;
> +
> +	if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_8GT_ECN_ERR051586) {
> +		/*
> +		 * ERR051586: Compliance with 8GT/s Receiver Impedance ECN
> +		 *
> +		 * The default value of GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
> +		 * is 1 which makes receiver non-compliant with the ZRX-DC
> +		 * parameter for 2.5 GT/s when operating at 8 GT/s or higher.
> +		 * It causes unnecessary timeout in L1.
> +		 *
> +		 * Workaround: Program GEN3_RELATED_OFF[GEN3_ZRXDC_NONCOMPL]
> +		 * to 0.
> +		 */
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> +		val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
> +		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +}
> +
>  static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
>  {
>  	struct imx_pcie *imx_pcie = to_imx_pcie(pcie);
> @@ -1304,6 +1331,7 @@ static const struct dw_pcie_host_ops imx_pcie_host_ops = {
>  static const struct dw_pcie_host_ops imx_pcie_host_dw_pme_ops = {
>  	.init = imx_pcie_host_init,
>  	.deinit = imx_pcie_host_exit,
> +	.post_init = imx_pcie_host_post_init,
>  };
>  
>  static const struct dw_pcie_ops dw_pcie_ops = {
> @@ -1403,6 +1431,7 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
>  	struct device *dev = pci->dev;
>  
>  	imx_pcie_host_init(pp);
> +	imx_pcie_host_post_init(pp);
>  	ep = &pci->ep;
>  	ep->ops = &pcie_ep_ops;
>  
> @@ -1812,6 +1841,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.variant = IMX95,
>  		.flags = IMX_PCIE_FLAG_HAS_SERDES |
>  			 IMX_PCIE_FLAG_HAS_LUT |
> +			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
>  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
>  		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
> @@ -1865,6 +1895,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  	[IMX95_EP] = {
>  		.variant = IMX95_EP,
>  		.flags = IMX_PCIE_FLAG_HAS_SERDES |
> +			 IMX_PCIE_FLAG_8GT_ECN_ERR051586 |
>  			 IMX_PCIE_FLAG_SUPPORT_64BIT,
>  		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
>  		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
> -- 
> 2.37.1
> 

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

Re: [PATCH v3 5/6] PCI: imx6: Add PLL clock lock check for i.MX95 PCIe

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:12AM +0800, Richard Zhu wrote:
> Add PLL clock lock check for i.MX95 PCIe.
> 

What are the implications of not waiting for PLL lock? I guess clock
instability.

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 28 +++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 35194b543551..40eeb02ffb5d 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -45,6 +45,9 @@
>  #define IMX95_PCIE_PHY_GEN_CTRL			0x0
>  #define IMX95_PCIE_REF_USE_PAD			BIT(17)
>  
> +#define IMX95_PCIE_PHY_MPLLA_CTRL		0x10
> +#define IMX95_PCIE_PHY_MPLL_STATE		BIT(30)
> +
>  #define IMX95_PCIE_SS_RW_REG_0			0xf0
>  #define IMX95_PCIE_REF_CLKEN			BIT(23)
>  #define IMX95_PCIE_PHY_CR_PARA_SEL		BIT(9)
> @@ -479,6 +482,23 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie)
>  		dev_err(dev, "PCIe PLL lock timeout\n");
>  }
>  
> +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie)
> +{
> +	u32 val;
> +	struct device *dev = imx_pcie->pci->dev;
> +
> +	if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr,
> +				     IMX95_PCIE_PHY_MPLLA_CTRL, val,
> +				     val & IMX95_PCIE_PHY_MPLL_STATE,
> +				     PHY_PLL_LOCK_WAIT_USLEEP_MAX,
> +				     PHY_PLL_LOCK_WAIT_TIMEOUT)) {
> +		dev_err(dev, "PCIe PLL lock timeout\n");
> +		return -ENODEV;

-ETIMEDOUT

- Mani

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

Re: [PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for i.MX95 PCIe

[Manivannan Sadhasivam]

On Fri, Mar 28, 2025 at 11:02:13AM +0800, Richard Zhu wrote:
> The look up table(LUT) setting would be lost during PCIe suspend on i.MX95.
> 
> To ensure proper functionality after resume, save and restore the LUT
> setting in suspend and resume operations.
> 

There should be a fixes tag pointing to the commit added suspend/resume support.

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 40eeb02ffb5d..d8f4608eb7da 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -138,6 +138,11 @@ struct imx_pcie_drvdata {
>  	const struct dw_pcie_host_ops *ops;
>  };
>  
> +struct imx_lut_data {
> +	u32 data1;
> +	u32 data2;
> +};
> +
>  struct imx_pcie {
>  	struct dw_pcie		*pci;
>  	struct gpio_desc	*reset_gpiod;
> @@ -157,6 +162,8 @@ struct imx_pcie {
>  	struct regulator	*vph;
>  	void __iomem		*phy_base;
>  
> +	/* LUT data for pcie */
> +	struct imx_lut_data	luts[IMX95_MAX_LUT];
>  	/* power domain for pcie */
>  	struct device		*pd_pcie;
>  	/* power domain for pcie phy */
> @@ -1505,6 +1512,42 @@ static void imx_pcie_msi_save_restore(struct imx_pcie *imx_pcie, bool save)
>  	}
>  }
>  
> +static void imx_pcie_lut_save(struct imx_pcie *imx_pcie)
> +{
> +	u32 data1, data2;
> +	int i;
> +
> +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL,
> +			     IMX95_PEO_LUT_RWA | i);
> +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1);
> +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, &data2);
> +		if (data1 & IMX95_PE0_LUT_VLD) {
> +			imx_pcie->luts[i].data1 = data1;
> +			imx_pcie->luts[i].data2 = data2;
> +		} else {
> +			imx_pcie->luts[i].data1 = 0;
> +			imx_pcie->luts[i].data2 = 0;
> +		}
> +	}
> +}
> +
> +static void imx_pcie_lut_restore(struct imx_pcie *imx_pcie)
> +{
> +	int i;
> +
> +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> +		if ((imx_pcie->luts[i].data1 & IMX95_PE0_LUT_VLD) == 0)
> +			continue;
> +
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1,
> +			     imx_pcie->luts[i].data1);
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2,
> +			     imx_pcie->luts[i].data2);
> +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL, i);
> +	}
> +}
> +
>  static int imx_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
> @@ -1513,6 +1556,8 @@ static int imx_pcie_suspend_noirq(struct device *dev)
>  		return 0;
>  
>  	imx_pcie_msi_save_restore(imx_pcie, true);
> +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> +		imx_pcie_lut_save(imx_pcie);
>  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
>  		/*
>  		 * The minimum for a workaround would be to set PERST# and to
> @@ -1557,6 +1602,8 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  		if (ret)
>  			return ret;
>  	}
> +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> +		imx_pcie_lut_restore(imx_pcie);
>  	imx_pcie_msi_save_restore(imx_pcie, false);
>  
>  	return 0;
> -- 
> 2.37.1
> 

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

RE: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not required

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 14:27
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not
> required
> 
> On Fri, Mar 28, 2025 at 11:02:08AM +0800, Richard Zhu wrote:
> > The current link setup procedure is one workaround to detect the
> > device behind PCIe switches on some i.MX6 platforms.
> >
> > To describe more accurately, change the flag name from
> > IMX_PCIE_FLAG_IMX_SPEED_CHANGE to
> IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND.
> >
> > Start PCIe link directly when this flag is not set on i.MX7 or later
> > platforms to simple and speed up link training.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> One observation below (not related to this change).
> 
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 34
> > +++++++++++----------------
> >  1 file changed, 14 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index c1f7904e3600..57aa777231ae 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -91,7 +91,7 @@ enum imx_pcie_variants {  };
> >
> >  #define IMX_PCIE_FLAG_IMX_PHY			BIT(0)
> > -#define IMX_PCIE_FLAG_IMX_SPEED_CHANGE		BIT(1)
> > +#define IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND	BIT(1)
> >  #define IMX_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
> >  #define IMX_PCIE_FLAG_HAS_PHYDRV		BIT(3)
> >  #define IMX_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> > @@ -860,6 +860,12 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
> >  	u32 tmp;
> >  	int ret;
> >
> > +	if (!(imx_pcie->drvdata->flags &
> > +	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
> > +		imx_pcie_ltssm_enable(dev);
> > +		return 0;
> 
> While looking into the code, I realized that we could skip waiting for link during
> the workaround in atleast one instance:
> 
> ```
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 5f267dd261b5..9dbfbd9de2da 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -875,11 +875,11 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>         /* Start LTSSM. */
>         imx_pcie_ltssm_enable(dev);
> 
> -       ret = dw_pcie_wait_for_link(pci);
> -       if (ret)
> -               goto err_reset_phy;
> -
>         if (pci->max_link_speed > 1) {
> +               ret = dw_pcie_wait_for_link(pci);
> +               if (ret)
> +                       goto err_reset_phy;
> +
>                 /* Allow faster modes after the link is up */
>                 dw_pcie_dbi_ro_wr_en(pci);
>                 tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> @@ -913,17 +913,10 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>                                 goto err_reset_phy;
>                         }
>                 }
> -
> -               /* Make sure link training is finished as well! */
> -               ret = dw_pcie_wait_for_link(pci);
> -               if (ret)
> -                       goto err_reset_phy;
>         } else {
>                 dev_info(dev, "Link: Only Gen1 is enabled\n");
>         }
> 
> -       tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
> -       dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
>         return 0;
> 
>  err_reset_phy:
> ```
> 
> dw_pcie_wait_for_link() in DWC core should serve the purpose.
Good suggestion. Thanks.
I can add another patch to optimize the workaround procedure without
 function changes.

Best Regards
Richard Zhu
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

RE: [PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 14:36
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 2/6] PCI: imx6: Toggle the cold reset for i.MX95 PCIe
> 
> On Fri, Mar 28, 2025 at 11:02:09AM +0800, Richard Zhu wrote:
> > Add the cold reset toggle for i.MX95 PCIe to align PHY's power up sequency.
> 
> What do you mean by 'cold' reset? Is it 'core' reset? I see both terminologies
> used in the code.
> 
Regarding my understands, the cold reset is one reset category of the core reset.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 42
> > +++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 57aa777231ae..6051b3b5928f 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -71,6 +71,9 @@
> >  #define IMX95_SID_MASK				GENMASK(5, 0)
> >  #define IMX95_MAX_LUT				32
> >
> > +#define IMX95_PCIE_RST_CTRL			0x3010
> > +#define IMX95_PCIE_COLD_RST			BIT(0)
> > +
> >  #define to_imx_pcie(x)	dev_get_drvdata((x)->dev)
> >
> >  enum imx_pcie_variants {
> > @@ -773,6 +776,43 @@ static int imx7d_pcie_core_reset(struct imx_pcie
> *imx_pcie, bool assert)
> >  	return 0;
> >  }
> >
> > +static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool
> > +assert) {
> > +	u32 val;
> > +
> > +	if (assert) {
> > +		/*
> > +		 * From i.MX95 PCIe PHY perspective, the COLD reset toggle
> > +		 * should be complete after power-up by the following sequence.
> > +		 *                 > 10us(at power-up)
> > +		 *                 > 10ns(warm reset)
> > +		 *               |<------------>|
> > +		 *                ______________
> > +		 * phy_reset ____/              \________________
> > +		 *                                   ____________
> > +		 * ref_clk_en_______________________/
> > +		 * Toggle COLD reset aligned with this sequence for i.MX95 PCIe.
> > +		 */
> > +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL,
> > +				IMX95_PCIE_COLD_RST);
> 
> Is this really COLD reset? Or CORE reset?
Cold reset is one reset type of core reset refer to my understand.
> 
> > +		/*
> > +		 * To make sure delay enough time, do regmap_read_bypassed
> > +		 * before udelay(). Since udelay() might not use MMIO, and cause
> > +		 * delay time less than setting value.
> > +		 */
> 
> This comment could be rephrased:
> 
> 		/*
> 		 * Make sure the write to IMX95_PCIE_RST_CTRL is flushed to the
> 		 * hardware by doing a read. Otherwise, there is no guarantee
> 		 * that the write has reached the hardware before udelay().
> 		 */
> 
Okay

Best Regards
Richard Zhu

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

RE: [PATCH v3 5/6] PCI: imx6: Add PLL clock lock check for i.MX95 PCIe

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 15:10
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 5/6] PCI: imx6: Add PLL clock lock check for i.MX95 PCIe
> 
> On Fri, Mar 28, 2025 at 11:02:12AM +0800, Richard Zhu wrote:
> > Add PLL clock lock check for i.MX95 PCIe.
> >
> 
> What are the implications of not waiting for PLL lock? I guess clock instability.
>
Yes, it is. 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 28
> > +++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 35194b543551..40eeb02ffb5d 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -45,6 +45,9 @@
> >  #define IMX95_PCIE_PHY_GEN_CTRL			0x0
> >  #define IMX95_PCIE_REF_USE_PAD			BIT(17)
> >
> > +#define IMX95_PCIE_PHY_MPLLA_CTRL		0x10
> > +#define IMX95_PCIE_PHY_MPLL_STATE		BIT(30)
> > +
> >  #define IMX95_PCIE_SS_RW_REG_0			0xf0
> >  #define IMX95_PCIE_REF_CLKEN			BIT(23)
> >  #define IMX95_PCIE_PHY_CR_PARA_SEL		BIT(9)
> > @@ -479,6 +482,23 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct
> imx_pcie *imx_pcie)
> >  		dev_err(dev, "PCIe PLL lock timeout\n");  }
> >
> > +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie
> > +*imx_pcie) {
> > +	u32 val;
> > +	struct device *dev = imx_pcie->pci->dev;
> > +
> > +	if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr,
> > +				     IMX95_PCIE_PHY_MPLLA_CTRL, val,
> > +				     val & IMX95_PCIE_PHY_MPLL_STATE,
> > +				     PHY_PLL_LOCK_WAIT_USLEEP_MAX,
> > +				     PHY_PLL_LOCK_WAIT_TIMEOUT)) {
> > +		dev_err(dev, "PCIe PLL lock timeout\n");
> > +		return -ENODEV;
> 
> -ETIMEDOUT
Okay

Best Regards
Richard Zhu
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

RE: [PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for i.MX95 PCIe

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 15:12
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 6/6] PCI: imx6: Save and restore the LUT setting for
> i.MX95 PCIe
> 
> On Fri, Mar 28, 2025 at 11:02:13AM +0800, Richard Zhu wrote:
> > The look up table(LUT) setting would be lost during PCIe suspend on i.MX95.
> >
> > To ensure proper functionality after resume, save and restore the LUT
> > setting in suspend and resume operations.
> >
> 
> There should be a fixes tag pointing to the commit added suspend/resume
> support.
> 
Okay, fixes tag would be added later.
Thanks.

Best Regards
Richard Zhu
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> - Mani
> 
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 47
> > +++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 40eeb02ffb5d..d8f4608eb7da 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -138,6 +138,11 @@ struct imx_pcie_drvdata {
> >  	const struct dw_pcie_host_ops *ops;
> >  };
> >
> > +struct imx_lut_data {
> > +	u32 data1;
> > +	u32 data2;
> > +};
> > +
> >  struct imx_pcie {
> >  	struct dw_pcie		*pci;
> >  	struct gpio_desc	*reset_gpiod;
> > @@ -157,6 +162,8 @@ struct imx_pcie {
> >  	struct regulator	*vph;
> >  	void __iomem		*phy_base;
> >
> > +	/* LUT data for pcie */
> > +	struct imx_lut_data	luts[IMX95_MAX_LUT];
> >  	/* power domain for pcie */
> >  	struct device		*pd_pcie;
> >  	/* power domain for pcie phy */
> > @@ -1505,6 +1512,42 @@ static void imx_pcie_msi_save_restore(struct
> imx_pcie *imx_pcie, bool save)
> >  	}
> >  }
> >
> > +static void imx_pcie_lut_save(struct imx_pcie *imx_pcie) {
> > +	u32 data1, data2;
> > +	int i;
> > +
> > +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> > +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL,
> > +			     IMX95_PEO_LUT_RWA | i);
> > +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1,
> &data1);
> > +		regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2,
> &data2);
> > +		if (data1 & IMX95_PE0_LUT_VLD) {
> > +			imx_pcie->luts[i].data1 = data1;
> > +			imx_pcie->luts[i].data2 = data2;
> > +		} else {
> > +			imx_pcie->luts[i].data1 = 0;
> > +			imx_pcie->luts[i].data2 = 0;
> > +		}
> > +	}
> > +}
> > +
> > +static void imx_pcie_lut_restore(struct imx_pcie *imx_pcie) {
> > +	int i;
> > +
> > +	for (i = 0; i < IMX95_MAX_LUT; i++) {
> > +		if ((imx_pcie->luts[i].data1 & IMX95_PE0_LUT_VLD) == 0)
> > +			continue;
> > +
> > +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1,
> > +			     imx_pcie->luts[i].data1);
> > +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2,
> > +			     imx_pcie->luts[i].data2);
> > +		regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL, i);
> > +	}
> > +}
> > +
> >  static int imx_pcie_suspend_noirq(struct device *dev)  {
> >  	struct imx_pcie *imx_pcie = dev_get_drvdata(dev); @@ -1513,6 +1556,8
> > @@ static int imx_pcie_suspend_noirq(struct device *dev)
> >  		return 0;
> >
> >  	imx_pcie_msi_save_restore(imx_pcie, true);
> > +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > +		imx_pcie_lut_save(imx_pcie);
> >  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
> >  		/*
> >  		 * The minimum for a workaround would be to set PERST# and to
> @@
> > -1557,6 +1602,8 @@ static int imx_pcie_resume_noirq(struct device *dev)
> >  		if (ret)
> >  			return ret;
> >  	}
> > +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > +		imx_pcie_lut_restore(imx_pcie);
> >  	imx_pcie_msi_save_restore(imx_pcie, false);
> >
> >  	return 0;
> > --
> > 2.37.1
> >
> 
> --
> மணிவண்ணன் சதாசிவம்

RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 15:08
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
> ready
> 
> On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready Through
> > Beacon or PERST# De-assertion
> 
> Is it possible to share the link to the erratum?
> 
Sorry, the erratum document isn't ready to be published yet.
> >
> > When the auxiliary power is not available, the controller cannot exit
> > from
> > L23 Ready with beacon or PERST# de-assertion when main power is not
> > removed.
> >
> 
> I don't understand how the presence of Vaux affects the controller. Same goes
> for PERST# deassertion. How does that relate to Vaux? Is this erratum for a
> specific endpoint behavior?
IMHO I don't know the exact details of the power supplies in this IP design.
Refer to my guess , maybe the beacon detect or wake-up logic in designs is
 relied on the status of SYS_AUX_PWR_DET signals in this case.

Best Regards
Richard Zhu
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Manivannan Sadhasivam]

On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 2025年4月2日 15:08
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
> > ready
> > 
> > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready Through
> > > Beacon or PERST# De-assertion
> > 
> > Is it possible to share the link to the erratum?
> > 
> Sorry, the erratum document isn't ready to be published yet.
> > >
> > > When the auxiliary power is not available, the controller cannot exit
> > > from
> > > L23 Ready with beacon or PERST# de-assertion when main power is not
> > > removed.
> > >
> > 
> > I don't understand how the presence of Vaux affects the controller. Same goes
> > for PERST# deassertion. How does that relate to Vaux? Is this erratum for a
> > specific endpoint behavior?
> IMHO I don't know the exact details of the power supplies in this IP design.
> Refer to my guess , maybe the beacon detect or wake-up logic in designs is
>  relied on the status of SYS_AUX_PWR_DET signals in this case.

Can you please try to get more details? I couldn't understand the errata.

- Mani

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

RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月2日 23:18
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
> ready
> 
> On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Sent: 2025年4月2日 15:08
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > not exit L23 ready
> > >
> > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
> > > > Through Beacon or PERST# De-assertion
> > >
> > > Is it possible to share the link to the erratum?
> > >
> > Sorry, the erratum document isn't ready to be published yet.
> > > >
> > > > When the auxiliary power is not available, the controller cannot
> > > > exit from
> > > > L23 Ready with beacon or PERST# de-assertion when main power is
> > > > not removed.
> > > >
> > >
> > > I don't understand how the presence of Vaux affects the controller.
> > > Same goes for PERST# deassertion. How does that relate to Vaux? Is
> > > this erratum for a specific endpoint behavior?
> > IMHO I don't know the exact details of the power supplies in this IP design.
> > Refer to my guess , maybe the beacon detect or wake-up logic in
> > designs is  relied on the status of SYS_AUX_PWR_DET signals in this case.
> 
> Can you please try to get more details? I couldn't understand the errata.
> 
Sure. Will contact designer and try to get more details.

Best Regards
Richard Zhu
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Hongxing Zhu]

> -----Original Message-----
> From: Hongxing Zhu
> Sent: 2025年4月3日 11:23
> To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit
> L23 ready
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 2025年4月2日 23:18
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not
> > exit L23 ready
> >
> > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Sent: 2025年4月2日 15:08
> > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > not exit L23 ready
> > > >
> > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
> > > > > Through Beacon or PERST# De-assertion
> > > >
> > > > Is it possible to share the link to the erratum?
> > > >
> > > Sorry, the erratum document isn't ready to be published yet.
> > > > >
> > > > > When the auxiliary power is not available, the controller cannot
> > > > > exit from
> > > > > L23 Ready with beacon or PERST# de-assertion when main power is
> > > > > not removed.
> > > > >
> > > >
> > > > I don't understand how the presence of Vaux affects the controller.
> > > > Same goes for PERST# deassertion. How does that relate to Vaux? Is
> > > > this erratum for a specific endpoint behavior?
> > > IMHO I don't know the exact details of the power supplies in this IP design.
> > > Refer to my guess , maybe the beacon detect or wake-up logic in
> > > designs is  relied on the status of SYS_AUX_PWR_DET signals in this case.
> >
> > Can you please try to get more details? I couldn't understand the errata.
> >
> Sure. Will contact designer and try to get more details.
Hi Mani:
Get some information from designs, the internal design logic is relied on the
 status of SYS_AUX_PWR_DET signal to handle the low power stuff.
So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW workaround.

Best Regards
Richard Zhu

> 
> Best Regards
> Richard Zhu
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Manivannan Sadhasivam]

On Tue, Apr 08, 2025 at 03:02:42AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Hongxing Zhu
> > Sent: 2025年4月3日 11:23
> > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org
> > Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit
> > L23 ready
> > 
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Sent: 2025年4月2日 23:18
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not
> > > exit L23 ready
> > >
> > > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > Sent: 2025年4月2日 15:08
> > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > > not exit L23 ready
> > > > >
> > > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
> > > > > > Through Beacon or PERST# De-assertion
> > > > >
> > > > > Is it possible to share the link to the erratum?
> > > > >
> > > > Sorry, the erratum document isn't ready to be published yet.
> > > > > >
> > > > > > When the auxiliary power is not available, the controller cannot
> > > > > > exit from
> > > > > > L23 Ready with beacon or PERST# de-assertion when main power is
> > > > > > not removed.
> > > > > >
> > > > >
> > > > > I don't understand how the presence of Vaux affects the controller.
> > > > > Same goes for PERST# deassertion. How does that relate to Vaux? Is
> > > > > this erratum for a specific endpoint behavior?
> > > > IMHO I don't know the exact details of the power supplies in this IP design.
> > > > Refer to my guess , maybe the beacon detect or wake-up logic in
> > > > designs is  relied on the status of SYS_AUX_PWR_DET signals in this case.
> > >
> > > Can you please try to get more details? I couldn't understand the errata.
> > >
> > Sure. Will contact designer and try to get more details.
> Hi Mani:
> Get some information from designs, the internal design logic is relied on the
>  status of SYS_AUX_PWR_DET signal to handle the low power stuff.
> So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW workaround.
> 

Ok. So due to the errata, when the link enters L23 Ready state, it cannot
transition to L3 when Vaux is not available. And the workaround requires setting
SYS_AUX_PWR_DET bit?

IIUC, the issue here is that the controller is not able to detect the presence
of Vaux in the L23 Ready state. So it relies on the SYS_AUX_PWR_DET bit. But
even in that case, how would you support the endpoint *with* Vaux?

- Mani

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

RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月10日 0:44
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
> ready
> 
> On Tue, Apr 08, 2025 at 03:02:42AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Hongxing Zhu
> > > Sent: 2025年4月3日 11:23
> > > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > not exit
> > > L23 ready
> > >
> > > > -----Original Message-----
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Sent: 2025年4月2日 23:18
> > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > not exit L23 ready
> > > >
> > > > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > > > -----Original Message-----
> > > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > Sent: 2025年4月2日 15:08
> > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > > may not exit L23 ready
> > > > > >
> > > > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
> > > > > > > Through Beacon or PERST# De-assertion
> > > > > >
> > > > > > Is it possible to share the link to the erratum?
> > > > > >
> > > > > Sorry, the erratum document isn't ready to be published yet.
> > > > > > >
> > > > > > > When the auxiliary power is not available, the controller
> > > > > > > cannot exit from
> > > > > > > L23 Ready with beacon or PERST# de-assertion when main power
> > > > > > > is not removed.
> > > > > > >
> > > > > >
> > > > > > I don't understand how the presence of Vaux affects the controller.
> > > > > > Same goes for PERST# deassertion. How does that relate to
> > > > > > Vaux? Is this erratum for a specific endpoint behavior?
> > > > > IMHO I don't know the exact details of the power supplies in this IP
> design.
> > > > > Refer to my guess , maybe the beacon detect or wake-up logic in
> > > > > designs is  relied on the status of SYS_AUX_PWR_DET signals in this
> case.
> > > >
> > > > Can you please try to get more details? I couldn't understand the errata.
> > > >
> > > Sure. Will contact designer and try to get more details.
> > Hi Mani:
> > Get some information from designs, the internal design logic is relied
> > on the  status of SYS_AUX_PWR_DET signal to handle the low power stuff.
> > So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW workaround.
> >
> 
> Ok. So due to the errata, when the link enters L23 Ready state, it cannot
> transition to L3 when Vaux is not available. And the workaround requires setting
> SYS_AUX_PWR_DET bit?
> 
Refer to the description of this errata, it just mentions the exist from
 L23 Ready state.
Yes, the workaround requires setting SYS_AUX_PWR_DET bit to 1b'1.

> IIUC, the issue here is that the controller is not able to detect the presence of
> Vaux in the L23 Ready state. So it relies on the SYS_AUX_PWR_DET bit. But even
> in that case, how would you support the endpoint *with* Vaux?
> 
This errata is only applied for i.MX95 dual PCIe mode controller.
The Vaux is not present for i.MX95 PCIe EP mode either.

Best Regards
Richard Zhu
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Manivannan Sadhasivam]

On Thu, Apr 10, 2025 at 02:45:51AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 2025年4月10日 0:44
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> > kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23
> > ready
> > 
> > On Tue, Apr 08, 2025 at 03:02:42AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Hongxing Zhu
> > > > Sent: 2025年4月3日 11:23
> > > > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > not exit
> > > > L23 ready
> > > >
> > > > > -----Original Message-----
> > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > Sent: 2025年4月2日 23:18
> > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > > not exit L23 ready
> > > > >
> > > > > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > Sent: 2025年4月2日 15:08
> > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > > > may not exit L23 ready
> > > > > > >
> > > > > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > > > > ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
> > > > > > > > Through Beacon or PERST# De-assertion
> > > > > > >
> > > > > > > Is it possible to share the link to the erratum?
> > > > > > >
> > > > > > Sorry, the erratum document isn't ready to be published yet.
> > > > > > > >
> > > > > > > > When the auxiliary power is not available, the controller
> > > > > > > > cannot exit from
> > > > > > > > L23 Ready with beacon or PERST# de-assertion when main power
> > > > > > > > is not removed.
> > > > > > > >
> > > > > > >
> > > > > > > I don't understand how the presence of Vaux affects the controller.
> > > > > > > Same goes for PERST# deassertion. How does that relate to
> > > > > > > Vaux? Is this erratum for a specific endpoint behavior?
> > > > > > IMHO I don't know the exact details of the power supplies in this IP
> > design.
> > > > > > Refer to my guess , maybe the beacon detect or wake-up logic in
> > > > > > designs is  relied on the status of SYS_AUX_PWR_DET signals in this
> > case.
> > > > >
> > > > > Can you please try to get more details? I couldn't understand the errata.
> > > > >
> > > > Sure. Will contact designer and try to get more details.
> > > Hi Mani:
> > > Get some information from designs, the internal design logic is relied
> > > on the  status of SYS_AUX_PWR_DET signal to handle the low power stuff.
> > > So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW workaround.
> > >
> > 
> > Ok. So due to the errata, when the link enters L23 Ready state, it cannot
> > transition to L3 when Vaux is not available. And the workaround requires setting
> > SYS_AUX_PWR_DET bit?
> > 
> Refer to the description of this errata, it just mentions the exist from
>  L23 Ready state.

Exiting from L23 Ready == entering L2/L3. And since you mentioned that Vaux is
not available, it is definitely entering L3.

> Yes, the workaround requires setting SYS_AUX_PWR_DET bit to 1b'1.
> 
> > IIUC, the issue here is that the controller is not able to detect the presence of
> > Vaux in the L23 Ready state. So it relies on the SYS_AUX_PWR_DET bit. But even
> > in that case, how would you support the endpoint *with* Vaux?
> > 
> This errata is only applied for i.MX95 dual PCIe mode controller.
> The Vaux is not present for i.MX95 PCIe EP mode either.
> 

First of all, does the controller really know whether Vaux is supplied to the
endpoint or not? AFAIK, it is up to the board designers to route Vaux and only
endpoint should care about it.

I still feel that this specific erratum is for fixing the issue with some
endpoints where Vaux is not supplied and the link doesn't exit L23 Ready. Again,
what would be the behavior if Vaux is supplied to the endpoint? You cannot just
say that the controller doesn't support Vaux, which is not a valid statement
IMO.

- Mani

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

RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Hongxing Zhu]

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2025年4月13日 23:39
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit
> L23 ready
> 
> On Thu, Apr 10, 2025 at 02:45:51AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Sent: 2025年4月10日 0:44
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > not exit L23 ready
> > >
> > > On Tue, Apr 08, 2025 at 03:02:42AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Hongxing Zhu
> > > > > Sent: 2025年4月3日 11:23
> > > > > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > linux-arm-kernel@lists.infradead.org;
> > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > may not exit
> > > > > L23 ready
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org>
> > > > > > Sent: 2025年4月2日 23:18
> > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > > may not exit L23 ready
> > > > > >
> > > > > > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Manivannan Sadhasivam
> > > > > > > > <manivannan.sadhasivam@linaro.org>
> > > > > > > > Sent: 2025年4月2日 15:08
> > > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95
> > > > > > > > PCIe may not exit L23 ready
> > > > > > > >
> > > > > > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > > > > > ERR051624: The Controller Without Vaux Cannot Exit L23
> > > > > > > > > Ready Through Beacon or PERST# De-assertion
> > > > > > > >
> > > > > > > > Is it possible to share the link to the erratum?
> > > > > > > >
> > > > > > > Sorry, the erratum document isn't ready to be published yet.
> > > > > > > > >
> > > > > > > > > When the auxiliary power is not available, the
> > > > > > > > > controller cannot exit from
> > > > > > > > > L23 Ready with beacon or PERST# de-assertion when main
> > > > > > > > > power is not removed.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I don't understand how the presence of Vaux affects the
> controller.
> > > > > > > > Same goes for PERST# deassertion. How does that relate to
> > > > > > > > Vaux? Is this erratum for a specific endpoint behavior?
> > > > > > > IMHO I don't know the exact details of the power supplies in
> > > > > > > this IP
> > > design.
> > > > > > > Refer to my guess , maybe the beacon detect or wake-up logic
> > > > > > > in designs is  relied on the status of SYS_AUX_PWR_DET
> > > > > > > signals in this
> > > case.
> > > > > >
> > > > > > Can you please try to get more details? I couldn't understand the
> errata.
> > > > > >
> > > > > Sure. Will contact designer and try to get more details.
> > > > Hi Mani:
> > > > Get some information from designs, the internal design logic is
> > > > relied on the  status of SYS_AUX_PWR_DET signal to handle the low
> power stuff.
> > > > So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW
> workaround.
> > > >
> > >
> > > Ok. So due to the errata, when the link enters L23 Ready state, it
> > > cannot transition to L3 when Vaux is not available. And the
> > > workaround requires setting SYS_AUX_PWR_DET bit?
> > >
> > Refer to the description of this errata, it just mentions the exist
> > from
> >  L23 Ready state.
> 
> Exiting from L23 Ready == entering L2/L3. And since you mentioned that Vaux
> is not available, it is definitely entering L3.
> 
> > Yes, the workaround requires setting SYS_AUX_PWR_DET bit to 1b'1.
> >
> > > IIUC, the issue here is that the controller is not able to detect
> > > the presence of Vaux in the L23 Ready state. So it relies on the
> > > SYS_AUX_PWR_DET bit. But even in that case, how would you support the
> endpoint *with* Vaux?
> > >
> > This errata is only applied for i.MX95 dual PCIe mode controller.
> > The Vaux is not present for i.MX95 PCIe EP mode either.
> >
> 
> First of all, does the controller really know whether Vaux is supplied to the
> endpoint or not? AFAIK, it is up to the board designers to route Vaux and only
> endpoint should care about it.
> 
> I still feel that this specific erratum is for fixing the issue with some endpoints
> where Vaux is not supplied and the link doesn't exit L23 Ready. Again, what
> would be the behavior if Vaux is supplied to the endpoint? You cannot just say
> that the controller doesn't support Vaux, which is not a valid statement IMO.
> 
Sorry, I miss-understand the question you posted in the previous reply.
I get the following answer from designers when the Vaux is supplied to the
 remote endpoint. Hope it can get ride of your concerns.
Q:
How about the situations when remote partner has the Vaux present?
For example, i.MX95 PCIe used as RC, and a endpoint device with one Vaux
 present is connected to i.MX95 PCIe RC.

A:
" As per my understanding it should work irrespective of vaux presence in remote partner."

Best Regards
Richard Zhu

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

Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit L23 ready

[Manivannan Sadhasivam]

On Mon, Apr 14, 2025 at 03:15:28AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 2025年4月13日 23:39
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may not exit
> > L23 ready
> > 
> > On Thu, Apr 10, 2025 at 02:45:51AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Sent: 2025年4月10日 0:44
> > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe may
> > > > not exit L23 ready
> > > >
> > > > On Tue, Apr 08, 2025 at 03:02:42AM +0000, Hongxing Zhu wrote:
> > > > > > -----Original Message-----
> > > > > > From: Hongxing Zhu
> > > > > > Sent: 2025年4月3日 11:23
> > > > > > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > Subject: RE: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > > may not exit
> > > > > > L23 ready
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org>
> > > > > > > Sent: 2025年4月2日 23:18
> > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95 PCIe
> > > > > > > may not exit L23 ready
> > > > > > >
> > > > > > > On Wed, Apr 02, 2025 at 07:59:26AM +0000, Hongxing Zhu wrote:
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Manivannan Sadhasivam
> > > > > > > > > <manivannan.sadhasivam@linaro.org>
> > > > > > > > > Sent: 2025年4月2日 15:08
> > > > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > > > > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > > > > > > > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > > > > > > > bhelgaas@google.com; shawnguo@kernel.org;
> > > > > > > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > > > > > > > festevam@gmail.com; linux-pci@vger.kernel.org;
> > > > > > > > > linux-arm-kernel@lists.infradead.org;
> > > > > > > > > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > > > > > > > > Subject: Re: [PATCH v3 3/6] PCI: imx6: Workaround i.MX95
> > > > > > > > > PCIe may not exit L23 ready
> > > > > > > > >
> > > > > > > > > On Fri, Mar 28, 2025 at 11:02:10AM +0800, Richard Zhu wrote:
> > > > > > > > > > ERR051624: The Controller Without Vaux Cannot Exit L23
> > > > > > > > > > Ready Through Beacon or PERST# De-assertion
> > > > > > > > >
> > > > > > > > > Is it possible to share the link to the erratum?
> > > > > > > > >
> > > > > > > > Sorry, the erratum document isn't ready to be published yet.
> > > > > > > > > >
> > > > > > > > > > When the auxiliary power is not available, the
> > > > > > > > > > controller cannot exit from
> > > > > > > > > > L23 Ready with beacon or PERST# de-assertion when main
> > > > > > > > > > power is not removed.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I don't understand how the presence of Vaux affects the
> > controller.
> > > > > > > > > Same goes for PERST# deassertion. How does that relate to
> > > > > > > > > Vaux? Is this erratum for a specific endpoint behavior?
> > > > > > > > IMHO I don't know the exact details of the power supplies in
> > > > > > > > this IP
> > > > design.
> > > > > > > > Refer to my guess , maybe the beacon detect or wake-up logic
> > > > > > > > in designs is  relied on the status of SYS_AUX_PWR_DET
> > > > > > > > signals in this
> > > > case.
> > > > > > >
> > > > > > > Can you please try to get more details? I couldn't understand the
> > errata.
> > > > > > >
> > > > > > Sure. Will contact designer and try to get more details.
> > > > > Hi Mani:
> > > > > Get some information from designs, the internal design logic is
> > > > > relied on the  status of SYS_AUX_PWR_DET signal to handle the low
> > power stuff.
> > > > > So, the SYS_AUX_PWR_DET is required to be 1b'1 in the SW
> > workaround.
> > > > >
> > > >
> > > > Ok. So due to the errata, when the link enters L23 Ready state, it
> > > > cannot transition to L3 when Vaux is not available. And the
> > > > workaround requires setting SYS_AUX_PWR_DET bit?
> > > >
> > > Refer to the description of this errata, it just mentions the exist
> > > from
> > >  L23 Ready state.
> > 
> > Exiting from L23 Ready == entering L2/L3. And since you mentioned that Vaux
> > is not available, it is definitely entering L3.
> > 
> > > Yes, the workaround requires setting SYS_AUX_PWR_DET bit to 1b'1.
> > >
> > > > IIUC, the issue here is that the controller is not able to detect
> > > > the presence of Vaux in the L23 Ready state. So it relies on the
> > > > SYS_AUX_PWR_DET bit. But even in that case, how would you support the
> > endpoint *with* Vaux?
> > > >
> > > This errata is only applied for i.MX95 dual PCIe mode controller.
> > > The Vaux is not present for i.MX95 PCIe EP mode either.
> > >
> > 
> > First of all, does the controller really know whether Vaux is supplied to the
> > endpoint or not? AFAIK, it is up to the board designers to route Vaux and only
> > endpoint should care about it.
> > 
> > I still feel that this specific erratum is for fixing the issue with some endpoints
> > where Vaux is not supplied and the link doesn't exit L23 Ready. Again, what
> > would be the behavior if Vaux is supplied to the endpoint? You cannot just say
> > that the controller doesn't support Vaux, which is not a valid statement IMO.
> > 
> Sorry, I miss-understand the question you posted in the previous reply.
> I get the following answer from designers when the Vaux is supplied to the
>  remote endpoint. Hope it can get ride of your concerns.
> Q:
> How about the situations when remote partner has the Vaux present?
> For example, i.MX95 PCIe used as RC, and a endpoint device with one Vaux
>  present is connected to i.MX95 PCIe RC.
> 
> A:
> " As per my understanding it should work irrespective of vaux presence in remote partner."

Okay, thanks for the confirmation. Please include this information in commit
message for documentation.

- Mani

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