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=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 06DEDC433E0 for ; Mon, 3 Aug 2020 12:42:34 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CD2422054F for ; Mon, 3 Aug 2020 12:42:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="RjHeVCCF"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=broadcom.com header.i=@broadcom.com header.b="ZRjMJ9i+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CD2422054F Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=broadcom.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=8bUKbZxvHd8uI0pLvqTKbam9Q8v51j+IfJ0P54JCxso=; b=RjHeVCCFrIys5Jq3KAzOugTyx BjQzt0OOumV/C1LS0z2biNRtQKKpYinoZrbK6RgFFF+PHb+qYzIa4wJiaPo+v0Q9lWsJyn4ur/Nwf XRNGm1LskjgRJIV3G8yrOgAbbiGSoWVlVuZBNkrGsHZ1V7kOHfGCh0YWdRPrO4nXXry2ZsnEuTmS2 fdomhPi2YMCT9ZAlZhbAGsB9whATRrzIvi37wFPH2Q6VLuPj9IAtVriCnDD1bGc3kx25mR3L2jp7r vBLI/H93O96ux5d0GFt8oO2PAJjeVxDsjJVqw0EQhmAknl5qxF6XLIwQFGM2tZfO7IOANHUJDFd62 sYvy6szUg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2Zll-0005m6-7h; Mon, 03 Aug 2020 12:40:45 +0000 Received: from mail-wm1-x341.google.com ([2a00:1450:4864:20::341]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2Zlh-0005ks-4l for linux-arm-kernel@lists.infradead.org; Mon, 03 Aug 2020 12:40:43 +0000 Received: by mail-wm1-x341.google.com with SMTP id q76so14068226wme.4 for ; Mon, 03 Aug 2020 05:40:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mkm6CObS36SJAMkOLwAunKXfddEBXbzFKEGDTTUK8x8=; b=ZRjMJ9i+3XtHFlvWGHZ8k1OJSiJIwYiuaYJjTSDvBsQo7p5Lw2lHEoeMLsDrOtbo5t Y/L6ct5WPSxA6i5eDn+OJ1XtcPhING+tXhwhkyjJXP10dSlFTTYrNBfbMF0GZXXGSSzG KHPqqWfMuDAAl6hwmgPB5+nYUkFTDd+lylEYs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mkm6CObS36SJAMkOLwAunKXfddEBXbzFKEGDTTUK8x8=; b=DGVnxk/TXu41s9khwuZfvTWfXN43gMo4OFP4WyBzYVFBXNERj1k/lEtYR8vFFZY000 XKFHIY9eOx4yHxul2JTFYy5Tae/jhSI7ECyZRwAcDybI5uk1VswzhghkcAyOST/8o6nR oapmgYX9g7MlJKpVkDiduVCISmp4bHCvJilv8TS2jcXvKs2GjVrzZmklGWrtjH3Q8Wsf MSkWl5Kcepyitb61IJoUaiozmZt+qJ+8Fvy6o2IF9F6boXbir/w+Qi+vbiw64CpV+nt8 JzMqZjy0nDCnib4w8yN7rQxN7GlYMW+Gs1xLxIBTABaJkTJAmqOz7ZJhKXaHZaH4Dj65 up3Q== X-Gm-Message-State: AOAM533FVqpUaCwHSU7CXpk9wzHvmFsQz29OUipRCvrjDpUQt5p4PRRT ahMFepKIDSczk/MLjvK127l7r4bM2LcqhEMbmBVOLw== X-Google-Smtp-Source: ABdhPJyhgboxKbPjxQ++0E3wx9USPAXOub0gcy5KJOARS+aCf/xp/ODALT+aGA7/GIEB4C7d3o1nsP0GO/rZzwXoO+I= X-Received: by 2002:a7b:c1c3:: with SMTP id a3mr16280931wmj.111.1596458438572; Mon, 03 Aug 2020 05:40:38 -0700 (PDT) MIME-Version: 1.0 References: <20200724203407.16972-1-james.quinlan@broadcom.com> <20200724203407.16972-10-james.quinlan@broadcom.com> In-Reply-To: From: Jim Quinlan Date: Mon, 3 Aug 2020 08:40:26 -0400 Message-ID: Subject: Re: [PATCH v9 09/12] PCI: brcmstb: Set additional internal memory DMA viewport sizes To: Nicolas Saenz Julienne X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200803_084041_501730_B05F7B0B X-CRM114-Status: GOOD ( 42.62 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Rob Herring , Lorenzo Pieralisi , "open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" , open list , Florian Fainelli , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , Bjorn Helgaas , Robin Murphy , Christoph Hellwig , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 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 fls64 On Sat, Aug 1, 2020 at 1:39 PM Nicolas Saenz Julienne wrote: > > Hi Jim, > > On Fri, 2020-07-24 at 16:33 -0400, Jim Quinlan wrote: > > The Raspberry Pi (RPI) is currently the only chip using this driver > > (pcie-brcmstb.c). There, only one memory controller is used, without an > > extension region, and the SCB0 viewport size is set to the size of the > > first and only dma-range region. Other BrcmSTB SOCs have more complicated > > memory configurations that require setting additional viewport sizes. > > > > BrcmSTB PCIe controllers are intimately connected to the memory > > controller(s) on the SOC. The SOC may have one to three memory > > controllers; they are indicated by the term SCBi. Each controller has a > > base region and an optional extension region. In physical memory, the base > > and extension regions of a controller are not adjacent, but in PCIe-space > > they are. > > > > There is a "viewport" for each memory controller that allows DMA from > > endpoint devices. Each viewport's size must be set to a power of two, and > > that size must be equal to or larger than the amount of memory each > > controller supports which is the sum of base region and its optional > > extension. Further, the 1-3 viewports are also adjacent in PCIe-space. > > > > Unfortunately the viewport sizes cannot be ascertained from the > > "dma-ranges" property so they have their own property, "brcm,scb-sizes". > > This is because dma-range information does not indicate what memory > > controller it is associated. For example, consider the following case > > where the size of one dma-range is 2GB and the second dma-range is 1GB: > > > > /* Case 1: SCB0 size set to 4GB */ > > dma-range0: 2GB (from memc0-base) > > dma-range1: 1GB (from memc0-extension) > > > > /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */ > > dma-range0: 2GB (from memc0-base) > > dma-range1: 1GB (from memc0-extension) > > > > By just looking at the dma-ranges information, one cannot tell which > > situation applies. That is why an additional property is needed. Its > > length indicates the number of memory controllers being used and each value > > indicates the viewport size. > > > > Note that the RPI DT does not have a "brcm,scb-sizes" property value, > > as it is assumed that it only requires one memory controller and no > > extension. So the optional use of "brcm,scb-sizes" will be backwards > > compatible. > > > > One last layer of complexity exists: all of the viewports sizes must be > > added and rounded up to a power of two to determine what the "BAR" size is. > > Further, an offset must be given that indicates the base PCIe address of > > this "BAR". The use of the term BAR is typically associated with endpoint > > devices, and the term is used here because the PCIe HW may be used as an RC > > or an EP. In the former case, all of the system memory appears in a single > > "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely > > used in the EP role and its system of mapping memory is an artifact that > > requires multiple dma-ranges regions. > > > > Signed-off-by: Jim Quinlan > > Acked-by: Florian Fainelli > > --- > > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++------- > > 1 file changed, 50 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > index 8dacb9d3b7b6..3ef2d37cc43b 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie, > > u64 *rc_bar2_offset) > > { > > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); > > - struct device *dev = pcie->dev; > > struct resource_entry *entry; > > + struct device *dev = pcie->dev; > > + u64 lowest_pcie_addr = ~(u64)0; > > + int ret, i = 0; > > + u64 size = 0; > > > > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM); > > - if (!entry) > > - return -ENODEV; > > + resource_list_for_each_entry(entry, &bridge->dma_ranges) { > > + u64 pcie_beg = entry->res->start - entry->offset; > > > > + size += entry->res->end - entry->res->start + 1; > > + if (pcie_beg < lowest_pcie_addr) > > + lowest_pcie_addr = pcie_beg; > > + } > > > > - /* > > - * The controller expects the inbound window offset to be calculated as > > - * the difference between PCIe's address space and CPU's. The offset > > - * provided by the firmware is calculated the opposite way, so we > > - * negate it. > > - */ > > - *rc_bar2_offset = -entry->offset; > > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start); > > + if (lowest_pcie_addr == ~(u64)0) { > > + dev_err(dev, "DT node has no dma-ranges\n"); > > + return -EINVAL; > > + } > > + > > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1, > > + PCIE_BRCM_MAX_MEMC); > > + > > + if (ret <= 0) { > > + /* Make an educated guess */ > > + pcie->num_memc = 1; > > + pcie->memc_size[0] = 1 << fls64(size - 1); > > You need to 1ULL here. Got it. Thanks, Jim > > Regards, > Nicolas > > > + } else { > > + pcie->num_memc = ret; > > + } > > + > > + /* Each memc is viewed through a "port" that is a power of 2 */ > > + for (i = 0, size = 0; i < pcie->num_memc; i++) > > + size += pcie->memc_size[i]; > > + > > + /* System memory starts at this address in PCIe-space */ > > + *rc_bar2_offset = lowest_pcie_addr; > > + /* The sum of all memc views must also be a power of 2 */ > > + *rc_bar2_size = 1ULL << fls64(size - 1); > > > > /* > > * We validate the inbound memory view even though we should trust > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel