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 CE0B8C54E4A for ; Fri, 8 Mar 2024 10:15:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=tbir1Jq2cTJv/zdKqoV/CfjxEb7XDP72KOSf9OiQiZ4=; b=YraMtnjaU4WGwh O3iRtriA/05WRoQEW7igGDtS0hl7qTPPHog7zbVeQvtw4+bQhLsIA8y6JxY49EhVU9tUVBlzurl7l WC0A67QyXuLG0XCGDbl+kJt6taT6kcv58zhGRgGoKUDGlVya1KffeLyIDTjF3o6pZa/WYCsz9FPKM SzrvWD/hJ5IJVloZeGEiWorOAkn/Ijzjc3YJsa7q14bmFJYnrc3V9m3z57XxkDo0wVkGVc852PWcj 1XSwellP5XX7xEzyjPWXPh12Z0PjekEM6YiDjlXcvegU1kAsKq9EXeK3hyhespQb7Luop4bT/KFak baT9AkAvCobFqYME3Rrg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1riXFu-00000008iSB-0PsC; Fri, 08 Mar 2024 10:15:10 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1riXFp-00000008iOp-2SwB for linux-arm-kernel@lists.infradead.org; Fri, 08 Mar 2024 10:15:07 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 4BCF7CE2847; Fri, 8 Mar 2024 10:15:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E6A22C43390; Fri, 8 Mar 2024 10:14:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1709892902; bh=RUpDGa9Rs6fU9BfFMEQzpLXZ5dNRS4tLDMMkYMlz10A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=azDcURbX6wPyiSgOborkCmRq1aJLfraoxkXDDCyOkkcYhnoXZDktTO4rQSTj2dLBe dJyVsKXRTCz/lpY0R/pg0Fna7y6mJQ0sOZ/8erlJ8i5bkRb1ZbKoYhUYoXRYYRzCl5 JWTjw0y7oj3egZys2daT0GDqqSaE5qK64Ov8Eq5yheZKKxT/smpklskIw2FAfVizd4 o00HzVVqduydu9paV8JKrl3GeFrsMPItPrGb/q2mSQ4JWv3c6VDOF0FYOVdqrpIyG7 8eCvrYxjUo+UqIYvKsfO3jC1szqnpt5hr0wSLCamo2pzzw+pERPuYfsU/vF372xN6V f1AC8A3XsX6nA== Date: Fri, 8 Mar 2024 11:14:51 +0100 From: Niklas Cassel To: Manivannan Sadhasivam Cc: Jingoo Han , Gustavo Pimentel , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Bjorn Helgaas , Marek Vasut , Yoshihiro Shimoda , Thierry Reding , Jonathan Hunter , Kishon Vijay Abraham I , Vidya Sagar , Vignesh Raghavendra , Richard Zhu , Lucas Stach , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Minghuan Lian , Mingkai Hu , Roy Zang , Kunihiko Hayashi , Masami Hiramatsu , Kishon Vijay Abraham I , Jesper Nilsson , Srikanth Thokala , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-arm-kernel@axis.com Subject: Re: [PATCH v9 08/10] PCI: dwc: ep: Add a generic dw_pcie_ep_linkdown() API to handle LINK_DOWN event Message-ID: References: <20240304-pci-dbi-rework-v9-0-29d433d99cda@linaro.org> <20240304-pci-dbi-rework-v9-8-29d433d99cda@linaro.org> <20240308054152.GD3789@thinkpad> <20240308094606.GG3789@thinkpad> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240308094606.GG3789@thinkpad> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240308_021506_015496_5A12FD31 X-CRM114-Status: GOOD ( 43.71 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 08, 2024 at 03:16:06PM +0530, Manivannan Sadhasivam wrote: > On Fri, Mar 08, 2024 at 09:56:33AM +0100, Niklas Cassel wrote: > > On Fri, Mar 08, 2024 at 11:11:52AM +0530, Manivannan Sadhasivam wrote: > > > On Thu, Mar 07, 2024 at 10:43:19PM +0100, Niklas Cassel wrote: > > > > On Mon, Mar 04, 2024 at 02:52:20PM +0530, Manivannan Sadhasivam wrote: > > > > > The PCIe link can go to LINK_DOWN state in one of the following scenarios: > > > > > > > > > > 1. Fundamental (PERST#)/hot/warm reset > > > > > 2. Link transition from L2/L3 to L0 > > > > > > > > > > In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the > > > > > state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize > > > > > them to function properly once the link comes back again. > > > > > > > > > > This is not a problem for drivers supporting PERST# IRQ, since they can > > > > > reinitialize the registers in the PERST# IRQ callback. But for the drivers > > > > > not supporting PERST#, there is no way they can reinitialize the registers > > > > > other than relying on LINK_DOWN IRQ received when the link goes down. So > > > > > let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the > > > > > non-sticky registers and also notifies the EPF drivers about link going > > > > > down. > > > > > > > > > > This API can also be used by the drivers supporting PERST# to handle the > > > > > scenario (2) mentioned above. > > > > > > > > > > Signed-off-by: Manivannan Sadhasivam > > > > > --- > > > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++---------- > > > > > drivers/pci/controller/dwc/pcie-designware.h | 5 ++ > > > > > 2 files changed, 72 insertions(+), 44 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > > index 278bdc9b2269..fed4c2936c78 100644 > > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > > @@ -14,14 +14,6 @@ > > > > > #include > > > > > #include > > > > > > > > > > -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > > > > -{ > > > > > - struct pci_epc *epc = ep->epc; > > > > > - > > > > > - pci_epc_linkup(epc); > > > > > -} > > > > > -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup); > > > > > - > > > > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) > > > > > { > > > > > struct pci_epc *epc = ep->epc; > > > > > @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap) > > > > > return 0; > > > > > } > > > > > > > > > > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > > > > > +{ > > > > > + unsigned int offset, ptm_cap_base; > > > > > + unsigned int nbars; > > > > > + u32 reg, i; > > > > > + > > > > > + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > > > > > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > > > > + > > > > > + dw_pcie_dbi_ro_wr_en(pci); > > > > > + > > > > > + if (offset) { > > > > > + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > > > > > + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >> > > > > > + PCI_REBAR_CTRL_NBAR_SHIFT; > > > > > + > > > > > + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) > > > > > + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > > > > > > > > If you look at PCI_REBAR_CAP, you will see that it is sticky, > > > > but you have to actually read the databook to see that: > > > > > > > > "The RESBAR_CTRL_REG_BAR_SIZE field is automatically updated > > > > when you write to RESBAR_CAP_REG_0_REG through the DBI." > > > > > > > > So the reason why we need to write this register, even though > > > > it is sticky, is to update the RESBAR_CTRL_REG_BAR_SIZE register, > > > > which is not sticky :) > > > > > > > > (Perhaps we should add that as a comment?) > > > > > > > > > > Yeah, makes sense. > > > > Note that I add a (unrelated) comment related to REBAR_CAP in this patch: > > https://lore.kernel.org/linux-pci/20240307111520.3303774-1-cassel@kernel.org/T/#u > > > > But once we move/add code to dw_pcie_ep_init_non_sticky_registers(), I think > > that it might be a good "rule" to have a small comment for each write in > > dw_pcie_ep_init_non_sticky_registers() which explains why the code should be > > in dw_pcie_ep_init_non_sticky_registers() instead of dw_pcie_ep_init_registers(), > > even if it just a small: > > > > /* Field PCI_XXX_YYY.ZZZ is non-sticky */ > > writel_dbi(pci, offset + PCI_XXX_YYY, 0); > > > > Why? The function name itself suggests that we are reinitializing non-sticky > registers. So a comment for each write is overkill. So that you know which field it is in the register that you are writing which you care about (which field it is that is non-sticky). But I see your point, perhaps it is overkill. Kind regards, Niklas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel