Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp817504yba; Fri, 26 Apr 2019 09:12:19 -0700 (PDT) X-Google-Smtp-Source: APXvYqy2ffd4ClMzaVJggSNOjaiB8cJkn+DpfpfeR8kcDFTq9+kcWo3MdgFmxERrrzMsSbHiZG5b X-Received: by 2002:a63:d408:: with SMTP id a8mr12167528pgh.184.1556295139892; Fri, 26 Apr 2019 09:12:19 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1556295139; cv=none; d=google.com; s=arc-20160816; b=Ac11xcA2+6HwrODEyvUSAFmexQbL+ATFrYtSDpvRMOWQDscs74ohol5dxWrCtbYEXj I0B5Q9CRrCDKDAMbxn9FErmuJsvKSehxT4+3ulWEMd2sEzBYNZ7CAc/mBRZDEdxwLcwP UaOA8+7j2qYWCJlQ2rgui/BXiHUDwCh4T/0Jm3LKaYd86VLZYLMQwdgqeU8aXip9iTuc 2EWX0RhKubqpvMN1hwdZbL4J7MhyncdaktAdRSh983FAKJsY1AwQaT//MzGKUTx0v/RP hHiSpM1iy4QFMIR1L0ZVmA7nCeG8pO4yi+MKB5UhfHJq38ZAYnTnG0FYWzqAO7Q2O8Zd eRmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=oT9O1u+AkVV1gD341t5qKuaVmOglyWKLY6uUzZ/1qag=; b=HmvHByV9e7XOFRK9nhvxM0c8gYsyrqKURuQpUvYv4PRAjKyG57LebibMjGK/1Ps7b3 aSQF1q0l9elxNlrduxx0iQc5A4MV8W+mDo+nDIVYwlE+n5HCpfPCZhTsgy5OMX3k+Pfe lX661szlnvH3MUooNsoBZwDWEVFsbknogJGfR7dlOapByCu7YWw6igPvH0y1tNv+7jLK rkQ8rqsF26tPJIYXEWXJ2Of9XgIslt70NtzVBRZOP+bs58DVtMqPCei1kEZe0HZWaVYn rNBM1W1tEixZzJZAx4sy1nPMN2Ps2vO/S9m+u16wnVKtKlU6/Sb+60x2OG4lJMyoasX2 U+mA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=NblDReoE; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id y22si27459497pfo.49.2019.04.26.09.12.03; Fri, 26 Apr 2019 09:12:19 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=NblDReoE; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726414AbfDZQKx (ORCPT + 99 others); Fri, 26 Apr 2019 12:10:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:54604 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726039AbfDZQKw (ORCPT ); Fri, 26 Apr 2019 12:10:52 -0400 Received: from localhost (173-25-63-173.client.mchsi.com [173.25.63.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id AE1C820B7C; Fri, 26 Apr 2019 16:10:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556295051; bh=AdIVBpnWD4EKRko4Qg1W+qoIdwT0atGxH5KyjTAjmLY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NblDReoEcEdKXjkOnUMB3hntAPt6i8qWP0cbksb59jsvVofe8n26ccSx++CJcH6TA gstldZ+aLEByveeMspV33ZGgll/aZW2RM5X9urMuzgIrxt/C0KcVbXD+8WzHBRVoYy 13E4n0CdIIy/WTkpKR23mrMPc4kCTiNIQLPHSsiw= Date: Fri, 26 Apr 2019 11:10:50 -0500 From: Bjorn Helgaas To: Remi Pommarel Cc: Lorenzo Pieralisi , Thomas Petazzoni , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Miquel Raynal Subject: Re: [PATCH] pci: aardvark: Wait for endpoint to be ready before training link Message-ID: <20190426161050.GA189964@google.com> References: <20190313213752.1246-1-repk@triplefau.lt> <20190423163215.GB26523@red-moon> <20190423222917.GN2754@voidbox.localdomain> <20190424165002.GA26089@e121166-lin.cambridge.arm.com> <20190425210826.GQ2754@voidbox.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190425210826.GQ2754@voidbox.localdomain> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 25, 2019 at 11:08:27PM +0200, Remi Pommarel wrote: > On Wed, Apr 24, 2019 at 05:50:02PM +0100, Lorenzo Pieralisi wrote: > > On Wed, Apr 24, 2019 at 12:29:18AM +0200, Remi Pommarel wrote: > > > On Tue, Apr 23, 2019 at 05:32:15PM +0100, Lorenzo Pieralisi wrote: > > > > On Wed, Mar 13, 2019 at 10:37:52PM +0100, Remi Pommarel wrote: > > > > > When configuring pcie reset pin from gpio (e.g. initially set by > > > > > u-boot) to pcie function this pin goes low for a brief moment > > > > > asserting the PERST# signal. Thus connected device enters fundamental > > > > > reset process and link configuration can only begin after a minimal > > > > > 100ms delay (see [1]). > > > > > > > > > > This makes sure that link is configured after at least 100ms from > > > > > beginning of probe() callback (shortly after the reset pin function > > > > > configuration switch through pinctrl subsytem). > > > > I am a bit lost, what's the connection between the probe() callback > > and the reset pin function configuration ? > > So currently u-boot configures the reset pin as a GPIO set to high. The > espressobin devicetree defines a default pinctrl to configure this pin > as a PCIe reset function. > > As you can see in drivers/base/dd.c, driver_probe_device() calls > really_probe() which first calls pinctrl_bind_pins() then shortly after > drv->probe() callback. The pinctrl_bind_pins() function applies the > default state. So here, just before drv->probe() gets called our reset > pin goes from GPIO function to PCIe reset one making it going low for a > short time during this transition. > > Because the pin goes low then gets back to high, PERST# signal is > asserted then deasserted and device enters fundamental reset process > just before drv->probe() is called. So in order to reduce the waiting > time to a minimum I sample jiffies at the very beginning of the probe > function, which is the closer spot from where PERST# is deasserted. > > To sum up: > > driver_probe_device() { > ... > really_probe() { > ... > pinctrl_bind_pins(); /* Here PERST# is asserted because pin configuration changes */ > ... > drv->probe(); Ah, I see. Hmmm. This definitely warrants a comment in advk_pcie_probe() about the connection. I appreciate that ab78029ecc34 ("drivers/pinctrl: grab default handles from device core") saves some boilerplate from drivers, but ... at the same time, it makes for some non-obvious implicit connections like this. I'm not sure whether having the boilerplate or the comment is worse. But I'm pretty sure the "no boilerplate, no comment" option is the worst of the three :) > > > > > [1] "PCI Express Base Specification", REV. 2.1 > > > > > PCI Express, March 4 2009, 6.6.1 Conventional Reset > > > > > +/* Endpoint can take up to 100ms to be ready after a reset */ > > > > > +#define ENDPOINT_RST_MS 100 > So doing that I do a msleep() of around 75-80ms instead of 100ms. So, > yes, are 20ms enough to justify that, or should we just go with a plain > msleep(100) to improve legibility. I vote for 100ms so it's easily tied to the spec. We *should* have a PCI core #define for that to make it even easier. PCI_PM_D3COLD_WAIT might be close, but we might need some cleanup in this area. There are a whole bunch of "msleep(100)" calls in drivers/pci that probably should use the same #define. Somebody should look through pci_af_flr(), pcie_flr(), pci_pm_reset(), pcie_wait_for_link(), to see if we can use a common constant. Bjorn