From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Rafael Wysocki <rafael@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, <rjw@rjwysocki.net>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
"ACPI Devel Maling List" <linux-acpi@vger.kernel.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
<linuxarm@huawei.com>, <devicetree@vger.kernel.org>,
linux-pci <linux-pci@vger.kernel.org>,
<linux-serial@vger.kernel.org>, Corey Minyard <minyard@acm.org>,
<liviu.dudau@arm.com>, Zou Rongrong <zourongrong@gmail.com>,
John Garry <john.garry@huawei.com>,
Gabriele Paoloni <gabriele.paoloni@huawei.com>,
<zhichang.yuan02@gmail.com>, <kantyzc@163.com>,
Wei Xu <xuwei5@hisilicon.com>
Subject: Re: [PATCH V7 0/7] LPC: legacy ISA I/O support
Date: Wed, 15 Mar 2017 12:05:17 +0800 [thread overview]
Message-ID: <58C8BD7D.7040207@hisilicon.com> (raw)
In-Reply-To: <CAK8P3a1f33ipk3Q8+wj9L6zP=ccn_AuhTfV0CC0+wAqrO-2beg@mail.gmail.com>
Hi, Arnd,
Many thanks for your review!
On 2017/3/14 16:39, Arnd Bergmann wrote:
> On Mon, Mar 13, 2017 at 3:42 AM, zhichang.yuan
> <yuanzhichang@hisilicon.com> wrote:
>> This patchset supports the IPMI-bt device attached to the Low-Pin-Count
>> interface implemented on Hisilicon Hip06/Hip07 SoC.
>> -----------
>> | LPC host|
>> | |
>> -----------
>> |
>> _____________V_______________LPC
>> | |
>> V V
>> ------------
>> | BT(ipmi)|
>> ------------
>>
>> When master accesses those peripherals beneath the Hip06/Hip07 LPC, a specific
>> LPC driver is needed to make LPC host generate the standard LPC I/O cycles with
>> the target peripherals'I/O port addresses. But on curent arm64 world, there is
>> no real I/O accesses. All the I/O operations through in/out pair are based on
>> MMIO which is not satisfied the I/O mechanism on Hip06/Hip07 LPC.
>> To solve this issue and keep the relevant existing peripherals' driver
>> untouched, this patchset implements:
>> - introduces a generic I/O space management framwork, LIBIO, to support I/O
>> operations of both MMIO buses and the host controllers which access their
>> peripherals with host local I/O addresses;
>> - redefines the in/out accessors to provide unified interfaces for MMIO and
>> legacy I/O. Based on the LIBIO, the calling of in/out() from upper-layer
>> drivers, such as ipmi-si, will be redirected to the corresponding
>> device-specific I/O hooks to perfrom the I/O accesses.
>> Based on this patch-set, all the I/O accesses to Hip06/Hip07 LPC peripherals
>> can be supported without any changes on the existing ipmi-si driver.
>
> Thanks for reposting this. I have a few high-level comments first, based on
> the walk through the code I did with Gabriele and John last week:
>
> - 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?
>
> - The name "libio" still needs to be changed, this is way too generic, as
> "I/O" can refer to many things in the kernel, and almost none of them
> are related to x86 programmed I/O ports in any way. My suggestion
> would be "generic_ioport", or possibly "libioport", "libpio" or "pci_io". Any
> of them would work for me, or someone else could come up with a better
> name that describes what it is.
Ok. We will make a better name:)
>
> - 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
def_bool y if PCI && (ARC || MN10300 || UNICORE32 || SPARC || MICROBLAZE || S390 || AVR32 || CRIS || BLACKFIN || XTENSA || ARM64)
2) Modify the ioport_map() defined in asm-generic/io.h
Add the checks to identify the input 'port' is MMIO, otherwise, return NULL;
Then is it enough to avoid the negative effect on the existing I/O framework?
>
> - 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.
Thanks,
Zhichang
> Arnd
>
> .
>
next prev parent reply other threads:[~2017-03-15 4:06 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-13 2:42 [PATCH V7 0/7] LPC: legacy ISA I/O support zhichang.yuan
2017-03-13 2:42 ` [PATCH V7 1/7] LIBIO: Introduce a generic PIO mapping method zhichang.yuan
2017-03-13 2:42 ` [PATCH V7 2/7] PCI: Apply the new generic I/O management on PCI IO hosts zhichang.yuan
2017-03-27 19:46 ` dann frazier
2017-03-13 2:42 ` [PATCH V7 3/7] OF: Add missing I/O range exception for indirect-IO devices zhichang.yuan
2017-03-13 2:42 ` [PATCH V7 4/7] LPC: Support the device-tree LPC host on Hip06/Hip07 zhichang.yuan
2017-03-13 2:42 ` [PATCH V7 5/7] ACPI: Delay the enumeration on the devices whose dependency has not met zhichang.yuan
2017-03-13 21:24 ` Rafael J. Wysocki
2017-03-14 8:14 ` Gabriele Paoloni
2017-03-16 2:21 ` zhichang.yuan
2017-03-16 10:13 ` Arnd Bergmann
2017-03-16 14:56 ` zhichang.yuan
2017-03-16 16:13 ` Gabriele Paoloni
2017-03-24 0:23 ` Gabriele Paoloni
2017-03-14 5:11 ` kbuild test robot
2017-03-14 6:10 ` kbuild test robot
2017-03-13 2:42 ` [PATCH V7 6/7] LIBIO: Support the dynamically logical PIO registration of ACPI host I/O zhichang.yuan
2017-03-14 4:27 ` kbuild test robot
2017-03-14 5:10 ` kbuild test robot
2017-03-13 2:42 ` [PATCH V7 7/7] LPC: Add the ACPI LPC support zhichang.yuan
2017-03-14 5:25 ` kbuild test robot
2017-03-14 8:39 ` [PATCH V7 0/7] LPC: legacy ISA I/O support Arnd Bergmann
2017-03-15 4:05 ` zhichang.yuan [this message]
2017-03-15 13:23 ` Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=58C8BD7D.7040207@hisilicon.com \
--to=yuanzhichang@hisilicon.com \
--cc=arnd@arndb.de \
--cc=benh@kernel.crashing.org \
--cc=bhelgaas@google.com \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=gabriele.paoloni@huawei.com \
--cc=john.garry@huawei.com \
--cc=kantyzc@163.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=liviu.dudau@arm.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=minyard@acm.org \
--cc=rafael@kernel.org \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=will.deacon@arm.com \
--cc=xuwei5@hisilicon.com \
--cc=zhichang.yuan02@gmail.com \
--cc=zourongrong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).