RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > You may do it on stack. Define your cell statically (but not const) > and apply resources just before mfd_add_devices() call. Ok thanks got it > There are examples in the existing drivers. Intel LPC comes to my mind > and perhaps PMC (Broxton), though latter has too much other stuff > around. Uh yes I see now in lpc_ich.c (base address is read from PCI config space and resources are set accordingly). Cheers Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > You may do it on stack. Define your cell statically (but not const) > and apply resources just before mfd_add_devices() call. Ok thanks got it > There are examples in the existing drivers. Intel LPC comes to my mind > and perhaps PMC (Broxton), though latter has too much other stuff > around. Uh yes I see now in lpc_ich.c (base address is read from PCI config space and resources are set accordingly). Cheers Gab > > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jul 4, 2017 at 6:14 PM, Gabriele Paoloniwrote: >> > In my case I'd like to have a platform device using the resources >> that are >> > parsed from the ACPI table (i.e. as it is done now by >> > acpi_create_platform_device()). >> >> So far so good. Nothing prevents you to do that. >> >> > If my understanding is correct, if I declared an mfd_cell for my IPMI >> child >> > the mfd subsystem would create a platform device for such child and >> > therefore acpi_create_platform_device() would fail to create a new >> platform >> > device as adev->physical_node_count will be non zero. >> > However as things stand now mfd_cell devices can only use the >> resources >> > that are statically defined in the code (and therefore not the ones >> in the >> > ACPI nodes)...am I right? >> >> You may file resources first and then register MFD cells. See many >> existing examples in the kernel. > > Well I had a look around the Kernel I have seen no mfd cells using > Resources that are not statically defined: > i.e. cell->resources in mfd_add_device() always points to statically > defined resource structures. > > Usually for ACPI devices first you need to parse the ACPI resources > from the table calling acpi_dev_get_resources(), then you iterate > over the resource list and fill the resource array by calling > acpi_platform_fill_resurces() (as in acpi_create_platform_device()) > > With respect to my case are you suggesting dynamically allocate a > resource array and fill it using the same fashion as > acpi_create_platform_device(), then point cell->resources to such > array before calling mfd_add_device() ? You may do it on stack. Define your cell statically (but not const) and apply resources just before mfd_add_devices() call. There are examples in the existing drivers. Intel LPC comes to my mind and perhaps PMC (Broxton), though latter has too much other stuff around. -- With Best Regards, Andy Shevchenko
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jul 4, 2017 at 6:14 PM, Gabriele Paoloni wrote: >> > In my case I'd like to have a platform device using the resources >> that are >> > parsed from the ACPI table (i.e. as it is done now by >> > acpi_create_platform_device()). >> >> So far so good. Nothing prevents you to do that. >> >> > If my understanding is correct, if I declared an mfd_cell for my IPMI >> child >> > the mfd subsystem would create a platform device for such child and >> > therefore acpi_create_platform_device() would fail to create a new >> platform >> > device as adev->physical_node_count will be non zero. >> > However as things stand now mfd_cell devices can only use the >> resources >> > that are statically defined in the code (and therefore not the ones >> in the >> > ACPI nodes)...am I right? >> >> You may file resources first and then register MFD cells. See many >> existing examples in the kernel. > > Well I had a look around the Kernel I have seen no mfd cells using > Resources that are not statically defined: > i.e. cell->resources in mfd_add_device() always points to statically > defined resource structures. > > Usually for ACPI devices first you need to parse the ACPI resources > from the table calling acpi_dev_get_resources(), then you iterate > over the resource list and fill the resource array by calling > acpi_platform_fill_resurces() (as in acpi_create_platform_device()) > > With respect to my case are you suggesting dynamically allocate a > resource array and fill it using the same fashion as > acpi_create_platform_device(), then point cell->resources to such > array before calling mfd_add_device() ? You may do it on stack. Define your cell statically (but not const) and apply resources just before mfd_add_devices() call. There are examples in the existing drivers. Intel LPC comes to my mind and perhaps PMC (Broxton), though latter has too much other stuff around. -- With Best Regards, Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > JFYI: Mika on vacation. Thanks for letting me know > > > I had a look into the MFD framework. If my understanding is correct > the mfd > > framework create a platform device for each declared mfd_cell that is > passed > > to mfd_add_devices(). > > Right. > > > However there is something that I do not quite understand: > > from > > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > > it seems that mfd_add_device() will create the platform device using > the > > resources that are statically declared in the respective mfd_cell. > > It's one possibility. > > > In my case I'd like to have a platform device using the resources > that are > > parsed from the ACPI table (i.e. as it is done now by > > acpi_create_platform_device()). > > So far so good. Nothing prevents you to do that. > > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > > the mfd subsystem would create a platform device for such child and > > therefore acpi_create_platform_device() would fail to create a new > platform > > device as adev->physical_node_count will be non zero. > > However as things stand now mfd_cell devices can only use the > resources > > that are statically defined in the code (and therefore not the ones > in the > > ACPI nodes)...am I right? > > You may file resources first and then register MFD cells. See many > existing examples in the kernel. Well I had a look around the Kernel I have seen no mfd cells using Resources that are not statically defined: i.e. cell->resources in mfd_add_device() always points to statically defined resource structures. Usually for ACPI devices first you need to parse the ACPI resources from the table calling acpi_dev_get_resources(), then you iterate over the resource list and fill the resource array by calling acpi_platform_fill_resurces() (as in acpi_create_platform_device()) With respect to my case are you suggesting dynamically allocate a resource array and fill it using the same fashion as acpi_create_platform_device(), then point cell->resources to such array before calling mfd_add_device() ? Thanks Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > JFYI: Mika on vacation. Thanks for letting me know > > > I had a look into the MFD framework. If my understanding is correct > the mfd > > framework create a platform device for each declared mfd_cell that is > passed > > to mfd_add_devices(). > > Right. > > > However there is something that I do not quite understand: > > from > > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > > it seems that mfd_add_device() will create the platform device using > the > > resources that are statically declared in the respective mfd_cell. > > It's one possibility. > > > In my case I'd like to have a platform device using the resources > that are > > parsed from the ACPI table (i.e. as it is done now by > > acpi_create_platform_device()). > > So far so good. Nothing prevents you to do that. > > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > > the mfd subsystem would create a platform device for such child and > > therefore acpi_create_platform_device() would fail to create a new > platform > > device as adev->physical_node_count will be non zero. > > However as things stand now mfd_cell devices can only use the > resources > > that are statically defined in the code (and therefore not the ones > in the > > ACPI nodes)...am I right? > > You may file resources first and then register MFD cells. See many > existing examples in the kernel. Well I had a look around the Kernel I have seen no mfd cells using Resources that are not statically defined: i.e. cell->resources in mfd_add_device() always points to statically defined resource structures. Usually for ACPI devices first you need to parse the ACPI resources from the table calling acpi_dev_get_resources(), then you iterate over the resource list and fill the resource array by calling acpi_platform_fill_resurces() (as in acpi_create_platform_device()) With respect to my case are you suggesting dynamically allocate a resource array and fill it using the same fashion as acpi_create_platform_device(), then point cell->resources to such array before calling mfd_add_device() ? Thanks Gab > > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jul 3, 2017 at 7:08 PM, Gabriele Paoloniwrote: JFYI: Mika on vacation. > I had a look into the MFD framework. If my understanding is correct the mfd > framework create a platform device for each declared mfd_cell that is passed > to mfd_add_devices(). Right. > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 > it seems that mfd_add_device() will create the platform device using the > resources that are statically declared in the respective mfd_cell. It's one possibility. > In my case I'd like to have a platform device using the resources that are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). So far so good. Nothing prevents you to do that. > If my understanding is correct, if I declared an mfd_cell for my IPMI child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in the > ACPI nodes)...am I right? You may file resources first and then register MFD cells. See many existing examples in the kernel. -- With Best Regards, Andy Shevchenko
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jul 3, 2017 at 7:08 PM, Gabriele Paoloni wrote: JFYI: Mika on vacation. > I had a look into the MFD framework. If my understanding is correct the mfd > framework create a platform device for each declared mfd_cell that is passed > to mfd_add_devices(). Right. > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 > it seems that mfd_add_device() will create the platform device using the > resources that are statically declared in the respective mfd_cell. It's one possibility. > In my case I'd like to have a platform device using the resources that are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). So far so good. Nothing prevents you to do that. > If my understanding is correct, if I declared an mfd_cell for my IPMI child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in the > ACPI nodes)...am I right? You may file resources first and then register MFD cells. See many existing examples in the kernel. -- With Best Regards, Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
+CC Lee Jones > -Original Message- > From: Gabriele Paoloni > Sent: 03 July 2017 17:08 > To: Gabriele Paoloni; Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > > Sent: 19 June 2017 11:05 > > To: Mika Westerberg > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > Hi Mika > > > > > -Original Message- > > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > > Sent: 19 June 2017 11:02 > > > To: Gabriele Paoloni > > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > > arm- > > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > > linux- > > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; > linux- > > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non- > MMIO > > > devices before scanning > > > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > > Many thanks for your response and your help here. > > > > > > > > I guess that as conclusion with respect to the current v9 > patchset > > we > > > can > > > > disregard the idea of MFD and modify the current v9 so that it > > > doesn't > > > > touch directly ACPI resources. > > > > Instead as I proposed before we can have the scan handler to > > > 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...? > > > > > > > > Do you think it as a viable solution? > > > > > > No, I think MFD + scan handler inside the MFD driver is the way to > > go. > > > We don't want to trash ACPI core with stuff that does not belong > > there > > > IMHO. > > > > Ok Many thanks I will investigate this direction > > I had a look into the MFD framework. If my understanding is correct the > mfd > framework create a platform device for each declared mfd_cell that is > passed > to mfd_add_devices(). > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > it seems that mfd_add_device() will create the platform device using > the > resources that are statically declared in the respective mfd_cell. > > In my case I'd like to have a platform device using the resources that > are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new > platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in > the > ACPI nodes)...am I right? > > Thanks > Gab > > > > > > > > > Also you don't need to modify acpi_default_enumeration() because > you > > > can > > > mark your device enumerated in the MFD driver. So all the dirty > > details > > > will be in the MFD driver and not in ACPI core. > > > > Ok got it :) > > > > Cheers > > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
+CC Lee Jones > -Original Message- > From: Gabriele Paoloni > Sent: 03 July 2017 17:08 > To: Gabriele Paoloni; Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > > Sent: 19 June 2017 11:05 > > To: Mika Westerberg > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > Hi Mika > > > > > -Original Message- > > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > > Sent: 19 June 2017 11:02 > > > To: Gabriele Paoloni > > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > > arm- > > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > > linux- > > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; > linux- > > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non- > MMIO > > > devices before scanning > > > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > > Many thanks for your response and your help here. > > > > > > > > I guess that as conclusion with respect to the current v9 > patchset > > we > > > can > > > > disregard the idea of MFD and modify the current v9 so that it > > > doesn't > > > > touch directly ACPI resources. > > > > Instead as I proposed before we can have the scan handler to > > > 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...? > > > > > > > > Do you think it as a viable solution? > > > > > > No, I think MFD + scan handler inside the MFD driver is the way to > > go. > > > We don't want to trash ACPI core with stuff that does not belong > > there > > > IMHO. > > > > Ok Many thanks I will investigate this direction > > I had a look into the MFD framework. If my understanding is correct the > mfd > framework create a platform device for each declared mfd_cell that is > passed > to mfd_add_devices(). > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > it seems that mfd_add_device() will create the platform device using > the > resources that are statically declared in the respective mfd_cell. > > In my case I'd like to have a platform device using the resources that > are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new > platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in > the > ACPI nodes)...am I right? > > Thanks > Gab > > > > > > > > > Also you don't need to modify acpi_default_enumeration() because > you > > > can > > > mark your device enumerated in the MFD driver. So all the dirty > > details > > > will be in the MFD driver and not in ACPI core. > > > > Ok got it :) > > > > Cheers > > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > Sent: 19 June 2017 11:05 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > Sent: 19 June 2017 11:02 > > To: Gabriele Paoloni > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > Many thanks for your response and your help here. > > > > > > I guess that as conclusion with respect to the current v9 patchset > we > > can > > > disregard the idea of MFD and modify the current v9 so that it > > doesn't > > > touch directly ACPI resources. > > > Instead as I proposed before we can have the scan handler to > > 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...? > > > > > > Do you think it as a viable solution? > > > > No, I think MFD + scan handler inside the MFD driver is the way to > go. > > We don't want to trash ACPI core with stuff that does not belong > there > > IMHO. > > Ok Many thanks I will investigate this direction I had a look into the MFD framework. If my understanding is correct the mfd framework create a platform device for each declared mfd_cell that is passed to mfd_add_devices(). However there is something that I do not quite understand: from http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 it seems that mfd_add_device() will create the platform device using the resources that are statically declared in the respective mfd_cell. In my case I'd like to have a platform device using the resources that are parsed from the ACPI table (i.e. as it is done now by acpi_create_platform_device()). If my understanding is correct, if I declared an mfd_cell for my IPMI child the mfd subsystem would create a platform device for such child and therefore acpi_create_platform_device() would fail to create a new platform device as adev->physical_node_count will be non zero. However as things stand now mfd_cell devices can only use the resources that are statically defined in the code (and therefore not the ones in the ACPI nodes)...am I right? Thanks Gab > > > > > Also you don't need to modify acpi_default_enumeration() because you > > can > > mark your device enumerated in the MFD driver. So all the dirty > details > > will be in the MFD driver and not in ACPI core. > > Ok got it :) > > Cheers > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > Sent: 19 June 2017 11:05 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > Sent: 19 June 2017 11:02 > > To: Gabriele Paoloni > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > Many thanks for your response and your help here. > > > > > > I guess that as conclusion with respect to the current v9 patchset > we > > can > > > disregard the idea of MFD and modify the current v9 so that it > > doesn't > > > touch directly ACPI resources. > > > Instead as I proposed before we can have the scan handler to > > 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...? > > > > > > Do you think it as a viable solution? > > > > No, I think MFD + scan handler inside the MFD driver is the way to > go. > > We don't want to trash ACPI core with stuff that does not belong > there > > IMHO. > > Ok Many thanks I will investigate this direction I had a look into the MFD framework. If my understanding is correct the mfd framework create a platform device for each declared mfd_cell that is passed to mfd_add_devices(). However there is something that I do not quite understand: from http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 it seems that mfd_add_device() will create the platform device using the resources that are statically declared in the respective mfd_cell. In my case I'd like to have a platform device using the resources that are parsed from the ACPI table (i.e. as it is done now by acpi_create_platform_device()). If my understanding is correct, if I declared an mfd_cell for my IPMI child the mfd subsystem would create a platform device for such child and therefore acpi_create_platform_device() would fail to create a new platform device as adev->physical_node_count will be non zero. However as things stand now mfd_cell devices can only use the resources that are statically defined in the code (and therefore not the ones in the ACPI nodes)...am I right? Thanks Gab > > > > > Also you don't need to modify acpi_default_enumeration() because you > > can > > mark your device enumerated in the MFD driver. So all the dirty > details > > will be in the MFD driver and not in ACPI core. > > Ok got it :) > > Cheers > Gab
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 30, 2017 at 11:28 AM, John Garrywrote: > On 30/06/2017 10:05, Mika Westerberg wrote: >> >> On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: >>> >>> On 16/06/2017 12:24, Rafael J. Wysocki wrote: >> >> >> It causes acpi_default_enumeration() to be called but it should be >> fine >> as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... >> >> >> In fact it may be that it is not sufficient in this case because the >> ACPI core might enumerate child devices before the LPC driver even >> gets >> a chance to probe so you would need to add also scan handler to the >> child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. >>> >>> For this, is it possible to just configure the ACPI table so we spoof >>> that >>> the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a >>> resource >>> of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi >>> to >>> solve this? >> >> >> But is the device connected to a I2C or SPI bus? If not, then it does >> not make much sense to declare it as I2C or SPI slave. Instead it should >> be platform device which is the type we use when there is no explicit >> bus specified in ACPI. >> > > No, it's not a SPI nor an I2C bus. I actually would say that my idea is > generally wrong, as the ACPI definition is not a real reflection of the > bus/slave. > > However, Rafael did suggest extending special I2C/SPI handling to them. In > this case, I don't see how the LPC slave can be identified like an I2C or > SPI slave is. I meant that it can be handled similarly (ie. as an exception from the default enumeration), such that the enumeration is delayed until the proper subsystem can enumerate those devices as appropriate. Sorry for the confusion. Thanks, Rafael
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 30, 2017 at 11:28 AM, John Garry wrote: > On 30/06/2017 10:05, Mika Westerberg wrote: >> >> On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: >>> >>> On 16/06/2017 12:24, Rafael J. Wysocki wrote: >> >> >> It causes acpi_default_enumeration() to be called but it should be >> fine >> as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... >> >> >> In fact it may be that it is not sufficient in this case because the >> ACPI core might enumerate child devices before the LPC driver even >> gets >> a chance to probe so you would need to add also scan handler to the >> child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. >>> >>> For this, is it possible to just configure the ACPI table so we spoof >>> that >>> the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a >>> resource >>> of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi >>> to >>> solve this? >> >> >> But is the device connected to a I2C or SPI bus? If not, then it does >> not make much sense to declare it as I2C or SPI slave. Instead it should >> be platform device which is the type we use when there is no explicit >> bus specified in ACPI. >> > > No, it's not a SPI nor an I2C bus. I actually would say that my idea is > generally wrong, as the ACPI definition is not a real reflection of the > bus/slave. > > However, Rafael did suggest extending special I2C/SPI handling to them. In > this case, I don't see how the LPC slave can be identified like an I2C or > SPI slave is. I meant that it can be handled similarly (ie. as an exception from the default enumeration), such that the enumeration is delayed until the proper subsystem can enumerate those devices as appropriate. Sorry for the confusion. Thanks, Rafael
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On 30/06/2017 10:05, Mika Westerberg wrote: On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: On 16/06/2017 12:24, Rafael J. Wysocki wrote: It causes acpi_default_enumeration() to be called but it should be fine as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... In fact it may be that it is not sufficient in this case because the ACPI core might enumerate child devices before the LPC driver even gets a chance to probe so you would need to add also scan handler to the child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. For this, is it possible to just configure the ACPI table so we spoof that the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to solve this? But is the device connected to a I2C or SPI bus? If not, then it does not make much sense to declare it as I2C or SPI slave. Instead it should be platform device which is the type we use when there is no explicit bus specified in ACPI. No, it's not a SPI nor an I2C bus. I actually would say that my idea is generally wrong, as the ACPI definition is not a real reflection of the bus/slave. However, Rafael did suggest extending special I2C/SPI handling to them. In this case, I don't see how the LPC slave can be identified like an I2C or SPI slave is. Thanks, John .
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On 30/06/2017 10:05, Mika Westerberg wrote: On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: On 16/06/2017 12:24, Rafael J. Wysocki wrote: It causes acpi_default_enumeration() to be called but it should be fine as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... In fact it may be that it is not sufficient in this case because the ACPI core might enumerate child devices before the LPC driver even gets a chance to probe so you would need to add also scan handler to the child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. For this, is it possible to just configure the ACPI table so we spoof that the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to solve this? But is the device connected to a I2C or SPI bus? If not, then it does not make much sense to declare it as I2C or SPI slave. Instead it should be platform device which is the type we use when there is no explicit bus specified in ACPI. No, it's not a SPI nor an I2C bus. I actually would say that my idea is generally wrong, as the ACPI definition is not a real reflection of the bus/slave. However, Rafael did suggest extending special I2C/SPI handling to them. In this case, I don't see how the LPC slave can be identified like an I2C or SPI slave is. Thanks, John .
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: > On 16/06/2017 12:24, Rafael J. Wysocki wrote: > > > > > >> > > > > > > >> > It causes acpi_default_enumeration() to be called but it should > > > > > >> > be fine > > > > > >> > as we are dealing with platform device anyway. > > > > >> > > > > >> I do not quite understand how declaring such MFD cell above would > > > > >> make sure > > > > >> that the LPC probe is called before the IPMI device is enumerated... > > > > > > > > In fact it may be that it is not sufficient in this case because the > > > > ACPI core might enumerate child devices before the LPC driver even gets > > > > a chance to probe so you would need to add also scan handler to the > > > > child devices and mark them already enumerated or something like that. > > Or extend the special I2C/SPI handling to them. > > > > For this, is it possible to just configure the ACPI table so we spoof that > the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource > of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to > solve this? But is the device connected to a I2C or SPI bus? If not, then it does not make much sense to declare it as I2C or SPI slave. Instead it should be platform device which is the type we use when there is no explicit bus specified in ACPI.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Thu, Jun 29, 2017 at 05:16:15PM +0100, John Garry wrote: > On 16/06/2017 12:24, Rafael J. Wysocki wrote: > > > > > >> > > > > > > >> > It causes acpi_default_enumeration() to be called but it should > > > > > >> > be fine > > > > > >> > as we are dealing with platform device anyway. > > > > >> > > > > >> I do not quite understand how declaring such MFD cell above would > > > > >> make sure > > > > >> that the LPC probe is called before the IPMI device is enumerated... > > > > > > > > In fact it may be that it is not sufficient in this case because the > > > > ACPI core might enumerate child devices before the LPC driver even gets > > > > a chance to probe so you would need to add also scan handler to the > > > > child devices and mark them already enumerated or something like that. > > Or extend the special I2C/SPI handling to them. > > > > For this, is it possible to just configure the ACPI table so we spoof that > the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource > of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to > solve this? But is the device connected to a I2C or SPI bus? If not, then it does not make much sense to declare it as I2C or SPI slave. Instead it should be platform device which is the type we use when there is no explicit bus specified in ACPI.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On 16/06/2017 12:24, Rafael J. Wysocki wrote: >> > >> > It causes acpi_default_enumeration() to be called but it should be fine >> > as we are dealing with platform device anyway. >> >> I do not quite understand how declaring such MFD cell above would make sure >> that the LPC probe is called before the IPMI device is enumerated... > > In fact it may be that it is not sufficient in this case because the > ACPI core might enumerate child devices before the LPC driver even gets > a chance to probe so you would need to add also scan handler to the > child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. For this, is it possible to just configure the ACPI table so we spoof that the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to solve this? This resource would/should need to be ignored for other purposes. John Thanks, Rafael
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On 16/06/2017 12:24, Rafael J. Wysocki wrote: >> > >> > It causes acpi_default_enumeration() to be called but it should be fine >> > as we are dealing with platform device anyway. >> >> I do not quite understand how declaring such MFD cell above would make sure >> that the LPC probe is called before the IPMI device is enumerated... > > In fact it may be that it is not sufficient in this case because the > ACPI core might enumerate child devices before the LPC driver even gets > a chance to probe so you would need to add also scan handler to the > child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. For this, is it possible to just configure the ACPI table so we spoof that the LPC slave (IPI0001), is an i2c/spi slave? Could we just add a resource of type ACPI_RESOURCE_TYPE_SERIAL_BUS, and common serial bus type i2c/spi to solve this? This resource would/should need to be ignored for other purposes. John Thanks, Rafael
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 19 June 2017 11:02 > To: Gabriele Paoloni > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > Many thanks for your response and your help here. > > > > I guess that as conclusion with respect to the current v9 patchset we > can > > disregard the idea of MFD and modify the current v9 so that it > doesn't > > touch directly ACPI resources. > > Instead as I proposed before we can have the scan handler to > 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...? > > > > Do you think it as a viable solution? > > No, I think MFD + scan handler inside the MFD driver is the way to go. > We don't want to trash ACPI core with stuff that does not belong there > IMHO. Ok Many thanks I will investigate this direction > > Also you don't need to modify acpi_default_enumeration() because you > can > mark your device enumerated in the MFD driver. So all the dirty details > will be in the MFD driver and not in ACPI core. Ok got it :) Cheers Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 19 June 2017 11:02 > To: Gabriele Paoloni > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > Many thanks for your response and your help here. > > > > I guess that as conclusion with respect to the current v9 patchset we > can > > disregard the idea of MFD and modify the current v9 so that it > doesn't > > touch directly ACPI resources. > > Instead as I proposed before we can have the scan handler to > 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...? > > > > Do you think it as a viable solution? > > No, I think MFD + scan handler inside the MFD driver is the way to go. > We don't want to trash ACPI core with stuff that does not belong there > IMHO. Ok Many thanks I will investigate this direction > > Also you don't need to modify acpi_default_enumeration() because you > can > mark your device enumerated in the MFD driver. So all the dirty details > will be in the MFD driver and not in ACPI core. Ok got it :) Cheers Gab
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > Many thanks for your response and your help here. > > I guess that as conclusion with respect to the current v9 patchset we can > disregard the idea of MFD and modify the current v9 so that it doesn't > touch directly ACPI resources. > Instead as I proposed before we can have the scan handler to 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...? > > Do you think it as a viable solution? No, I think MFD + scan handler inside the MFD driver is the way to go. We don't want to trash ACPI core with stuff that does not belong there IMHO. Also you don't need to modify acpi_default_enumeration() because you can mark your device enumerated in the MFD driver. So all the dirty details will be in the MFD driver and not in ACPI core.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > Many thanks for your response and your help here. > > I guess that as conclusion with respect to the current v9 patchset we can > disregard the idea of MFD and modify the current v9 so that it doesn't > touch directly ACPI resources. > Instead as I proposed before we can have the scan handler to 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...? > > Do you think it as a viable solution? No, I think MFD + scan handler inside the MFD driver is the way to go. We don't want to trash ACPI core with stuff that does not belong there IMHO. Also you don't need to modify acpi_default_enumeration() because you can mark your device enumerated in the MFD driver. So all the dirty details will be in the MFD driver and not in ACPI core.
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Rafael, Mika, Lorenzo > -Original Message- > From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of > Rafael J. Wysocki > Sent: 16 June 2017 13:23 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Gabriele Paoloni; Lorenzo Pieralisi; Rafael J. > Wysocki; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerberg > <mika.westerb...@linux.intel.com> wrote: > > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > >> > In fact it may be that it is not sufficient in this case because > the > >> > ACPI core might enumerate child devices before the LPC driver even > gets > >> > a chance to probe so you would need to add also scan handler to > the > >> > child devices and mark them already enumerated or something like > that. > >> > >> Or extend the special I2C/SPI handling to them. > > > > Sure but those have I2c/SpiSerialBus() resources which we can use to > > identify them but for the ipmi thing there is nothing else than _HID > so > > we would need to keep a list of such devices in ACPI core. > > OK, so adding a scan handler for that would be the way to go IMO. Many thanks for your response and your help here. I guess that as conclusion with respect to the current v9 patchset we can disregard the idea of MFD and modify the current v9 so that it doesn't touch directly ACPI resources. Instead as I proposed before we can have the scan handler to 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...? Do you think it as a viable solution? Thanks Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Rafael, Mika, Lorenzo > -Original Message- > From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of > Rafael J. Wysocki > Sent: 16 June 2017 13:23 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Gabriele Paoloni; Lorenzo Pieralisi; Rafael J. > Wysocki; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerberg > wrote: > > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > >> > In fact it may be that it is not sufficient in this case because > the > >> > ACPI core might enumerate child devices before the LPC driver even > gets > >> > a chance to probe so you would need to add also scan handler to > the > >> > child devices and mark them already enumerated or something like > that. > >> > >> Or extend the special I2C/SPI handling to them. > > > > Sure but those have I2c/SpiSerialBus() resources which we can use to > > identify them but for the ipmi thing there is nothing else than _HID > so > > we would need to keep a list of such devices in ACPI core. > > OK, so adding a scan handler for that would be the way to go IMO. Many thanks for your response and your help here. I guess that as conclusion with respect to the current v9 patchset we can disregard the idea of MFD and modify the current v9 so that it doesn't touch directly ACPI resources. Instead as I proposed before we can have the scan handler to 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...? Do you think it as a viable solution? Thanks Gab
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerbergwrote: > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: >> > In fact it may be that it is not sufficient in this case because the >> > ACPI core might enumerate child devices before the LPC driver even gets >> > a chance to probe so you would need to add also scan handler to the >> > child devices and mark them already enumerated or something like that. >> >> Or extend the special I2C/SPI handling to them. > > Sure but those have I2c/SpiSerialBus() resources which we can use to > identify them but for the ipmi thing there is nothing else than _HID so > we would need to keep a list of such devices in ACPI core. OK, so adding a scan handler for that would be the way to go IMO.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerberg wrote: > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: >> > In fact it may be that it is not sufficient in this case because the >> > ACPI core might enumerate child devices before the LPC driver even gets >> > a chance to probe so you would need to add also scan handler to the >> > child devices and mark them already enumerated or something like that. >> >> Or extend the special I2C/SPI handling to them. > > Sure but those have I2c/SpiSerialBus() resources which we can use to > identify them but for the ipmi thing there is nothing else than _HID so > we would need to keep a list of such devices in ACPI core. OK, so adding a scan handler for that would be the way to go IMO.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > > In fact it may be that it is not sufficient in this case because the > > ACPI core might enumerate child devices before the LPC driver even gets > > a chance to probe so you would need to add also scan handler to the > > child devices and mark them already enumerated or something like that. > > Or extend the special I2C/SPI handling to them. Sure but those have I2c/SpiSerialBus() resources which we can use to identify them but for the ipmi thing there is nothing else than _HID so we would need to keep a list of such devices in ACPI core.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > > In fact it may be that it is not sufficient in this case because the > > ACPI core might enumerate child devices before the LPC driver even gets > > a chance to probe so you would need to add also scan handler to the > > child devices and mark them already enumerated or something like that. > > Or extend the special I2C/SPI handling to them. Sure but those have I2c/SpiSerialBus() resources which we can use to identify them but for the ipmi thing there is nothing else than _HID so we would need to keep a list of such devices in ACPI core.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 10:33 AM, Mika Westerberg <mika.westerb...@linux.intel.com> wrote: > On Thu, Jun 15, 2017 at 06:01:02PM +, Gabriele Paoloni wrote: >> Hi Mika >> >> > -Original Message- >> > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- >> > ow...@vger.kernel.org] On Behalf Of Mika Westerberg >> > Sent: 13 June 2017 21:04 >> > To: Gabriele Paoloni >> > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; >> > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; >> > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- >> > ker...@lists.infradead.org; mark.rutl...@arm.com; >> > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- >> > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- >> > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) >> > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO >> > devices before scanning >> > >> > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: >> > > I am not very familiar with Linux MFD however the main issue here is >> > that >> > > 1) for IPMI we want to re-use the standard IPMI driver without >> > touching it: >> > >see >> > > >> > >static const struct acpi_device_id acpi_ipmi_match[] = { >> > > { "IPI0001", 0 }, >> > > { }, >> > >}; >> > > >> > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard >> > driver >> > >of an LPC child) >> > > >> > > 2) We need a way to guarantee that all LPC children are not >> > enumerated >> > >by acpi_default_enumeration() (so for example if an ipmi node is >> > an LPC# >> > >child it should not be enumerated, otherwise it should be) >> > >Currently acpi_default_enumeration() skips spi/i2c slaves by >> > checking: >> > >1) if the acpi resource type is a serial bus >> > >2) if the type of serial bus descriptor is I2C or SPI >> > > >> > >For LPC we cannot leverage on any ACPI property to "recognize" >> > that our >> > >devices are LPC children; hence before I proposed for >> > acpi_default_enumeration() >> > >to skip devices that have already been enumerated (by calling >> > >acpi_device_enumerated() ). >> > > >> > > So in the current scenario, how do you think that MFD can help? >> > >> > If you look at Documentation/acpi/enumeration.txt there is a chapter >> > "MFD devices". I think it pretty much maches what you have here. An LPC >> > device (MFD device) and bunch of child devices. The driver for your LPC >> > device can specify _HID for each child device. Those are then mached by >> > the MFD ACPI code to the corresponding ACPI nodes from which platform >> > devices are created including "IPI0001". >> >> So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: >> >> static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { >> .pnpid = "IPI0001", >> }; >> >> correct? > > Yes. > >> > >> > It causes acpi_default_enumeration() to be called but it should be fine >> > as we are dealing with platform device anyway. >> >> I do not quite understand how declaring such MFD cell above would make sure >> that the LPC probe is called before the IPMI device is enumerated... > > In fact it may be that it is not sufficient in this case because the > ACPI core might enumerate child devices before the LPC driver even gets > a chance to probe so you would need to add also scan handler to the > child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. Thanks, Rafael
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Fri, Jun 16, 2017 at 10:33 AM, Mika Westerberg wrote: > On Thu, Jun 15, 2017 at 06:01:02PM +, Gabriele Paoloni wrote: >> Hi Mika >> >> > -Original Message- >> > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- >> > ow...@vger.kernel.org] On Behalf Of Mika Westerberg >> > Sent: 13 June 2017 21:04 >> > To: Gabriele Paoloni >> > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; >> > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; >> > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- >> > ker...@lists.infradead.org; mark.rutl...@arm.com; >> > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- >> > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- >> > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) >> > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO >> > devices before scanning >> > >> > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: >> > > I am not very familiar with Linux MFD however the main issue here is >> > that >> > > 1) for IPMI we want to re-use the standard IPMI driver without >> > touching it: >> > >see >> > > >> > >static const struct acpi_device_id acpi_ipmi_match[] = { >> > > { "IPI0001", 0 }, >> > > { }, >> > >}; >> > > >> > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard >> > driver >> > >of an LPC child) >> > > >> > > 2) We need a way to guarantee that all LPC children are not >> > enumerated >> > >by acpi_default_enumeration() (so for example if an ipmi node is >> > an LPC# >> > >child it should not be enumerated, otherwise it should be) >> > >Currently acpi_default_enumeration() skips spi/i2c slaves by >> > checking: >> > >1) if the acpi resource type is a serial bus >> > >2) if the type of serial bus descriptor is I2C or SPI >> > > >> > >For LPC we cannot leverage on any ACPI property to "recognize" >> > that our >> > >devices are LPC children; hence before I proposed for >> > acpi_default_enumeration() >> > >to skip devices that have already been enumerated (by calling >> > >acpi_device_enumerated() ). >> > > >> > > So in the current scenario, how do you think that MFD can help? >> > >> > If you look at Documentation/acpi/enumeration.txt there is a chapter >> > "MFD devices". I think it pretty much maches what you have here. An LPC >> > device (MFD device) and bunch of child devices. The driver for your LPC >> > device can specify _HID for each child device. Those are then mached by >> > the MFD ACPI code to the corresponding ACPI nodes from which platform >> > devices are created including "IPI0001". >> >> So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: >> >> static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { >> .pnpid = "IPI0001", >> }; >> >> correct? > > Yes. > >> > >> > It causes acpi_default_enumeration() to be called but it should be fine >> > as we are dealing with platform device anyway. >> >> I do not quite understand how declaring such MFD cell above would make sure >> that the LPC probe is called before the IPMI device is enumerated... > > In fact it may be that it is not sufficient in this case because the > ACPI core might enumerate child devices before the LPC driver even gets > a chance to probe so you would need to add also scan handler to the > child devices and mark them already enumerated or something like that. Or extend the special I2C/SPI handling to them. Thanks, Rafael
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Thu, Jun 15, 2017 at 06:01:02PM +, Gabriele Paoloni wrote: > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Mika Westerberg > > Sent: 13 June 2017 21:04 > > To: Gabriele Paoloni > > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > > > I am not very familiar with Linux MFD however the main issue here is > > that > > > 1) for IPMI we want to re-use the standard IPMI driver without > > touching it: > > >see > > > > > >static const struct acpi_device_id acpi_ipmi_match[] = { > > > { "IPI0001", 0 }, > > > { }, > > >}; > > > > > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard > > driver > > >of an LPC child) > > > > > > 2) We need a way to guarantee that all LPC children are not > > enumerated > > >by acpi_default_enumeration() (so for example if an ipmi node is > > an LPC# > > >child it should not be enumerated, otherwise it should be) > > >Currently acpi_default_enumeration() skips spi/i2c slaves by > > checking: > > >1) if the acpi resource type is a serial bus > > >2) if the type of serial bus descriptor is I2C or SPI > > > > > >For LPC we cannot leverage on any ACPI property to "recognize" > > that our > > >devices are LPC children; hence before I proposed for > > acpi_default_enumeration() > > >to skip devices that have already been enumerated (by calling > > >acpi_device_enumerated() ). > > > > > > So in the current scenario, how do you think that MFD can help? > > > > If you look at Documentation/acpi/enumeration.txt there is a chapter > > "MFD devices". I think it pretty much maches what you have here. An LPC > > device (MFD device) and bunch of child devices. The driver for your LPC > > device can specify _HID for each child device. Those are then mached by > > the MFD ACPI code to the corresponding ACPI nodes from which platform > > devices are created including "IPI0001". > > So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: > > static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { > .pnpid = "IPI0001", > }; > > correct? Yes. > > > > It causes acpi_default_enumeration() to be called but it should be fine > > as we are dealing with platform device anyway. > > I do not quite understand how declaring such MFD cell above would make sure > that the LPC probe is called before the IPMI device is enumerated... In fact it may be that it is not sufficient in this case because the ACPI core might enumerate child devices before the LPC driver even gets a chance to probe so you would need to add also scan handler to the child devices and mark them already enumerated or something like that. > Could you point me to the code that does this? Check for example drivers/mfd/intel-lpss.c.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Thu, Jun 15, 2017 at 06:01:02PM +, Gabriele Paoloni wrote: > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Mika Westerberg > > Sent: 13 June 2017 21:04 > > To: Gabriele Paoloni > > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > > > I am not very familiar with Linux MFD however the main issue here is > > that > > > 1) for IPMI we want to re-use the standard IPMI driver without > > touching it: > > >see > > > > > >static const struct acpi_device_id acpi_ipmi_match[] = { > > > { "IPI0001", 0 }, > > > { }, > > >}; > > > > > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard > > driver > > >of an LPC child) > > > > > > 2) We need a way to guarantee that all LPC children are not > > enumerated > > >by acpi_default_enumeration() (so for example if an ipmi node is > > an LPC# > > >child it should not be enumerated, otherwise it should be) > > >Currently acpi_default_enumeration() skips spi/i2c slaves by > > checking: > > >1) if the acpi resource type is a serial bus > > >2) if the type of serial bus descriptor is I2C or SPI > > > > > >For LPC we cannot leverage on any ACPI property to "recognize" > > that our > > >devices are LPC children; hence before I proposed for > > acpi_default_enumeration() > > >to skip devices that have already been enumerated (by calling > > >acpi_device_enumerated() ). > > > > > > So in the current scenario, how do you think that MFD can help? > > > > If you look at Documentation/acpi/enumeration.txt there is a chapter > > "MFD devices". I think it pretty much maches what you have here. An LPC > > device (MFD device) and bunch of child devices. The driver for your LPC > > device can specify _HID for each child device. Those are then mached by > > the MFD ACPI code to the corresponding ACPI nodes from which platform > > devices are created including "IPI0001". > > So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: > > static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { > .pnpid = "IPI0001", > }; > > correct? Yes. > > > > It causes acpi_default_enumeration() to be called but it should be fine > > as we are dealing with platform device anyway. > > I do not quite understand how declaring such MFD cell above would make sure > that the LPC probe is called before the IPMI device is enumerated... In fact it may be that it is not sufficient in this case because the ACPI core might enumerate child devices before the LPC driver even gets a chance to probe so you would need to add also scan handler to the child devices and mark them already enumerated or something like that. > Could you point me to the code that does this? Check for example drivers/mfd/intel-lpss.c.
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Mika Westerberg > Sent: 13 June 2017 21:04 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > > I am not very familiar with Linux MFD however the main issue here is > that > > 1) for IPMI we want to re-use the standard IPMI driver without > touching it: > >see > > > >static const struct acpi_device_id acpi_ipmi_match[] = { > > { "IPI0001", 0 }, > > { }, > >}; > > > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard > driver > >of an LPC child) > > > > 2) We need a way to guarantee that all LPC children are not > enumerated > >by acpi_default_enumeration() (so for example if an ipmi node is > an LPC# > >child it should not be enumerated, otherwise it should be) > >Currently acpi_default_enumeration() skips spi/i2c slaves by > checking: > >1) if the acpi resource type is a serial bus > >2) if the type of serial bus descriptor is I2C or SPI > > > >For LPC we cannot leverage on any ACPI property to "recognize" > that our > >devices are LPC children; hence before I proposed for > acpi_default_enumeration() > >to skip devices that have already been enumerated (by calling > >acpi_device_enumerated() ). > > > > So in the current scenario, how do you think that MFD can help? > > If you look at Documentation/acpi/enumeration.txt there is a chapter > "MFD devices". I think it pretty much maches what you have here. An LPC > device (MFD device) and bunch of child devices. The driver for your LPC > device can specify _HID for each child device. Those are then mached by > the MFD ACPI code to the corresponding ACPI nodes from which platform > devices are created including "IPI0001". So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { .pnpid = "IPI0001", }; correct? > > It causes acpi_default_enumeration() to be called but it should be fine > as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... Could you point me to the code that does this? Many Thanks Gab > > Once the platform device is created your ipmi driver will be probed by > the driver core. > > Does that make sense?
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Mika Westerberg > Sent: 13 June 2017 21:04 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > > I am not very familiar with Linux MFD however the main issue here is > that > > 1) for IPMI we want to re-use the standard IPMI driver without > touching it: > >see > > > >static const struct acpi_device_id acpi_ipmi_match[] = { > > { "IPI0001", 0 }, > > { }, > >}; > > > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard > driver > >of an LPC child) > > > > 2) We need a way to guarantee that all LPC children are not > enumerated > >by acpi_default_enumeration() (so for example if an ipmi node is > an LPC# > >child it should not be enumerated, otherwise it should be) > >Currently acpi_default_enumeration() skips spi/i2c slaves by > checking: > >1) if the acpi resource type is a serial bus > >2) if the type of serial bus descriptor is I2C or SPI > > > >For LPC we cannot leverage on any ACPI property to "recognize" > that our > >devices are LPC children; hence before I proposed for > acpi_default_enumeration() > >to skip devices that have already been enumerated (by calling > >acpi_device_enumerated() ). > > > > So in the current scenario, how do you think that MFD can help? > > If you look at Documentation/acpi/enumeration.txt there is a chapter > "MFD devices". I think it pretty much maches what you have here. An LPC > device (MFD device) and bunch of child devices. The driver for your LPC > device can specify _HID for each child device. Those are then mached by > the MFD ACPI code to the corresponding ACPI nodes from which platform > devices are created including "IPI0001". So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.: static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = { .pnpid = "IPI0001", }; correct? > > It causes acpi_default_enumeration() to be called but it should be fine > as we are dealing with platform device anyway. I do not quite understand how declaring such MFD cell above would make sure that the LPC probe is called before the IPMI device is enumerated... Could you point me to the code that does this? Many Thanks Gab > > Once the platform device is created your ipmi driver will be probed by > the driver core. > > Does that make sense?
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > I am not very familiar with Linux MFD however the main issue here is that > 1) for IPMI we want to re-use the standard IPMI driver without touching it: >see > >static const struct acpi_device_id acpi_ipmi_match[] = { > { "IPI0001", 0 }, > { }, >}; > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver >of an LPC child) > > 2) We need a way to guarantee that all LPC children are not enumerated >by acpi_default_enumeration() (so for example if an ipmi node is an LPC# >child it should not be enumerated, otherwise it should be) >Currently acpi_default_enumeration() skips spi/i2c slaves by checking: >1) if the acpi resource type is a serial bus >2) if the type of serial bus descriptor is I2C or SPI > >For LPC we cannot leverage on any ACPI property to "recognize" that our >devices are LPC children; hence before I proposed for > acpi_default_enumeration() >to skip devices that have already been enumerated (by calling >acpi_device_enumerated() ). > > So in the current scenario, how do you think that MFD can help? If you look at Documentation/acpi/enumeration.txt there is a chapter "MFD devices". I think it pretty much maches what you have here. An LPC device (MFD device) and bunch of child devices. The driver for your LPC device can specify _HID for each child device. Those are then mached by the MFD ACPI code to the corresponding ACPI nodes from which platform devices are created including "IPI0001". It causes acpi_default_enumeration() to be called but it should be fine as we are dealing with platform device anyway. Once the platform device is created your ipmi driver will be probed by the driver core. Does that make sense?
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote: > I am not very familiar with Linux MFD however the main issue here is that > 1) for IPMI we want to re-use the standard IPMI driver without touching it: >see > >static const struct acpi_device_id acpi_ipmi_match[] = { > { "IPI0001", 0 }, > { }, >}; > >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver >of an LPC child) > > 2) We need a way to guarantee that all LPC children are not enumerated >by acpi_default_enumeration() (so for example if an ipmi node is an LPC# >child it should not be enumerated, otherwise it should be) >Currently acpi_default_enumeration() skips spi/i2c slaves by checking: >1) if the acpi resource type is a serial bus >2) if the type of serial bus descriptor is I2C or SPI > >For LPC we cannot leverage on any ACPI property to "recognize" that our >devices are LPC children; hence before I proposed for > acpi_default_enumeration() >to skip devices that have already been enumerated (by calling >acpi_device_enumerated() ). > > So in the current scenario, how do you think that MFD can help? If you look at Documentation/acpi/enumeration.txt there is a chapter "MFD devices". I think it pretty much maches what you have here. An LPC device (MFD device) and bunch of child devices. The driver for your LPC device can specify _HID for each child device. Those are then mached by the MFD ACPI code to the corresponding ACPI nodes from which platform devices are created including "IPI0001". It causes acpi_default_enumeration() to be called but it should be fine as we are dealing with platform device anyway. Once the platform device is created your ipmi driver will be probed by the driver core. Does that make sense?
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 13 June 2017 16:10 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote: > > > Is there an example ASL showing how these LPC devices are > > > currently presented in ACPI? > > > > Please find below the asl sketch for our LPC and IPMI > > > > // > > // LPC > > // > > > > Scope(_SB) { > > Device (LPC0) { > > Name (_HID, "HISI0191") // HiSi LPC > > Name (_CRS, ResourceTemplate () { > > Memory32Fixed (ReadWrite, 0xa01b, 0x1000) > > }) > > } > > > > Device (LPC0.IPMI) { > > Name (_HID, "IPI0001") > > Method (_IFT) { > > Return (0x03) > > } > > Name (LORS, ResourceTemplate() { > > QWordIO ( > > ResourceConsumer, > > MinNotFixed, // _MIF > > MaxNotFixed, // _MAF > > PosDecode, > > EntireRange, > > 0x0, // _GRA > > 0xe4,// _MIN > > 0x3fff, // _MAX > > 0x0, // _TRA > > 0x04,// _LEN > > , , > > BTIO > > ) > > }) > > CreateQWordField (LORS, BTIO._MIN, CMIN) > > CreateQWordField (LORS, BTIO._MAX, CMAX) > > CreateQWordField (LORS, BTIO._LEN, CLEN) > > > > Method (_PRS, 0) { > > Return (LORS) > > } > > > > Method (_CRS, 0) { > > Return (LORS) > > } > > Method (_SRS, 1) { > > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) > > Store (IMIN, CMIN) > > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) > > Store (IMAX, CMAX) > > } > > } > > [...] > > } > > Thanks. So this looks pretty much like normal Linux MFD device which we > already have support for adding ACPI bindings to child devices. It > should also support splitting resources to child devices IIRC. I am not very familiar with Linux MFD however the main issue here is that 1) for IPMI we want to re-use the standard IPMI driver without touching it: see static const struct acpi_device_id acpi_ipmi_match[] = { { "IPI0001", 0 }, { }, }; in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver of an LPC child) 2) We need a way to guarantee that all LPC children are not enumerated by acpi_default_enumeration() (so for example if an ipmi node is an LPC# child it should not be enumerated, otherwise it should be) Currently acpi_default_enumeration() skips spi/i2c slaves by checking: 1) if the acpi resource type is a serial bus 2) if the type of serial bus descriptor is I2C or SPI For LPC we cannot leverage on any ACPI property to "recognize" that our devices are LPC children; hence before I proposed for acpi_default_enumeration() to skip devices that have already been enumerated (by calling acpi_device_enumerated() ). So in the current scenario, how do you think that MFD can help? Do you see any possible solution? Many thanks Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 13 June 2017 16:10 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote: > > > Is there an example ASL showing how these LPC devices are > > > currently presented in ACPI? > > > > Please find below the asl sketch for our LPC and IPMI > > > > // > > // LPC > > // > > > > Scope(_SB) { > > Device (LPC0) { > > Name (_HID, "HISI0191") // HiSi LPC > > Name (_CRS, ResourceTemplate () { > > Memory32Fixed (ReadWrite, 0xa01b, 0x1000) > > }) > > } > > > > Device (LPC0.IPMI) { > > Name (_HID, "IPI0001") > > Method (_IFT) { > > Return (0x03) > > } > > Name (LORS, ResourceTemplate() { > > QWordIO ( > > ResourceConsumer, > > MinNotFixed, // _MIF > > MaxNotFixed, // _MAF > > PosDecode, > > EntireRange, > > 0x0, // _GRA > > 0xe4,// _MIN > > 0x3fff, // _MAX > > 0x0, // _TRA > > 0x04,// _LEN > > , , > > BTIO > > ) > > }) > > CreateQWordField (LORS, BTIO._MIN, CMIN) > > CreateQWordField (LORS, BTIO._MAX, CMAX) > > CreateQWordField (LORS, BTIO._LEN, CLEN) > > > > Method (_PRS, 0) { > > Return (LORS) > > } > > > > Method (_CRS, 0) { > > Return (LORS) > > } > > Method (_SRS, 1) { > > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) > > Store (IMIN, CMIN) > > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) > > Store (IMAX, CMAX) > > } > > } > > [...] > > } > > Thanks. So this looks pretty much like normal Linux MFD device which we > already have support for adding ACPI bindings to child devices. It > should also support splitting resources to child devices IIRC. I am not very familiar with Linux MFD however the main issue here is that 1) for IPMI we want to re-use the standard IPMI driver without touching it: see static const struct acpi_device_id acpi_ipmi_match[] = { { "IPI0001", 0 }, { }, }; in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver of an LPC child) 2) We need a way to guarantee that all LPC children are not enumerated by acpi_default_enumeration() (so for example if an ipmi node is an LPC# child it should not be enumerated, otherwise it should be) Currently acpi_default_enumeration() skips spi/i2c slaves by checking: 1) if the acpi resource type is a serial bus 2) if the type of serial bus descriptor is I2C or SPI For LPC we cannot leverage on any ACPI property to "recognize" that our devices are LPC children; hence before I proposed for acpi_default_enumeration() to skip devices that have already been enumerated (by calling acpi_device_enumerated() ). So in the current scenario, how do you think that MFD can help? Do you see any possible solution? Many thanks Gab
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote: > > Is there an example ASL showing how these LPC devices are > > currently presented in ACPI? > > Please find below the asl sketch for our LPC and IPMI > > // > // LPC > // > > Scope(_SB) { > Device (LPC0) { > Name (_HID, "HISI0191") // HiSi LPC > Name (_CRS, ResourceTemplate () { > Memory32Fixed (ReadWrite, 0xa01b, 0x1000) > }) > } > > Device (LPC0.IPMI) { > Name (_HID, "IPI0001") > Method (_IFT) { > Return (0x03) > } > Name (LORS, ResourceTemplate() { > QWordIO ( > ResourceConsumer, > MinNotFixed, // _MIF > MaxNotFixed, // _MAF > PosDecode, > EntireRange, > 0x0, // _GRA > 0xe4,// _MIN > 0x3fff, // _MAX > 0x0, // _TRA > 0x04,// _LEN > , , > BTIO > ) > }) > CreateQWordField (LORS, BTIO._MIN, CMIN) > CreateQWordField (LORS, BTIO._MAX, CMAX) > CreateQWordField (LORS, BTIO._LEN, CLEN) > > Method (_PRS, 0) { > Return (LORS) > } > > Method (_CRS, 0) { > Return (LORS) > } > Method (_SRS, 1) { > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) > Store (IMIN, CMIN) > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) > Store (IMAX, CMAX) > } > } > [...] > } Thanks. So this looks pretty much like normal Linux MFD device which we already have support for adding ACPI bindings to child devices. It should also support splitting resources to child devices IIRC.
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote: > > Is there an example ASL showing how these LPC devices are > > currently presented in ACPI? > > Please find below the asl sketch for our LPC and IPMI > > // > // LPC > // > > Scope(_SB) { > Device (LPC0) { > Name (_HID, "HISI0191") // HiSi LPC > Name (_CRS, ResourceTemplate () { > Memory32Fixed (ReadWrite, 0xa01b, 0x1000) > }) > } > > Device (LPC0.IPMI) { > Name (_HID, "IPI0001") > Method (_IFT) { > Return (0x03) > } > Name (LORS, ResourceTemplate() { > QWordIO ( > ResourceConsumer, > MinNotFixed, // _MIF > MaxNotFixed, // _MAF > PosDecode, > EntireRange, > 0x0, // _GRA > 0xe4,// _MIN > 0x3fff, // _MAX > 0x0, // _TRA > 0x04,// _LEN > , , > BTIO > ) > }) > CreateQWordField (LORS, BTIO._MIN, CMIN) > CreateQWordField (LORS, BTIO._MAX, CMAX) > CreateQWordField (LORS, BTIO._LEN, CLEN) > > Method (_PRS, 0) { > Return (LORS) > } > > Method (_CRS, 0) { > Return (LORS) > } > Method (_SRS, 1) { > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) > Store (IMIN, CMIN) > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) > Store (IMAX, CMAX) > } > } > [...] > } Thanks. So this looks pretty much like normal Linux MFD device which we already have support for adding ACPI bindings to child devices. It should also support splitting resources to child devices IIRC.
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 13 June 2017 09:49 > To: Lorenzo Pieralisi > Cc: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote: > > 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) > > (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) > > > > 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). > > Is there an example ASL showing how these LPC devices are > currently presented in ACPI? Please find below the asl sketch for our LPC and IPMI // // LPC // Scope(_SB) { Device (LPC0) { Name (_HID, "HISI0191") // HiSi LPC Name (_CRS, ResourceTemplate () { Memory32Fixed (ReadWrite, 0xa01b, 0x1000) }) } Device (LPC0.IPMI) { Name (_HID, "IPI0001") Method (_IFT) { Return (0x03) } Name (LORS, ResourceTemplate() { QWordIO ( ResourceConsumer, MinNotFixed, // _MIF MaxNotFixed, // _MAF PosDecode, EntireRange, 0x0, // _GRA 0xe4,// _MIN 0x3fff, // _MAX 0x0, // _TRA 0x04,// _LEN , , BTIO ) }) CreateQWordField (LORS, BTIO._MIN, CMIN) CreateQWordField (LORS, BTIO._MAX, CMAX) CreateQWordField (LORS, BTIO._LEN, CLEN) Method (_PRS, 0) { Return (LORS) } Method (_CRS, 0) { Return (LORS) } Method (_SRS, 1) { CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) Store (IMIN, CMIN) CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) Store (IMAX, CMAX) } } [...] } Many thanks Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 13 June 2017 09:49 > To: Lorenzo Pieralisi > Cc: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote: > > 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) > > (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) > > > > 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). > > Is there an example ASL showing how these LPC devices are > currently presented in ACPI? Please find below the asl sketch for our LPC and IPMI // // LPC // Scope(_SB) { Device (LPC0) { Name (_HID, "HISI0191") // HiSi LPC Name (_CRS, ResourceTemplate () { Memory32Fixed (ReadWrite, 0xa01b, 0x1000) }) } Device (LPC0.IPMI) { Name (_HID, "IPI0001") Method (_IFT) { Return (0x03) } Name (LORS, ResourceTemplate() { QWordIO ( ResourceConsumer, MinNotFixed, // _MIF MaxNotFixed, // _MAF PosDecode, EntireRange, 0x0, // _GRA 0xe4,// _MIN 0x3fff, // _MAX 0x0, // _TRA 0x04,// _LEN , , BTIO ) }) CreateQWordField (LORS, BTIO._MIN, CMIN) CreateQWordField (LORS, BTIO._MAX, CMAX) CreateQWordField (LORS, BTIO._LEN, CLEN) Method (_PRS, 0) { Return (LORS) } Method (_CRS, 0) { Return (LORS) } Method (_SRS, 1) { CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN) Store (IMIN, CMIN) CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX) Store (IMAX, CMAX) } } [...] } Many thanks Gab
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote: > 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) > (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) > > 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). Is there an example ASL showing how these LPC devices are currently presented in ACPI?
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote: > 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) > (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) > > 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). Is there an example ASL showing how these LPC devices are currently presented in ACPI?
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo, Rafael > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 12 June 2017 16:57 > To: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; Mika > Westerberg > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; 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 +, 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(_device_handler); > > > > > > > > > > diff
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo, Rafael > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 12 June 2017 16:57 > To: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; Mika > Westerberg > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; 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 +, 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(_device_handler); > > > > > > > > > > diff --
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 +, 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) (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) 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). 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(_device_handler); > > > > > > > > diff --git a/include/acpi/acpi_indirect_pio.h > > > b/include/acpi/acpi_indirect_pio.h > > > > new file mode 100644 > > > > index 000..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> > > > + * Author: Zhichang Yuan > > > > + * > > > > + * 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 > > > > > > > >
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 +, 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) (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) 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). 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(_device_handler); > > > > > > > > diff --git a/include/acpi/acpi_indirect_pio.h > > > b/include/acpi/acpi_indirect_pio.h > > > > new file mode 100644 > > > > index 000..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 > > > > + * Author: Zhichang Yuan > > > > + * > > > > + * 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 > > > > > > > >
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gab, Rafael, On Wed, May 31, 2017 at 10:24:47AM +, 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. Thoughts ? Thanks, Lorenzo > > > acpi_scan_add_handler(_device_handler); > > > > > > diff --git a/include/acpi/acpi_indirect_pio.h > > b/include/acpi/acpi_indirect_pio.h > > > new file mode 100644 > > > index 000..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> > > + * Author: Zhichang Yuan > > > + * > > > + * 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 > > > > > >
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gab, Rafael, On Wed, May 31, 2017 at 10:24:47AM +, 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. Thoughts ? Thanks, Lorenzo > > > acpi_scan_add_handler(_device_handler); > > > > > > diff --git a/include/acpi/acpi_indirect_pio.h > > b/include/acpi/acpi_indirect_pio.h > > > new file mode 100644 > > > index 000..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 > > > + * Author: Zhichang Yuan > > > + * > > > + * 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 > > > > > >
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo Many thanks for reviewing > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 30 May 2017 14:24 > To: Gabriele Paoloni > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Gab, > > On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote: > > From: Gabriele <gabriele.paol...@huawei.com> > > > > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices > access > > I/O with some special host-local I/O ports known on x86. As their I/O > > space are not memory mapped like PCI/PCIE MMIO host bridges, this > patch is > > meant to support a new class of I/O host controllers where the local > IO > > ports of the children devices are translated into the Indirect I/O > address > > space. > > Through the handler attach callback, all the I/O translations are > done > > before starting the enumeration on children devices and the > translated > > addresses are replaced in the children resources. > > I do not understand why this is done through a scan handler and to > be frank I do not see how this mechanism is supposed to be a generic > kernel layer, possibly used by other platforms, when there is no notion > in ACPI to cater for that. Well, the main reason is that we need to translate the bus addresses of the LPC children before the respective children's drivers probe. > > As far as I am concerned this patch code should live in the Hisilicon > LPC driver - as things stand it is neither ACPI generic code nor ARM64 > specific, it is code to make ACPI work like DT bindings without any > ACPI > binding at all; I still have some concerns from an ACPI standpoint > below. Well the reason why this patch cannot live in the LPC driver probe is that AFAIK currently there is no way to guarantee the LPC probe to be called before the children probe. With respect to the ACPI concerns please see below. > > > Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com> > > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> > > --- > > drivers/acpi/arm64/Makefile| 1 + > > drivers/acpi/arm64/acpi_indirect_pio.c | 301 > + > > drivers/acpi/internal.h| 5 + > > drivers/acpi/scan.c| 1 + > > include/acpi/acpi_indirect_pio.h | 24 +++ > > 5 files changed, 332 insertions(+) > > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c > > create mode 100644 include/acpi/acpi_indirect_pio.h > > > > diff --git a/drivers/acpi/arm64/Makefile > b/drivers/acpi/arm64/Makefile > > index 1017def..3944775 100644 > > --- a/drivers/acpi/arm64/Makefile > > +++ b/drivers/acpi/arm64/Makefile > > @@ -1,2 +1,3 @@ > > obj-$(CONFIG_ACPI_IORT)+= iort.o > > obj-$(CONFIG_ACPI_GTDT)+= gtdt.o > > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o > > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c > b/drivers/acpi/arm64/acpi_indirect_pio.c > > new file mode 100644 > > index 000..7813f73 > > --- /dev/null > > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c > > @@ -0,0 +1,301 @@ > > +/* > > + * ACPI support for indirect-PIO bus. > > + * > > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved. > > + * Author: Gabriele Paoloni <gabriele.paol...@huawei.com> > > + * Author: Zhichang Yuan <yuanzhich...@hisilicon.com> > > + * > > + * 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. > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include > > + > > +ACPI_MODULE_NAME("indirect PIO"); > > + > > +#define INDIRECT_PIO_INFO(desc) ((unsigned long)) > > + > > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res, > > + void *data) > > +{ > > + int *res_cnt = data; > > + > > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) > >
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo Many thanks for reviewing > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 30 May 2017 14:24 > To: Gabriele Paoloni > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Gab, > > On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote: > > From: Gabriele > > > > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices > access > > I/O with some special host-local I/O ports known on x86. As their I/O > > space are not memory mapped like PCI/PCIE MMIO host bridges, this > patch is > > meant to support a new class of I/O host controllers where the local > IO > > ports of the children devices are translated into the Indirect I/O > address > > space. > > Through the handler attach callback, all the I/O translations are > done > > before starting the enumeration on children devices and the > translated > > addresses are replaced in the children resources. > > I do not understand why this is done through a scan handler and to > be frank I do not see how this mechanism is supposed to be a generic > kernel layer, possibly used by other platforms, when there is no notion > in ACPI to cater for that. Well, the main reason is that we need to translate the bus addresses of the LPC children before the respective children's drivers probe. > > As far as I am concerned this patch code should live in the Hisilicon > LPC driver - as things stand it is neither ACPI generic code nor ARM64 > specific, it is code to make ACPI work like DT bindings without any > ACPI > binding at all; I still have some concerns from an ACPI standpoint > below. Well the reason why this patch cannot live in the LPC driver probe is that AFAIK currently there is no way to guarantee the LPC probe to be called before the children probe. With respect to the ACPI concerns please see below. > > > Signed-off-by: zhichang.yuan > > Signed-off-by: Gabriele Paoloni > > --- > > drivers/acpi/arm64/Makefile| 1 + > > drivers/acpi/arm64/acpi_indirect_pio.c | 301 > + > > drivers/acpi/internal.h| 5 + > > drivers/acpi/scan.c| 1 + > > include/acpi/acpi_indirect_pio.h | 24 +++ > > 5 files changed, 332 insertions(+) > > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c > > create mode 100644 include/acpi/acpi_indirect_pio.h > > > > diff --git a/drivers/acpi/arm64/Makefile > b/drivers/acpi/arm64/Makefile > > index 1017def..3944775 100644 > > --- a/drivers/acpi/arm64/Makefile > > +++ b/drivers/acpi/arm64/Makefile > > @@ -1,2 +1,3 @@ > > obj-$(CONFIG_ACPI_IORT)+= iort.o > > obj-$(CONFIG_ACPI_GTDT)+= gtdt.o > > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o > > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c > b/drivers/acpi/arm64/acpi_indirect_pio.c > > new file mode 100644 > > index 000..7813f73 > > --- /dev/null > > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c > > @@ -0,0 +1,301 @@ > > +/* > > + * ACPI support for indirect-PIO bus. > > + * > > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved. > > + * Author: Gabriele Paoloni > > + * Author: Zhichang Yuan > > + * > > + * 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. > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include > > + > > +ACPI_MODULE_NAME("indirect PIO"); > > + > > +#define INDIRECT_PIO_INFO(desc) ((unsigned long)) > > + > > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res, > > + void *data) > > +{ > > + int *res_cnt = data; > > + > > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) > > + (*res_cnt)++; > > + > > + return AE_OK; > > +} > > + > > +static acpi_status acpi_read_one_logicpiores(struct acpi_resource >
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gab, On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote: > From: Gabriele> > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access > I/O with some special host-local I/O ports known on x86. As their I/O > space are not memory mapped like PCI/PCIE MMIO host bridges, this patch is > meant to support a new class of I/O host controllers where the local IO > ports of the children devices are translated into the Indirect I/O address > space. > Through the handler attach callback, all the I/O translations are done > before starting the enumeration on children devices and the translated > addresses are replaced in the children resources. I do not understand why this is done through a scan handler and to be frank I do not see how this mechanism is supposed to be a generic kernel layer, possibly used by other platforms, when there is no notion in ACPI to cater for that. As far as I am concerned this patch code should live in the Hisilicon LPC driver - as things stand it is neither ACPI generic code nor ARM64 specific, it is code to make ACPI work like DT bindings without any ACPI binding at all; I still have some concerns from an ACPI standpoint below. > Signed-off-by: zhichang.yuan > Signed-off-by: Gabriele Paoloni > --- > drivers/acpi/arm64/Makefile| 1 + > drivers/acpi/arm64/acpi_indirect_pio.c | 301 > + > drivers/acpi/internal.h| 5 + > drivers/acpi/scan.c| 1 + > include/acpi/acpi_indirect_pio.h | 24 +++ > 5 files changed, 332 insertions(+) > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c > create mode 100644 include/acpi/acpi_indirect_pio.h > > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 1017def..3944775 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c > b/drivers/acpi/arm64/acpi_indirect_pio.c > new file mode 100644 > index 000..7813f73 > --- /dev/null > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c > @@ -0,0 +1,301 @@ > +/* > + * ACPI support for indirect-PIO bus. > + * > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved. > + * Author: Gabriele Paoloni > + * Author: Zhichang Yuan > + * > + * 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. > + */ > + > +#include > +#include > +#include > + > +#include > + > +ACPI_MODULE_NAME("indirect PIO"); > + > +#define INDIRECT_PIO_INFO(desc) ((unsigned long)) > + > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res, > +void *data) > +{ > + int *res_cnt = data; > + > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) > + (*res_cnt)++; > + > + return AE_OK; > +} > + > +static acpi_status acpi_read_one_logicpiores(struct acpi_resource *res, > + void *data) > +{ > + struct acpi_resource **resource = data; > + > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) { > + memcpy((*resource), res, sizeof(struct acpi_resource)); > + (*resource)->length = sizeof(struct acpi_resource); > + (*resource)->type = res->type; > + (*resource)++; > + } > + > + return AE_OK; > +} > + > +static acpi_status > +acpi_build_logicpiores_template(struct acpi_device *adev, > + struct acpi_buffer *buffer) > +{ > + acpi_handle handle = adev->handle; > + struct acpi_resource *resource; > + acpi_status status; > + int res_cnt = 0; > + > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + acpi_count_logic_iores, _cnt); > + if (ACPI_FAILURE(status)) { > + dev_err(>dev, "can't evaluate _CRS: %d\n", status); > + return -EINVAL; > + } > + > + if (!res_cnt) { > + dev_err(>dev, "no logic IO resources\n"); > + return -ENODEV; > + } > + > + buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1); > + buffer->pointer = kzalloc(buffer->length, GFP_KERNEL); > + if (!buffer->pointer) > + return -ENOMEM; > + > + resource = (struct acpi_resource *)buffer->pointer; > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + acpi_read_one_logicpiores, ); > + if (ACPI_FAILURE(status)) { > + kfree(buffer->pointer); > + dev_err(>dev, "can't evaluate _CRS: %d\n", status); > +
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gab, On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote: > From: Gabriele > > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access > I/O with some special host-local I/O ports known on x86. As their I/O > space are not memory mapped like PCI/PCIE MMIO host bridges, this patch is > meant to support a new class of I/O host controllers where the local IO > ports of the children devices are translated into the Indirect I/O address > space. > Through the handler attach callback, all the I/O translations are done > before starting the enumeration on children devices and the translated > addresses are replaced in the children resources. I do not understand why this is done through a scan handler and to be frank I do not see how this mechanism is supposed to be a generic kernel layer, possibly used by other platforms, when there is no notion in ACPI to cater for that. As far as I am concerned this patch code should live in the Hisilicon LPC driver - as things stand it is neither ACPI generic code nor ARM64 specific, it is code to make ACPI work like DT bindings without any ACPI binding at all; I still have some concerns from an ACPI standpoint below. > Signed-off-by: zhichang.yuan > Signed-off-by: Gabriele Paoloni > --- > drivers/acpi/arm64/Makefile| 1 + > drivers/acpi/arm64/acpi_indirect_pio.c | 301 > + > drivers/acpi/internal.h| 5 + > drivers/acpi/scan.c| 1 + > include/acpi/acpi_indirect_pio.h | 24 +++ > 5 files changed, 332 insertions(+) > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c > create mode 100644 include/acpi/acpi_indirect_pio.h > > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 1017def..3944775 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c > b/drivers/acpi/arm64/acpi_indirect_pio.c > new file mode 100644 > index 000..7813f73 > --- /dev/null > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c > @@ -0,0 +1,301 @@ > +/* > + * ACPI support for indirect-PIO bus. > + * > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved. > + * Author: Gabriele Paoloni > + * Author: Zhichang Yuan > + * > + * 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. > + */ > + > +#include > +#include > +#include > + > +#include > + > +ACPI_MODULE_NAME("indirect PIO"); > + > +#define INDIRECT_PIO_INFO(desc) ((unsigned long)) > + > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res, > +void *data) > +{ > + int *res_cnt = data; > + > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) > + (*res_cnt)++; > + > + return AE_OK; > +} > + > +static acpi_status acpi_read_one_logicpiores(struct acpi_resource *res, > + void *data) > +{ > + struct acpi_resource **resource = data; > + > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO)) { > + memcpy((*resource), res, sizeof(struct acpi_resource)); > + (*resource)->length = sizeof(struct acpi_resource); > + (*resource)->type = res->type; > + (*resource)++; > + } > + > + return AE_OK; > +} > + > +static acpi_status > +acpi_build_logicpiores_template(struct acpi_device *adev, > + struct acpi_buffer *buffer) > +{ > + acpi_handle handle = adev->handle; > + struct acpi_resource *resource; > + acpi_status status; > + int res_cnt = 0; > + > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + acpi_count_logic_iores, _cnt); > + if (ACPI_FAILURE(status)) { > + dev_err(>dev, "can't evaluate _CRS: %d\n", status); > + return -EINVAL; > + } > + > + if (!res_cnt) { > + dev_err(>dev, "no logic IO resources\n"); > + return -ENODEV; > + } > + > + buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1); > + buffer->pointer = kzalloc(buffer->length, GFP_KERNEL); > + if (!buffer->pointer) > + return -ENOMEM; > + > + resource = (struct acpi_resource *)buffer->pointer; > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + acpi_read_one_logicpiores, ); > + if (ACPI_FAILURE(status)) { > + kfree(buffer->pointer); > + dev_err(>dev, "can't evaluate _CRS: %d\n", status); > + return -EINVAL; > + } > + > + resource->type = ACPI_RESOURCE_TYPE_END_TAG; > + resource->length = sizeof(struct acpi_resource); > +
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gabriele, [auto build test ERROR on linus/master] [also build test ERROR on v4.12-rc2 next-20170525] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gabriele-Paoloni/LPC-legacy-ISA-I-O-support/20170526-033719 config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `acpi_scan_init': >> (.init.text+0x6742): undefined reference to `acpi_indirectio_scan_init' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Gabriele, [auto build test ERROR on linus/master] [also build test ERROR on v4.12-rc2 next-20170525] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gabriele-Paoloni/LPC-legacy-ISA-I-O-support/20170526-033719 config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `acpi_scan_init': >> (.init.text+0x6742): undefined reference to `acpi_indirectio_scan_init' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip