Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp30448813rwd; Thu, 6 Jul 2023 06:11:08 -0700 (PDT) X-Google-Smtp-Source: APBJJlG70w7eL5Jdza85E0oEkm63iibmlOxeQck4fDSbCKfR/qvcA7NXRTS1n+51wIpHeYXMFmq6 X-Received: by 2002:a17:903:234e:b0:1b8:9bd4:297 with SMTP id c14-20020a170903234e00b001b89bd40297mr1419173plh.15.1688649068490; Thu, 06 Jul 2023 06:11:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688649068; cv=none; d=google.com; s=arc-20160816; b=Pc5S/sT5eF1iSF3VyedOSUWvEQ6biiiH+GrGWjgAGcAkr+Yq2w1eI7UKqXPOxDtDYH Q+1HmAf3Amh3pNS1+idAjNA4/U5ET807xiSKoAvqttxv7DmFepv5a7xRGhAs85v8Prq7 Ko1JvIIPjl8peWO8BXPnMWluErbBBn1GNxjLKQEQ7kAnr0PpXtBRSliUgAYLhBzAJXlE WFIrl69EbDqRVOjv36LhUvXTYp27EE8NdGmkTV2EeGXQ4iEr8cM79ZswoSPc048SHhH/ ghupWPTaQbEdW7mv+w/w5m/E9BQE6zYhw82FaROuGQtM37+yD4VxX2UbVvSL4wbMdb7D rcPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=vELZbnMmNkmoaEABtrT3W1r0gqpXFf7Q4yaW/PDd3ic=; fh=ijmUOsZVRYTTodG0OqLg3EGpS0KT8DMM4e4dzgUtRrM=; b=fgsWehzjdvYyx8/nK2Nry/QtKMImHUvvw2BYDBcVP9hHiMFu53mbj5oBTB2dsis8zK Zed3qnRqrayYq0yYCBmAbqm41i1gUV+fLsXIIPUjHfxQBmszOAe3SUKxzIYvUT9AgpQH JgYA2Gl3qaMXtUG4dKEGQb2tKh9ltBxOpdy7woSFZctFOASrV52VhPtx8cSgIRhEVoLk 8l0DcH7Gnz9jVuAS1ApvdtHmRaLAYhcRvfbluI4WWLgb9P13IlI2MZ3dKYE/oCohLW4p MnPtofYy/blxfbJoT6bUFgJ2g/wkMbXtXjc44o3GUenSkeG6SEm714+54kB/W5QNbDd7 3s/w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=uriX3xdK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id n8-20020a170902e54800b001b20e2ba8e1si1438158plf.23.2023.07.06.06.10.55; Thu, 06 Jul 2023 06:11:08 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=uriX3xdK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229919AbjGFM6X (ORCPT + 99 others); Thu, 6 Jul 2023 08:58:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59072 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229843AbjGFM6W (ORCPT ); Thu, 6 Jul 2023 08:58:22 -0400 Received: from mail-pg1-x534.google.com (mail-pg1-x534.google.com [IPv6:2607:f8b0:4864:20::534]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B0CCD1BD9 for ; Thu, 6 Jul 2023 05:58:19 -0700 (PDT) Received: by mail-pg1-x534.google.com with SMTP id 41be03b00d2f7-55779047021so483737a12.3 for ; Thu, 06 Jul 2023 05:58:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1688648299; x=1691240299; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=vELZbnMmNkmoaEABtrT3W1r0gqpXFf7Q4yaW/PDd3ic=; b=uriX3xdK4G/u9PN6mBy8tlT2/KJ0P8M7jWAuBjRLJ/Sih3nSDbV8mAztivxafEVHNJ 8YJ5Ev71rDYVS9sAbJ4BjQPZARo+x8auRWKyfab4iBTVE92X8XvjqEj4HXMOEd3dmHV3 spkEelpiOCMbkw46ss1wqPgSEKkaaChQthhVO5EiD0Yd/fWwaE/0elDbZImJXl0pdZeu 3wlswkkrivh5CCwZ0/WXW+SkRAtnye0ZCgZSHBwcSbbQKjr/yfHhKnaB5ceempEX5KIP FfBlzoShO2K9CpwEf5NbX1i3xMioI2FI5vXnTigvPtnjTNnEychuIEnnCP2M0qxpXUOf b5bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688648299; x=1691240299; h=in-reply-to:content-transfer-encoding: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=vELZbnMmNkmoaEABtrT3W1r0gqpXFf7Q4yaW/PDd3ic=; b=hzk8rVpP/5U5D5kCOuCq1jsDkMmuNrrerfKhNzkQPseA8BSCJf291KjAKTLCndDpEI iwtMUwghmLP0TUow2dArtkUJGoPCXuiWMkmgmJkxxUnihzSKJURjDUCJXsSSqYyVIdss 5PMLF4YEJgGgBAF2Ynlh2iKoJQGLSQJ4RhXylUaSas65N33DTi8cOabC/d0R3tHS+Hyw YIxxCDJebCVV19f4zFAaDsGGsNKrqkbV2J36WSBUfznEr3rF2N1+itedFwRsl2GgdSuh 1exPNe1+Nc+yz0q6LKqCDCvYsbfDqDtKplnv+pXxEb/02NuKMGAdxd8uH7g4qBQn/05r g9bg== X-Gm-Message-State: ABy/qLYqrZgNUXvw2qsk4c7TBGToEHeAIG1/FhxqcSTG9Q7rxPg3ekbo 5mn9yCl7Gw6kmn3yxel/kRNc X-Received: by 2002:a17:90b:1d81:b0:263:5eea:e820 with SMTP id pf1-20020a17090b1d8100b002635eeae820mr1179773pjb.26.1688648298965; Thu, 06 Jul 2023 05:58:18 -0700 (PDT) Received: from thinkpad ([117.216.120.100]) by smtp.gmail.com with ESMTPSA id t3-20020a17090b018300b002635db431a0sm1275171pjs.45.2023.07.06.05.58.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jul 2023 05:58:18 -0700 (PDT) Date: Thu, 6 Jul 2023 18:28:11 +0530 From: Manivannan Sadhasivam To: Johan Hovold Cc: Lorenzo Pieralisi , Jingoo Han , Gustavo Pimentel , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Bjorn Helgaas , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Bjorn Andersson , Sajid Dalvi , Ajay Agarwal Subject: Re: [PATCH] Revert "PCI: dwc: Wait for link up only if link is started" Message-ID: <20230706125811.GD4808@thinkpad> References: <20230706082610.26584-1-johan+linaro@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230706082610.26584-1-johan+linaro@kernel.org> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 06, 2023 at 10:26:10AM +0200, Johan Hovold wrote: > This reverts commit da56a1bfbab55189595e588f1d984bdfb5cf5924. > > A recent commit broke controller probe by returning an error in case the > link does not come up during host initialisation. > > As explained in commit 886a9c134755 ("PCI: dwc: Move link handling into > common code") and as indicated by the comment "Ignore errors, the link > may come up later" in the code, waiting for link up and ignoring errors > is the intended behaviour: > > Let's standardize this to succeed as there are usecases where > devices (and the link) appear later even without hotplug. For > example, a reconfigured FPGA device. > > Reverting the offending commit specifically fixes a regression on > Qualcomm platforms like the Lenovo ThinkPad X13s which no longer reach > the interconnect sync state if a slot does not have a device populated > (e.g. an optional modem). > > Note that enabling asynchronous probing by default as was done for > Qualcomm platforms by commit c0e1eb441b1d ("PCI: qcom: Enable async > probe by default"), should take care of any related boot time concerns. > > Finally, note that the intel-gw driver is the only driver currently not > providing a start_link callback and instead starts the link in its > host_init callback, and which may avoid an additional one-second timeout > during probe by making the link-up wait conditional. If anyone cares, > that can be done in a follow-up patch with a proper motivation. > The offending commit is bogus since it makes the intel-gw _special_ w.r.t waiting for the link up. Most of the drivers call dw_pcie_host_init() during the probe time and they all have to wait for 1 sec if the slot is empty. As Johan noted, intel-gw should make use of the async probe to avoid the boot delay instead of adding a special case. > Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") > Reported-by: Bjorn Andersson > Cc: Sajid Dalvi > Cc: Ajay Agarwal > Signed-off-by: Johan Hovold Reviewed-by: Manivannan Sadhasivam - Mani > --- > .../pci/controller/dwc/pcie-designware-host.c | 13 ++++-------- > drivers/pci/controller/dwc/pcie-designware.c | 20 +++++++------------ > drivers/pci/controller/dwc/pcie-designware.h | 1 - > 3 files changed, 11 insertions(+), 23 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index cf61733bf78d..9952057c8819 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -485,20 +485,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > if (ret) > goto err_remove_edma; > > - if (dw_pcie_link_up(pci)) { > - dw_pcie_print_link_status(pci); > - } else { > + if (!dw_pcie_link_up(pci)) { > ret = dw_pcie_start_link(pci); > if (ret) > goto err_remove_edma; > - > - if (pci->ops && pci->ops->start_link) { > - ret = dw_pcie_wait_for_link(pci); > - if (ret) > - goto err_stop_link; > - } > } > > + /* Ignore errors, the link may come up later */ > + dw_pcie_wait_for_link(pci); > + > bridge->sysdata = pp; > > ret = pci_host_probe(bridge); > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index df092229e97d..8e33e6e59e68 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -644,20 +644,9 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index) > dw_pcie_writel_atu(pci, dir, index, PCIE_ATU_REGION_CTRL2, 0); > } > > -void dw_pcie_print_link_status(struct dw_pcie *pci) > -{ > - u32 offset, val; > - > - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > - val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); > - > - dev_info(pci->dev, "PCIe Gen.%u x%u link up\n", > - FIELD_GET(PCI_EXP_LNKSTA_CLS, val), > - FIELD_GET(PCI_EXP_LNKSTA_NLW, val)); > -} > - > int dw_pcie_wait_for_link(struct dw_pcie *pci) > { > + u32 offset, val; > int retries; > > /* Check if the link is up or not */ > @@ -673,7 +662,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci) > return -ETIMEDOUT; > } > > - dw_pcie_print_link_status(pci); > + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > + val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); > + > + dev_info(pci->dev, "PCIe Gen.%u x%u link up\n", > + FIELD_GET(PCI_EXP_LNKSTA_CLS, val), > + FIELD_GET(PCI_EXP_LNKSTA_NLW, val)); > > return 0; > } > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 615660640801..79713ce075cc 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -429,7 +429,6 @@ void dw_pcie_setup(struct dw_pcie *pci); > void dw_pcie_iatu_detect(struct dw_pcie *pci); > int dw_pcie_edma_detect(struct dw_pcie *pci); > void dw_pcie_edma_remove(struct dw_pcie *pci); > -void dw_pcie_print_link_status(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { > -- > 2.39.3 > -- மணிவண்ணன் சதாசிவம்