Received: by 2002:a05:7412:e794:b0:fa:551:50a7 with SMTP id o20csp2189000rdd; Fri, 12 Jan 2024 02:00:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IEfbWDI4rSHwcwRjfulWAjFyv70FLY5S40f0fLEbnwbjOeh4dlwnfQCJ4SU+hnkF7yIXAPc X-Received: by 2002:a17:906:c095:b0:a2c:aaa5:1a10 with SMTP id f21-20020a170906c09500b00a2caaa51a10mr806997ejz.5.1705053636511; Fri, 12 Jan 2024 02:00:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705053636; cv=none; d=google.com; s=arc-20160816; b=F3aBxw0nOfJiAGytVPc0f8V0HQ1jmKm73wMB6S/KUP3lFHHT24GHnNVeaEXejtzkA7 i0ZmCj3bdEou9Vgot/tS/5nakCvz2fGpIXNyQyCoJO1DpGmWx4GTVFbDrvsDHatwAUHX LynsaDKs8IyAwrvWKj3tpgy7EKbksfwofLxZM55iNi764FT8eU3Dv0T0V/iN/blEyFBi dUYymHSm4ChYtuLGqxVajiXoDNfL5lTb5NvilDRb6sXIslGlmxPAOKph3pmOlHi3qhRT 6kL5Hv3j4C7B0H1QQpaCGViNnBoiufsG4RIY+mdvB+mUn5Pvt5n/QXR9YN5o1Py2sfE0 md4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:list-unsubscribe:list-subscribe:list-id:precedence :references:message-id:subject:cc:to:from:date:dkim-signature; bh=dRYoalgnwKVYfdSiaBfDRevKcT4wc2GOJqsALypTOmU=; fh=xHZl3rvqCVBrwxvuHl2saYOtkS2jA/Ojht/4Zmi3Dtg=; b=0n95RJaEQqPcZtvVkXo9ZA4zJ7Lq3+TOu1uaPYRrxZkDubxj2IulV4Zf9zcQShpDeL dqh883smnymbFakQ7p3oIgoEIIFUSeW8rZHD3Vg0C3OUQOel8PoqL/sXXmr5ltQ6t6Wd gAxmeSdV5gCs4fpJP1nFttEPtJWaOIa1mj3NdeIPylZ3uMj19Upf16xIwVprVscRM9WF Xg23zlev/U4imEuhFp1Yx0WEtMX9j2wM8V1wyrWqjoO8xiNSIcBLl+r8RaVdwBQXUBFk UitPjbFEC9HmaaCx4HvrKloCvODWk882NaWqP3zsfzJLfT88xxn7PaMR9iCznuvPHw8s 11PQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=OpOcctWq; spf=pass (google.com: domain of linux-kernel+bounces-24535-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24535-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id i22-20020a170906445600b00a27a0bf598dsi1185184ejp.1036.2024.01.12.02.00.36 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 02:00:36 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-24535-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=OpOcctWq; spf=pass (google.com: domain of linux-kernel+bounces-24535-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24535-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 1C8C71F23F2B for ; Fri, 12 Jan 2024 10:00:36 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 39CA85D8EA; Fri, 12 Jan 2024 10:00:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="OpOcctWq" Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 00FCA5D739 for ; Fri, 12 Jan 2024 10:00:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-28ca63fd071so3380045a91.3 for ; Fri, 12 Jan 2024 02:00:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705053624; x=1705658424; darn=vger.kernel.org; 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=dRYoalgnwKVYfdSiaBfDRevKcT4wc2GOJqsALypTOmU=; b=OpOcctWqWBe3ajchwYGTl3RxOwVKYQiBfBqLvQPnN9D8/8aHpnJz3q8CFxEqBfsUz5 JCacvo4unw0zaCQBdj4M6F5i5WoCuGpCHuOYKV7nE+rvVqchOgHOsECliAMxK3y51HLF rzi2gvAQUP7nPGrQ9WApRk9fws2Q2gJarzBju1s7MBOYwc9VHLYA7fkVglDsvqCeeFiE HMZv7RO/ch4OuvWpErEJumEsS1cnaCnNgBIOKwgGGRrYTpMJX1G+kmIsRzVirT88jPKj A+6UBi8td/SRVZ17mRI3eMvlyjP5TqYKFmvFKPuFutppLVyrVbJiu7TXitHS5rFPrKyL 6Y8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705053624; x=1705658424; 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=dRYoalgnwKVYfdSiaBfDRevKcT4wc2GOJqsALypTOmU=; b=qzZ5S1fABEaZzqjRB1fpRu5KnUW1hOSoj92qfbFmyTqt0oEnv6i1cT63r4YmjY+O7E DyX+QwW0NNjzmQ5f2ukifB+Jm/r/OupcSqEbGrPHw4+H33lcBIMp1PhKXfaPhcfHouy0 2THc4fe6Udd1ujLgbDqQzH/udo4JB8EHX1BshpuMt7U7S5SCcoi28nvioDdzRVB0dWV3 QhEzvW7/7PDg32AxB74K6o+7BvWZsGwFrpmN+1oO6lhzYz5Dr9v6jbFkP6JLxfGUpBy0 XVykVL/yoyA5aPo2gcgqsnyi49csxtbdMiKlcf2b3iNG3DP3FgfMHJiAwcC4LzCUsM/s zmaw== X-Gm-Message-State: AOJu0YyUqj7mRxIlxHc+tQZQI7cbEZwZGVtYxtlJqGzGE55YnMqLGI6O VcIwHBOXw/TQIzz32Jdr5AUJYQCnQkfz X-Received: by 2002:a17:90a:898b:b0:28d:2b9d:e273 with SMTP id v11-20020a17090a898b00b0028d2b9de273mr917862pjn.74.1705053624143; Fri, 12 Jan 2024 02:00:24 -0800 (PST) Received: from google.com (108.93.126.34.bc.googleusercontent.com. [34.126.93.108]) by smtp.gmail.com with ESMTPSA id px12-20020a17090b270c00b0028e17b2f27esm58229pjb.13.2024.01.12.02.00.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 02:00:23 -0800 (PST) Date: Fri, 12 Jan 2024 15:30:15 +0530 From: Ajay Agarwal To: Manivannan Sadhasivam Cc: Johan Hovold , 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 Subject: Re: [PATCH] Revert "PCI: dwc: Wait for link up only if link is started" Message-ID: References: <20230706082610.26584-1-johan+linaro@kernel.org> <20230706125811.GD4808@thinkpad> <20230711073719.GA36617@thinkpad> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230711073719.GA36617@thinkpad> On Tue, Jul 11, 2023 at 01:07:19PM +0530, Manivannan Sadhasivam wrote: > On Mon, Jul 10, 2023 at 09:51:22PM +0530, Ajay Agarwal wrote: > > On Fri, Jul 07, 2023 at 02:47:56PM +0200, Johan Hovold wrote: > > > On Thu, Jul 06, 2023 at 06:28:11PM +0530, Manivannan Sadhasivam wrote: > > > > On Thu, Jul 06, 2023 at 10:26:10AM +0200, Johan Hovold wrote: > > > > > > > > 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. > > Mani, can you please explain how my commit made the intel-gw driver > > special? The intel driver actually fails the dw_pcie_host_init if the > > link does not come up. That was my motivation behind adding the fail > > logic in the core driver as well. > > Your commit ended up failing the probe, if dw_pcie_wait_for_link() fails for > SoCs defining start_link() callback, which is the case for all the drivers > except intel-gw. I take back my _special_ argument since it was special before > your commit and now you just made its behavior applicable to all SoCs. > You are right. I should not have returned an error from the dw_pcie_wait_for_link check. Raised v5 with the error return removed: https://lore.kernel.org/all/20240112093006.2832105-1-ajayagarwal@google.com/ > > > > > > Just to clarify, the intel-gw driver starts the link and waits for link > > > up in its host_init() callback, which is called during probe. That wait > > > could possibly just be dropped in favour of the one in > > > dw_pcie_host_init() and/or the driver could be reworked to implement > > > start_link(). > > > > > > Either way, the call in dw_pcie_host_init() will only add an additional > > > 1 second delay in cases where the link did *not* come up. > > > > > > > As Johan noted, intel-gw should make use of the async probe to avoid the boot > > > > delay instead of adding a special case. > > > > > > Indeed. > > > > > > Johan > > Johan, Mani > > My apologies for adding this regression in some of the SOCs. > > May I suggest to keep my patch and make the following change instead? > > This shall keep the existing behavior as is, and save the boot time > > for drivers that do not define the start_link()? > > > > No, IMO the offending commit was wrong in serving its purpose so a revert makes > sense. Because, if the intention was to reduce the boot delay then it did not > fix that because dw_pcie_wait_for_link() is still called from intel-gw's > host_init() callback. You just skipped another instance which is there in > dw_pcie_host_init(). > > So to fix this issue properly intel-gw needs to do 2 things: > > 1. Move the ltssm_enable to start_link() callback and get rid of > dw_pcie_wait_for_link() from its host_init() callback. If there is any special > reason to not do this way, please explain. > > 2. Enable async probe so that other drivers can continue probing while this > driver waits for the link to be up. This will almost make the delay negligible. > > The above 2 should be done in separate patches. > > - Mani > Mani, the intention is not to fix the intel-gw driver in any manner. It calls dw_pcie_wait_for_link explicitly in the probe path and checks for the error as well. So it has to live with the delay and the probe failure if the link does not come up. My intention is just to get rid of the 1 sec delay for the drivers that do not define the start_link callback, and hence do not expect that the link will come up during probe anyway. > > ``` > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index cf61733bf78d..af6a7cd060b1 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -492,11 +492,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > 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; > > - } > > + if (pci->ops && pci->ops->start_link) > > + dw_pcie_wait_for_link(pci); > > } > > > > bridge->sysdata = pp; > > ``` > > -- > மணிவண்ணன் சதாசிவம்