Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753772AbdCONXX (ORCPT ); Wed, 15 Mar 2017 09:23:23 -0400 Received: from mail-ot0-f196.google.com ([74.125.82.196]:34338 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751458AbdCONXT (ORCPT ); Wed, 15 Mar 2017 09:23:19 -0400 MIME-Version: 1.0 In-Reply-To: <58C8BD7D.7040207@hisilicon.com> References: <1489372963-9000-1-git-send-email-yuanzhichang@hisilicon.com> <58C8BD7D.7040207@hisilicon.com> From: Arnd Bergmann Date: Wed, 15 Mar 2017 14:23:12 +0100 X-Google-Sender-Auth: lHwOPd5KqFpVY5omN8KMaRBGhVE Message-ID: Subject: Re: [PATCH V7 0/7] LPC: legacy ISA I/O support To: "zhichang.yuan" Cc: Catalin Marinas , Will Deacon , Rob Herring , Frank Rowand , Bjorn Helgaas , Rafael Wysocki , Mark Rutland , rjw@rjwysocki.net, Linux ARM , ACPI Devel Maling List , 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, Zou Rongrong , John Garry , Gabriele Paoloni , zhichang.yuan02@gmail.com, kantyzc@163.com, Wei Xu Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3414 Lines: 73 On Wed, Mar 15, 2017 at 5:05 AM, zhichang.yuan wrote: >> - I think the libio framework is more generic than it needs to be, but as >> Alex really liked it this way and it was done like this based on his earlier >> comments, I think that's ok. >> >> - after we went back and forth on the ACPI implementation, we concluded >> that it is correct to do the same as on DT and completely abstract the >> number space for I/O ports. No code should rely on a Linux port number >> to have any particular relation to the physical address or the the address >> on a PCI or LPC bus. > Thanks again for your helps in Linaro Connect! > I think we are heading for this direction, is it? Yes, I think so. >> - I'm pretty sure the current implementation is broken for the ioport_map >> function that tries to turn an IORESOURCE_IO number into a pointer. >> Forcing CONFIG_GENERIC_IOMAP on would solve this, but also >> make all MMIO operations slower, which we probably don't want. >> It's probably enough to add a check in ioport_map() to see if the range >> is mapped into a virtual address or not. > > > Yes, I think our LIBIO will break the ioport_map() at this moment. > I try to solve this issue. Could you help to check the following ideas? > I am not deeper understanding the whole I/O framework, the following maybe not correct:( > > ioport_map seems architecture-dependent. For our LIBIO, we don't want to replace the existing I/O > frameworks which support MMIO at this moment. Can we add these two revise to solve this issue? > 1) Make LIBIO only target for non GENERIC_IOMAP platforms > > config LIBIO > bool "Generic logical IO management" > depends on !GENERIC_IOMAP I don't think there is even a problem with GENERIC_IOMAP: If both are set, passing a low number as a pointer will turn an ioread32() into an inl(), which is implemented by libio. > def_bool y if PCI && (ARC || MN10300 || UNICORE32 || SPARC || MICROBLAZE || S390 || AVR32 || CRIS || BLACKFIN || XTENSA || ARM64) I think most of these architectures just use their own inb/outb functions, and should not use libio at all. It's also possible that they use an older way of mapping I/O ports by calling ioremap() on the physical address and treating the pointer as a 32-bit I/O port number (relying on the PCI_IOBASE=0 default). Architectures doing that might have other issues with libio, and I wouldn't try converting those. > 2) Modify the ioport_map() defined in asm-generic/io.h > Add the checks to identify the input 'port' is MMIO, otherwise, return NULL; This seems fine. >> - We could simplify the lookup a bit by using the trick from arch/ia64 >> of using an array instead of linked list for walking the port numbers. >> There, the upper bits of the port number refer to an address space >> number while the lower bits refer to the bus address within that >> address space. This should work just as well as the current >> implementation but would be a little easier to understand. Maybe >> Bjorn can comment on this too, as I think he was involved with the >> ia64 implementation. >> > Yes, It will be more efficient. > > But the issue remained here is still how to coexist with the existing I/O frameworks. > I will continue to look into. Ok. Let's wait for Bjorn to reply on this idea before you spend too much time on this though. Arnd