From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 10A70CA0EED for ; Thu, 21 Aug 2025 01:07:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=eH87NJGI4Ym4B4he8U6C+rotDmjwkZldCjn9jAswb4k=; b=0D2zqQETbWMgoL p+OrRb81URhZVP9uskvTFOc2/+6mBk8XTrVyLV/m61iJvkRzzP8N7pgqbxLAXAtLWmH06W27m9TW8 DQBuvsVbYoeAE6WImPBA24lOiVP071MpGSxxtlB3UJ3E2ZvQpif13SGkixydNYJrn9ioAEnO8jr9n n28fqxmGswiu4Vwwi+UB3dkfCyVi/PdI5BJVcoYeBR3Z4oyor8N3NmyGOI+IyM0cNigx9n/X+RYPO frud9inv7JWS/1kMg9kX34c6MkdPLnZgd1qbc2yUCXqYmO4ynSN0AKgmhX17XzGy8ntYfpeTpcvtR YkKgPdJOg69feKTs15/g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uotm0-0000000FNic-2o0Q; Thu, 21 Aug 2025 01:07:24 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uopX0-0000000EqZg-08mi for linux-arm-kernel@lists.infradead.org; Wed, 20 Aug 2025 20:35:39 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id AE6E5468FD; Wed, 20 Aug 2025 20:35:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6B738C4CEE7; Wed, 20 Aug 2025 20:35:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1755722137; bh=TsKHYxNZLLNav2fl4VlSgG/S6ch3wqYz+Pns7R4RxD0=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=tEf1V7syOSrPzXUXLULwOAU4Rkjql9oMaQBC6Gwq3yNFOJOO7Bj+g0HBiN9WpMFRL WgxoS9fajMm8VOQLwWG7B+45FaorAbW6VNVcePZS/+B+6rgCHP1U8iuj/pIJe8Wzq+ ih69AN7Cci3vJwwNBroHTt7UJtHl+ehko9DMi/4quqvF7WgTS/dvOZlBB4syFmHLBI JurFBvSQ+UMEchxxzU5ESUlqSPONBknCwg+Ij1+MguCl48xEXEVTsRJPbQRLtj0Li8 yC1NVsKWKZD6ZycoK8wPPlAeImllbbJOMW3PVD5r8OuoOmdr1QGxgi+eUtAQQzvGav D0xctMv3Yel8w== Date: Wed, 20 Aug 2025 15:35:36 -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: <20250820203536.GA639059@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250820081048.2279057-3-hongxing.zhu@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250820_133538_110364_7A8C1256 X-CRM114-Status: GOOD ( 25.27 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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); 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