From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751973AbdBBXAY (ORCPT ); Thu, 2 Feb 2017 18:00:24 -0500 Received: from mail-oi0-f67.google.com ([209.85.218.67]:33476 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751500AbdBBXAV (ORCPT ); Thu, 2 Feb 2017 18:00:21 -0500 MIME-Version: 1.0 In-Reply-To: <068bbe37-42f4-e245-f900-c639c68404b9@huawei.com> References: <1485241525-201782-1-git-send-email-yuanzhichang@hisilicon.com> <1485241525-201782-3-git-send-email-yuanzhichang@hisilicon.com> <068bbe37-42f4-e245-f900-c639c68404b9@huawei.com> From: "Rafael J. Wysocki" Date: Fri, 3 Feb 2017 00:00:20 +0100 X-Google-Sender-Auth: tlDHVc9QLQBssCj24OXDjm5wMTg Message-ID: Subject: Re: [PATCH V6 2/5] PCI: Adapt pci_register_io_range() for indirect-IO and PCI I/O translation To: John Garry Cc: "zhichang.yuan" , Catalin Marinas , Will Deacon , Rob Herring , Frank Rowand , Bjorn Helgaas , "Rafael J. Wysocki" , Mark Rutland , brian.starkey@arm.com, Olof Johansson , Arnd Bergmann , "linux-arm-kernel@lists.infradead.org" , Lorenzo Pieralisi , Benjamin Herrenschmidt , Linux Kernel Mailing List , linuxarm@huawei.com, "devicetree@vger.kernel.org" , Linux PCI , "linux-serial@vger.kernel.org" , Corey Minyard , liviu.dudau@arm.com, zourongrong@gmail.com, Gabriele Paoloni , zhichang.yuan02@gmail.com, kantyzc@163.com, xuwei5@hisilicon.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 2, 2017 at 6:36 PM, John Garry wrote: > Hi Rafael, > > Could you kindly check the minor changes to drivers/acpi/pci_root.c? It > would also be appreciated if you could review the ACPI-relevant parts in > lib/extio.c, in [PATCH V5 5/5] LPC: Add the ACPI LPC support If you want patches to be reviewed from the ACPI perspective, please CC them to linux-acpi. I'm not the only person who may be interested in them. The change is pci_root.c looks OK to me, but it's Bjorn's call anyway. Thanks, Rafael > On 24/01/2017 07:05, 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. >> 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. >> >> Signed-off-by: zhichang.yuan >> Signed-off-by: Gabriele Paoloni >> Signed-off-by: Arnd Bergmann >> --- >> drivers/acpi/pci_root.c | 12 +++++------- >> drivers/of/address.c | 8 ++------ >> drivers/pci/pci.c | 44 ++++++++++++++++++++++++++++++++++++++++---- >> include/linux/pci.h | 7 +++++-- >> 4 files changed, 52 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index bf601d4..6cadf05 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -730,7 +730,8 @@ static void acpi_pci_root_validate_resources(struct >> device *dev, >> } >> } >> >> -static void acpi_pci_root_remap_iospace(struct resource_entry *entry) >> +static void acpi_pci_root_remap_iospace(struct fwnode_handle *node, >> + struct resource_entry *entry) >> { >> #ifdef PCI_IOBASE >> struct resource *res = entry->res; >> @@ -739,11 +740,7 @@ static void acpi_pci_root_remap_iospace(struct >> resource_entry *entry) >> resource_size_t length = resource_size(res); >> unsigned long port; >> >> - if (pci_register_io_range(cpu_addr, length)) >> - goto err; >> - >> - port = pci_address_to_pio(cpu_addr); >> - if (port == (unsigned long)-1) >> + if (pci_register_io_range(node, cpu_addr, length, &port)) >> goto err; >> >> res->start = port; >> @@ -781,7 +778,8 @@ int acpi_pci_probe_root_resources(struct >> acpi_pci_root_info *info) >> else { >> resource_list_for_each_entry_safe(entry, tmp, list) { >> if (entry->res->flags & IORESOURCE_IO) >> - acpi_pci_root_remap_iospace(entry); >> + >> acpi_pci_root_remap_iospace(&device->fwnode, >> + entry); >> >> if (entry->res->flags & IORESOURCE_DISABLED) >> resource_list_destroy_entry(entry); >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index 02b2903..d85d228 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -2,6 +2,7 @@ >> #define pr_fmt(fmt) "OF: " fmt >> >> #include >> +#include >> #include >> #include >> #include >> @@ -323,14 +324,9 @@ int of_pci_range_to_resource(struct of_pci_range >> *range, >> >> if (res->flags & IORESOURCE_IO) { >> unsigned long port; >> - err = pci_register_io_range(range->cpu_addr, range->size); >> + err = pci_register_io_range(&np->fwnode, range->cpu_addr, >> range->size, &port); >> if (err) >> goto invalid_range; >> - port = pci_address_to_pio(range->cpu_addr); >> - if (port == (unsigned long)-1) { >> - err = -EINVAL; >> - goto invalid_range; >> - } >> res->start = port; >> } else { >> if ((sizeof(resource_size_t) < 8) && >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index a881c0d..5289221 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -3241,6 +3241,7 @@ int pci_request_regions_exclusive(struct pci_dev >> *pdev, const char *res_name) >> #ifdef PCI_IOBASE >> struct io_range { >> struct list_head list; >> + struct fwnode_handle *node; >> phys_addr_t start; >> resource_size_t size; >> }; >> @@ -3253,7 +3254,8 @@ struct io_range { >> * Record the PCI IO range (expressed as CPU physical address + size). >> * Return a negative value if an error has occured, zero otherwise >> */ >> -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) >> { >> int err = 0; >> >> @@ -3261,10 +3263,31 @@ int __weak pci_register_io_range(phys_addr_t addr, >> resource_size_t size) >> struct io_range *range; >> resource_size_t allocated_size = 0; >> >> + /* >> + * As indirect-IO which support multiple bus instances is >> introduced, >> + * the input 'addr' is probably not page-aligned. If the PCI I/O >> + * ranges are registered after indirect-IO, there is risk that the >> + * start logical PIO assigned to PCI I/O is not page-aligned. >> + * This will cause some I/O subranges are not remapped or >> overlapped >> + * in pci_remap_iospace() handling. >> + */ >> + WARN_ON(addr != IO_RANGE_IOEXT && !(addr & PAGE_MASK)); >> + /* >> + * MMIO will call ioremap, it is better to align size with >> PAGE_SIZE, >> + * then the return linux virtual PIO is page-aligned. >> + */ >> + if (size & PAGE_MASK) >> + size = PAGE_ALIGN(size); >> + >> /* check if the range hasn't been previously recorded */ >> spin_lock(&io_range_lock); >> list_for_each_entry(range, &io_range_list, list) { >> - if (addr >= range->start && addr + size <= range->start + >> size) { >> + if (node == range->node) >> + goto end_register; >> + >> + if (addr != IO_RANGE_IOEXT && >> + addr >= range->start && >> + addr + size <= range->start + size) { >> /* range already registered, bail out */ >> goto end_register; >> } >> @@ -3290,6 +3313,7 @@ int __weak pci_register_io_range(phys_addr_t addr, >> resource_size_t size) >> goto end_register; >> } >> >> + range->node = node; >> range->start = addr; >> range->size = size; >> >> @@ -3297,6 +3321,14 @@ int __weak pci_register_io_range(phys_addr_t addr, >> resource_size_t size) >> >> end_register: >> spin_unlock(&io_range_lock); >> + >> + *port = allocated_size; >> +#else >> + /* >> + * powerpc and microblaze have their own registration, >> + * just look up the value here >> + */ >> + *port = pci_address_to_pio(addr); >> #endif >> >> return err; >> @@ -3315,7 +3347,9 @@ phys_addr_t pci_pio_to_address(unsigned long pio) >> >> spin_lock(&io_range_lock); >> list_for_each_entry(range, &io_range_list, list) { >> - if (pio >= allocated_size && pio < allocated_size + >> range->size) { >> + if (range->start != IO_RANGE_IOEXT && >> + pio >= allocated_size && >> + pio < allocated_size + range->size) { >> address = range->start + pio - allocated_size; >> break; >> } >> @@ -3336,7 +3370,9 @@ unsigned long __weak pci_address_to_pio(phys_addr_t >> address) >> >> spin_lock(&io_range_lock); >> list_for_each_entry(res, &io_range_list, list) { >> - if (address >= res->start && address < res->start + >> res->size) { >> + if (res->start != IO_RANGE_IOEXT && >> + address >= res->start && >> + address < res->start + res->size) { >> addr = address - res->start + offset; >> break; >> } >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index e2d1a12..8d91af8 100644 >> --- 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 */ >> +#define IO_RANGE_IOEXT (resource_size_t)(-1ull) >> /* >> * 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); >> > >