* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 8:10 ` [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low Richard Zhu
@ 2025-08-20 15:13 ` Frank Li
2025-08-20 20:35 ` Bjorn Helgaas
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: Frank Li @ 2025-08-20 15:13 UTC (permalink / raw)
To: Richard Zhu
Cc: l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas, shawnguo,
s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
linux-kernel
On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock.
>
> Since the reference clock may be required by i.MX PCIe host too. To make
> sure this clock is available even when the CLKREQ# isn't driven low by
> the card(e.x no card connected
some old PCIe standard slot card have not connect CLKREQ# signal because
PCIe standard have not defined it at that time.
>), 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 in this case.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf6..a73632b47e2d3 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;
> @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> IMX95_PCIE_REF_CLKEN,
> IMX95_PCIE_REF_CLKEN);
>
> + /* Force CLKREQ# low by override */
> + regmap_update_bits(imx_pcie->iomuxc_gpr,
> + IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> return 0;
> }
>
> @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> +}
> +
> static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1345,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 +1774,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 +1904,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 +1915,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 +1926,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 +1947,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
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 8:10 ` [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low Richard Zhu
2025-08-20 15:13 ` Frank Li
@ 2025-08-20 20:35 ` Bjorn Helgaas
2025-08-20 21:02 ` Frank Li
2025-08-20 21:40 ` Bjorn Helgaas
2025-09-08 6:06 ` Manivannan Sadhasivam
3 siblings, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 20:35 UTC (permalink / raw)
To: Richard Zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock.
>
> Since the reference clock may be required by i.MX PCIe host too. To make
> sure this clock is available even when the CLKREQ# isn't driven low by
> the card(e.x no card connected), 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 in this case.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf6..a73632b47e2d3 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;
> @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> IMX95_PCIE_REF_CLKEN,
> IMX95_PCIE_REF_CLKEN);
>
> + /* Force CLKREQ# low by override */
> + regmap_update_bits(imx_pcie->iomuxc_gpr,
> + IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> return 0;
> }
>
> @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> +}
> +
> static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1345,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);
It seems racy to clear the override when the link is up.
Since it sounds like the i.MX host requires refclock all the time, why
not keep the override permanently?
Obviously it must be ok to keep the override for a while, because
there is some interval between the link coming up and the call of
.clr_clkreq_override().
Would something bad happen if we *never* called
.clr_clkreq_override()?
Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 20:35 ` Bjorn Helgaas
@ 2025-08-20 21:02 ` Frank Li
2025-08-20 21:40 ` Bjorn Helgaas
0 siblings, 1 reply; 19+ messages in thread
From: Frank Li @ 2025-08-20 21:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, mani, robh,
bhelgaas, shawnguo, s.hauer, kernel, festevam, linux-pci,
linux-arm-kernel, imx, linux-kernel
On Wed, Aug 20, 2025 at 03:35:36PM -0500, Bjorn Helgaas wrote:
> On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock.
> >
> > Since the reference clock may be required by i.MX PCIe host too. To make
> > sure this clock is available even when the CLKREQ# isn't driven low by
> > the card(e.x no card connected), 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 in this case.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf6..a73632b47e2d3 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;
> > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> > IMX95_PCIE_REF_CLKEN,
> > IMX95_PCIE_REF_CLKEN);
> >
> > + /* Force CLKREQ# low by override */
> > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > + IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > return 0;
> > }
> >
> > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> > +}
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> > +}
> > +
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -1322,6 +1345,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);
>
> It seems racy to clear the override when the link is up.
>
> Since it sounds like the i.MX host requires refclock all the time, why
> not keep the override permanently?
It will Lost l1ss support if enable override permanetly. I think other
platform have similar issues (at least dwc controller). Most platform use
m.2 socket, which pull down clk_req by EP side devices.
Only standard PCIe slots, which clkreq have not connect by EP devices
because stardard PCIe slots add #clkreq signal happen recent years (maybe
after miniPCIe slot spec). Some old PCIe devices have not connect it.
Even latest PCIe devices, I saw have jumper in PCIe card to controller
#clkreq.
>
> Obviously it must be ok to keep the override for a while, because
> there is some interval between the link coming up and the call of
> .clr_clkreq_override().
>
> Would something bad happen if we *never* called
> .clr_clkreq_override()?
clock will always running, lost L1SS power save feature.
Frank
>
> Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 21:02 ` Frank Li
@ 2025-08-20 21:40 ` Bjorn Helgaas
2025-08-21 6:28 ` Hongxing Zhu
0 siblings, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 21:40 UTC (permalink / raw)
To: Frank Li
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, mani, robh,
bhelgaas, shawnguo, s.hauer, kernel, festevam, linux-pci,
linux-arm-kernel, imx, linux-kernel
On Wed, Aug 20, 2025 at 05:02:28PM -0400, Frank Li wrote:
> On Wed, Aug 20, 2025 at 03:35:36PM -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> > > The CLKREQ# is an open drain, active low signal that is driven low by
> > > the card to request reference clock.
> > >
> > > Since the reference clock may be required by i.MX PCIe host too. To make
> > > sure this clock is available even when the CLKREQ# isn't driven low by
> > > the card(e.x no card connected), force CLKREQ# override active low for
> > > i.MX PCIe host during initialization.
What is the effect of refclk not being available when no card is
connected? I guess something is wrong with the i.MX PCIe host? Maybe
register accesses don't work? Something else?
How would a user know that something is wrong when the slot is empty?
Presumably this patch fixes whatever is wrong in that case.
> > > 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 in this case.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > > drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> > > 1 file changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 80e48746bbaf6..a73632b47e2d3 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;
> > > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> > > IMX95_PCIE_REF_CLKEN,
> > > IMX95_PCIE_REF_CLKEN);
> > >
> > > + /* Force CLKREQ# low by override */
> > > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > > + IMX95_PCIE_SS_RW_REG_1,
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > > return 0;
> > > }
> > >
> > > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> > > +}
> > > +
> > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > > +{
> > > + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> > > +}
> > > +
> > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > {
> > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > @@ -1322,6 +1345,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);
> >
> > It seems racy to clear the override when the link is up.
> >
> > Since it sounds like the i.MX host requires refclock all the time, why
> > not keep the override permanently?
>
> It will Lost l1ss support if enable override permanetly. I think other
> platform have similar issues (at least dwc controller). Most platform use
> m.2 socket, which pull down clk_req by EP side devices.
>
> Only standard PCIe slots, which clkreq have not connect by EP devices
> because stardard PCIe slots add #clkreq signal happen recent years (maybe
> after miniPCIe slot spec). Some old PCIe devices have not connect it.
>
> Even latest PCIe devices, I saw have jumper in PCIe card to controller
> #clkreq.
>
> > Obviously it must be ok to keep the override for a while, because
> > there is some interval between the link coming up and the call of
> > .clr_clkreq_override().
> >
> > Would something bad happen if we *never* called
> > .clr_clkreq_override()?
>
> clock will always running, lost L1SS power save feature.
Thanks! Let's include a little of this back story in the next rev of
the commit log and the comment near the .clr_clkreq_override() call.
Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 21:40 ` Bjorn Helgaas
@ 2025-08-21 6:28 ` Hongxing Zhu
0 siblings, 0 replies; 19+ messages in thread
From: Hongxing Zhu @ 2025-08-21 6:28 UTC (permalink / raw)
To: Bjorn Helgaas, Frank Li
Cc: 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
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年8月21日 5:40
> To: Frank Li <frank.li@nxp.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.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 v1 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
>
> On Wed, Aug 20, 2025 at 05:02:28PM -0400, Frank Li wrote:
> > On Wed, Aug 20, 2025 at 03:35:36PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> > > > The CLKREQ# is an open drain, active low signal that is driven low
> > > > by the card to request reference clock.
> > > >
> > > > Since the reference clock may be required by i.MX PCIe host too.
> > > > To make sure this clock is available even when the CLKREQ# isn't
> > > > driven low by the card(e.x no card connected), force CLKREQ#
> > > > override active low for i.MX PCIe host during initialization.
>
> What is the effect of refclk not being available when no card is connected?
> I guess something is wrong with the i.MX PCIe host? Maybe register
> accesses don't work? Something else?
If the refclk is not available, but mandatory required by RC host when no
card is connected to driven active low. There would be PCIe RC driver probe
failure because of the pll lock error return or the failure of PHY power on.
>
> How would a user know that something is wrong when the slot is empty?
> Presumably this patch fixes whatever is wrong in that case.
The error message would be dumped to inform users.
Best Regards
Richard Zhu
>
> > > > 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 in this case.
> > > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-imx6.c | 35
> > > > +++++++++++++++++++++++++++
> > > > 1 file changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 80e48746bbaf6..a73632b47e2d3 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;
> > > > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie
> *imx_pcie)
> > > > IMX95_PCIE_REF_CLKEN,
> > > > IMX95_PCIE_REF_CLKEN);
> > > >
> > > > + /* Force CLKREQ# low by override */
> > > > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > > > + IMX95_PCIE_SS_RW_REG_1,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > > > return 0;
> > > > }
> > > >
> > > > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false); }
> > > > +
> > > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0); }
> > > > +
> > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp) {
> > > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6
> > > > +1345,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);
> > >
> > > It seems racy to clear the override when the link is up.
> > >
> > > Since it sounds like the i.MX host requires refclock all the time,
> > > why not keep the override permanently?
> >
> > It will Lost l1ss support if enable override permanetly. I think other
> > platform have similar issues (at least dwc controller). Most platform
> > use
> > m.2 socket, which pull down clk_req by EP side devices.
> >
> > Only standard PCIe slots, which clkreq have not connect by EP devices
> > because stardard PCIe slots add #clkreq signal happen recent years
> > (maybe after miniPCIe slot spec). Some old PCIe devices have not connect
> it.
> >
> > Even latest PCIe devices, I saw have jumper in PCIe card to controller
> > #clkreq.
> >
> > > Obviously it must be ok to keep the override for a while, because
> > > there is some interval between the link coming up and the call of
> > > .clr_clkreq_override().
> > >
> > > Would something bad happen if we *never* called
> > > .clr_clkreq_override()?
> >
> > clock will always running, lost L1SS power save feature.
>
> Thanks! Let's include a little of this back story in the next rev of the
> commit log and the comment near the .clr_clkreq_override() call.
>
> Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 8:10 ` [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low Richard Zhu
2025-08-20 15:13 ` Frank Li
2025-08-20 20:35 ` Bjorn Helgaas
@ 2025-08-20 21:40 ` Bjorn Helgaas
2025-08-20 22:25 ` Frank Li
2025-09-08 6:06 ` Manivannan Sadhasivam
3 siblings, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 21:40 UTC (permalink / raw)
To: Richard Zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock.
>
> Since the reference clock may be required by i.MX PCIe host too. To make
> sure this clock is available even when the CLKREQ# isn't driven low by
> the card(e.x no card connected), 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 in this case.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf6..a73632b47e2d3 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;
> @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> IMX95_PCIE_REF_CLKEN,
> IMX95_PCIE_REF_CLKEN);
>
> + /* Force CLKREQ# low by override */
> + regmap_update_bits(imx_pcie->iomuxc_gpr,
> + IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> return 0;
> }
>
> @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> +}
> +
> static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1345,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 +1774,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");
Is there a DT binding update that goes along with this? I see
"supports-clkreq" mentioned in
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml, but
nowhere else.
Should this be some kind of generic thing? CLKREQ# itself is part of
the PCIe base spec, so it's definitely not device-specific.
But I'm not sure what this property is telling us. Based on the
patch:
- We assert CLKREQ# on IMX95 (and on IMX95_EP, which seems sort of
weird) regardless of "supports-clkreq".
- We call .clr_clkreq_override() to stop asserting CLKREQ# on
IMX8MQ, IMX8MM, IMX8MP, and IMX95 (but not IMX95_EP), but only if
they have "supports-clkreq" in devicetree.
So I don't know whether or how CLKREQ# is asserted on IMX8MQ, IMX8MM,
and IMX8MP.
And I don't know why we assert CLKREQ# on IMX95_EP but apparently
never stop asserting it.
And I don't know why we assert CLKREQ# regardless of "supports-clkreq"
but only stop asserting it if "supports-clkreq" is present.
> 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 +1904,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 +1915,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 +1926,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 +1947,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
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 21:40 ` Bjorn Helgaas
@ 2025-08-20 22:25 ` Frank Li
0 siblings, 0 replies; 19+ messages in thread
From: Frank Li @ 2025-08-20 22:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, mani, robh,
bhelgaas, shawnguo, s.hauer, kernel, festevam, linux-pci,
linux-arm-kernel, imx, linux-kernel
On Wed, Aug 20, 2025 at 04:40:10PM -0500, Bjorn Helgaas wrote:
> On Wed, Aug 20, 2025 at 04:10:48PM +0800, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock.
> >
> > Since the reference clock may be required by i.MX PCIe host too. To make
> > sure this clock is available even when the CLKREQ# isn't driven low by
> > the card(e.x no card connected), 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 in this case.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf6..a73632b47e2d3 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;
> > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> > IMX95_PCIE_REF_CLKEN,
> > IMX95_PCIE_REF_CLKEN);
> >
> > + /* Force CLKREQ# low by override */
> > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > + IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > return 0;
> > }
> >
> > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> > +}
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> > +}
> > +
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -1322,6 +1345,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 +1774,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");
>
> Is there a DT binding update that goes along with this? I see
> "supports-clkreq" mentioned in
> Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml, but
> nowhere else.
It is already defined dtschema/schemas/pci/pci-bus-common.yaml although I
like it should "clkreq-broken" because most system support it now.
>
> Should this be some kind of generic thing? CLKREQ# itself is part of
> the PCIe base spec, so it's definitely not device-specific.
>
> But I'm not sure what this property is telling us. Based on the
> patch:
It is quite long story about clkreq signal. The old version before L1SS
PCIe controller #CLKREQ (#CLKREQ_RC)
EP #CLKREQ (#CLKREQ_EP)
There are OR gate in board design OR(#CLKREQ_RC, #CLKREQ_EP) to controller
ref clk gate.
In this type board design, supports-clkreq should be set false.
After L1SS support, there are ECN at PCIe. Require connect RC and EP's #clkreq
(open drain) together and use one pull up register, see below diagram.
VCC
─┬──
│
│
┌┴┐
│ │
│ │ 10K
│ │
└┬┘
┌────────┐ │ #clkreq┌───────────┐
│ RC ┼───┼────────┼ EP │
│ │ │ │ │
└────────┘ │ └───────────┘
│
┌────────────┐
│ CLK Gate │
│ │ │
└───┴────────┘
So RC's phy part can power on/off by #clkreq signal beside ref clock gate.
Rollback more years, there are #clkreq signal at stardard pcie slots.
https://en.wikipedia.org/wiki/PCI_Express, PIN12 CLKREQ# is added by ECN
(maybe after miniPCIe). Before ECN, PIN12 is reversed.
>
> - We assert CLKREQ# on IMX95 (and on IMX95_EP, which seems sort of
> weird) regardless of "supports-clkreq".
>
> - We call .clr_clkreq_override() to stop asserting CLKREQ# on
> IMX8MQ, IMX8MM, IMX8MP, and IMX95 (but not IMX95_EP), but only if
> they have "supports-clkreq" in devicetree.
>
> So I don't know whether or how CLKREQ# is asserted on IMX8MQ, IMX8MM,
> and IMX8MP.
>
> And I don't know why we assert CLKREQ# on IMX95_EP but apparently
> never stop asserting it.
It is default for EP by spec define. We have not support L1SS for EP side
yet. But it should not neccessary to overwrite it because IP default pull
clkreq to low when work at EP mode. but not do that at RC mode.
>
> And I don't know why we assert CLKREQ# regardless of "supports-clkreq"
> but only stop asserting it if "supports-clkreq" is present.
See above diagram, ref clk should be alway running after power up. clkreq
is power saving feature. Only board design match #clkreq ECN requirement,
we can enough such power saving feature.
Before enable L1SS, EP card always pull #clkreq to low. But there are
"broken" design, other from board or EP card, which product before ECN
public.
Frank
>
> > 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 +1904,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 +1915,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 +1926,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 +1947,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
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-08-20 8:10 ` [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low Richard Zhu
` (2 preceding siblings ...)
2025-08-20 21:40 ` Bjorn Helgaas
@ 2025-09-08 6:06 ` Manivannan Sadhasivam
2025-09-08 15:26 ` Frank Li
2025-09-09 7:17 ` Hongxing Zhu
3 siblings, 2 replies; 19+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-08 6:06 UTC (permalink / raw)
To: Richard Zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> The CLKREQ# is an open drain, active low signal that is driven low by
> the card to request reference clock.
>
> Since the reference clock may be required by i.MX PCIe host too.
Add some info on why the refclk is needed by the host.
> To make
> sure this clock is available even when the CLKREQ# isn't driven low by
> the card(e.x no card connected), force CLKREQ# override active low for
> i.MX PCIe host during initialization.
>
CLKREQ# override is not a spec defined feature. So you need to explain what it
does first.
> 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 in this case.
>
Why do you need to depend on 'supports-clkreq' property? Don't you already know
if your platform supports CLKREQ# or not? None of the upstream DTS has the
'supports-clkreq' property set and the NXP binding also doesn't enable this
property.
So I'm wondering how you are suddenly using this property. The property implies
that when not set to true, CLKREQ# is not supported by the platform. So when the
driver starts using this property, all the old DTS based platforms are not going
to release CLKREQ# from driving low, so L1SS will not be entered for them. Do
you really want it to happen?
- Mani
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80e48746bbaf6..a73632b47e2d3 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;
> @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> IMX95_PCIE_REF_CLKEN,
> IMX95_PCIE_REF_CLKEN);
>
> + /* Force CLKREQ# low by override */
> + regmap_update_bits(imx_pcie->iomuxc_gpr,
> + IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> return 0;
> }
>
> @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> +}
> +
> +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> +{
> + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> +}
> +
> static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1322,6 +1345,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 +1774,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 +1904,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 +1915,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 +1926,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 +1947,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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-09-08 6:06 ` Manivannan Sadhasivam
@ 2025-09-08 15:26 ` Frank Li
2025-09-08 15:49 ` Manivannan Sadhasivam
2025-09-09 7:17 ` Hongxing Zhu
1 sibling, 1 reply; 19+ messages in thread
From: Frank Li @ 2025-09-08 15:26 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Mon, Sep 08, 2025 at 11:36:02AM +0530, Manivannan Sadhasivam wrote:
> On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock.
> >
> > Since the reference clock may be required by i.MX PCIe host too.
>
> Add some info on why the refclk is needed by the host.
>
> > To make
> > sure this clock is available even when the CLKREQ# isn't driven low by
> > the card(e.x no card connected), force CLKREQ# override active low for
> > i.MX PCIe host during initialization.
> >
>
> CLKREQ# override is not a spec defined feature. So you need to explain what it
> does first.
>
> > 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 in this case.
> >
>
> Why do you need to depend on 'supports-clkreq' property? Don't you already know
> if your platform supports CLKREQ# or not? None of the upstream DTS has the
> 'supports-clkreq' property set and the NXP binding also doesn't enable this
> property.
It is history reason. Supposed all the boards which supports L1SS need set
'supports-clkreq' in dts. L1SS require board design use open drain connect
RC's clk-req and EP's clk-req together, which come from one ECN of PCI
spec.
But most M.2 slot now, which support L1SS, so most platform default enable
L1SS or default 'supports-clkreq' on.
Ideally, 'supports-clkreq' should use revert logic like 'clk-req-broken'.
but 'supports-clkreq' already come into stardard PCIe binding now.
One of i.MX95 boards use standard PCIe slot, PIN 12
12 CLKREQ# Ground Clock Request Signal[26]
which is reserved at old PCIe standard, so some old PCIe card float this
pin.
So I think most dts in kernel tree should add 'supports-clkreq' property
if they use M.2 and connect CLK_REQ# as below [1]
============================================
VCC
---
|
R (10K)
|
CLK_REQ# (RC)------ CLK_REQ#(EP)
NOT add supports-clkreq if connect as below [2]
==========================================
CLK_REQ# (RC) ---> |---------|
| OR GATE | ---> control ref clock
CLK_REQ#(EP) ---> |-------- |
>
> So I'm wondering how you are suddenly using this property. The property implies
> that when not set to true, CLKREQ# is not supported by the platform. So when the
> driver starts using this property, all the old DTS based platforms are not going
> to release CLKREQ# from driving low, so L1SS will not be entered for them. Do
> you really want it to happen?
Actually, some old board use [2]. we will add supports-clkreq if board
design use [1], so correct reflect board design.
Frank
>
> - Mani
>
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 35 +++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf6..a73632b47e2d3 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;
> > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
> > IMX95_PCIE_REF_CLKEN,
> > IMX95_PCIE_REF_CLKEN);
> >
> > + /* Force CLKREQ# low by override */
> > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > + IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > return 0;
> > }
> >
> > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false);
> > +}
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0);
> > +}
> > +
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -1322,6 +1345,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 +1774,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 +1904,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 +1915,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 +1926,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 +1947,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
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-09-08 15:26 ` Frank Li
@ 2025-09-08 15:49 ` Manivannan Sadhasivam
2025-09-08 16:32 ` Frank Li
0 siblings, 1 reply; 19+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-08 15:49 UTC (permalink / raw)
To: Frank Li
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Mon, Sep 08, 2025 at 11:26:11AM GMT, Frank Li wrote:
> On Mon, Sep 08, 2025 at 11:36:02AM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > > The CLKREQ# is an open drain, active low signal that is driven low by
> > > the card to request reference clock.
> > >
> > > Since the reference clock may be required by i.MX PCIe host too.
> >
> > Add some info on why the refclk is needed by the host.
> >
> > > To make
> > > sure this clock is available even when the CLKREQ# isn't driven low by
> > > the card(e.x no card connected), force CLKREQ# override active low for
> > > i.MX PCIe host during initialization.
> > >
> >
> > CLKREQ# override is not a spec defined feature. So you need to explain what it
> > does first.
> >
> > > 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 in this case.
> > >
> >
> > Why do you need to depend on 'supports-clkreq' property? Don't you already know
> > if your platform supports CLKREQ# or not? None of the upstream DTS has the
> > 'supports-clkreq' property set and the NXP binding also doesn't enable this
> > property.
>
> It is history reason. Supposed all the boards which supports L1SS need set
> 'supports-clkreq' in dts. L1SS require board design use open drain connect
> RC's clk-req and EP's clk-req together, which come from one ECN of PCI
> spec.
>
> But most M.2 slot now, which support L1SS, so most platform default enable
> L1SS or default 'supports-clkreq' on.
>
> Ideally, 'supports-clkreq' should use revert logic like 'clk-req-broken'.
> but 'supports-clkreq' already come into stardard PCIe binding now.
>
> One of i.MX95 boards use standard PCIe slot, PIN 12
> 12 CLKREQ# Ground Clock Request Signal[26]
> which is reserved at old PCIe standard, so some old PCIe card float this
> pin.
>
Ok. IIUC, i.MX platforms doesn't always support CLKREQ#, as the pin might not be
wired on some connectors. So if the driver turns off the override, CLKREQ# will
be driven high, but the endpoint wouldn't get a chance to drive it low and it
won't receive the refclk.
Is my understanding correct?
I'm wondering in those cases, why can't you keep the CLKREQ# pin to be in
active low state by defining the initial pinctrl state in DT? Can't you change
the pinctrl state of CLKREQ#?
> So I think most dts in kernel tree should add 'supports-clkreq' property
> if they use M.2 and connect CLK_REQ# as below [1]
> ============================================
> VCC
> ---
> |
> R (10K)
> |
> CLK_REQ# (RC)------ CLK_REQ#(EP)
>
> NOT add supports-clkreq if connect as below [2]
> ==========================================
>
> CLK_REQ# (RC) ---> |---------|
> | OR GATE | ---> control ref clock
> CLK_REQ#(EP) ---> |-------- |
>
>
> >
> > So I'm wondering how you are suddenly using this property. The property implies
> > that when not set to true, CLKREQ# is not supported by the platform. So when the
> > driver starts using this property, all the old DTS based platforms are not going
> > to release CLKREQ# from driving low, so L1SS will not be entered for them. Do
> > you really want it to happen?
>
> Actually, some old board use [2]. we will add supports-clkreq if board
> design use [1], so correct reflect board design.
>
Ok, thanks for clarifying.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-09-08 15:49 ` Manivannan Sadhasivam
@ 2025-09-08 16:32 ` Frank Li
2025-09-09 7:14 ` Hongxing Zhu
0 siblings, 1 reply; 19+ messages in thread
From: Frank Li @ 2025-09-08 16:32 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Richard Zhu, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
shawnguo, s.hauer, kernel, festevam, linux-pci, linux-arm-kernel,
imx, linux-kernel
On Mon, Sep 08, 2025 at 09:19:40PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 08, 2025 at 11:26:11AM GMT, Frank Li wrote:
> > On Mon, Sep 08, 2025 at 11:36:02AM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > > > The CLKREQ# is an open drain, active low signal that is driven low by
> > > > the card to request reference clock.
> > > >
> > > > Since the reference clock may be required by i.MX PCIe host too.
> > >
> > > Add some info on why the refclk is needed by the host.
> > >
> > > > To make
> > > > sure this clock is available even when the CLKREQ# isn't driven low by
> > > > the card(e.x no card connected), force CLKREQ# override active low for
> > > > i.MX PCIe host during initialization.
> > > >
> > >
> > > CLKREQ# override is not a spec defined feature. So you need to explain what it
> > > does first.
> > >
> > > > 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 in this case.
> > > >
> > >
> > > Why do you need to depend on 'supports-clkreq' property? Don't you already know
> > > if your platform supports CLKREQ# or not? None of the upstream DTS has the
> > > 'supports-clkreq' property set and the NXP binding also doesn't enable this
> > > property.
> >
> > It is history reason. Supposed all the boards which supports L1SS need set
> > 'supports-clkreq' in dts. L1SS require board design use open drain connect
> > RC's clk-req and EP's clk-req together, which come from one ECN of PCI
> > spec.
> >
> > But most M.2 slot now, which support L1SS, so most platform default enable
> > L1SS or default 'supports-clkreq' on.
> >
> > Ideally, 'supports-clkreq' should use revert logic like 'clk-req-broken'.
> > but 'supports-clkreq' already come into stardard PCIe binding now.
> >
> > One of i.MX95 boards use standard PCIe slot, PIN 12
> > 12 CLKREQ# Ground Clock Request Signal[26]
> > which is reserved at old PCIe standard, so some old PCIe card float this
> > pin.
> >
>
> Ok. IIUC, i.MX platforms doesn't always support CLKREQ#, as the pin might not be
> wired on some connectors. So if the driver turns off the override, CLKREQ# will
> be driven high, but the endpoint wouldn't get a chance to drive it low and it
CLKREQ# will be float and pull up by pull up resistor. The old endpoint
(PCIe standard slot) float this pin also because it is reversed at old
PCIe standard. So ref clock is off at that case.
> won't receive the refclk.
>
> Is my understanding correct?
Basic is correct with some small problem.
It is caused by two common PCIe problem.
- stadarnd PCIe slot define change PIN12 from reserved to CLKREQ#, which is
ECN, before ECN, all EP device float this pin. after ECN, EP device pull
this pin down.
- use logic [2] to design boards, which just impact L1SS, the basic function
should work.
Frank
>
> I'm wondering in those cases, why can't you keep the CLKREQ# pin to be in
> active low state by defining the initial pinctrl state in DT? Can't you change
> the pinctrl state of CLKREQ#?
>
> > So I think most dts in kernel tree should add 'supports-clkreq' property
> > if they use M.2 and connect CLK_REQ# as below [1]
> > ============================================
> > VCC
> > ---
> > |
> > R (10K)
> > |
> > CLK_REQ# (RC)------ CLK_REQ#(EP)
> >
> > NOT add supports-clkreq if connect as below [2]
> > ==========================================
> >
> > CLK_REQ# (RC) ---> |---------|
> > | OR GATE | ---> control ref clock
> > CLK_REQ#(EP) ---> |-------- |
> >
> >
> > >
> > > So I'm wondering how you are suddenly using this property. The property implies
> > > that when not set to true, CLKREQ# is not supported by the platform. So when the
> > > driver starts using this property, all the old DTS based platforms are not going
> > > to release CLKREQ# from driving low, so L1SS will not be entered for them. Do
> > > you really want it to happen?
> >
> > Actually, some old board use [2]. we will add supports-clkreq if board
> > design use [1], so correct reflect board design.
> >
>
> Ok, thanks for clarifying.
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-09-08 16:32 ` Frank Li
@ 2025-09-09 7:14 ` Hongxing Zhu
0 siblings, 0 replies; 19+ messages in thread
From: Hongxing Zhu @ 2025-09-09 7:14 UTC (permalink / raw)
To: Frank Li, Manivannan Sadhasivam
Cc: l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@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
Hi Frank:
I'm very appreciated that you kindly help to add the explanations.
Best Regards
Richard Zhu
> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: 2025年9月9日 0:33
> To: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kwilczynski@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 v1 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
>
> On Mon, Sep 08, 2025 at 09:19:40PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Sep 08, 2025 at 11:26:11AM GMT, Frank Li wrote:
> > > On Mon, Sep 08, 2025 at 11:36:02AM +0530, Manivannan Sadhasivam
> wrote:
> > > > On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > > > > The CLKREQ# is an open drain, active low signal that is driven
> > > > > low by the card to request reference clock.
> > > > >
> > > > > Since the reference clock may be required by i.MX PCIe host too.
> > > >
> > > > Add some info on why the refclk is needed by the host.
> > > >
> > > > > To make
> > > > > sure this clock is available even when the CLKREQ# isn't driven
> > > > > low by the card(e.x no card connected), force CLKREQ# override
> > > > > active low for i.MX PCIe host during initialization.
> > > > >
> > > >
> > > > CLKREQ# override is not a spec defined feature. So you need to
> > > > explain what it does first.
> > > >
> > > > > 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 in this case.
> > > > >
> > > >
> > > > Why do you need to depend on 'supports-clkreq' property? Don't you
> > > > already know if your platform supports CLKREQ# or not? None of the
> > > > upstream DTS has the 'supports-clkreq' property set and the NXP
> > > > binding also doesn't enable this property.
> > >
> > > It is history reason. Supposed all the boards which supports L1SS
> > > need set 'supports-clkreq' in dts. L1SS require board design use
> > > open drain connect RC's clk-req and EP's clk-req together, which
> > > come from one ECN of PCI spec.
> > >
> > > But most M.2 slot now, which support L1SS, so most platform default
> > > enable L1SS or default 'supports-clkreq' on.
> > >
> > > Ideally, 'supports-clkreq' should use revert logic like 'clk-req-broken'.
> > > but 'supports-clkreq' already come into stardard PCIe binding now.
> > >
> > > One of i.MX95 boards use standard PCIe slot, PIN 12
> > > 12 CLKREQ# Ground Clock Request Signal[26]
> > > which is reserved at old PCIe standard, so some old PCIe card float
> > > this pin.
> > >
> >
> > Ok. IIUC, i.MX platforms doesn't always support CLKREQ#, as the pin
> > might not be wired on some connectors. So if the driver turns off the
> > override, CLKREQ# will be driven high, but the endpoint wouldn't get a
> > chance to drive it low and it
>
> CLKREQ# will be float and pull up by pull up resistor. The old endpoint (PCIe
> standard slot) float this pin also because it is reversed at old PCIe standard.
> So ref clock is off at that case.
>
> > won't receive the refclk.
> >
> > Is my understanding correct?
>
> Basic is correct with some small problem.
>
> It is caused by two common PCIe problem.
> - stadarnd PCIe slot define change PIN12 from reserved to CLKREQ#, which is
> ECN, before ECN, all EP device float this pin. after ECN, EP device pull this
> pin down.
> - use logic [2] to design boards, which just impact L1SS, the basic function
> should work.
>
> Frank
> >
> > I'm wondering in those cases, why can't you keep the CLKREQ# pin to be
> > in active low state by defining the initial pinctrl state in DT? Can't
> > you change the pinctrl state of CLKREQ#?
> >
> > > So I think most dts in kernel tree should add 'supports-clkreq'
> > > property if they use M.2 and connect CLK_REQ# as below [1]
> > > ============================================
> > > VCC
> > > ---
> > > |
> > > R (10K)
> > > |
> > > CLK_REQ# (RC)------ CLK_REQ#(EP)
> > >
> > > NOT add supports-clkreq if connect as below [2]
> > > ==========================================
> > >
> > > CLK_REQ# (RC) ---> |---------|
> > > | OR GATE | ---> control ref clock
> > > CLK_REQ#(EP) ---> |-------- |
> > >
> > >
> > > >
> > > > So I'm wondering how you are suddenly using this property. The
> > > > property implies that when not set to true, CLKREQ# is not
> > > > supported by the platform. So when the driver starts using this
> > > > property, all the old DTS based platforms are not going to release
> > > > CLKREQ# from driving low, so L1SS will not be entered for them. Do you
> really want it to happen?
> > >
> > > Actually, some old board use [2]. we will add supports-clkreq if
> > > board design use [1], so correct reflect board design.
> > >
> >
> > Ok, thanks for clarifying.
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
2025-09-08 6:06 ` Manivannan Sadhasivam
2025-09-08 15:26 ` Frank Li
@ 2025-09-09 7:17 ` Hongxing Zhu
1 sibling, 0 replies; 19+ messages in thread
From: Hongxing Zhu @ 2025-09-09 7:17 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@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
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: 2025年9月8日 14:06
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kwilczynski@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 v1 2/2] PCI: imx6: Add a method to handle CLKREQ#
> override active low
>
> On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > The CLKREQ# is an open drain, active low signal that is driven low by
> > the card to request reference clock.
> >
> > Since the reference clock may be required by i.MX PCIe host too.
>
> Add some info on why the refclk is needed by the host.
>
Hi Mani:
Thanks for your comments.
How about add the following information?
Either internal system PLL or external crystal oscillator can be used as PCIe
reference clock for i.MX PCIe RC controller.
The reference clock used by the i.MX PCIe RC controller may come from an
external crystal oscillator controlled by the clkreq# signal.
> > To make
> > sure this clock is available even when the CLKREQ# isn't driven low by
> > the card(e.x no card connected), force CLKREQ# override active low for
> > i.MX PCIe host during initialization.
> >
>
> CLKREQ# override is not a spec defined feature. So you need to explain what
> it does first.
Okay. Would add the following descriptions.
"Some i.MX PCIe have the CLKREQ# override mechanism, that can force CLKREQ#
active low."
>
> > 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 in this case.
> >
>
> Why do you need to depend on 'supports-clkreq' property? Don't you
> already know if your platform supports CLKREQ# or not? None of the
> upstream DTS has the 'supports-clkreq' property set and the NXP binding
> also doesn't enable this property.
>
> So I'm wondering how you are suddenly using this property. The property
> implies that when not set to true, CLKREQ# is not supported by the platform.
> So when the driver starts using this property, all the old DTS based
> platforms are not going to release CLKREQ# from driving low, so L1SS will not
> be entered for them. Do you really want it to happen?
>
> - Mani
>
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 35
> > +++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80e48746bbaf6..a73632b47e2d3 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;
> > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie
> *imx_pcie)
> > IMX95_PCIE_REF_CLKEN,
> > IMX95_PCIE_REF_CLKEN);
> >
> > + /* Force CLKREQ# low by override */
> > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > + IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > return 0;
> > }
> >
> > @@ -1298,6 +1309,18 @@ 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_enable_ref_clk(imx_pcie, false); }
> > +
> > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
> > +{
> > + regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_1,
> > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0); }
> > +
> > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp) {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6 +1345,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 +1774,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 +1904,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 +1915,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 +1926,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 +1947,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
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 19+ messages in thread