Re: [PATCH v3 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low
From: Manivannan Sadhasivam <mani@kernel.org>
Date: 2025-09-22 06:12:45
Also in:
imx, linux-pci, lkml
On Mon, Sep 22, 2025 at 10:37:41AM +0800, Richard Zhu wrote:
quoted hunk ↗ jump to hunk
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 | 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 80e48746bbaf..a73632b47e2d 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);
This should be: imx95_pcie_clkreq_override(imx_pcie, true); refer below...
quoted hunk ↗ jump to hunk
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);
Just noticed this discrepancy. 'imx8mm_pcie_enable_ref_clk(, false)' is enabling
the CLKREQ# override, thereby enabling the refclk. But only for imx8mm, this
helper is called as imx8mm_pcie_enable_ref_clk(). But for imx95, the equivalent
function is called as imx95_pcie_clr_clkreq_override(). This is causing
confusion.
Maybe you should just call both functions as:
imx8mm_pcie_clkreq_override(imx_pcie, bool enable);
imx95_pcie_clkreq_override(imx_pcie, bool enable);
Then,
imx8mm_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
{
imx8mm_pcie_clkreq_override(imx_pcie, false)
}
imx95_pcie_clr_clkreq_override(struct imx_pcie *imx_pcie)
{
imx95_pcie_clkreq_override(imx_pcie, false)
}
and populate the clr_clkreq_override() callback.
- Mani
--
மணிவண்ணன் சதாசிவம்