From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller Date: Fri, 13 May 2016 13:31:31 +0200 Message-ID: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-pci-owner@vger.kernel.org To: Jayachandran C Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Rafael Wysocki , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , Wangyijing , Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" List-Id: linux-acpi@vger.kernel.org On Fri, May 13, 2016 at 1:25 PM, Jayachandran C wrote: > On Tue, May 10, 2016 at 8:49 PM, Tomasz Nowicki wrote: >> This patch is going to implement generic PCI host controller for >> ACPI world, similar to what pci-host-generic.c driver does for DT world. >> >> All such drivers, which we have seen so far, were implemented within >> arch/ directory since they had some arch assumptions (x86 and ia64). >> However, they all are doing similar thing, so it makes sense to find >> some common code and abstract it into the generic driver. >> >> In order to handle PCI config space regions properly, we define new >> MCFG interface which does sanity checks on MCFG table and keeps its >> root pointer. User is able to lookup MCFG regions based on that root >> pointer and specified domain:bus_start:bus_end touple. We are using >> pci_mmcfg_late_init old prototype to avoid another function name. >> >> The implementation of pci_acpi_scan_root() looks up the MCFG entries >> and sets up a new mapping (regions are not mapped until host controller ask >> for it). Generic PCI functions are used for accessing config space. >> Driver selects PCI_ECAM and uses functions from drivers/pci/ecam.h >> to create and access ECAM mappings. >> >> As mentioned in Kconfig help section, ACPI_PCI_HOST_GENERIC choice >> should be made on a per-architecture basis. >> >> Signed-off-by: Tomasz Nowicki >> Signed-off-by: Jayachandran C >> --- > [....] > >> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h >> index 1ad2176..1cccf57 100644 >> --- a/drivers/pci/ecam.h >> +++ b/drivers/pci/ecam.h >> @@ -45,6 +45,11 @@ struct pci_config_window { >> void __iomem *win; /* 64-bit single mapping */ >> void __iomem **winp; /* 32-bit per bus mapping */ >> }; >> +#ifdef CONFIG_ACPI_PCI_HOST_GENERIC >> + struct acpi_device *companion; /* ACPI companion device */ >> +#endif >> + int domain; >> + >> }; > > Using struct pci_config_window to pass along domain and > companion looks bad. I think there are two possible options > to do this better: > > 1. add a 'struct fwnode_handle *' or 'struct device *parent_dev' > instead of the companion and domain fields above. In case of > ACPI either of them can be used to get the acpi_device and > both domain and companion can be set from that. > > 2. make pci_config_window fully embeddable by moving allocation > out of pci_ecam_create to its callers. Then it can be embedded > into acpi_pci_generic_root_info, and container_of can be used > to get acpi info from ->sysdata. > > The first option should be easier to implement but the second may > be better on long run. I would leave it to the Bjorn or Rafael to > suggest which is preferred. Personally, I'd probably try to use fwnode_handle, but the second option makes sense too in principle. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932109AbcEMLbf (ORCPT ); Fri, 13 May 2016 07:31:35 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:32980 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932066AbcEMLbd (ORCPT ); Fri, 13 May 2016 07:31:33 -0400 MIME-Version: 1.0 In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> Date: Fri, 13 May 2016 13:31:31 +0200 X-Google-Sender-Auth: Ig-UMIGEIW7ijCcGWJ1hD40pgII Message-ID: Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller From: "Rafael J. Wysocki" To: Jayachandran C Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Rafael Wysocki , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , Wangyijing , Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" , Jon Masters , Andrea Gallo , Duc Dang , jeremy.linton@arm.com, liudongdong3@huawei.com, Christopher Covington Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 13, 2016 at 1:25 PM, Jayachandran C wrote: > On Tue, May 10, 2016 at 8:49 PM, Tomasz Nowicki wrote: >> This patch is going to implement generic PCI host controller for >> ACPI world, similar to what pci-host-generic.c driver does for DT world. >> >> All such drivers, which we have seen so far, were implemented within >> arch/ directory since they had some arch assumptions (x86 and ia64). >> However, they all are doing similar thing, so it makes sense to find >> some common code and abstract it into the generic driver. >> >> In order to handle PCI config space regions properly, we define new >> MCFG interface which does sanity checks on MCFG table and keeps its >> root pointer. User is able to lookup MCFG regions based on that root >> pointer and specified domain:bus_start:bus_end touple. We are using >> pci_mmcfg_late_init old prototype to avoid another function name. >> >> The implementation of pci_acpi_scan_root() looks up the MCFG entries >> and sets up a new mapping (regions are not mapped until host controller ask >> for it). Generic PCI functions are used for accessing config space. >> Driver selects PCI_ECAM and uses functions from drivers/pci/ecam.h >> to create and access ECAM mappings. >> >> As mentioned in Kconfig help section, ACPI_PCI_HOST_GENERIC choice >> should be made on a per-architecture basis. >> >> Signed-off-by: Tomasz Nowicki >> Signed-off-by: Jayachandran C >> --- > [....] > >> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h >> index 1ad2176..1cccf57 100644 >> --- a/drivers/pci/ecam.h >> +++ b/drivers/pci/ecam.h >> @@ -45,6 +45,11 @@ struct pci_config_window { >> void __iomem *win; /* 64-bit single mapping */ >> void __iomem **winp; /* 32-bit per bus mapping */ >> }; >> +#ifdef CONFIG_ACPI_PCI_HOST_GENERIC >> + struct acpi_device *companion; /* ACPI companion device */ >> +#endif >> + int domain; >> + >> }; > > Using struct pci_config_window to pass along domain and > companion looks bad. I think there are two possible options > to do this better: > > 1. add a 'struct fwnode_handle *' or 'struct device *parent_dev' > instead of the companion and domain fields above. In case of > ACPI either of them can be used to get the acpi_device and > both domain and companion can be set from that. > > 2. make pci_config_window fully embeddable by moving allocation > out of pci_ecam_create to its callers. Then it can be embedded > into acpi_pci_generic_root_info, and container_of can be used > to get acpi info from ->sysdata. > > The first option should be easier to implement but the second may > be better on long run. I would leave it to the Bjorn or Rafael to > suggest which is preferred. Personally, I'd probably try to use fwnode_handle, but the second option makes sense too in principle. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: MIME-Version: 1.0 Sender: rjwysocki@gmail.com In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> Date: Fri, 13 May 2016 13:31:31 +0200 Message-ID: Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller From: "Rafael J. Wysocki" To: Jayachandran C Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Rafael Wysocki , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , Wangyijing , Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" , Jon Masters , Andrea Gallo , Duc Dang , jeremy.linton@arm.com, liudongdong3@huawei.com, Christopher Covington Content-Type: text/plain; charset=UTF-8 List-ID: On Fri, May 13, 2016 at 1:25 PM, Jayachandran C wrote: > On Tue, May 10, 2016 at 8:49 PM, Tomasz Nowicki wrote: >> This patch is going to implement generic PCI host controller for >> ACPI world, similar to what pci-host-generic.c driver does for DT world. >> >> All such drivers, which we have seen so far, were implemented within >> arch/ directory since they had some arch assumptions (x86 and ia64). >> However, they all are doing similar thing, so it makes sense to find >> some common code and abstract it into the generic driver. >> >> In order to handle PCI config space regions properly, we define new >> MCFG interface which does sanity checks on MCFG table and keeps its >> root pointer. User is able to lookup MCFG regions based on that root >> pointer and specified domain:bus_start:bus_end touple. We are using >> pci_mmcfg_late_init old prototype to avoid another function name. >> >> The implementation of pci_acpi_scan_root() looks up the MCFG entries >> and sets up a new mapping (regions are not mapped until host controller ask >> for it). Generic PCI functions are used for accessing config space. >> Driver selects PCI_ECAM and uses functions from drivers/pci/ecam.h >> to create and access ECAM mappings. >> >> As mentioned in Kconfig help section, ACPI_PCI_HOST_GENERIC choice >> should be made on a per-architecture basis. >> >> Signed-off-by: Tomasz Nowicki >> Signed-off-by: Jayachandran C >> --- > [....] > >> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h >> index 1ad2176..1cccf57 100644 >> --- a/drivers/pci/ecam.h >> +++ b/drivers/pci/ecam.h >> @@ -45,6 +45,11 @@ struct pci_config_window { >> void __iomem *win; /* 64-bit single mapping */ >> void __iomem **winp; /* 32-bit per bus mapping */ >> }; >> +#ifdef CONFIG_ACPI_PCI_HOST_GENERIC >> + struct acpi_device *companion; /* ACPI companion device */ >> +#endif >> + int domain; >> + >> }; > > Using struct pci_config_window to pass along domain and > companion looks bad. I think there are two possible options > to do this better: > > 1. add a 'struct fwnode_handle *' or 'struct device *parent_dev' > instead of the companion and domain fields above. In case of > ACPI either of them can be used to get the acpi_device and > both domain and companion can be set from that. > > 2. make pci_config_window fully embeddable by moving allocation > out of pci_ecam_create to its callers. Then it can be embedded > into acpi_pci_generic_root_info, and container_of can be used > to get acpi info from ->sysdata. > > The first option should be easier to implement but the second may > be better on long run. I would leave it to the Bjorn or Rafael to > suggest which is preferred. Personally, I'd probably try to use fwnode_handle, but the second option makes sense too in principle. From mboxrd@z Thu Jan 1 00:00:00 1970 From: rafael@kernel.org (Rafael J. Wysocki) Date: Fri, 13 May 2016 13:31:31 +0200 Subject: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, May 13, 2016 at 1:25 PM, Jayachandran C wrote: > On Tue, May 10, 2016 at 8:49 PM, Tomasz Nowicki wrote: >> This patch is going to implement generic PCI host controller for >> ACPI world, similar to what pci-host-generic.c driver does for DT world. >> >> All such drivers, which we have seen so far, were implemented within >> arch/ directory since they had some arch assumptions (x86 and ia64). >> However, they all are doing similar thing, so it makes sense to find >> some common code and abstract it into the generic driver. >> >> In order to handle PCI config space regions properly, we define new >> MCFG interface which does sanity checks on MCFG table and keeps its >> root pointer. User is able to lookup MCFG regions based on that root >> pointer and specified domain:bus_start:bus_end touple. We are using >> pci_mmcfg_late_init old prototype to avoid another function name. >> >> The implementation of pci_acpi_scan_root() looks up the MCFG entries >> and sets up a new mapping (regions are not mapped until host controller ask >> for it). Generic PCI functions are used for accessing config space. >> Driver selects PCI_ECAM and uses functions from drivers/pci/ecam.h >> to create and access ECAM mappings. >> >> As mentioned in Kconfig help section, ACPI_PCI_HOST_GENERIC choice >> should be made on a per-architecture basis. >> >> Signed-off-by: Tomasz Nowicki >> Signed-off-by: Jayachandran C >> --- > [....] > >> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h >> index 1ad2176..1cccf57 100644 >> --- a/drivers/pci/ecam.h >> +++ b/drivers/pci/ecam.h >> @@ -45,6 +45,11 @@ struct pci_config_window { >> void __iomem *win; /* 64-bit single mapping */ >> void __iomem **winp; /* 32-bit per bus mapping */ >> }; >> +#ifdef CONFIG_ACPI_PCI_HOST_GENERIC >> + struct acpi_device *companion; /* ACPI companion device */ >> +#endif >> + int domain; >> + >> }; > > Using struct pci_config_window to pass along domain and > companion looks bad. I think there are two possible options > to do this better: > > 1. add a 'struct fwnode_handle *' or 'struct device *parent_dev' > instead of the companion and domain fields above. In case of > ACPI either of them can be used to get the acpi_device and > both domain and companion can be set from that. > > 2. make pci_config_window fully embeddable by moving allocation > out of pci_ecam_create to its callers. Then it can be embedded > into acpi_pci_generic_root_info, and container_of can be used > to get acpi info from ->sysdata. > > The first option should be easier to implement but the second may > be better on long run. I would leave it to the Bjorn or Rafael to > suggest which is preferred. Personally, I'd probably try to use fwnode_handle, but the second option makes sense too in principle.