On 2013-12-9 19:50, Catalin Marinas wrote: > On Mon, Dec 09, 2013 at 04:12:24AM +0000, Hanjun Guo wrote: >> On 2013-12-7 1:23, Arnd Bergmann wrote: >>> On Friday 06 December 2013, Tomasz Nowicki wrote: >>>> On 05.12.2013 23:04, Arnd Bergmann wrote: >>>>> On Wednesday 04 December 2013, Hanjun Guo wrote: >>>>>> On 2013年12月04日 00:41, Matthew Garrett wrote: >>>>>>> Given the number of #ifdefs you're adding, wouldn't it make more sense >>>>>>> to just add stub functions to include/linux/pci.h? >>>>>> >>>>>> Thanks for the suggestion :) >>>>>> >>>>>> I can add stub functions in include/linux/pci.h for raw_pci_read()/ >>>>>> raw_pci_write(), then can remove #ifdefs for acpi_os_read/write_pci_configuration(). >>>>> >>>>> Actually I wonder about the usefulness of this patch in either form: Since ACPI >>>>> on ARM64 is only for servers, I would very much expect them to always come with >>>>> PCI, either physical host bridges with attached devices, or logical PCI functions >>>>> used to describe the on-SoC I/O devices. Even in case of virtual machines, you'd >>>>> normally use PCI as the method to communicate data about the virtio channels. >>>>> >>>>> Can you name a realistic use-case where you'd want ACPI but not PCI? >>>> >>>> Yes you can describe SoC I/O devices using logical PCI functions only if >>>> they are on PCI, correct me if I am wrong. Also, devices can be placed >>>> only on IOMEM (like for ARM SoC) and it is hard to predict which way >>>> vendors chose. So way don't let it be configurable? ACPI spec says >>>> nothing like PCI is needed for ACPI, AFAIK. >>> >>> You are right that today's ARM SoCs basically never use PCI to describe >>> internal devices (IIRC VIA VT8500 is an exception, but their PCI was >>> just a software fabrication). >>> >>> However, when we're talking about ACPI on ARM64, that is nothing like classic >>> ARM SoCs: As Jon Masters mentioned, this is about new server hardware following >>> a (still secret, but hopefully not much longer) hardware specification that is >>> explicitly designed to allow interoperability between vendors, so they >>> must have put some thought into how to make the hardware discoverable. It >>> seems that they are modeling things after how it's done on x86, and the >>> only sensible way to have discoverable hardware there is PCI. This is >>> also what all x86 SoCs do. >> >> I think the concern here is that ACPI is only for server platform or not. >> >> Since ACPI has lots of content related to power management, I think ACPI >> can be used for mobile devices and other platform too, not only for ARM >> servers, and with this patch, we can support both requirement. > > 'Can be used' is one thing, will it really be used is another? I don't > think so, it was (well, is) difficult enough to make the transition to > FDT, I don't see how ACPI would solve the current issues. > > I see ACPI as a server distro requirement and there are indeed benefits > in abstracting the hardware behind standard description, AML. Of course, > this would work even better with probe-able buses like PCIe and I'm > pretty sure this would be the case on high-end servers. But even if a > server distro like RHEL supports a SoC without PCIe, I would expect them > to only provide a single binary Image with CONFIG_PCI enabled. > > This patch is small enough and allows ACPI build with !CONFIG_PCI for > the time being but longer term I would expect such SoCs without PCI to > be able to run on a kernel with CONFIG_PCI enabled. Yes, we will support PCI in ACPI in the long run, and we just make PCI optional for ACPI in this patch. Actually, I had reworked this patch and make the code with minimal changes to ACPI code: Not all the ARM64 targets that are using ACPI have PCI, so introduce some stub functions to make ACPI core run without CONFIG_PCI on ARM64. pcibios_penalize_isa_irq() is arch dependent, introduce asm/pci.h to include it. Since ACPI on X86 and IA64 depends on PCI, it will not break X86 and IA64 with this patch. Signed-off-by: Graeme Gregory <graeme.gregory@xxxxxxxxxx> Signed-off-by: Al Stone <al.stone@xxxxxxxxxx> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx> --- arch/arm64/include/asm/pci.h | 13 +++++++++++++ drivers/acpi/Makefile | 2 +- drivers/acpi/internal.h | 5 +++++ include/linux/pci.h | 32 +++++++++++++++++++++++--------- 4 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 arch/arm64/include/asm/pci.h diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h new file mode 100644 index 0000000..e682c25 --- /dev/null +++ b/arch/arm64/include/asm/pci.h @@ -0,0 +1,13 @@ +#ifndef ASMARM_PCI_H +#define ASMARM_PCI_H + +#ifdef __KERNEL__ + +static inline void pcibios_penalize_isa_irq(int irq, int active) +{ + /* We don't do dynamic PCI IRQ allocation */ +} + +#endif /* __KERNEL__ */ + +#endif diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 0331f91..d8cebe3 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -38,7 +38,7 @@ acpi-y += acpi_processor.o acpi-y += processor_core.o acpi-y += ec.o acpi-$(CONFIG_ACPI_DOCK) += dock.o -acpi-y += pci_root.o pci_link.o pci_irq.o +acpi-$(CONFIG_PCI) += pci_root.o pci_link.o pci_irq.o acpi-$(CONFIG_X86_INTEL_LPSS) += acpi_lpss.o acpi-y += acpi_platform.o acpi-y += power.o diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index b125fdb..b1ef8fa 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -26,8 +26,13 @@ acpi_status acpi_os_initialize1(void); int init_acpi_device_notify(void); int acpi_scan_init(void); +#ifdef CONFIG_PCI void acpi_pci_root_init(void); void acpi_pci_link_init(void); +#else +static inline void acpi_pci_root_init(void) {} +static inline void acpi_pci_link_init(void) {} +#endif /* CONFIG_PCI */ void acpi_processor_init(void); void acpi_platform_init(void); int acpi_sysfs_init(void); diff --git a/include/linux/pci.h b/include/linux/pci.h index 1084a15..28334dd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -541,15 +541,6 @@ struct pci_ops { int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); }; -/* - * ACPI needs to be able to access PCI config space before we've done a - * PCI bus scan and created pci_bus structures. - */ -int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn, - int reg, int len, u32 *val); -int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn, - int reg, int len, u32 val); - struct pci_bus_region { resource_size_t start; resource_size_t end; @@ -1280,6 +1271,15 @@ typedef int (*arch_set_vga_state_t)(struct pci_dev *pdev, bool decode, unsigned int command_bits, u32 flags); void pci_register_set_vga_state(arch_set_vga_state_t func); +/* + * ACPI needs to be able to access PCI config space before we've done a + * PCI bus scan and created pci_bus structures. + */ +int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn, + int reg, int len, u32 *val); +int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn, + int reg, int len, u32 val); + #else /* CONFIG_PCI is not enabled */ /* @@ -1476,6 +1476,20 @@ static inline int pci_domain_nr(struct pci_bus *bus) static inline struct pci_dev *pci_dev_get(struct pci_dev *dev) { return NULL; } +static inline struct pci_bus *pci_find_bus(int domain, int busnr) +{ return NULL; } + +static inline int pci_bus_write_config_byte(struct pci_bus *bus, + unsigned int devfn, int where, u8 val); +{ return -ENODEV; } + +static inline int raw_pci_read(unsigned int domain, unsigned int bus, + unsigned int devfn, int reg, int len, u32 *val); +{ return -EINVAL; } +static inline int raw_pci_write(unsigned int domain, unsigned int bus, + unsigned int devfn, int reg, int len, u32 val); +{return -EINVAL; } + #define dev_is_pci(d) (false) #define dev_is_pf(d) (false) #define dev_num_vf(d) (0) -- Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html