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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0320DEE49A0 for ; Mon, 21 Aug 2023 20:14:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=K/JythBV0IiieD6RSq0n81toIEAxwv/ss3g2T7pTLlE=; b=vpnnUgRbCvddL9 d8u52DGMBsXu28wwNeoMBvtK49Kg7lvhERaJNRvHM03tyuSYzoAGi0vX8ADExLRSU88FtzJxKIsfL Mh6hVrMKgh9InODBH6fSmUV4Fqb2xzP8RnZN07fMngLTqut5cXLBLxII6jMFIojO7pEATgRltxxRz 1HNsHeKWVsL4ovB01X7fK7yYTerkQ6SoAihLybVQzXrA7+Xczc+FgE4bOoOzJschvsXJd4bMHyk+Z WDMk1a+baIWsGecP3l8jTAcS6mV72lg1tPh+HTui0tcBkXu5HF9n0WY1e4xrjYW4sSCREznpWG1CU 1G4MxNowK3i3cAPj7c7Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qYBHr-00Edu6-1D; Mon, 21 Aug 2023 20:14:07 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qYBHo-00EdtU-1d for linux-arm-kernel@lists.infradead.org; Mon, 21 Aug 2023 20:14:05 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 29BC762767; Mon, 21 Aug 2023 20:14:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 473B8C433C9; Mon, 21 Aug 2023 20:14:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1692648842; bh=JhMi2+GzkL/Eutsnn0WRkJmqRV/RT+NL9pChlj1qKxg=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=pFML8CHrMw2oGOkdn9OVZjl55kwjR+o5S9LOycl7oWvHoW2IVsKCDNnYbEZg8JxG8 pqlwWlBITKbPT/TgHfiIPCLERXTm4AedvP04nB51oir6EjYC7uBPEDGzg/EwufW5IJ +IqN9vm8M9QXfta87NvEXP0yLRPeufnGDF+DaWQVNfhgrC3UblLDFKfxEStv2mfboi apQUDBSc1HL82SziZzJe4Wx6qook+z5rtIeUt4j6bjBXDjovI2bP/toa+SMwQcO9pk D0wrUUaufDSBnabOIXvxU57pLmV2SrkIbHMbxPNz9iHlb5KpFUj/7XYhkNYYQGL9FD Th2PuvUlLhkhw== Date: Mon, 21 Aug 2023 15:14:00 -0500 From: Bjorn Helgaas To: Thippeswamy Havalige Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, bhelgaas@google.com, lpieralisi@kernel.org, robh@kernel.org, kw@linux.com, michal.simek@amd.com, krzysztof.kozlowski+dt@linaro.org, bharat.kumar.gogada@amd.com Subject: Re: [PATCH v6 3/3] PCI: xilinx-xdma: Add Xilinx XDMA Root Port driver Message-ID: <20230821201400.GA367570@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230818093507.24435-4-thippeswamy.havalige@amd.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230821_131404_633102_949A767F X-CRM114-Status: GOOD ( 26.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Aug 18, 2023 at 03:05:07PM +0530, Thippeswamy Havalige wrote: > Add support for Xilinx XDMA Soft IP core as Root Port. > > The Zynq UltraScale+ MPSoCs devices support XDMA soft IP module in > programmable logic. > > The integrated XDMA soft IP block has integrated bridge function that > can act as PCIe Root Port. > > Signed-off-by: Thippeswamy Havalige > Signed-off-by: Bharat Kumar Gogada > --- > changes in v6: > - Replaced chained irq's with regular interrupts. Thanks a million for working this out! Trivial comments below, wait a couple days before reposting in case there are other comments. > +static inline bool xilinx_pl_dma_pcie_link_up(struct pl_dma_pcie *port) > +{ > + return (pcie_read(port, XILINX_PCIE_DMA_REG_PSCR) & > + XILINX_PCIE_DMA_REG_PSCR_LNKUP) ? 1 : 0; This function returns bool, so I think true/false would be more appropriate than 1/0. > +static bool xilinx_pl_dma_pcie_valid_device(struct pci_bus *bus, > + unsigned int devfn) > +{ > + struct pl_dma_pcie *port = bus->sysdata; > + > + /* Check if link is up when trying to access downstream ports */ > + if (!pci_is_root_bus(bus)) { > + /* > + * Checking whether link is up here is a last line of defence, > + * if the link goes down after we check for link-up, we have a > + * problem: if a PIO request is initiated while link-down, the > + * whole controller hangs, and even after link comes up again, > + * previous PIO requests won't work, and a reset of the whole > + * PCIe controller is needed. Henceforth we need link-up check > + * here to avoid sending PIO request when link is down. This > + * check is racy by definition and does not make controller hang > + * if the link goes down after this check is performed. This comment doesn't make sense to me. "If PIO request initiated while link-down, controller hangs ... This check is racy and does not make controller hang if link goes down." Which is it? My *guess* is that this check narrows the window but doesn't close it, so if xilinx_pl_dma_pcie_link_up() finds the link up, but the link goes down before pci_generic_config_read() initiates the PIO request, the controller hangs, and a reset is required. > + */ > + if (!xilinx_pl_dma_pcie_link_up(port)) > + return false; > + } else if (devfn > 0) > + /* Only one device down on each root port */ > + return false; > + > + return true; > +} > +/* INTx error interrupts are Xilinx controller specific interrupt, used to > + * notify user about error's such as cfg timeout, slave unsupported requests, s/error's/errors/ > + * fatal and non fatal error etc. > + err = devm_request_irq(dev, irq, xilinx_pl_dma_pcie_intr_handler, > + IRQF_SHARED | IRQF_NO_THREAD, intr_cause[i].sym, port); Rewrap to fit in 80 columns. > + /* Needed for MSI DECODE MODE */ > + pcie_write(port, XILINX_PCIE_DMA_IDR_ALL_MASK, XILINX_PCIE_DMA_REG_MSI_LOW_MASK); > + pcie_write(port, XILINX_PCIE_DMA_IDR_ALL_MASK, XILINX_PCIE_DMA_REG_MSI_HI_MASK); Rewrap. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel