From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754637AbdAaAME (ORCPT ); Mon, 30 Jan 2017 19:12:04 -0500 Received: from mail.kernel.org ([198.145.29.136]:37038 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754472AbdAaAMC (ORCPT ); Mon, 30 Jan 2017 19:12:02 -0500 Date: Mon, 30 Jan 2017 18:10:30 -0600 From: Bjorn Helgaas To: "zhichang.yuan" Cc: catalin.marinas@arm.com, will.deacon@arm.com, robh+dt@kernel.org, frowand.list@gmail.com, bhelgaas@google.com, rafael@kernel.org, mark.rutland@arm.com, brian.starkey@arm.com, olof@lixom.net, arnd@arndb.de, linux-arm-kernel@lists.infradead.org, lorenzo.pieralisi@arm.com, benh@kernel.crashing.org, linux-kernel@vger.kernel.org, linuxarm@huawei.com, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, linux-serial@vger.kernel.org, minyard@acm.org, liviu.dudau@arm.com, zourongrong@gmail.com, john.garry@huawei.com, gabriele.paoloni@huawei.com, zhichang.yuan02@gmail.com, kantyzc@163.com, xuwei5@hisilicon.com Subject: Re: [PATCH V6 2/5] PCI: Adapt pci_register_io_range() for indirect-IO and PCI I/O translation Message-ID: <20170131001030.GB20550@bhelgaas-glaptop.roam.corp.google.com> References: <1485241525-201782-1-git-send-email-yuanzhichang@hisilicon.com> <1485241525-201782-3-git-send-email-yuanzhichang@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1485241525-201782-3-git-send-email-yuanzhichang@hisilicon.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 24, 2017 at 03:05:22PM +0800, zhichang.yuan wrote: > After indirect-IO is introduced, system must can assigned indirect-IO devices > with logical I/O ranges which are different from those for PCI I/O devices. > Otherwise, I/O accessors can't identify whether the I/O port is for memory > mapped I/O or indirect-IO. Maybe: We must assign logical I/O port space for indirect I/O such that the I/O accessors can tell whether a logical I/O port refers to memory- mapped I/O space or indirect I/O space. > As current helper, pci_register_io_range(), is used for PCI I/O ranges > registration and translation, indirect-IO devices should also apply these > helpers to manage the I/O ranges. It will be easy to ensure the assigned > logical I/O ranges unique. > But for indirect-IO devices, there is no cpu address. The current > pci_register_io_range() can not work for this case. > > This patch makes some changes on the pci_register_io_range() to support the > I/O range registration with device's fwnode also. After this, the indirect-IO > devices can register the device-local I/O range to system logical I/O and > easily perform the translation between device-local I/O range and sytem > logical I/O range. > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size) > +int __weak pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr, > + resource_size_t size, unsigned long *port) Why is this __weak? It looks like it's been __weak since its introduction by 41f8bba7f555 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()"), but I don't see any other implementations of it. Can you add a patch that does nothing but make this non-weak? > +#else > + /* > + * powerpc and microblaze have their own registration, > + * just look up the value here Can you include a pointer to the powerpc and microblaze registration code here? It's conceivable that somebody could generalize this enough to support powerpc and microblaze as well. > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -34,6 +34,9 @@ > > #include > > +/* the macro below flags an invalid cpu address > + * and is used by IO special hosts */ s/cpu/CPU/ Use conventional multi-line comment style: /* * IO_RANGE_IOEXT flags an invalid CPU address ... */ > +#define IO_RANGE_IOEXT (resource_size_t)(-1ull) And put this close to related things, e.g., pci_register_io_range(), instead of just dropping it in at the top of the file. > /* > * The PCI interface treats multi-function devices as independent > * devices. The slot/function address of each device is encoded > @@ -1197,8 +1200,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus, > resource_size_t), > void *alignf_data); > > - > -int pci_register_io_range(phys_addr_t addr, resource_size_t size); > +int pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr, > + resource_size_t size, unsigned long *port); > unsigned long pci_address_to_pio(phys_addr_t addr); > phys_addr_t pci_pio_to_address(unsigned long pio); > int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr); > -- > 1.9.1 >