[PATCH v5 0/2] PCI: imx6: Add a method to handle CLKREQ# override

[PATCH v5 0/2] PCI: imx6: Add a method to handle CLKREQ# override

[Richard Zhu]

The CLKREQ# is an open drain, active low signal that is driven low by
the card to request reference clock. It's an optional signal added in
PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
reserved.

Since the reference clock controlled by CLKREQ# may be required by i.MX
PCIe host too. To make sure this clock is ready even when the CLKREQ#
isn't driven low by the card(e.x the scenario described above), force
CLKREQ# override active low for i.MX PCIe host during initialization.

The CLKREQ# override can be cleared safely when supports-clkreq is
present and PCIe link is up later. Because the CLKREQ# would be driven
low by the card at this time.

Main changes in v5:
- New create imx8mm_pcie_clkreq_override() and keep the original
  enable_ref_clk callback function.

Main changes in v4:
- To align the function name when add the CLKREQ# override clear, rename
imx8mm_pcie_enable_ref_clk(), clean up codes refer to Mani' suggestions.

Main changes in v3:
- Rebase to v6.17-rc1.
- Update the commit message refer to Bjorn's suggestions.

Main changes in v2:
- Update the commit message, and collect the reviewed-by tag.

[PATCH v5 1/2] PCI: dwc: Invoke post_init in dw_pcie_resume_noirq()
[PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override

drivers/pci/controller/dwc/pci-imx6.c             | 43 ++++++++++++++++++++++++++++++++++++++++++-
drivers/pci/controller/dwc/pcie-designware-host.c |  3 +++
2 files changed, 45 insertions(+), 1 deletion(-)

[PATCH v5 1/2] PCI: dwc: Invoke post_init in dw_pcie_resume_noirq()

[Richard Zhu]

If the ops has post_init callback, invoke it in dw_pcie_resume_noirq().

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

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b501..f24f4cd5c278 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1079,6 +1079,9 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
 	if (ret)
 		return ret;
 
+	if (pci->pp.ops->post_init)
+		pci->pp.ops->post_init(&pci->pp);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dw_pcie_resume_noirq);
-- 
2.37.1

[PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Richard Zhu]

The CLKREQ# is an open drain, active low signal that is driven low by
the card to request reference clock. It's an optional signal added in
PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
reserved.

Since the reference clock controlled by CLKREQ# may be required by i.MX
PCIe host too. To make sure this clock is ready even when the CLKREQ#
isn't driven low by the card(e.x the scenario described above), force
CLKREQ# override active low for i.MX PCIe host during initialization.

The CLKREQ# override can be cleared safely when supports-clkreq is
present and PCIe link is up later. Because the CLKREQ# would be driven
low by the card at this time.

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

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 80e48746bbaf..6b03b1111d06 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -52,6 +52,8 @@
 #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_CLKREQ_OVERRIDE_EN		BIT(8)
+#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
 #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
 
 #define IMX95_PE0_GEN_CTRL_1			0x1050
@@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
 	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
 	int (*core_reset)(struct imx_pcie *pcie, bool assert);
 	int (*wait_pll_lock)(struct imx_pcie *pcie);
+	void (*clr_clkreq_override)(struct imx_pcie *pcie);
 	const struct dw_pcie_host_ops *ops;
 };
 
@@ -149,6 +152,7 @@ struct imx_pcie {
 	struct gpio_desc	*reset_gpiod;
 	struct clk_bulk_data	*clks;
 	int			num_clks;
+	bool			supports_clkreq;
 	struct regmap		*iomuxc_gpr;
 	u16			msi_ctrl;
 	u32			controller_id;
@@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
 	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
 }
 
