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 CD843C7EE23 for ; Wed, 31 May 2023 17:12:59 +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=CrBp7a5vD8iXOotbRqFLhZ3BmmxIUKlv/zymlhf/Doo=; b=cUo+VgAur5Pgr9 MzrDdeulGwNtMiFZame9ZRx8QPPGL5Fl/zHApzyRpUWZz4WG3Yi6Lsq0i7EzECa7S84XGGKtvda7r FgTmEW/3cgXaxps1dvHfIT+ZIycHPFws1E0vNkfy4ENi9OUbifYKRymthbd+vHPeo+2YLO1/4XRJM L9tBRXBeI2SFSbZCjBbqlnpL2yjPMeNZy7lEFChKexNgAtUWQCdv3SphKmESk+YktvYT64xDK5ur/ OBQiKRqsmCFgisI//wX6qla7DRyjEeAsNOElEUw9YhOIJKI13XqnvvHo7Q1b/zbnuq6CoJiFLRU0d iFnEhxVutTLpREAR7YnQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q4PMx-000Zu6-11; Wed, 31 May 2023 17:12:19 +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 1q4PMt-000Zt0-12 for linux-arm-kernel@lists.infradead.org; Wed, 31 May 2023 17:12:17 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D86B7637B0; Wed, 31 May 2023 17:12:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1AD87C4339C; Wed, 31 May 2023 17:12:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685553134; bh=+Ml1j+mRUJgmXvowGAHL7NQaz1pxH/evVJuvs2hRgso=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=BYw8uvSKtQqAvjDQ1+aUtduoiKuESJASt6I+ZqQvJA20//6FeGNg1f866MI0WmXrT ruslzv5W28Kb6NwsYa/Ad9LZOTko8KfxpqzYAE5WtJWktKj6c06tq4gtDVwv/0Pwaa mDJqQwfoyV5LITBF0ptaPtkQU62PGfvfZrYX/EoeTekMyu8tbdC4xAoE58FqEESk2p eFIBgNmgyA68cfWPAbwwdKubtp/VUXFKB8qg6me5gxLsMyb5ASRluj4WLsfXQxe4Un /blsTaEObx/P02q5JlLcYRt8k4n4G5SotCTPWArugBDQtmsIg6K2A530cZDiM7CjtV unjSWwsaoEZtA== Date: Wed, 31 May 2023 12:12:12 -0500 From: Bjorn Helgaas To: Thippeswamy Havalige Cc: krzysztof.kozlowski@linaro.org, bhelgaas@google.com, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, lorenzo.pieralisi@arm.com, linux-arm-kernel@lists.infradead.org, bharat.kumar.gogada@amd.com, michals@amd.com, nagaradhesh.yeleswarapu@amd.com Subject: Re: [PATCH v4 3/3] PCI: xilinx-xdma: Add Xilinx XDMA Root Port driver Message-ID: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230531083825.985584-4-thippeswamy.havalige@amd.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230531_101215_439315_01988015 X-CRM114-Status: GOOD ( 20.58 ) 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 Wed, May 31, 2023 at 02:08:25PM +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 > |Reported-by: kernel test robot > |Reported-by: Dan Carpenter > |Closes: https://lore.kernel.org/r/202305261250.2cs1phTS-lkp@intel.com/ Not relevant. These are basically review comments. > + Add support for the Xilinx PL DMA PCIe host bridge, > + The controller is an soft IP which can act as Root Port. > + If you know your system provides xilinx PCIe host controller > + bridge DMA as soft IP say Y; if you are not sure, say N. s/is an soft/is soft/ s/xilinx/Xilinx/ > +#define XILINX_PCIE_DMA_REG_IMR 0x0000013c > +#define XILINX_PCIE_DMA_REG_MSIBASE1 0x0000014c > +#define XILINX_PCIE_DMA_REG_MSI_HI_MASK 0x0000017c > ... > +#define XILINX_PCIE_DMA_IMR_ALL_MASK 0x0FF30FE9 > +#define XILINX_PCIE_DMA_IDR_ALL_MASK 0xFFFFFFFF Pick upper-case hex or lower-case hex and use it consistently. > +static inline bool xilinx_pl_dma_pcie_linkup(struct pl_dma_pcie *port) Name this *_pcie_link_up() (not *_pcie_linkup()) to match other drivers. > +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)) { > + if (!xilinx_pl_dma_pcie_linkup(port)) > + return false; > + } else if (devfn > 0) > + /* Only one device down on each root port */ > + return false; > + > + return true; > +} > + > +static void __iomem *xilinx_pl_dma_pcie_map_bus(struct pci_bus *bus, > + unsigned int devfn, int where) > +{ > + struct pl_dma_pcie *port = bus->sysdata; > + > + if (!xilinx_pl_dma_pcie_valid_device(bus, devfn)) > + return NULL; Checking whether the link is up is racy because the link may be up, so xilinx_pl_dma_pcie_valid_device() returns true, then the link may go down before the read below. What happens then? If it's an error that you can recover from, it would better to skip the link up check and just handle the error. > + return port->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where); > +} > + /*set the Bridge enable bit */ /* Set ... */ (add space before "Set" and capitalize it) Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel