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=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT 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 7C51EC10F0B for ; Wed, 3 Apr 2019 11:31:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 490F82082C for ; Wed, 3 Apr 2019 11:31:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726591AbfDCLbd (ORCPT ); Wed, 3 Apr 2019 07:31:33 -0400 Received: from foss.arm.com ([217.140.101.70]:38424 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725935AbfDCLbc (ORCPT ); Wed, 3 Apr 2019 07:31:32 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 53E8B80D; Wed, 3 Apr 2019 04:31:32 -0700 (PDT) Received: from e107981-ln.cambridge.arm.com (e107981-ln.cambridge.arm.com [10.1.197.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 879B03F59C; Wed, 3 Apr 2019 04:31:30 -0700 (PDT) Date: Wed, 3 Apr 2019 12:31:24 +0100 From: Lorenzo Pieralisi To: Srinath Mannam Cc: Ray Jui , Bjorn Helgaas , Ray Jui , Scott Branden , BCM Kernel Feedback , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Linux Kernel Mailing List , Abhishek Shah Subject: Re: [PATCH v4 2/2] PCI: iproc: Add outbound configuration for 32-bit I/O region Message-ID: <20190403113124.GA16233@e107981-ln.cambridge.arm.com> References: <1551415936-30174-3-git-send-email-srinath.mannam@broadcom.com> <20190329173515.GA10367@e107981-ln.cambridge.arm.com> <20190401164416.GA8616@e107981-ln.cambridge.arm.com> <20190402102639.GB22708@e107981-ln.cambridge.arm.com> <20190402133851.GA26122@e107981-ln.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 03, 2019 at 08:41:44AM +0530, Srinath Mannam wrote: > Hi Lorenzo, > > Please see my reply below, > > On Tue, Apr 2, 2019 at 7:08 PM Lorenzo Pieralisi > wrote: > > > > On Tue, Apr 02, 2019 at 04:16:13PM +0530, Srinath Mannam wrote: > > > > [...] > > > > > > Ok - I start to understand. What does it mean in HW terms that your > > > > 32bit AXI address region size is 32MB ? Please explain to me in details. > > > > > > > In our PCIe controller HW, AXI address from 0x42000000 to 0x44000000 > > > of 32MB size and . > > > AXI address from 0x400000000 to 0x480000000 of 2GB size are provided > > > to map ob address. > > > First IO region is inside 32bit address and second IO region is > > > outside 32bit address. > > > This code change is to map first IO region(0x42000000 to 0x44000000). > > > > > > > IIUC you are using an OARR0 of 128MB in size to map a 32MB address > > > > region, that's what I understand this patch does (and the lowest index > > > > corresponds to the smallest possible size - it is far from clear by > > > > looking at the patch). > > > Yes, lowest index corresponds to smallest possible size (128MB). > > > In our controller we have multiple windows like OARR0, OARR1, OARR2, > > > OARR3 all supports multiple sizes from 128MB to 1024MB. > > > These details are given at the top of this driver file, as shown > > > below. all windows supports 128MB size still we must use OARR0 window > > > to configure first IO region(0x42000000 to 0x44000000). > > > > > > static const struct iproc_pcie_ob_map paxb_v2_ob_map[] = { > > > { > > > /* OARR0/OMAP0 */ > > > .window_sizes = { 128, 256 }, > > > .nr_sizes = 2, > > > }, > > > { > > > /* OARR1/OMAP1 */ > > > .window_sizes = { 128, 256 }, > > > .nr_sizes = 2, > > > }, > > > { > > > /* OARR2/OMAP2 */ > > > .window_sizes = { 128, 256, 512, 1024 }, > > > .nr_sizes = 4, > > > }, > > > { > > > /* OARR3/OMAP3 */ > > > .window_sizes = { 128, 256, 512, 1024 }, > > > .nr_sizes = 4, > > > }, > > > }; > > > > Ok so this patch allows mapping an AXI I/O window that is smaller > > than OARR possible sizes, why it was not done from the beginning > > I really do not know. > > > Same Iproc driver we use for multiple SOCs, in previous SOCs does not > have 32-bit AXI address region to map ob. > In the present SOC, 32-bit AXI address region is available so that > this change is added. > > > Now explain this to me please: > > > > > This patch add outbound window configuration to map below 32-bit I/O range > > > with corresponding PCI memory, which helps to access I/O region in ARM > > > 32-bit and one to one mapping of I/O region to PCI memory. > > > > > > Ex: > > > 1. ranges DT property given for current driver is, > > > ranges = <0x83000000 0x0 0x40000000 0x4 0x00000000 0 0x40000000>; > > > I/O region address is 0x400000000 > > > 2. ranges DT property can be given after this patch, > > > ranges = <0x83000000 0x0 0x42000000 0x0 0x42000000 0 0x2000000>; > > > I/O region address is 0x42000000 > > > > Why 1:1 AXI<->PCI address mapping is not possible in (1), how does the > > current code works on 32-bit systems and what's the benefit your change > > is bringing. > non-prefetchable memory range can only support 32-bit addresses, so > that we have taken 32-bit PCI bus address in (1). > current code does not work in 32-bit systems. In the present SOC with > this new change we can access from 32-bit CPU. Thank you. I rewrote the log and pushed patches to pci/iproc, please have a look (Ray/Scott please do have a look too) and report back if that's fine. Do you agree that the initial commit was lacking _significant_ information ? Please remember that the commit log plays a fundamental part in understanding a change and this one is a very important one so I am being pedantic on it. Thanks, Lorenzo