Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932076AbbFCKVa (ORCPT ); Wed, 3 Jun 2015 06:21:30 -0400 Received: from mga11.intel.com ([192.55.52.93]:29214 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753143AbbFCKVW (ORCPT ); Wed, 3 Jun 2015 06:21:22 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,546,1427785200"; d="scan'208";a="740052119" Message-ID: <556ED51C.4070302@linux.intel.com> Date: Wed, 03 Jun 2015 18:21:16 +0800 From: Jiang Liu Organization: Intel User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Lorenzo Pieralisi CC: "hanjun.guo@linaro.org" , "Rafael J . Wysocki" , Bjorn Helgaas , Marc Zyngier , Liviu Dudau , Yijing Wang , Catalin Marinas , Will Deacon , Lv Zheng , "lenb @ kernel . org" , LKML , "linux-pci@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "x86 @ kernel . org" , "linux-arm-kernel@lists.infradead.org" , Arnd Bergmann Subject: Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI References: <1433225576-8215-1-git-send-email-jiang.liu@linux.intel.com> <1433225576-8215-6-git-send-email-jiang.liu@linux.intel.com> <20150602093525.GA23543@red-moon> <556EBE6D.90907@linaro.org> <556ECA93.7040800@linux.intel.com> <20150603100343.GA27917@red-moon> In-Reply-To: <20150603100343.GA27917@red-moon> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4068 Lines: 104 On 2015/6/3 18:03, Lorenzo Pieralisi wrote: > On Wed, Jun 03, 2015 at 10:36:19AM +0100, Jiang Liu wrote: >> On 2015/6/3 16:44, Hanjun Guo wrote: >>> On 2015???06???02??? 17:35, Lorenzo Pieralisi wrote: >>>> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote: >>>>> From: Hanjun Guo >>>>> >>>>> ARM64 ACPI based PCI host bridge init needs a arch dependent >>>>> struct pci_controller to accommodate common PCI host bridge >>>>> code which is introduced later, or it will lead to compile >>>>> errors on ARM64. >>>>> >>>>> Signed-off-by: Hanjun Guo >>>>> Tested-by: Suravee Suthikulpanit >>>>> CC: Arnd Bergmann >>>>> CC: Catalin Marinas >>>>> CC: Liviu Dudau >>>>> CC: Lorenzo Pieralisi >>>>> CC: Will Deacon >>>>> Signed-off-by: Jiang Liu >>>>> --- >>>>> arch/arm64/include/asm/pci.h | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h >>>>> index b008a72f8bc0..70884957f253 100644 >>>>> --- a/arch/arm64/include/asm/pci.h >>>>> +++ b/arch/arm64/include/asm/pci.h >>>>> @@ -10,6 +10,16 @@ >>>>> #include >>>>> #include >>>>> >>>>> +struct acpi_device; >>>>> + >>>>> +struct pci_controller { >>>>> +#ifdef CONFIG_ACPI >>>>> + struct acpi_device *companion; /* ACPI companion device */ >>>>> +#endif >>>>> + int segment; /* PCI domain */ >>>>> + int node; /* NUMA node */ >>>>> +}; >>>> >>>> There is nothing ARM64 specific in this structure. The only >>>> reason I see you want to keep it arch specific is the iommu >>>> pointer on x86, >>> >>> And also plarform_data for IA64 too. >>> >>>> but I think we should find a way to make >>>> the common bits shared across archs (ie the struct above) and >>>> add (maybe a void*) to the generic struct to cater for arch >>>> specific data. >>>> >>>> Thoughts ? >>> >>> We discussed this already, it has limitations to make it >>> common to all archs, I think the limitation are: >>> >>> - struct pci_controller are also used for other archs >>> such as PowerPC and Tile, they will not use it for >>> ACPI purpose, so we can not used for all archs. >>> >>> - if we let struct pci_controller defined only for archs >>> using ACPI, such as introduce it in linux/acpi.h, we still >>> can not satisfy that the struct pci_controller is not >>> only used for ACPI case on x86, it will be used for >>> non-ACPI too. >>> >>> So it's pretty difficult to share it with across archs to me, >>> any more ideas? >> Hi Hanjun and Lorenzo, >> As mentioned by Hanjun, I have no idea yet about how to >> consolidating "struct pci_controller" further. One possible >> way is to move "struct pci_controller" related code into >> arch, but apparently that will reduce code reusing. > > I guess you can't move that struct pci_controller to generic code > since it is present on other archs too (with completely different > members). > > What you can do is creating a new struct (ie same purpose of pci_controller > with a different name) common to all archs that contains the common bits > + a void* data that contains arch specific data, and convert x86 and ia64 > to using it. > > It is weird to be forced to declare a pci_controller structure in arm64 > code with 0 arch specific data in it. Hi Lorenzo, I have thought to consolidate pci_controller for x86 and ia64, but that will make the change set much more bigger. How about to consolidate pci_controller by another patch set. That will be easier for review. Thanks! Gerry > > Lorenzo > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/