From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2B0893314DF; Wed, 20 Aug 2025 21:40:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755726012; cv=none; b=c4+IAtq9sbOlBy8NSSIU5QOLGN1cUfAO9xmddVxgCdQdEtxUI+dhWowez8mCFlKF8CXbe0Tg5UM+cloFwH3CRFtfc+ZfyUO4Tlyd62e1MDjEZhCom4X3HagoFI+BEW1WSdtH0E2mP0WxbvIWw0TS8bbHzRuzXb1teaqDB8sfy6g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755726012; c=relaxed/simple; bh=TnjR1aCGk2Nq7S3fmi9JhV4UrNQPN7ZoCC3P9TmTk/c=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition:In-Reply-To; b=QKZo7e2DJO6l+QvjrKW3KjneojxhtpUIH/tjW+nAqmJEsf6gaJKvPtmG5jGKpR6DrXcbO2UYhRViSkGfN2wUbTlfLj/rHKXzrik1SFEZDy9O8luJGjWBkPnmczBE8jad0ZGdvygSck8wKx33GN3KI3rodvGBVkyC4Eq4rtmh0hY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=EB2MrzxD; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="EB2MrzxD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C227FC4CEE7; Wed, 20 Aug 2025 21:40:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1755726012; bh=TnjR1aCGk2Nq7S3fmi9JhV4UrNQPN7ZoCC3P9TmTk/c=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=EB2MrzxDN4Sb38enZTXidWOdB7HEEOR6z36T2iHb4uABIuS1nWfXPolbV5H+SCisT uY5QC1SIsnmpRsSLsZ3iYCkTtUD/RUD/nS4lITHXcIMGgeS97culcYIofMCN297+6H 8knk/wPBTxxpw0PlJK9FPQLpDnfRwEsX5GeusZF5w2S32QUcbPag8eOKgfZQU7219v nxJw5+16g6U5aujPw1xsxzmY8U7ryoQEIl5u5E2OHPdf2U7pusPs1j2JC+uqvREUf5 zusTilsJuVL6Qhq/6cHuTKdklBdLSAmHyjX7HbqLFCBI2CZx5Pu/EcbrJotLkcUQuC h2743WoUljPNw== Date: Wed, 20 Aug 2025 16:40:10 -0500 From: Bjorn Helgaas To: Richard Zhu Cc: frank.li@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 Message-ID: <20250820214010.GA641668@bhelgaas> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250820081048.2279057-3-hongxing.zhu@nxp.com> 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 > --- > 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 >