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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4EC9C433EF for ; Fri, 12 Nov 2021 16:08:06 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3B25B60FBF for ; Fri, 12 Nov 2021 16:08:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3B25B60FBF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2827C82FD4; Fri, 12 Nov 2021 17:08:04 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1636733284; bh=7nGVCfnVzqxuoc2y1ZPUSFsnujaNFeNk0ve8Fl6Jahs=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=ePxSxJN70yzPgCvDTUG2MdKQxZbsLSRGSB8iVtxgYiOxSv5MV/G8PO9Xy6+AuxkUZ rGqR+FKPH0vC0irWWKAL2r0XNFppa9YIh2QeR+6sR7hLTGH/rsLxjPX4jJ9/NXgh5C 6Fi30zsatyp4sHG3TUWZP7mT5GrB2WhjzNHEAQ9B2ZzP8+T/E2tGENHQngNMcvI5Jg QMbun/llxYja+mvypw8sKOtbF2TIOlWaI+vzAHZVSI0sa/dTh/aED5l6A3NGkYVKXU aVO1cHcC0GZMQ329Pjou/D5ZDzfyj2t3UQN2SHZrwRDfqCPFzZFzcp7NvHAsukk9// uolPFY8VQKQ/g== Received: by phobos.denx.de (Postfix, from userid 109) id 27A2D82FBC; Fri, 12 Nov 2021 17:08:02 +0100 (CET) Received: from mout-u-204.mailbox.org (mout-u-204.mailbox.org [91.198.250.253]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3337D82FF0 for ; Fri, 12 Nov 2021 17:07:58 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp202.mailbox.org (unknown [91.198.250.118]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-204.mailbox.org (Postfix) with ESMTPS id 4HrNlx5n2lzQkDy; Fri, 12 Nov 2021 17:07:57 +0100 (CET) Message-ID: <596206f8-7521-85e3-81c7-d7ba5dd8eacb@denx.de> Date: Fri, 12 Nov 2021 17:07:51 +0100 MIME-Version: 1.0 Subject: Re: [PATCH u-boot-marvell 01/10] pci: pci_mvebu: Wait 100ms for Link Up in mvebu_pcie_probe() Content-Language: en-US To: =?UTF-8?Q?Pali_Roh=c3=a1r?= Cc: =?UTF-8?Q?Marek_Beh=c3=ban?= , u-boot@lists.denx.de, =?UTF-8?Q?Marek_Beh=c3=ban?= References: <20211111153549.29111-1-kabel@kernel.org> <20211111153549.29111-2-kabel@kernel.org> <2a877aa7-115a-44c8-c940-a24578f7682c@denx.de> <20211112154423.exvxswqygxm2qbh3@pali> From: Stefan Roese In-Reply-To: <20211112154423.exvxswqygxm2qbh3@pali> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.35 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 11/12/21 16:44, Pali Rohár wrote: > On Friday 12 November 2021 14:59:31 Stefan Roese wrote: >> On 11/11/21 16:35, Marek Behún wrote: >>> From: Pali Rohár >>> >>> Function mvebu_pcie_probe() configures PCIe controller and sometimes it >>> takes some time for PCIe card to bring link up. Delay mvebu_pcie_probe() >>> for 100ms to ensure that PCIe link is up after function finish. In the >>> case when no card is connected to the PCIe slot, this will delay probe >>> time by 100ms, which should not be problematic. >> >> Where does this 100ms come from? From tests with your PCIe devices / >> card? > > pci-aardvark.c has similar wait timeout, but up to the 1s. > > In PCIe base spec 4.0 in section 6.6.1 Conventional Reset is written: > With a Downstream Port that does not support Link speeds greater than > 5.0 GT/s, software must wait a minimum of 100 ms before sending a > Configuration Request to the device immediately below that Port. > > So I think that waiting 100ms is reasonable... But I do not know what > should be correct here as proper initialization needs more steps... > For more details see my email sent to linux-pci: > https://lore.kernel.org/linux-pci/20211022183808.jdeo7vntnagqkg7g@pali/ > > I saw that more drivers in kernel are using different timeouts at > different levels and they are between 1ms-150ms. It is just mess. Agreed. So let's tick with this 100ms for now. Thanks, Stefan >> Please see another comment below... >> >>> This change fixes detection and initialization of some QCA98xx cards on >>> the first serdes when configured in x1 mode. Default configuration of >>> the first serdes on A385 is x4 mode, so it looks as if some delay is >>> required when x4 is changed to x1 and card correctly links with A385. >>> Other PCIe serdes ports on A385 are x1-only, and so they don't have this >>> problem. >>> >>> (We need this patch because in the following patch we are moving the >>> configuration change from x4 to x1 from serdes driver to PCIe mvebu >>> driver, because the corresponding register lives in the address space >>> of the PCIe controller. Without that this explicit timeout is not >>> needed, because there is an implicit timeout between change from x4 to >>> x1 and probe of PCIe mvebu driver, due to the first being run in SPL >>> and the second in U-Boot proper.) >>> >>> Signed-off-by: Pali Rohár >>> Signed-off-by: Marek Behún >>> --- >>> drivers/pci/pci_mvebu.c | 23 +++++++++++++++++++++++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c >>> index 14cd82db6f..a3364d5a59 100644 >>> --- a/drivers/pci/pci_mvebu.c >>> +++ b/drivers/pci/pci_mvebu.c >>> @@ -22,6 +22,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -70,6 +71,9 @@ DECLARE_GLOBAL_DATA_PTR; >>> #define PCIE_DEBUG_CTRL 0x1a60 >>> #define PCIE_DEBUG_SOFT_RESET BIT(20) >>> +#define LINK_WAIT_RETRIES 100 >>> +#define LINK_WAIT_TIMEOUT 1000 >> >> Wouldn't it be easier read, if this was defines like this: >> >> #define LINK_TIMEOUT_MS 100 >> #define LINK_WAIT_TIMEOUT_US 1000 >> #define LINK_WAIT_RETRIES ((LINK_TIMEOUT_MS * 1000) / LINK_WAIT_TIMEOUT_US) >> >> Other than this: >> >> Reviewed-by: Stefan Roese >> >> Thanks, >> Stefan >> >>> + >>> struct mvebu_pcie { >>> struct pci_controller hose; >>> void __iomem *base; >>> @@ -106,6 +110,23 @@ static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie) >>> return !(val & PCIE_STAT_LINK_DOWN); >>> } >>> +static void mvebu_pcie_wait_for_link(struct mvebu_pcie *pcie) >>> +{ >>> + int retries; >>> + >>> + /* check if the link is up or not */ >>> + for (retries = 0; retries < LINK_WAIT_RETRIES; retries++) { >>> + if (mvebu_pcie_link_up(pcie)) { >>> + printf("%s: Link up\n", pcie->name); >>> + return; >>> + } >>> + >>> + udelay(LINK_WAIT_TIMEOUT); >>> + } >>> + >>> + printf("%s: Link down\n", pcie->name); >>> +} >>> + >>> static void mvebu_pcie_set_local_bus_nr(struct mvebu_pcie *pcie, int busno) >>> { >>> u32 stat; >>> @@ -477,6 +498,8 @@ static int mvebu_pcie_probe(struct udevice *dev) >>> pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] = >>> PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16); >>> + mvebu_pcie_wait_for_link(pcie); >>> + >>> return 0; >>> } >>> >> >> Viele Grüße, >> Stefan Roese >> >> -- >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de Viele Grüße, Stefan Roese -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de