Hi Lorenzo, Rafael > -----Original Message----- > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@xxxxxxx] > Sent: 12 June 2017 16:57 > To: Gabriele Paoloni; rafael@xxxxxxxxxx; Rafael J. Wysocki; Mika > Westerberg > Cc: catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; robh+dt@xxxxxxxxxx; > frowand.list@xxxxxxxxx; bhelgaas@xxxxxxxxxx; arnd@xxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; mark.rutland@xxxxxxx; > brian.starkey@xxxxxxx; olof@xxxxxxxxx; benh@xxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; Linuxarm; linux- > pci@xxxxxxxxxxxxxxx; minyard@xxxxxxx; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > [+Mika] > > Gab, Rafael, > > On Tue, Jun 06, 2017 at 09:55:53AM +0100, Lorenzo Pieralisi wrote: > > Hi Gab, Rafael, > > > > On Wed, May 31, 2017 at 10:24:47AM +0000, Gabriele Paoloni wrote: > > > > [...] > > > > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > > > > index e39ec7b..37dd23c 100644 > > > > > --- a/drivers/acpi/scan.c > > > > > +++ b/drivers/acpi/scan.c > > > > > @@ -2035,6 +2035,7 @@ int __init acpi_scan_init(void) > > > > > acpi_int340x_thermal_init(); > > > > > acpi_amba_init(); > > > > > acpi_watchdog_init(); > > > > > + acpi_indirectio_scan_init(); > > > > Unfortunately this is becoming a pattern and we are ending up > > with a static ordering of "subsystems" init (even though for this > > LPC series it is just the Hisilicon driver that requires this call) > > and I am not sure I see any way of avoiding it. I think that's always > > been the case in x86, with fewer subsystems/kernel paths to care > > about, I wanted to flag this up though to check your opinion since > > I am not sure this is the right direction we are taking. > > > > I also think that relying on _DEP to build any dependency is not > > entirely a) usable (owing to legacy bindings and previous _DEP > misuse) > > and b) compliant with ACPI bindings given that _DEP has to be used > > for operation regions only. > > I had a more in-depth look at this series and from my understanding > the problem are the following to manage the LPC bindings in ACPI. > > (1) Child devices of an LPC controller require special handling when > filling their resources (ie they need to be translated - in DT > that's guaranteed by the "isa" binding, in ACPI it has to be > done by new code) Correct, LPC resources need to be translated in a virtual IO port address space. We cannot strictly follow the ISA bindings as the LPC host does not define a mapping (through the "range" property) between a CPU address range and an LPC address range. Instead LPC has got his own bus accessors; therefore the bus address range that LPC operates on is directly mapped into the IO port address range and the IO in/out standard accessors (include/asm-generic/io.h) are redefined to use the LPC accessors for the virtual IO port address range that corresponds to LPC. > (2) In DT systems, LPC child devices are created by the LPC bus > controller driver through an of_platform_populate() call with > the LPC controller node as the fwnode root. For ACPI to work > the same way there must be a way to prevent LPC children to > be enumerated in acpi_default_enumeration() something like > I2C does (and then the LPC driver would enumerate its children as > DT does) Correct. > > I am not sure how (1) and (2) can be managed with current ACPI bindings > and kernel code - I suspect it may be done by mirroring what's done > for I2C but I am not sure, that's why I CC'ed Mika (ie the LPC adapter > is matched as a platform device and it takes care of enumerating its > children - problem though is preventing enumeration from core ACPI > code). Yes my idea was to have a scan handler that enumerate the children devices and translate its addresses filling dev->resources[] and at the same time we can modify acpi_default_enumeration() to check acpi_device_enumerated() before continuing with device enumeration...? Many thanks Gab > > I will let Gabriele and Hisilicon guys chime in if I missed something, > which is likely, please let me know your opinion on how this code > can be made functional on ACPI systems - it is uncharted territory. > > Thank you ! > Lorenzo > > > > > Thoughts ? > > > > Thanks, > > Lorenzo > > > > > > > acpi_scan_add_handler(&generic_device_handler); > > > > > > > > > > diff --git a/include/acpi/acpi_indirect_pio.h > > > > b/include/acpi/acpi_indirect_pio.h > > > > > new file mode 100644 > > > > > index 0000000..efc5c43 > > > > > --- /dev/null > > > > > +++ b/include/acpi/acpi_indirect_pio.h > > > > > @@ -0,0 +1,24 @@ > > > > > +/* > > > > > + * ACPI support for indirect-PIO bus. > > > > > + * > > > > > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved. > > > > > + * Author: Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> > > > > > + * Author: Zhichang Yuan <yuanzhichang@xxxxxxxxxxxxx> > > > > > + * > > > > > + * This program is free software; you can redistribute it > and/or > > > > modify > > > > > + * it under the terms of the GNU General Public License > version 2 as > > > > > + * published by the Free Software Foundation. > > > > > + */ > > > > > + > > > > > +#ifndef _ACPI_INDIRECT_PIO_H > > > > > +#define _ACPI_INDIRECT_PIO_H > > > > > + > > > > > +struct indirect_pio_device_desc { > > > > > + void *pdata; /* device relevant info data */ > > > > > + int (*pre_setup)(struct acpi_device *adev, void *pdata); > > > > > +}; > > > > > + > > > > > +int acpi_set_logic_pio_resource(struct device *child, > > > > > + struct device *hostdev); > > > > > + > > > > > +#endif > > > > > -- > > > > > 2.7.4 > > > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
