Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754871AbcKJJNe (ORCPT ); Thu, 10 Nov 2016 04:13:34 -0500 Received: from mout.kundenserver.de ([212.227.17.10]:58018 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753297AbcKJJN3 (ORCPT ); Thu, 10 Nov 2016 04:13:29 -0500 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: "zhichang.yuan" , "mark.rutland@arm.com" , "devicetree@vger.kernel.org" , "lorenzo.pieralisi@arm.com" , Gabriele Paoloni , "minyard@acm.org" , "linux-pci@vger.kernel.org" , "benh@kernel.crashing.org" , John Garry , "will.deacon@arm.com" , "linux-kernel@vger.kernel.org" , "xuwei (O)" , Linuxarm , "zourongrong@gmail.com" , "robh+dt@kernel.org" , "kantyzc@163.com" , "linux-serial@vger.kernel.org" , "catalin.marinas@arm.com" , "olof@lixom.net" , "liviu.dudau@arm.com" , "bhelgaas@google.com" , "zhichang.yuan02@gmail.com" Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06 Date: Thu, 10 Nov 2016 10:12:21 +0100 Message-ID: <17821285.aIcTyCGn5n@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <5824165A.4040303@hisilicon.com> References: <1478576829-112707-1-git-send-email-yuanzhichang@hisilicon.com> <2825537.ADCNsGqGxn@wuerfel> <5824165A.4040303@hisilicon.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:dR3m0vNyUtC+5E3iHdpEwLqIOGUpBe+AOho0ESa6g8AR8sZIsXx RDIOb4HLsdkh2m1dqo4NbxHSeGh3gYqkp3YyKH6fsT7gd/oIxL9Mq+e26iMm0iwA7XBJ1FI kozZ4v33zyC345hEloeAL3FvMlTj+f1P590OC91apH0Q7Y9SriwA11UXrtvnRnawPHtUv7H j6iyTT9ooBHBKM2kfhZjw== X-UI-Out-Filterresults: notjunk:1;V01:K0:lvc+iPAs3pk=:xmttKsa0LQtP/6NhL1Tn1b Ys+g0gwBvCCW7Fs4eAjDF3BJTU6WuM5+fotM00/zrJaOr+FDD3Klc9WPJwRqLCBDOsCa6XDh0 kBFJRLJxOmIryaSnLbmbrM6D5iJaOe+51Vp/Wyqoj1Yf5FLG2N6dZX8XWAIUReieAcJzs0VRZ e94FgLgK6b9NvunOq6nlmu26Y12euCErnR0JYgIWnGszwB1nJ1enyDIdz0pQOb0rlsYtzK+z7 /xIFQIdfyVem3yncDQn8mMouBTzAuoU/s1MFWKkwkSbui1DR2mHDUMb37X2c8YHWGqvqlOXyE MHSvxT4jFIYXAVOt9/1XjEzmGptzuL4N9BPKg2s+KuSMa/IHM2VD8PobENIn3nvu35Jh5FEhw xvSwM2A7DwZPKiZBe7ACP8sk1ZvNWp9XTuW4ttqhZVKRKbQ7c22a2lGEDrVKmmJtJr4wcu5td kESrZ9voPZ5jDTTiJPIu3RSbj4J6mJL84EiZQ1bG7zt0PBbOcwZh9qrAf1AqWeqHmODxYvOcG bFrHMOjYB/05JLfrZrIyV0TPeVXlvDDI4RSAnz7hepIZa9eZu7rhsy3l9FuI38jFaR7PXOb++ 11uepuIwU/3jzm3cAU/ikQSFd2TsqmEOF858C+dS8SeP1BceCL4WUdyoYnXEjqbZkEqpEiJek DdCptQKQYptcd1CIFNhikVVan5KHl6gsTtOUI11xwFsYzE7AeXH9wEqCe+B+57/x3VW7yDB6i qoyfjSGYwWLiD8l1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2756 Lines: 61 On Thursday, November 10, 2016 2:40:26 PM CET zhichang.yuan wrote: > On 2016/11/10 5:34, Arnd Bergmann wrote: > > On Wednesday, November 9, 2016 12:10:43 PM CET Gabriele Paoloni wrote: > >>> On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote: > >>>> + /* > >>>> + * The first PCIBIOS_MIN_IO is reserved specifically for > >>> indirectIO. > >>>> + * It will separate indirectIO range from pci host bridge to > >>>> + * avoid the possible PIO conflict. > >>>> + * Set the indirectIO range directly here. > >>>> + */ > >>>> + lpcdev->io_ops.start = 0; > >>>> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1; > >>>> + lpcdev->io_ops.devpara = lpcdev; > >>>> + lpcdev->io_ops.pfin = hisilpc_comm_in; > >>>> + lpcdev->io_ops.pfout = hisilpc_comm_out; > >>>> + lpcdev->io_ops.pfins = hisilpc_comm_ins; > >>>> + lpcdev->io_ops.pfouts = hisilpc_comm_outs; > >>> > >>> I have to look at patch 2 in more detail again, after missing a few > >>> review > >>> rounds. I'm still a bit skeptical about hardcoding a logical I/O port > >>> range here, and would hope that we can just go through the same > >>> assignment of logical port ranges that we have for PCI buses, > >>> decoupling > >>> the bus addresses from the linux-internal ones. > >> > >> The point here is that we want to avoid any conflict/overlap between > >> the LPC I/O space and the PCI I/O space. With the assignment above > >> we make sure that LPC never interfere with PCI I/O space. > > > > But we already abstract the PCI I/O space using dynamic registration. > > There is no need to hardcode the logical address for ISA, though > > I think we can hardcode the bus address to start at zero here. > > Do you means that we can pick up the maximal I/O address from all children's > device resources?? The driver should not look at the resources of its children, just register a range of addresses dynamically, as I suggested in an earlier review. Your current version has if (arm64_extio_ops->pfout) \ arm64_extio_ops->pfout(arm64_extio_ops->devpara,\ addr, value, sizeof(type)); \ Instead, just subtract the start of the range from the logical port number to transform it back into a bus-local port number: if (arm64_extio_ops->pfout) \ arm64_extio_ops->pfout(arm64_extio_ops->devpara,\ addr - arm64_extio_ops->start, value, sizeof(type)); \ We know that the ISA/LPC bus can only have up to 65536 ports, so you can register all of those, or possibly limit it further to 1024 or 4096 ports, whichever matches the bus implementation. Arnd