From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752594AbcEMLmR (ORCPT ); Fri, 13 May 2016 07:42:17 -0400 Received: from mail-lf0-f46.google.com ([209.85.215.46]:33640 "EHLO mail-lf0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752088AbcEMLmN (ORCPT ); Fri, 13 May 2016 07:42:13 -0400 Subject: Re: [PATCH V7 08/11] pci, acpi: Support for ACPI based generic PCI host controller To: "Rafael J. Wysocki" , Jayachandran C References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-9-git-send-email-tn@semihalf.com> Cc: Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , 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 From: Tomasz Nowicki Message-ID: <5735BD91.3020206@semihalf.com> Date: Fri, 13 May 2016 13:42:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13.05.2016 13:31, Rafael J. Wysocki wrote: > 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. > Thanks for suggestions. I will try to use fwnode_handle Tomasz