On 2015/5/27 17:47, Liviu Dudau wrote: > On Tue, May 26, 2015 at 06:20:40PM +0100, Jiang Liu wrote: >> On 2015/5/27 0:58, Liviu Dudau wrote: >>> On Tue, May 26, 2015 at 01:49:14PM +0100, Hanjun Guo wrote: >>>> 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. >>> >>> Hi Hanjun, >>> >>> Two questions: why don't you introduce this patch next to the >>> one that is going to make use of it (or even merge it there)? >>> Second, why is the whole struct pci_controller not surrounded >>> by #ifdef CONFIG_ACPI as you are implying that this is needed >>> only for ACPI? >>> >>> Btw, looking through the whole series I'm not (yet) convinced >>> that this is needed at all. >> Hi Liviu, >> This structure is required by the requested patch set >> at http://patchwork.ozlabs.org/patch/472249/, which consolidates >> the common code to support PCI host bridge into ACPI core. >> Thanks! >> Gerry > > Hi Jiang, > > Thanks for pointing me on the right answer, I've missed that series! > Probably not the best place to comment on that series here, but I > wonder why did you not made the pci_controller structure available > in a more generic header file that can be included so that arches > don't have to redefine the structure every time. After all, you are > trying to consolidate things. > > Oh, and pci_controller name throws a lot of false negatives, maybe > a more specific one (acpi_pci_controller?) would make things clear? Hi Liviu, It's a trade-off. Once I tried to rename it too, but gave up later. There are several reasons to keep it as is: 1) Several architectures define pci_controller to support PCI root bus. 2) struct pci_controller is a generic concept, I guess, and ACPI code extends pci_controller to host some ACPI specific data on IA64 and x86. 3) It will cause big code changes if we rename pci_controller to something else. Thanks! Gerry > > Best regards, > Liviu > >> >>> >>> Best regards, >>> Liviu >>> >>>> >>>> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx> >>>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx> >>>> CC: Arnd Bergmann <arnd@xxxxxxxx> >>>> CC: Catalin Marinas <catalin.marinas@xxxxxxx> >>>> CC: Liviu Dudau <Liviu.Dudau@xxxxxxx> >>>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@xxxxxxx> >>>> CC: Will Deacon <will.deacon@xxxxxxx> >>>> --- >>>> 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 b008a72..7088495 100644 >>>> --- a/arch/arm64/include/asm/pci.h >>>> +++ b/arch/arm64/include/asm/pci.h >>>> @@ -10,6 +10,16 @@ >>>> #include <asm-generic/pci-bridge.h> >>>> #include <asm-generic/pci-dma-compat.h> >>>> >>>> +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 */ >>>> +}; >>>> + >>>> #define PCIBIOS_MIN_IO 0x1000 >>>> #define PCIBIOS_MIN_MEM 0 >>>> >>>> -- >>>> 1.9.1 >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
