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 X-Spam-Level: X-Spam-Status: No, score=-19.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8E760C4338F for ; Mon, 9 Aug 2021 07:53:44 +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 BB7BB610E7 for ; Mon, 9 Aug 2021 07:53:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BB7BB610E7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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 3C02881671; Mon, 9 Aug 2021 09:53:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="UDEvs282"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CCE2381671; Mon, 9 Aug 2021 09:53:38 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1D6F980FC5 for ; Mon, 9 Aug 2021 09:53:35 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pali@kernel.org Received: by mail.kernel.org (Postfix) with ESMTPSA id 2D21E60720; Mon, 9 Aug 2021 07:53:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628495613; bh=f1bpMuxgfWPEiUa07DP0z27W9949N/SdzmDU4EtL6Y0=; h=From:To:Cc:Subject:Date:From; b=UDEvs282FzpPM48enQnjSgr+4HsyNIsuG91xK791OrJLk1Gnt7EHBhIj+rT2HHgR3 aOBeSuDQXgUe9HmlLyiQOR+dk/NLp+NsaHmfpeZc0cb7mcEcMwfgTyIJlCeVAVl4sB WwIXgXQSj/PuhMUEMoBGs4dbHaWfGmL8GfZZX0dV7jOxBwVr9KxpA0Cj2XvUHYSvwe mXbPfvV8jhRGeXTwy/h3MdNIPVD8hFWlZhwSR2mi/xTCcPTqrGeUpueGeDUaQ0pMKS i+OGcNmGfW0trIPqKNrZeeg5bCcvg3lBdBY3N5xA5aWXlD7mHglwdCl1YcPuNK/p2y nDrKRouf1zHNQ== Received: by pali.im (Postfix) id D8F79C7C; Mon, 9 Aug 2021 09:53:30 +0200 (CEST) From: =?UTF-8?q?Pali=20Roh=C3=A1r?= To: Stefan Roese Cc: =?UTF-8?q?Marek=20Beh=C3=BAn?= , Konstantin Porotchkin , u-boot@lists.denx.de Subject: [PATCH] arm: a37xx: pci: Fix handling PIO config error responses Date: Mon, 9 Aug 2021 09:53:13 +0200 Message-Id: <20210809075313.24868-1-pali@kernel.org> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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 Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to OS is allowed only for 4-byte PCI_VENDOR_ID config read request and only when CRSSVE bit in Root Port PCIe device is enabled. In all other error PCIe Root Complex must return all-ones. So implement this logic in pci-aardvark.c driver properly. aardvark HW does not have Root Port PCIe device and U-Boot does not implement emulation of this device. So expect that CRSSVE bit is set as U-Boot can already handle CRS value for PCI_VENDOR_ID config read request. More callers of pci_bus_read_config() function in U-Boot do not check for return value, but check readback value. Therefore always fill readback value in pcie_advk_read_config() function. On error fill all-ones of correct size as it is required for PCIe Root Complex. And also correctly propagates error from failed config write request to return value of pcie_advk_write_config() function. Most U-Boot callers ignores this return value, but it is a good idea to return correct value from function. These issues about return value of failed config read requests, including special handling of CRS were reported by Lorenzo and Bjorn for Linux kernel driver pci-aardvark together with quotes from PCIe r4.0 spec, see details: https://lore.kernel.org/linux-pci/20210624213345.3617-1-pali@kernel.org/t/#u Signed-off-by: Pali Rohár --- drivers/pci/pci-aardvark.c | 52 +++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index 1b9bae7cca72..815b26162f15 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -177,7 +177,6 @@ #define LINK_MAX_RETRIES 10 #define LINK_WAIT_TIMEOUT 100000 -#define CFG_RD_UR_VAL 0xFFFFFFFF #define CFG_RD_CRS_VAL 0xFFFF0001 /** @@ -263,12 +262,12 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie) * pcie_advk_check_pio_status() - Validate PIO status and get the read result * * @pcie: Pointer to the PCI bus - * @read: Read from or write to configuration space - true(read) false(write) - * @read_val: Pointer to the read result, only valid when read is true + * @allow_crs: Only for read requests, if CRS response is allowed + * @read_val: Pointer to the read result * */ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, - bool read, + bool allow_crs, uint *read_val) { uint reg; @@ -286,22 +285,16 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, break; } /* Get the read result */ - if (read) + if (read_val) *read_val = advk_readl(pcie, PIO_RD_DATA); /* No error */ strcomp_status = NULL; break; case PIO_COMPLETION_STATUS_UR: - if (read) { - /* For reading, UR is not an error status. */ - *read_val = CFG_RD_UR_VAL; - strcomp_status = NULL; - } else { - strcomp_status = "UR"; - } + strcomp_status = "UR"; break; case PIO_COMPLETION_STATUS_CRS: - if (read) { + if (allow_crs && read_val) { /* For reading, CRS is not an error status. */ *read_val = CFG_RD_CRS_VAL; strcomp_status = NULL; @@ -352,6 +345,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct pcie_advk *pcie = dev_get_priv(bus); + bool allow_crs; uint reg; int ret; @@ -364,13 +358,17 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, return 0; } + allow_crs = (offset == PCI_VENDOR_ID) && (size == 4); + if (advk_readl(pcie, PIO_START)) { dev_err(pcie->dev, "Previous PIO read/write transfer is still running\n"); - if (offset != PCI_VENDOR_ID) - return -EINVAL; - *valuep = CFG_RD_CRS_VAL; - return 0; + if (allow_crs) { + *valuep = CFG_RD_CRS_VAL; + return 0; + } + *valuep = pci_get_ff(size); + return -EINVAL; } /* Program the control register */ @@ -392,16 +390,20 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, advk_writel(pcie, 1, PIO_START); if (!pcie_advk_wait_pio(pcie)) { - if (offset != PCI_VENDOR_ID) - return -EINVAL; - *valuep = CFG_RD_CRS_VAL; - return 0; + if (allow_crs) { + *valuep = CFG_RD_CRS_VAL; + return 0; + } + *valuep = pci_get_ff(size); + return -EINVAL; } /* Check PIO status and get the read result */ - ret = pcie_advk_check_pio_status(pcie, true, ®); - if (ret) + ret = pcie_advk_check_pio_status(pcie, allow_crs, ®); + if (ret) { + *valuep = pci_get_ff(size); return ret; + } dev_dbg(pcie->dev, "(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, reg); @@ -511,9 +513,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, } /* Check PIO status */ - pcie_advk_check_pio_status(pcie, false, ®); - - return 0; + return pcie_advk_check_pio_status(pcie, false, NULL); } /** -- 2.20.1