From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jayachandran C Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller Date: Fri, 13 May 2016 16:55:48 +0530 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: Received: from mail-yw0-f177.google.com ([209.85.161.177]:33347 "EHLO mail-yw0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751631AbcEML02 (ORCPT ); Fri, 13 May 2016 07:26:28 -0400 Received: by mail-yw0-f177.google.com with SMTP id t10so112612153ywa.0 for ; Fri, 13 May 2016 04:26:28 -0700 (PDT) In-Reply-To: <1462893601-8937-9-git-send-email-tn@semihalf.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Tomasz Nowicki Cc: 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.lin 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. JC. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752666AbcEML0c (ORCPT ); Fri, 13 May 2016 07:26:32 -0400 Received: from mail-yw0-f169.google.com ([209.85.161.169]:33347 "EHLO mail-yw0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752088AbcEML02 (ORCPT ); Fri, 13 May 2016 07:26:28 -0400 MIME-Version: 1.0 In-Reply-To: <1462893601-8937-9-git-send-email-tn@semihalf.com> References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> From: Jayachandran C Date: Fri, 13 May 2016 16:55:48 +0530 Message-ID: Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller To: Tomasz Nowicki Cc: 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 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. JC. From mboxrd@z Thu Jan 1 00:00:00 1970 From: jchandra@broadcom.com (Jayachandran C) Date: Fri, 13 May 2016 16:55:48 +0530 Subject: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller In-Reply-To: <1462893601-8937-9-git-send-email-tn@semihalf.com> 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 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. JC.