+static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
+{
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
+			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
+			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
+			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
+			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0);
+}
+
 static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 {
 	/*
@@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
 	return 0;
 }
 
-static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
+static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
 {
 	int offset = imx_pcie_grp_offset(imx_pcie);
 
@@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
 	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
 			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
 			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
+}
+
+static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
+{
+	imx8mm_pcie_clkreq_override(imx_pcie, enable);
 	return 0;
 }
 
@@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 		regulator_disable(imx_pcie->vpcie);
 }
 
+static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
+{
+	imx8mm_pcie_clkreq_override(imx_pcie, false);
+}
+
+static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
+{
+	imx95_pcie_clkreq_override(imx_pcie, false);
+}
+
 static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1322,6 +1351,12 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
 		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
 		dw_pcie_dbi_ro_wr_dis(pci);
 	}
+
+	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
+	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
+		if (imx_pcie->drvdata->clr_clkreq_override)
+			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
+	}
 }
 
 /*
@@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	pci->max_link_speed = 1;
 	of_property_read_u32(node, "fsl,max-link-speed", &pci->max_link_speed);
 
+	imx_pcie->supports_clkreq =
+		of_property_read_bool(node, "supports-clkreq");
 	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
 	if (IS_ERR(imx_pcie->vpcie)) {
 		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV)
@@ -1873,6 +1910,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
 		.init_phy = imx8mq_pcie_init_phy,
 		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
+		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
 	},
 	[IMX8MM] = {
 		.variant = IMX8MM,
@@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
+		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
 	},
 	[IMX8MP] = {
 		.variant = IMX8MP,
@@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
+		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
 	},
 	[IMX8Q] = {
 		.variant = IMX8Q,
@@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.core_reset = imx95_pcie_core_reset,
 		.init_phy = imx95_pcie_init_phy,
 		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
+		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
 	},
 	[IMX8MQ_EP] = {
 		.variant = IMX8MQ_EP,
-- 
2.37.1

Re: [PATCH v5 0/2] PCI: imx6: Add a method to handle CLKREQ# override

[Manivannan Sadhasivam]

On Tue, 23 Sep 2025 15:39:11 +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock. It's an optional signal added in
> PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> reserved.
> 
> Since the reference clock controlled by CLKREQ# may be required by i.MX
> PCIe host too. To make sure this clock is ready even when the CLKREQ#
> isn't driven low by the card(e.x the scenario described above), force
> CLKREQ# override active low for i.MX PCIe host during initialization.
> 
> [...]

Applied, thanks!

[1/2] PCI: dwc: Invoke post_init in dw_pcie_resume_noirq()
      commit: 27afd094ea4be542594058d685c7af4654ab8de7
[2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
      commit: dc853fb85d6b90e0805660c1dbb38ef7641af839

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Tue, Sep 23, 2025 at 03:39:13PM +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock. It's an optional signal added in
> PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> reserved.
> 
> Since the reference clock controlled by CLKREQ# may be required by i.MX
> PCIe host too. To make sure this clock is ready even when the CLKREQ#
> isn't driven low by the card(e.x the scenario described above), force
> CLKREQ# override active low for i.MX PCIe host during initialization.
> 
> The CLKREQ# override can be cleared safely when supports-clkreq is
> present and PCIe link is up later. Because the CLKREQ# would be driven
> low by the card at this time.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 43 ++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf..6b03b1111d06 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -52,6 +52,8 @@
>  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
>  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
>  
>  #define IMX95_PE0_GEN_CTRL_1			0x1050
> @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
>  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
>  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
>  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
>  	const struct dw_pcie_host_ops *ops;
>  };
>  
> @@ -149,6 +152,7 @@ struct imx_pcie {
>  	struct gpio_desc	*reset_gpiod;
>  	struct clk_bulk_data	*clks;
>  	int			num_clks;
> +	bool			supports_clkreq;
>  	struct regmap		*iomuxc_gpr;
>  	u16			msi_ctrl;
>  	u32			controller_id;
> @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
>  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
>  }
>  
> +static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
> +{
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0);
> +}
> +
>  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
>  {
>  	/*
> @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  	return 0;
>  }
>  
> -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
> +static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
>  {
>  	int offset = imx_pcie_grp_offset(imx_pcie);
>  
> @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
>  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
>  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
> +}
> +
> +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
> +{
> +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
>  	return 0;
>  }
>  
> @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  		regulator_disable(imx_pcie->vpcie);
>  }
>  
> +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> +	imx8mm_pcie_clkreq_override(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> +	imx95_pcie_clkreq_override(imx_pcie, false);
> +}
> +
>  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1351,12 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
>  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
>  		dw_pcie_dbi_ro_wr_dis(pci);
>  	}
> +
> +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> +		if (imx_pcie->drvdata->clr_clkreq_override)
> +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> +	}
>  }
>  
>  /*
> @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	pci->max_link_speed = 1;
>  	of_property_read_u32(node, "fsl,max-link-speed", &pci->max_link_speed);
>  
> +	imx_pcie->supports_clkreq =
> +		of_property_read_bool(node, "supports-clkreq");
>  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
>  	if (IS_ERR(imx_pcie->vpcie)) {
>  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV)
> @@ -1873,6 +1910,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
>  		.init_phy = imx8mq_pcie_init_phy,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
>  	},
>  	[IMX8MM] = {
>  		.variant = IMX8MM,
> @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_off[0] = IOMUXC_GPR12,
>  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
>  	},
>  	[IMX8MP] = {
>  		.variant = IMX8MP,
> @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_off[0] = IOMUXC_GPR12,
>  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,

imx8mm_pcie_enable_ref_clk() and imx8mm_pcie_clr_clkreq_override()
both call imx8mm_pcie_clkreq_override().

All these devices that use imx8mm_pcie_clr_clkreq_override() also
enable the refclk in .init() by overriding clkreq, so .post_init()
clears that override.

>  	},
>  	[IMX8Q] = {
>  		.variant = IMX8Q,
> @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.core_reset = imx95_pcie_core_reset,
>  		.init_phy = imx95_pcie_init_phy,
>  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,

But here there's no .enable_ref_clk() method, so I guess this device
must override clkreq automatically and the driver doesn't have to
enable that?

>  	},
>  	[IMX8MQ_EP] = {
>  		.variant = IMX8MQ_EP,
> -- 
> 2.37.1
> 

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Tue, Sep 23, 2025 at 03:39:13PM +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock. It's an optional signal added in
> PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> reserved.
> 
> Since the reference clock controlled by CLKREQ# may be required by i.MX
> PCIe host too. To make sure this clock is ready even when the CLKREQ#
> isn't driven low by the card(e.x the scenario described above), force
> CLKREQ# override active low for i.MX PCIe host during initialization.
> 
> The CLKREQ# override can be cleared safely when supports-clkreq is
> present and PCIe link is up later. Because the CLKREQ# would be driven
> low by the card at this time.

What happens if we clear the CLKREQ# override (so the host doesn't
assert it), and the link is up but the card never asserts CLKREQ#
(since it's an optional signal)?

Does the i.MX host still work?

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 43 ++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf..6b03b1111d06 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -52,6 +52,8 @@
>  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
>  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
>  
>  #define IMX95_PE0_GEN_CTRL_1			0x1050
> @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
>  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
>  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
>  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
>  	const struct dw_pcie_host_ops *ops;
>  };
>  
> @@ -149,6 +152,7 @@ struct imx_pcie {
>  	struct gpio_desc	*reset_gpiod;
>  	struct clk_bulk_data	*clks;
>  	int			num_clks;
> +	bool			supports_clkreq;
>  	struct regmap		*iomuxc_gpr;
>  	u16			msi_ctrl;
>  	u32			controller_id;
> @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
>  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
>  }
>  
> +static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
> +{
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0);
> +}
> +
>  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
>  {
>  	/*
> @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  	return 0;
>  }
>  
> -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
> +static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie, bool enable)
>  {
>  	int offset = imx_pcie_grp_offset(imx_pcie);
>  
> @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
>  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
>  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
> +}
> +
> +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
> +{
> +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
>  	return 0;
>  }
>  
> @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  		regulator_disable(imx_pcie->vpcie);
>  }
>  
> +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> +	imx8mm_pcie_clkreq_override(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> +	imx95_pcie_clkreq_override(imx_pcie, false);
> +}
> +
>  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1351,12 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
>  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
>  		dw_pcie_dbi_ro_wr_dis(pci);
>  	}
> +
> +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> +		if (imx_pcie->drvdata->clr_clkreq_override)
> +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> +	}
>  }
>  
>  /*
> @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	pci->max_link_speed = 1;
>  	of_property_read_u32(node, "fsl,max-link-speed", &pci->max_link_speed);
>  
> +	imx_pcie->supports_clkreq =
> +		of_property_read_bool(node, "supports-clkreq");
>  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
>  	if (IS_ERR(imx_pcie->vpcie)) {
>  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV)
> @@ -1873,6 +1910,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
>  		.init_phy = imx8mq_pcie_init_phy,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
>  	},
>  	[IMX8MM] = {
>  		.variant = IMX8MM,
> @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_off[0] = IOMUXC_GPR12,
>  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
>  	},
>  	[IMX8MP] = {
>  		.variant = IMX8MP,
> @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.mode_off[0] = IOMUXC_GPR12,
>  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
>  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
>  	},
>  	[IMX8Q] = {
>  		.variant = IMX8Q,
> @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
>  		.core_reset = imx95_pcie_core_reset,
>  		.init_phy = imx95_pcie_init_phy,
>  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
>  	},
>  	[IMX8MQ_EP] = {
>  		.variant = IMX8MQ_EP,
> -- 
> 2.37.1
> 

RE: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年9月26日 5:58
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; jingoohan1@gmail.com;
> l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> mani@kernel.org; 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 v5 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
> 
> On Tue, Sep 23, 2025 at 03:39:13PM +0800, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock. It's an optional signal added in
> > PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> > reserved.
> >
> > Since the reference clock controlled by CLKREQ# may be required by
> > i.MX PCIe host too. To make sure this clock is ready even when the
> > CLKREQ# isn't driven low by the card(e.x the scenario described
> > above), force CLKREQ# override active low for i.MX PCIe host during
> initialization.
> >
> > The CLKREQ# override can be cleared safely when supports-clkreq is
> > present and PCIe link is up later. Because the CLKREQ# would be driven
> > low by the card at this time.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > ++++++++++++++++++++++++++-
> >  1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf..6b03b1111d06 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -52,6 +52,8 @@
> >  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
> >  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
> >
> >  #define IMX95_PE0_GEN_CTRL_1			0x1050
> > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> >  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> >  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
> >  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> > +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
> >  	const struct dw_pcie_host_ops *ops;
> >  };
> >
> > @@ -149,6 +152,7 @@ struct imx_pcie {
> >  	struct gpio_desc	*reset_gpiod;
> >  	struct clk_bulk_data	*clks;
> >  	int			num_clks;
> > +	bool			supports_clkreq;
> >  	struct regmap		*iomuxc_gpr;
> >  	u16			msi_ctrl;
> >  	u32			controller_id;
> > @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const struct
> imx_pcie *imx_pcie)
> >  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> > }
> >
> > +static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > +bool enable) {
> > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0); }
> > +
> >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> >  	/*
> > @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct imx_pcie
> *imx_pcie, bool enable)
> >  	return 0;
> >  }
> >
> > -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > enable)
> > +static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > +bool enable)
> >  {
> >  	int offset = imx_pcie_grp_offset(imx_pcie);
> >
> > @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct
> imx_pcie *imx_pcie, bool enable)
> >  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
> >  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> >  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
> > +}
> > +
> > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > +enable) {
> > +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
> >  	return 0;
> >  }
> >
> > @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct dw_pcie_rp
> *pp)
> >  		regulator_disable(imx_pcie->vpcie);
> >  }
> >
> > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > +*imx_pcie) {
> > +	imx8mm_pcie_clkreq_override(imx_pcie, false); }
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > +	imx95_pcie_clkreq_override(imx_pcie, false); }
> > +
> >  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6 +1351,12
> @@
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> >  		dw_pcie_dbi_ro_wr_dis(pci);
> >  	}
> > +
> > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > +	}
> >  }
> >
> >  /*
> > @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> >  	pci->max_link_speed = 1;
> >  	of_property_read_u32(node, "fsl,max-link-speed",
> > &pci->max_link_speed);
> >
> > +	imx_pcie->supports_clkreq =
> > +		of_property_read_bool(node, "supports-clkreq");
> >  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
> >  	if (IS_ERR(imx_pcie->vpcie)) {
> >  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV) @@ -1873,6 +1910,7 @@
> > static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
> >  		.init_phy = imx8mq_pcie_init_phy,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8MM] = {
> >  		.variant = IMX8MM,
> > @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_off[0] = IOMUXC_GPR12,
> >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8MP] = {
> >  		.variant = IMX8MP,
> > @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_off[0] = IOMUXC_GPR12,
> >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> 
> imx8mm_pcie_enable_ref_clk() and imx8mm_pcie_clr_clkreq_override() both
> call imx8mm_pcie_clkreq_override().
> 
> All these devices that use imx8mm_pcie_clr_clkreq_override() also enable the
> refclk in .init() by overriding clkreq, so .post_init() clears that override.
> 
On i.MX8M, the override clkreq# operations are wrapped as .enable_ref_clk().
This would block L1ss feature enablement, if the override active low is not
 cleared. This patch-set paves the path to support L1ss feature on i.MX8M PCIes.
> >  	},
> >  	[IMX8Q] = {
> >  		.variant = IMX8Q,
> > @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.core_reset = imx95_pcie_core_reset,
> >  		.init_phy = imx95_pcie_init_phy,
> >  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> > +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
> 
> But here there's no .enable_ref_clk() method, so I guess this device must
> override clkreq automatically and the driver doesn't have to enable that?
Wow, it's my fault that I made a mistake when sent out the v4 after v3 review
 around. "imx95_pcie_clkreq_override(imx_pcie, true);" should be added when I
 pack the codes into a new function in v4.

https://lore.kernel.org/imx/hsmebnz6opoj45zztdd2svmdtrwwwrngjaidpltbunnkdvvdqz@lhyejtlwkkes/

--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -281,6 +281,8 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
                           IMX95_PCIE_REF_CLKEN,
                           IMX95_PCIE_REF_CLKEN);

+       imx95_pcie_clkreq_override(imx_pcie, true);
+

Thanks a lot for your carefully review here.

Hi Mani:
The v5 patch-set miss the changes listed above, please backout the applied v5
 patch-set. Should I re-post next version?
Sorry to bring inconvenience to you.

Best Regards
Richard Zhu
> 
> >  	},
> >  	[IMX8MQ_EP] = {
> >  		.variant = IMX8MQ_EP,
> > --
> > 2.37.1
> >

RE: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年9月26日 6:04
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; jingoohan1@gmail.com;
> l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> mani@kernel.org; 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 v5 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
> 
> On Tue, Sep 23, 2025 at 03:39:13PM +0800, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock. It's an optional signal added in
> > PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> > reserved.
> >
> > Since the reference clock controlled by CLKREQ# may be required by
> > i.MX PCIe host too. To make sure this clock is ready even when the
> > CLKREQ# isn't driven low by the card(e.x the scenario described
> > above), force CLKREQ# override active low for i.MX PCIe host during
> initialization.
> >
> > The CLKREQ# override can be cleared safely when supports-clkreq is
> > present and PCIe link is up later. Because the CLKREQ# would be driven
> > low by the card at this time.
> 
> What happens if we clear the CLKREQ# override (so the host doesn't assert
> it), and the link is up but the card never asserts CLKREQ# (since it's an
> optional signal)?
> 
> Does the i.MX host still work?
Hi Bjorn:
Thanks for your kindly concerns.
The CLKREQ# override active low only be cleared when link is up and
 supports-clkreq is present. In the other words, there is a remote endpoint
 device, and the CLKREQ# would be driven active low by this endpoint device.

Best Regards
Richard Zhu
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > ++++++++++++++++++++++++++-
> >  1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf..6b03b1111d06 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -52,6 +52,8 @@
> >  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
> >  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
> >
> >  #define IMX95_PE0_GEN_CTRL_1			0x1050
> > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> >  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> >  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
> >  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> > +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
> >  	const struct dw_pcie_host_ops *ops;
> >  };
> >
> > @@ -149,6 +152,7 @@ struct imx_pcie {
> >  	struct gpio_desc	*reset_gpiod;
> >  	struct clk_bulk_data	*clks;
> >  	int			num_clks;
> > +	bool			supports_clkreq;
> >  	struct regmap		*iomuxc_gpr;
> >  	u16			msi_ctrl;
> >  	u32			controller_id;
> > @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const
> struct imx_pcie *imx_pcie)
> >  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> > }
> >
> > +static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > +bool enable) {
> > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0); }
> > +
> >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> >  	/*
> > @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct
> imx_pcie *imx_pcie, bool enable)
> >  	return 0;
> >  }
> >
> > -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > enable)
> > +static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > +bool enable)
> >  {
> >  	int offset = imx_pcie_grp_offset(imx_pcie);
> >
> > @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct
> imx_pcie *imx_pcie, bool enable)
> >  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
> >  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> >  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
> > +}
> > +
> > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > +enable) {
> > +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
> >  	return 0;
> >  }
> >
> > @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct
> dw_pcie_rp *pp)
> >  		regulator_disable(imx_pcie->vpcie);
> >  }
> >
> > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > +*imx_pcie) {
> > +	imx8mm_pcie_clkreq_override(imx_pcie, false); }
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > +	imx95_pcie_clkreq_override(imx_pcie, false); }
> > +
> >  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6 +1351,12
> @@
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> >  		dw_pcie_dbi_ro_wr_dis(pci);
> >  	}
> > +
> > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > +	}
> >  }
> >
> >  /*
> > @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> >  	pci->max_link_speed = 1;
> >  	of_property_read_u32(node, "fsl,max-link-speed",
> > &pci->max_link_speed);
> >
> > +	imx_pcie->supports_clkreq =
> > +		of_property_read_bool(node, "supports-clkreq");
> >  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
> >  	if (IS_ERR(imx_pcie->vpcie)) {
> >  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV) @@ -1873,6 +1910,7 @@
> > static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
> >  		.init_phy = imx8mq_pcie_init_phy,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8MM] = {
> >  		.variant = IMX8MM,
> > @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_off[0] = IOMUXC_GPR12,
> >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8MP] = {
> >  		.variant = IMX8MP,
> > @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.mode_off[0] = IOMUXC_GPR12,
> >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8Q] = {
> >  		.variant = IMX8Q,
> > @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  		.core_reset = imx95_pcie_core_reset,
> >  		.init_phy = imx95_pcie_init_phy,
> >  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> > +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
> >  	},
> >  	[IMX8MQ_EP] = {
> >  		.variant = IMX8MQ_EP,
> > --
> > 2.37.1
> >

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > On Tue, Sep 23, 2025 at 03:39:13PM +0800, Richard Zhu wrote:
> > > The CLKREQ# is an open drain, active low signal that is driven low by
> > > the card to request reference clock. It's an optional signal added in
> > > PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be driven low if it's
> > > reserved.
> > >
> > > Since the reference clock controlled by CLKREQ# may be required by
> > > i.MX PCIe host too. To make sure this clock is ready even when the
> > > CLKREQ# isn't driven low by the card(e.x the scenario described
> > > above), force CLKREQ# override active low for i.MX PCIe host during
> > initialization.
> > >
> > > The CLKREQ# override can be cleared safely when supports-clkreq is
> > > present and PCIe link is up later. Because the CLKREQ# would be driven
> > > low by the card at this time.
> > 
> > What happens if we clear the CLKREQ# override (so the host doesn't assert
> > it), and the link is up but the card never asserts CLKREQ# (since it's an
> > optional signal)?
> > 
> > Does the i.MX host still work?
>
> The CLKREQ# override active low only be cleared when link is up and
>  supports-clkreq is present. In the other words, there is a remote endpoint
>  device, and the CLKREQ# would be driven active low by this endpoint device.

Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in CEM
r2.0, so even if the endpoint is present and the link is up, the
endpoint will not assert CLKREQ#.

Will the i.MX host still work?

IIUC, CLKREQ# is required for ASPM L1 PM Substates.  Maybe the CLKREQ#
override should only be cleared if the endpoint advertises L1 PM
Substates support?

Sorry, I'm just really confused about this.  Here's another question:
Even if the endpoint is designed to CEM r4.0, it supports L1 PM
Substates, and it asserts CLKREQ#, my understanding is that the
endpoint won't assert CLKREQ# *all* the time.  For example, when the
link enters L1, the device deasserts CLKREQ# (CEM r5.0, sec 2.8.2).

What happens to the i.MX host when the endpoint isn't asserting
CLKREQ#?  I guess i.MX doesn't need refclk in that situation?

> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > > ++++++++++++++++++++++++++-
> > >  1 file changed, 42 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 80e48746bbaf..6b03b1111d06 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -52,6 +52,8 @@
> > >  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> > > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
> > >  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
> > >
> > >  #define IMX95_PE0_GEN_CTRL_1			0x1050
> > > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> > >  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> > >  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
> > >  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> > > +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
> > >  	const struct dw_pcie_host_ops *ops;
> > >  };
> > >
> > > @@ -149,6 +152,7 @@ struct imx_pcie {
> > >  	struct gpio_desc	*reset_gpiod;
> > >  	struct clk_bulk_data	*clks;
> > >  	int			num_clks;
> > > +	bool			supports_clkreq;
> > >  	struct regmap		*iomuxc_gpr;
> > >  	u16			msi_ctrl;
> > >  	u32			controller_id;
> > > @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const
> > struct imx_pcie *imx_pcie)
> > >  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> > > }
> > >
> > > +static void  imx95_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > > +bool enable) {
> > > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> > > +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0); }
> > > +
> > >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> > >  	/*
> > > @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct
> > imx_pcie *imx_pcie, bool enable)
> > >  	return 0;
> > >  }
> > >
> > > -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > > enable)
> > > +static void imx8mm_pcie_clkreq_override(struct imx_pcie *imx_pcie,
> > > +bool enable)
> > >  {
> > >  	int offset = imx_pcie_grp_offset(imx_pcie);
> > >
> > > @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct
> > imx_pcie *imx_pcie, bool enable)
> > >  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
> > >  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> > >  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN : 0);
> > > +}
> > > +
> > > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool
> > > +enable) {
> > > +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
> > >  	return 0;
> > >  }
> > >
> > > @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct
> > dw_pcie_rp *pp)
> > >  		regulator_disable(imx_pcie->vpcie);
> > >  }
> > >
> > > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > > +*imx_pcie) {
> > > +	imx8mm_pcie_clkreq_override(imx_pcie, false); }
> > > +
> > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > > +{
> > > +	imx95_pcie_clkreq_override(imx_pcie, false); }
> > > +
> > >  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)  {
> > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6 +1351,12
> > @@
> > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > >  		dw_pcie_dbi_ro_wr_dis(pci);
> > >  	}
> > > +
> > > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > > +	}
> > >  }
> > >
> > >  /*
> > > @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct platform_device
> > *pdev)
> > >  	pci->max_link_speed = 1;
> > >  	of_property_read_u32(node, "fsl,max-link-speed",
> > > &pci->max_link_speed);
> > >
> > > +	imx_pcie->supports_clkreq =
> > > +		of_property_read_bool(node, "supports-clkreq");
> > >  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
> > >  	if (IS_ERR(imx_pcie->vpcie)) {
> > >  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV) @@ -1873,6 +1910,7 @@
> > > static const struct imx_pcie_drvdata drvdata[] = {
> > >  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
> > >  		.init_phy = imx8mq_pcie_init_phy,
> > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > >  	},
> > >  	[IMX8MM] = {
> > >  		.variant = IMX8MM,
> > > @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> > >  		.mode_off[0] = IOMUXC_GPR12,
> > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > >  	},
> > >  	[IMX8MP] = {
> > >  		.variant = IMX8MP,
> > > @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> > >  		.mode_off[0] = IOMUXC_GPR12,
> > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > >  	},
> > >  	[IMX8Q] = {
> > >  		.variant = IMX8Q,
> > > @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> > >  		.core_reset = imx95_pcie_core_reset,
> > >  		.init_phy = imx95_pcie_init_phy,
> > >  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> > > +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
> > >  	},
> > >  	[IMX8MQ_EP] = {
> > >  		.variant = IMX8MQ_EP,
> > > --
> > > 2.37.1
> > >

RE: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年9月26日 10:45
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; jingoohan1@gmail.com;
> l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> mani@kernel.org; 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 v5 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
> 
> On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > The CLKREQ# is an open drain, active low signal that is driven low
> > > > by the card to request reference clock. It's an optional signal
> > > > added in PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be
> > > > driven low if it's reserved.
> > > >
> > > > Since the reference clock controlled by CLKREQ# may be required by
> > > > i.MX PCIe host too. To make sure this clock is ready even when the
> > > > CLKREQ# isn't driven low by the card(e.x the scenario described
> > > > above), force CLKREQ# override active low for i.MX PCIe host
> > > > during
> > > initialization.
> > > >
> > > > The CLKREQ# override can be cleared safely when supports-clkreq is
> > > > present and PCIe link is up later. Because the CLKREQ# would be
> > > > driven low by the card at this time.
> > >
> > > What happens if we clear the CLKREQ# override (so the host doesn't
> > > assert it), and the link is up but the card never asserts CLKREQ#
> > > (since it's an optional signal)?
> > >
> > > Does the i.MX host still work?
> >
> > The CLKREQ# override active low only be cleared when link is up and
> > supports-clkreq is present. In the other words, there is a remote
> > endpoint  device, and the CLKREQ# would be driven active low by this
> endpoint device.
> 
> Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in CEM
> r2.0, so even if the endpoint is present and the link is up, the endpoint will not
> assert CLKREQ#.
> 
> Will the i.MX host still work?
Yes, i.MX hos still work. 
If the endpoint designed to CEM r2.0, and CLKREQ# is reserved. The property
 suppots-clkreq wouldn't present in this scenario. Thus, the CLKREQ# override
 active low set by host driver wouldn't be cleared later, although the link is
 up and an endpoint is present.

> 
> IIUC, CLKREQ# is required for ASPM L1 PM Substates.  Maybe the CLKREQ#
> override should only be cleared if the endpoint advertises L1 PM Substates
> support?
> 
> Sorry, I'm just really confused about this.  Here's another question:
> Even if the endpoint is designed to CEM r4.0, it supports L1 PM Substates, and
> it asserts CLKREQ#, my understanding is that the endpoint won't assert
> CLKREQ# *all* the time.  For example, when the link enters L1, the device
> deasserts CLKREQ# (CEM r5.0, sec 2.8.2).
> 
> What happens to the i.MX host when the endpoint isn't asserting CLKREQ#?  I
> guess i.MX doesn't need refclk in that situation?
Yes, it is. When L1ss is entered, i.MX PCIe doesn't need refclk. In the other
words, the REFCLK can be gated off when L1ss is entered.
I had observed the toggling of CLKREQ# and REFCLK by the Oscilloscope when
L1ss is enabled. CLKRQ# is high, and no REFCLK toggling, if there are no
activities on the bus. CLKREQ# would be driven to active low and REFCLK start
to toggle when any side try to issue the TLPs.

Best Regards
Richard Zhu
> 
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > > > ++++++++++++++++++++++++++-
> > > >  1 file changed, 42 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 80e48746bbaf..6b03b1111d06 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -52,6 +52,8 @@
> > > >  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> > > > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
> > > >  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
> > > >
> > > >  #define IMX95_PE0_GEN_CTRL_1			0x1050
> > > > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> > > >  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> > > >  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
> > > >  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> > > > +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
> > > >  	const struct dw_pcie_host_ops *ops;  };
> > > >
> > > > @@ -149,6 +152,7 @@ struct imx_pcie {
> > > >  	struct gpio_desc	*reset_gpiod;
> > > >  	struct clk_bulk_data	*clks;
> > > >  	int			num_clks;
> > > > +	bool			supports_clkreq;
> > > >  	struct regmap		*iomuxc_gpr;
> > > >  	u16			msi_ctrl;
> > > >  	u32			controller_id;
> > > > @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const
> > > struct imx_pcie *imx_pcie)
> > > >  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 :
> > > > IOMUXC_GPR14; }
> > > >
> > > > +static void  imx95_pcie_clkreq_override(struct imx_pcie
> > > > +*imx_pcie, bool enable) {
> > > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> > > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> > > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0); }
> > > > +
> > > >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> > > >  	/*
> > > > @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct
> > > imx_pcie *imx_pcie, bool enable)
> > > >  	return 0;
> > > >  }
> > > >
> > > > -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie,
> > > > bool
> > > > enable)
> > > > +static void imx8mm_pcie_clkreq_override(struct imx_pcie
> > > > +*imx_pcie, bool enable)
> > > >  {
> > > >  	int offset = imx_pcie_grp_offset(imx_pcie);
> > > >
> > > > @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct
> > > imx_pcie *imx_pcie, bool enable)
> > > >  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
> > > >  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> > > >  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN :
> 0);
> > > > +}
> > > > +
> > > > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie,
> > > > +bool
> > > > +enable) {
> > > > +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
> > > >  	return 0;
> > > >  }
> > > >
> > > > @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct
> > > dw_pcie_rp *pp)
> > > >  		regulator_disable(imx_pcie->vpcie);
> > > >  }
> > > >
> > > > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > +	imx8mm_pcie_clkreq_override(imx_pcie, false); }
> > > > +
> > > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > +	imx95_pcie_clkreq_override(imx_pcie, false); }
> > > > +
> > > >  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)  {
> > > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6
> > > > +1351,12
> > > @@
> > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > > >  		dw_pcie_dbi_ro_wr_dis(pci);
> > > >  	}
> > > > +
> > > > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > > > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > > > +	}
> > > >  }
> > > >
> > > >  /*
> > > > @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  	pci->max_link_speed = 1;
> > > >  	of_property_read_u32(node, "fsl,max-link-speed",
> > > > &pci->max_link_speed);
> > > >
> > > > +	imx_pcie->supports_clkreq =
> > > > +		of_property_read_bool(node, "supports-clkreq");
> > > >  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev,
> "vpcie");
> > > >  	if (IS_ERR(imx_pcie->vpcie)) {
> > > >  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV) @@ -1873,6 +1910,7
> @@
> > > > static const struct imx_pcie_drvdata drvdata[] = {
> > > >  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
> > > >  		.init_phy = imx8mq_pcie_init_phy,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MM] = {
> > > >  		.variant = IMX8MM,
> > > > @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.mode_off[0] = IOMUXC_GPR12,
> > > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MP] = {
> > > >  		.variant = IMX8MP,
> > > > @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.mode_off[0] = IOMUXC_GPR12,
> > > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8Q] = {
> > > >  		.variant = IMX8Q,
> > > > @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.core_reset = imx95_pcie_core_reset,
> > > >  		.init_phy = imx95_pcie_init_phy,
> > > >  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> > > > +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MQ_EP] = {
> > > >  		.variant = IMX8MQ_EP,
> > > > --
> > > > 2.37.1
> > > >

RE: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年9月26日 10:45
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; jingoohan1@gmail.com;
> l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> mani@kernel.org; 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 v5 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
> 
> On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > The CLKREQ# is an open drain, active low signal that is driven low
> > > > by the card to request reference clock. It's an optional signal
> > > > added in PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be
> > > > driven low if it's reserved.
> > > >
> > > > Since the reference clock controlled by CLKREQ# may be required by
> > > > i.MX PCIe host too. To make sure this clock is ready even when the
> > > > CLKREQ# isn't driven low by the card(e.x the scenario described
> > > > above), force CLKREQ# override active low for i.MX PCIe host
> > > > during
> > > initialization.
> > > >
> > > > The CLKREQ# override can be cleared safely when supports-clkreq is
> > > > present and PCIe link is up later. Because the CLKREQ# would be
> > > > driven low by the card at this time.
> > >
> > > What happens if we clear the CLKREQ# override (so the host doesn't
> > > assert it), and the link is up but the card never asserts CLKREQ#
> > > (since it's an optional signal)?
> > >
> > > Does the i.MX host still work?
> >
> > The CLKREQ# override active low only be cleared when link is up and
> > supports-clkreq is present. In the other words, there is a remote
> > endpoint  device, and the CLKREQ# would be driven active low by this
> endpoint device.
> 
> Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in CEM
> r2.0, so even if the endpoint is present and the link is up, the endpoint will not
> assert CLKREQ#.
> 
> Will the i.MX host still work?
> 
> IIUC, CLKREQ# is required for ASPM L1 PM Substates.  Maybe the CLKREQ#
> override should only be cleared if the endpoint advertises L1 PM Substates
> support?
CLKREQ# override active low only be cleared when the endpoint advertises that
 it has L1 PM Substates support or it always drives CLKREQ# low.
Hope this explanation can resolve the confusion you have.

Best Regards
Richard Zhu
> 
> Sorry, I'm just really confused about this.  Here's another question:
> Even if the endpoint is designed to CEM r4.0, it supports L1 PM Substates, and
> it asserts CLKREQ#, my understanding is that the endpoint won't assert
> CLKREQ# *all* the time.  For example, when the link enters L1, the device
> deasserts CLKREQ# (CEM r5.0, sec 2.8.2).
> 
> What happens to the i.MX host when the endpoint isn't asserting CLKREQ#?  I
> guess i.MX doesn't need refclk in that situation?
> 
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > > > ++++++++++++++++++++++++++-
> > > >  1 file changed, 42 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 80e48746bbaf..6b03b1111d06 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -52,6 +52,8 @@
> > > >  #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_CLKREQ_OVERRIDE_EN		BIT(8)
> > > > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL		BIT(9)
> > > >  #define IMX95_PCIE_SYS_AUX_PWR_DET		BIT(31)
> > > >
> > > >  #define IMX95_PE0_GEN_CTRL_1			0x1050
> > > > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> > > >  	int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> > > >  	int (*core_reset)(struct imx_pcie *pcie, bool assert);
> > > >  	int (*wait_pll_lock)(struct imx_pcie *pcie);
> > > > +	void (*clr_clkreq_override)(struct imx_pcie *pcie);
> > > >  	const struct dw_pcie_host_ops *ops;  };
> > > >
> > > > @@ -149,6 +152,7 @@ struct imx_pcie {
> > > >  	struct gpio_desc	*reset_gpiod;
> > > >  	struct clk_bulk_data	*clks;
> > > >  	int			num_clks;
> > > > +	bool			supports_clkreq;
> > > >  	struct regmap		*iomuxc_gpr;
> > > >  	u16			msi_ctrl;
> > > >  	u32			controller_id;
> > > > @@ -239,6 +243,16 @@ static unsigned int imx_pcie_grp_offset(const
> > > struct imx_pcie *imx_pcie)
> > > >  	return imx_pcie->controller_id == 1 ? IOMUXC_GPR16 :
> > > > IOMUXC_GPR14; }
> > > >
> > > > +static void  imx95_pcie_clkreq_override(struct imx_pcie
> > > > +*imx_pcie, bool enable) {
> > > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_EN,
> > > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_EN : 0);
> > > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > +			   IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > > +			   enable ? IMX95_PCIE_CLKREQ_OVERRIDE_VAL : 0); }
> > > > +
> > > >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> > > >  	/*
> > > > @@ -685,7 +699,7 @@ static int imx6q_pcie_enable_ref_clk(struct
> > > imx_pcie *imx_pcie, bool enable)
> > > >  	return 0;
> > > >  }
> > > >
> > > > -static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie,
> > > > bool
> > > > enable)
> > > > +static void imx8mm_pcie_clkreq_override(struct imx_pcie
> > > > +*imx_pcie, bool enable)
> > > >  {
> > > >  	int offset = imx_pcie_grp_offset(imx_pcie);
> > > >
> > > > @@ -695,6 +709,11 @@ static int imx8mm_pcie_enable_ref_clk(struct
> > > imx_pcie *imx_pcie, bool enable)
> > > >  	regmap_update_bits(imx_pcie->iomuxc_gpr, offset,
> > > >  			   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> > > >  			   enable ? IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN :
> 0);
> > > > +}
> > > > +
> > > > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie,
> > > > +bool
> > > > +enable) {
> > > > +	imx8mm_pcie_clkreq_override(imx_pcie, enable);
> > > >  	return 0;
> > > >  }
> > > >
> > > > @@ -1298,6 +1317,16 @@ static void imx_pcie_host_exit(struct
> > > dw_pcie_rp *pp)
> > > >  		regulator_disable(imx_pcie->vpcie);
> > > >  }
> > > >
> > > > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > +	imx8mm_pcie_clkreq_override(imx_pcie, false); }
> > > > +
> > > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > +	imx95_pcie_clkreq_override(imx_pcie, false); }
> > > > +
> > > >  static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)  {
> > > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6
> > > > +1351,12
> > > @@
> > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > > >  		dw_pcie_dbi_ro_wr_dis(pci);
> > > >  	}
> > > > +
> > > > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > > > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > > > +	}
> > > >  }
> > > >
> > > >  /*
> > > > @@ -1745,6 +1780,8 @@ static int imx_pcie_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  	pci->max_link_speed = 1;
> > > >  	of_property_read_u32(node, "fsl,max-link-speed",
> > > > &pci->max_link_speed);
> > > >
> > > > +	imx_pcie->supports_clkreq =
> > > > +		of_property_read_bool(node, "supports-clkreq");
> > > >  	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev,
> "vpcie");
> > > >  	if (IS_ERR(imx_pcie->vpcie)) {
> > > >  		if (PTR_ERR(imx_pcie->vpcie) != -ENODEV) @@ -1873,6 +1910,7
> @@
> > > > static const struct imx_pcie_drvdata drvdata[] = {
> > > >  		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
> > > >  		.init_phy = imx8mq_pcie_init_phy,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MM] = {
> > > >  		.variant = IMX8MM,
> > > > @@ -1883,6 +1921,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.mode_off[0] = IOMUXC_GPR12,
> > > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MP] = {
> > > >  		.variant = IMX8MP,
> > > > @@ -1893,6 +1932,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.mode_off[0] = IOMUXC_GPR12,
> > > >  		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > > >  		.enable_ref_clk = imx8mm_pcie_enable_ref_clk,
> > > > +		.clr_clkreq_override = imx8mm_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8Q] = {
> > > >  		.variant = IMX8Q,
> > > > @@ -1913,6 +1953,7 @@ static const struct imx_pcie_drvdata drvdata[]
> = {
> > > >  		.core_reset = imx95_pcie_core_reset,
> > > >  		.init_phy = imx95_pcie_init_phy,
> > > >  		.wait_pll_lock = imx95_pcie_wait_for_phy_pll_lock,
> > > > +		.clr_clkreq_override = imx95_pcie_clr_clkreq_override,
> > > >  	},
> > > >  	[IMX8MQ_EP] = {
> > > >  		.variant = IMX8MQ_EP,
> > > > --
> > > > 2.37.1
> > > >

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Fri, Sep 26, 2025 at 03:23:43AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>

> > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > The CLKREQ# is an open drain, active low signal that is driven low
> > > > > by the card to request reference clock. It's an optional signal
> > > > > added in PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be
> > > > > driven low if it's reserved.
> > > > >
> > > > > Since the reference clock controlled by CLKREQ# may be required by
> > > > > i.MX PCIe host too. To make sure this clock is ready even when the
> > > > > CLKREQ# isn't driven low by the card(e.x the scenario described
> > > > > above), force CLKREQ# override active low for i.MX PCIe host
> > > > > during initialization.
> > > > >
> > > > > The CLKREQ# override can be cleared safely when supports-clkreq is
> > > > > present and PCIe link is up later. Because the CLKREQ# would be
> > > > > driven low by the card at this time.
> > > >
> > > > What happens if we clear the CLKREQ# override (so the host doesn't
> > > > assert it), and the link is up but the card never asserts CLKREQ#
> > > > (since it's an optional signal)?
> > > >
> > > > Does the i.MX host still work?
> > >
> > > The CLKREQ# override active low only be cleared when link is up
> > > and supports-clkreq is present. In the other words, there is a
> > > remote endpoint  device, and the CLKREQ# would be driven active
> > > low by this endpoint device.
> > 
> > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > CEM r2.0, so even if the endpoint is present and the link is up,
> > the endpoint will not assert CLKREQ#.
> > 
> > Will the i.MX host still work?
> > 
> > IIUC, CLKREQ# is required for ASPM L1 PM Substates.  Maybe the
> > CLKREQ# override should only be cleared if the endpoint advertises
> > L1 PM Substates support?
>
> CLKREQ# override active low only be cleared when the endpoint
> advertises that it has L1 PM Substates support or it always drives
> CLKREQ# low.

What?  That's not what the patch does.  It calls .clr_clkreq_override()
whenever the link is up and devicetree contains 'support-clkreq'.

A device advertises L1 PM Substates support by putting the L1 PM
Substates Capability in its config space.

> > > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > > >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > > > >  		dw_pcie_dbi_ro_wr_dis(pci);
> > > > >  	}
> > > > > +
> > > > > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > > > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > > > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > > > > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > > > > +	}

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Fri, Sep 26, 2025 at 03:08:30AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>

> > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > The CLKREQ# is an open drain, active low signal that is
> > > > > driven low by the card to request reference clock. It's an
> > > > > optional signal added in PCIe CEM r4.0, sec 2. Thus, this
> > > > > signal wouldn't be driven low if it's reserved.
> > > > >
> > > > > Since the reference clock controlled by CLKREQ# may be
> > > > > required by i.MX PCIe host too. To make sure this clock is
> > > > > ready even when the CLKREQ# isn't driven low by the card(e.x
> > > > > the scenario described above), force CLKREQ# override active
> > > > > low for i.MX PCIe host during initialization.
> > > > >
> > > > > The CLKREQ# override can be cleared safely when
> > > > > supports-clkreq is present and PCIe link is up later.
> > > > > Because the CLKREQ# would be driven low by the card at this
> > > > > time.
> > > >
> > > > What happens if we clear the CLKREQ# override (so the host
> > > > doesn't assert it), and the link is up but the card never
> > > > asserts CLKREQ# (since it's an optional signal)?
> > > >
> > > > Does the i.MX host still work?
> > >
> > > The CLKREQ# override active low only be cleared when link is up
> > > and supports-clkreq is present. In the other words, there is a
> > > remote endpoint  device, and the CLKREQ# would be driven active
> > > low by this endpoint device.
> > 
> > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > CEM r2.0, so even if the endpoint is present and the link is up,
> > the endpoint will not assert CLKREQ#.
> > 
> > Will the i.MX host still work?

> Yes, i.MX host still work. 
> If the endpoint designed to CEM r2.0, and CLKREQ# is reserved. The
> property suppots-clkreq wouldn't present in this scenario. Thus, the
> CLKREQ# override active low set by host driver wouldn't be cleared
> later, although the link is up and an endpoint is present.

Do you mean 'supports-clkreq' describes the *endpoint*, and you need
to change the devicetree depending on which endpoint is connected?

The schema says 'supports-clkreq' tells us whether CLKREQ# signal
routing exists, not whether the downstream device actually supports
CLKREQ#:

  https://github.com/devicetree-org/dt-schema/blob/4b28bc79fdc552f3e0b870ef1362bb711925f4f3/dtschema/schemas/pci/pci-bus-common.yaml#L155

I don't see 'supports-clkreq' in any devicetree related to imx6, so
I'm not sure this patch is needed yet.  Does it fix an existing
problem?

If it enables some future functionality, maybe we should defer it
until we're actually ready to enable that functionality?

Bjorn

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Frank Li]

On Fri, Sep 26, 2025 at 03:25:21PM -0500, Bjorn Helgaas wrote:
> On Fri, Sep 26, 2025 at 03:08:30AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
>
> > > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > > The CLKREQ# is an open drain, active low signal that is
> > > > > > driven low by the card to request reference clock. It's an
> > > > > > optional signal added in PCIe CEM r4.0, sec 2. Thus, this
> > > > > > signal wouldn't be driven low if it's reserved.
> > > > > >
> > > > > > Since the reference clock controlled by CLKREQ# may be
> > > > > > required by i.MX PCIe host too. To make sure this clock is
> > > > > > ready even when the CLKREQ# isn't driven low by the card(e.x
> > > > > > the scenario described above), force CLKREQ# override active
> > > > > > low for i.MX PCIe host during initialization.
> > > > > >
> > > > > > The CLKREQ# override can be cleared safely when
> > > > > > supports-clkreq is present and PCIe link is up later.
> > > > > > Because the CLKREQ# would be driven low by the card at this
> > > > > > time.
> > > > >
> > > > > What happens if we clear the CLKREQ# override (so the host
> > > > > doesn't assert it), and the link is up but the card never
> > > > > asserts CLKREQ# (since it's an optional signal)?
> > > > >
> > > > > Does the i.MX host still work?
> > > >
> > > > The CLKREQ# override active low only be cleared when link is up
> > > > and supports-clkreq is present. In the other words, there is a
> > > > remote endpoint  device, and the CLKREQ# would be driven active
> > > > low by this endpoint device.
> > >
> > > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > > CEM r2.0, so even if the endpoint is present and the link is up,
> > > the endpoint will not assert CLKREQ#.
> > >
> > > Will the i.MX host still work?
>
> > Yes, i.MX host still work.
> > If the endpoint designed to CEM r2.0, and CLKREQ# is reserved. The
> > property suppots-clkreq wouldn't present in this scenario. Thus, the
> > CLKREQ# override active low set by host driver wouldn't be cleared
> > later, although the link is up and an endpoint is present.
>
> Do you mean 'supports-clkreq' describes the *endpoint*, and you need
> to change the devicetree depending on which endpoint is connected?

It is NOT descript *endpoint*. supports-clkreq descript the board design,
which connect CLKREQ# signal. Because standard slot's CLKREQ# (PIN12) is
reserved in beggin, so some old PCIe card have not pull down this signal as
latest spec requirement.

PCIe Standard slot with INTEL E2000 1G ethernet card, which is producted
around 10 year ago, PIN12 is reserved.

So we don't set supports-clkreq for stardard PCI slot, only set it for
M.2 slot. So stardard PCI slot in imx95 evk can support most cards. We have
not vendor card lists, which already connect/not connect CLKREQ#, so we
have to fallback to disconnect CLKREQ# situation by clarm our evk board
have not connect CLKREQ# to make all card works, eventhough it lost power
save feature. work is more impantant then power saving.

>
> The schema says 'supports-clkreq' tells us whether CLKREQ# signal
> routing exists, not whether the downstream device actually supports
> CLKREQ#:
>
>   https://github.com/devicetree-org/dt-schema/blob/4b28bc79fdc552f3e0b870ef1362bb711925f4f3/dtschema/schemas/pci/pci-bus-common.yaml#L155
>
> I don't see 'supports-clkreq' in any devicetree related to imx6, so
> I'm not sure this patch is needed yet.  Does it fix an existing
> problem?

The patch adding 'supports-clkreq' in dts is on going. No funtional broken
because it just impact l1ss power saving features.

>
> If it enables some future functionality, maybe we should defer it
> until we're actually ready to enable that functionality?

Actually, it fixes i.MX95 19x19 EVK second slot problem. At least
INTEL E2000 1G ethernet card can't work at i.MX95 EVK boards at main
stream kernel without this patch.

Frank
>
> Bjorn

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Frank Li]

On Fri, Sep 26, 2025 at 03:24:31PM -0500, Bjorn Helgaas wrote:
> On Fri, Sep 26, 2025 at 03:23:43AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
>
> > > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > > The CLKREQ# is an open drain, active low signal that is driven low
> > > > > > by the card to request reference clock. It's an optional signal
> > > > > > added in PCIe CEM r4.0, sec 2. Thus, this signal wouldn't be
> > > > > > driven low if it's reserved.
> > > > > >
> > > > > > Since the reference clock controlled by CLKREQ# may be required by
> > > > > > i.MX PCIe host too. To make sure this clock is ready even when the
> > > > > > CLKREQ# isn't driven low by the card(e.x the scenario described
> > > > > > above), force CLKREQ# override active low for i.MX PCIe host
> > > > > > during initialization.
> > > > > >
> > > > > > The CLKREQ# override can be cleared safely when supports-clkreq is
> > > > > > present and PCIe link is up later. Because the CLKREQ# would be
> > > > > > driven low by the card at this time.
> > > > >
> > > > > What happens if we clear the CLKREQ# override (so the host doesn't
> > > > > assert it), and the link is up but the card never asserts CLKREQ#
> > > > > (since it's an optional signal)?
> > > > >
> > > > > Does the i.MX host still work?
> > > >
> > > > The CLKREQ# override active low only be cleared when link is up
> > > > and supports-clkreq is present. In the other words, there is a
> > > > remote endpoint  device, and the CLKREQ# would be driven active
> > > > low by this endpoint device.
> > >
> > > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > > CEM r2.0, so even if the endpoint is present and the link is up,
> > > the endpoint will not assert CLKREQ#.
> > >
> > > Will the i.MX host still work?
> > >
> > > IIUC, CLKREQ# is required for ASPM L1 PM Substates.  Maybe the
> > > CLKREQ# override should only be cleared if the endpoint advertises
> > > L1 PM Substates support?
> >
> > CLKREQ# override active low only be cleared when the endpoint
> > advertises that it has L1 PM Substates support or it always drives
> > CLKREQ# low.
>
> What?  That's not what the patch does.  It calls .clr_clkreq_override()
> whenever the link is up and devicetree contains 'support-clkreq'.
>
> A device advertises L1 PM Substates support by putting the L1 PM
> Substates Capability in its config space.

Regardless L1SS state, EP will pull down CLKREQ# by spec requirement.
'support-clkreq' indicate board design connect this signal, so host needn't
force it to low.

Additional support-clkreq require use open drain to connect both EP and RC's
CLKREQ#, some old board design use OR gate, which should not claim
support-clkreq.

The key point if CLKREQ# is connected. If clkreq# connected between EP
and RC, we can release override.

Frank

>
> > > > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > > > >  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > > > > >  		dw_pcie_dbi_ro_wr_dis(pci);
> > > > > >  	}
> > > > > > +
> > > > > > +	/* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > > > > +	if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > > > > +		if (imx_pcie->drvdata->clr_clkreq_override)
> > > > > > +			imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > > > > > +	}

Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

[Bjorn Helgaas]

On Fri, Sep 26, 2025 at 06:46:28PM -0400, Frank Li wrote:
> On Fri, Sep 26, 2025 at 03:25:21PM -0500, Bjorn Helgaas wrote:
> > On Fri, Sep 26, 2025 at 03:08:30AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> >
> > > > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bjorn Helgaas <helgaas@kernel.org> On Tue, Sep 23, 2025 at
> > > > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > > > The CLKREQ# is an open drain, active low signal that is
> > > > > > > driven low by the card to request reference clock. It's an
> > > > > > > optional signal added in PCIe CEM r4.0, sec 2. Thus, this
> > > > > > > signal wouldn't be driven low if it's reserved.
> > > > > > >
> > > > > > > Since the reference clock controlled by CLKREQ# may be
> > > > > > > required by i.MX PCIe host too. To make sure this clock is
> > > > > > > ready even when the CLKREQ# isn't driven low by the card(e.x
> > > > > > > the scenario described above), force CLKREQ# override active
> > > > > > > low for i.MX PCIe host during initialization.
> > > > > > >
> > > > > > > The CLKREQ# override can be cleared safely when
> > > > > > > supports-clkreq is present and PCIe link is up later.
> > > > > > > Because the CLKREQ# would be driven low by the card at this
> > > > > > > time.
> > > > > >
> > > > > > What happens if we clear the CLKREQ# override (so the host
> > > > > > doesn't assert it), and the link is up but the card never
> > > > > > asserts CLKREQ# (since it's an optional signal)?
> > > > > >
> > > > > > Does the i.MX host still work?
> > > > >
> > > > > The CLKREQ# override active low only be cleared when link is up
> > > > > and supports-clkreq is present. In the other words, there is a
> > > > > remote endpoint  device, and the CLKREQ# would be driven active
> > > > > low by this endpoint device.
> > > >
> > > > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > > > CEM r2.0, so even if the endpoint is present and the link is up,
> > > > the endpoint will not assert CLKREQ#.
> > > >
> > > > Will the i.MX host still work?
> >
> > > Yes, i.MX host still work.
> > > If the endpoint designed to CEM r2.0, and CLKREQ# is reserved. The
> > > property suppots-clkreq wouldn't present in this scenario. Thus, the
> > > CLKREQ# override active low set by host driver wouldn't be cleared
> > > later, although the link is up and an endpoint is present.
> >
> > Do you mean 'supports-clkreq' describes the *endpoint*, and you need
> > to change the devicetree depending on which endpoint is connected?
> 
> It is NOT descript *endpoint*. supports-clkreq descript the board design,
> which connect CLKREQ# signal. Because standard slot's CLKREQ# (PIN12) is
> reserved in beggin, so some old PCIe card have not pull down this signal as
> latest spec requirement.
> 
> PCIe Standard slot with INTEL E2000 1G ethernet card, which is producted
> around 10 year ago, PIN12 is reserved.
> 
> So we don't set supports-clkreq for stardard PCI slot, only set it for
> M.2 slot. So stardard PCI slot in imx95 evk can support most cards. We have
> not vendor card lists, which already connect/not connect CLKREQ#, so we
> have to fallback to disconnect CLKREQ# situation by clarm our evk board
> have not connect CLKREQ# to make all card works, eventhough it lost power
> save feature. work is more impantant then power saving.
> 
> > The schema says 'supports-clkreq' tells us whether CLKREQ# signal
> > routing exists, not whether the downstream device actually supports
> > CLKREQ#:
> >
> >   https://github.com/devicetree-org/dt-schema/blob/4b28bc79fdc552f3e0b870ef1362bb711925f4f3/dtschema/schemas/pci/pci-bus-common.yaml#L155
> >
> > I don't see 'supports-clkreq' in any devicetree related to imx6, so
> > I'm not sure this patch is needed yet.  Does it fix an existing
> > problem?
> 
> The patch adding 'supports-clkreq' in dts is on going. No funtional broken
> because it just impact l1ss power saving features.
> 
> > If it enables some future functionality, maybe we should defer it
> > until we're actually ready to enable that functionality?
> 
> Actually, it fixes i.MX95 19x19 EVK second slot problem. At least
> INTEL E2000 1G ethernet card can't work at i.MX95 EVK boards at main
> stream kernel without this patch.

I deferred these two patches so we have time to tidy these up:

  - Coordinate with adding 'supports-clkreq' in devicetrees.

  - Fix the imx95 refclk enable that was missed in the v5 series.

  - Consider making imx95 refclk enable parallel to the other
    versions, e.g., by using .enable_ref_clk() instead of doing it in
    imx95_pcie_init_phy().

  - Describe the "i.MX95 19x19 EVK second slot problem" in the commit
    log.  Possibly split that into a second patch if it can be
    separated from the CLKREQ# override.  It sounds like this part
    doesn't depend on 'supports-clkreq' in a devicetree?

Maybe we can also figure out how to explain why CLKREQ# override is an
issue for imx6 but not for other DWC-based drivers.

Bjorn