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 57235C5320E for ; Tue, 20 Aug 2024 07:06:13 +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:References: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:List-Owner; bh=LsF6Nk9oBlSWv0G1oFX6tHfAMXIERFk3AxXMBDi43S4=; b=sfWYhD01RIoG6WkJ6LwsheEzbs /aqNnORgNGy+93CFl+w0RAxW6bkIB735NVwsR40Chxk11VON0GwJM4LLrAixa+BKoTj1bqfz572qL mEX7ONiv1GkpRyWdlH+3UMfDMTOB8JH1fpPcFW4DyFrmF1Uj2tTLzZMsLukvnFI/ZvyVimjnlDGH3 ws2AwL6lFXVn4K9O+jNHrsBBEaRUGgEYgiwJk0oN4ubGeQvUFMymYNK+FJOvIpKcsEAc79e7qgvT6 iJ2CipVnJqj+PSAJz5N6fknWA2WTm3GD4Mv8i4AdZsg4bCUOejyvUJ5J9zId/7/JxUj13qhLr5NKn XgPG6wyg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgIwJ-00000004864-44XS; Tue, 20 Aug 2024 07:05:59 +0000 Received: from mail-ej1-x636.google.com ([2a00:1450:4864:20::636]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgIvb-000000047v0-1EWT for linux-arm-kernel@lists.infradead.org; Tue, 20 Aug 2024 07:05:16 +0000 Received: by mail-ej1-x636.google.com with SMTP id a640c23a62f3a-a7b2dbd81e3so710309666b.1 for ; Tue, 20 Aug 2024 00:05:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724137513; x=1724742313; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=LsF6Nk9oBlSWv0G1oFX6tHfAMXIERFk3AxXMBDi43S4=; b=ZTvreuZc+E193bXmYzUMMByJfQr/MZcmDgmODJEU39+vWbHtJsLsS4+QeqCSwQE5GD SbS5dDZT1P26/dhjnTBkGWt4+Qc1GiYVIS2Kzre+q3KGEbMD7ERGHpdXG9h4fcgrGy41 BZXqBsTy+PxBkZ+dNQwG10JAxCDaGt9BnLbNr6xd5R+jMouuwVU0qSvSgrK5YyCL0N4b Ywh+V09Z3pnCRBtYDUdz8rRFFAStuPDADN3ztS3GalcPV3ZXFrGqzVW81KQsTDtalter pmIoLdRGKZy61yQA7vys+3GRTQ32CqMaLREiwf60GbmKhjP0SChkbgzZAsqJaEwSOuGD zqoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724137513; x=1724742313; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=LsF6Nk9oBlSWv0G1oFX6tHfAMXIERFk3AxXMBDi43S4=; b=UIWnk7zvUQmfvPBBOIAcLJHPYLkgyz+GeJh5ac0p5yiGUARYALZEbKW3gu3ItxMKcE zpqz5DBy6SP65dwcRQTC/Wq3QwJiSmvLp/OsagecXKMnxvIIE0Q0q2d+QvSwy2WQtIkd pXLpC/UzrPvcKPDves+pJbF7pFCVcfw8haEGPHMO8WRqih6TGPdN5/5giCuONIFmHu4/ PZ4TgQGmqfXbTdWg2ZSgD07EH3vXnzoT8qB/p3uHl8vz3CzVS5BPW1cor4uAyCVFcwwn 44C9kD+qjXZDxDPPL0eVpOmOQiq33fGDWixFFyPoIwxuIOHK+rV6RekW6mCtkWOqjbCV XZiA== X-Forwarded-Encrypted: i=1; AJvYcCX2f0ii/GTR5MOO0U98oTZjff+iecHheqbvoYz9XSEwzOG/2aSV0nIJnButfbnQzd2S4aDkrwqEU8cr15tbgMqdBIPN/MTmbHQCGaFkFvbcg6DpRsM= X-Gm-Message-State: AOJu0YzVxyvPTx1e2B3queT8NkAZA02PCqs5RsqI4MzxpSudFu6+tWrq etDdUGTqk99Wu49dTqkFI5B3Rj69Dpvq3yvLReh+msEHz18XC+yH X-Google-Smtp-Source: AGHT+IGIdq3GdLoqQGTFyWn1TS/t53HJCAInq/O8v+Afgq7IVNwWy6BOOUHntTjFl6AEvA9/pKq7NQ== X-Received: by 2002:a17:907:e221:b0:a71:ddb8:9394 with SMTP id a640c23a62f3a-a8392957bd8mr965538566b.40.1724137512903; Tue, 20 Aug 2024 00:05:12 -0700 (PDT) Received: from eichest-laptop ([77.109.188.34]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a83838c7444sm725745566b.6.2024.08.20.00.05.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Aug 2024 00:05:12 -0700 (PDT) Date: Tue, 20 Aug 2024 09:05:10 +0200 From: Stefan Eichenberger To: Frank Li Cc: hongxing.zhu@nxp.com, l.stach@pengutronix.de, lpieralisi@kernel.org, kw@linux.com, robh@kernel.org, bhelgaas@google.com, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, francesco.dolcini@toradex.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev, linux-kernel@vger.kernel.org, Stefan Eichenberger Subject: Re: [PATCH v1 3/3] PCI: imx6: reset link on resume Message-ID: References: <20240819090428.17349-1-eichest@gmail.com> <20240819090428.17349-4-eichest@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240820_000515_362522_DEA233BB X-CRM114-Status: GOOD ( 48.53 ) 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 Mon, Aug 19, 2024 at 10:39:45AM -0400, Frank Li wrote: > On Mon, Aug 19, 2024 at 11:03:19AM +0200, Stefan Eichenberger wrote: > > From: Stefan Eichenberger > > > > According to the https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf errata, > > Can you show errata number here? > I will include it in the next version of the patch. If I understand it correct it is ERR005723. > > the i.MX6Q PCIe controller does not support suspend/resume. So suspend > > and resume was omitted. However, this does not seem to work because it > > looks like the PCIe link is still expecting a reset. If we do not reset > > the link, we end up with a frozen system after resume. The last message > > we see is: > > ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, > > device inaccessible > > > > Besides resetting the link, we also need to enable msi again, otherwise > > DMA access will not work and we can still end up with a frozen system. > > With these changes we can suspend and resume the system properly with a > > PCIe device attached. This was tested with a Compex WLE900VX miniPCIe > > Wifi module. > > > > Signed-off-by: Stefan Eichenberger > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 45 ++++++++++++++++++++++++++- > > 1 file changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index f17561791e35a..751243f4c519e 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -1213,14 +1213,57 @@ static int imx6_pcie_suspend_noirq(struct device *dev) > > return 0; > > } > > > > +static int imx6_pcie_reset_link(struct imx6_pcie *imx6_pcie) > > +{ > > + int ret; > > + > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > > + IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18); > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > > + IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16); > > + > > + /* Reset the PCIe device */ > > + gpiod_set_value_cansleep(imx6_pcie->reset_gpiod, 1); > > + > > + ret = imx6_pcie_enable_ref_clk(imx6_pcie); > > + if (ret) { > > + dev_err(imx6_pcie->pci->dev, "unable to enable pcie ref clock\n"); > > + return ret; > > + } > > + > > + imx6_pcie_deassert_reset_gpio(imx6_pcie); > > In my patch https://lore.kernel.org/linux-pci/Zr4XG6r+HnbIlu8S@lizhi-Precision-Tower-5810/T/#mc5f38934b6cef95eca90f1a6a63b3193e45179de > > imx6qp_pcie_core_reset() and imx6q_pcie_core_reset() is not symatic for > assert/desert() to match origin code. I plan fix it after above patch > merged. > > Does it work if make above code symatic? > I will give it a try with your patches applied and let you know. > > + > > + /* > > + * Setup the root complex again and enable msi. Without this PCIe will > > + * not work in msi mode and drivers will crash if they try to access > > + * the device memory area > > + */ > > + dw_pcie_setup_rc(&imx6_pcie->pci->pp); > > + if (pci_msi_enabled()) { > > + u32 val; > > + u8 offset = dw_pcie_find_capability(imx6_pcie->pci, PCI_CAP_ID_MSI); > > + > > + val = dw_pcie_readw_dbi(imx6_pcie->pci, offset + PCI_MSI_FLAGS); > > + val |= PCI_MSI_FLAGS_ENABLE; > > + dw_pcie_writew_dbi(imx6_pcie->pci, offset + PCI_MSI_FLAGS, val); > > + } > > there are already have imx6_pcie_msi_save_restore(imx6_pcie, true); in > suspend/resume, why need addtional one here? > I took the part from the probe function and added it here. I will see if I can rework that part. > > + > > + return 0; > > +} > > + > > static int imx6_pcie_resume_noirq(struct device *dev) > > { > > int ret; > > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > > struct dw_pcie_rp *pp = &imx6_pcie->pci->pp; > > > > + /* > > + * Even though the i.MX6Q does not support suspend/resume, we need to > > + * reset the link after resume or the memory mapped PCIe I/O space will > > + * be inaccessible. This will cause the system to freeze. > > + */ > > if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_SUPPORTS_SUSPEND)) > > - return 0; > > + return imx6_pcie_reset_link(imx6_pcie); > > If reset everything, I supposed we can add IMX6_PCIE_FLAG_SUPPORTS_SUSPEND > at driver data. > This didn't work for the current version. It seems we do too much in the suspend function and therefore it is still not working. However, maybe it is really better to just add the flag and try to make suspend/resume work by using the function pointers you introduced. I will have a look at it, thanks. > > > > ret = imx6_pcie_host_init(pp); > > if (ret) > > -- > > 2.43.0 > >