lkml.org 
[lkml]   [2018]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v13 7/9] ACPI: Translate the I/O range of non-MMIO devices before scanning
From
Date
On 14/02/2018 16:16, Lorenzo Pieralisi wrote:
> On Wed, Feb 14, 2018 at 01:45:31AM +0800, John Garry wrote:
>> On some platforms (such as arm64-based hip06/hip07), access to legacy
>> ISA/LPC devices through access IO space is required, similar to x86
>> platforms. As the I/O for these devices are not memory mapped like
>> PCI/PCIE MMIO host bridges, they require special low-level device
>> operations through some host to generate IO accesses, i.e. a non-
>> transparent bridge.
>>
>> Through the logical PIO framework, hosts are able to register address
>> ranges in the logical PIO space for IO accesses. For hosts which require
>> a LLDD to generate the IO accesses, through the logical PIO framework
>> the host also registers accessors as a backend to generate the physical
>> bus transactions for IO space accesses (called indirect IO).
>>
>> When describing the indirect IO child device in APCI tables, the IO
>> resource is the host-specific address for the child (generally a
>> bus address).
>> An example is as follows:
>> Device (LPC0) {
>> Name (_HID, "HISI0191") // HiSi LPC
>> Name (_CRS, ResourceTemplate () {
>> Memory32Fixed (ReadWrite, 0xa01b0000, 0x1000)
>> })
>> }
>>
>> Device (LPC0.IPMI) {
>> Name (_HID, "IPI0001")
>> Name (LORS, ResourceTemplate() {
>> QWordIO (
>> ResourceConsumer,
>> MinNotFixed, // _MIF
>> MaxNotFixed, // _MAF
>> PosDecode,
>> EntireRange,
>> 0x0, // _GRA
>> 0xe4, // _MIN
>> 0x3fff, // _MAX
>> 0x0, // _TRA
>> 0x04, // _LEN
>> , ,
>> BTIO
>> )
>> })
>>
>> Since the IO resource for the child is a host-specific address,
>> special translation are required to retrieve the logical PIO address
>> for that child.
>

Hi Lorenzo,

> The problem I have with this patchset and with pretending that the ACPI
> bits are generic is that the rules used to translate resources (I am
> referring to LPC0.IPMI above) are documented _nowhere_ which means that
> making this series generic code is just wishful thinking - there are no
> bindings backing it, it will never ever be used on a platform different
> from the one you are pushing this code for and I stated this already.
>

Right, it is working on the presumption that this is how all "indirectio
IO" hosts and children should/would be described in DSDT.

> Reworded differently - this is a Hisilicon driver it is not generic ACPI
> code; I can't see how it can be used on a multitude of platforms unless
> you specify FW level bindings.
>
>> To overcome the problem of associating this logical PIO address
>> with the child device, a scan handler is added to scan the ACPI
>> namespace for known indirect IO hosts. This scan handler creates an
>> MFD per child with the translated logical PIO address as it's IO
>> resource, as a substitute for the normal platform device which ACPI
>> would create during device enumeration.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> ---
>> drivers/acpi/arm64/Makefile | 1 +
>> drivers/acpi/arm64/acpi_indirectio.c | 250 +++++++++++++++++++++++++++++++++++
>
> See above (and I do not understand what arm64 has to do with it).

Nothing apart from only being used by arm64 platforms today, which is
circumstantial.

>
> I understand you need to find a place to add the:
>
> acpi_indirect_io_scan_init()
>
> to be called from core ACPI code because ACPI can't handle probe
> dependencies in any other way but other than that this patch is
> a Hisilicon ACPI driver - there is nothing generic in it (or at
> least there are no standard bindings to make it so).
>
> Whether a callback from ACPI core code (acpi_scan_init()) to a driver
> specific hook is sane or not that's the question and the only reason
> why you want to add this in drivers/acpi/arm64 rather than, say,
> drivers/bus (as you do for the DT driver).
>
> I do not know Rafael's opinion on the above, I would like to help
> you make forward progress but please understand my concerns, mostly
> on FW side.
>

I did mention an alternative in my "ping" in v12 patch 7/9 (Feb 1), but
no response to this specific note so I kept on the same path.

Here's what I then wrote:
"I think another solution - which you may prefer - is to avoid adding
this scan handler (and all this other scan code) and add a check like
acpi_is_serial_bus_slave() [which checks the device parent versus a list
of known indirectIO hosts] to not enumerate these children, and do it
from the LLDD host probe instead (https://lkml.org/lkml/2017/6/16/250)"

Please consider this.

> Thanks,
> Lorenzo
>
>> drivers/acpi/internal.h | 5 +
>> drivers/acpi/scan.c | 1 +
>> 4 files changed, 257 insertions(+)
>> create mode 100644 drivers/acpi/arm64/acpi_indirectio.c
>>

Cheers,
John



\
 
 \ /
  Last update: 2018-02-14 17:54    [W:0.158 / U:1.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site