ACPI: Allow PCI to be disabled for reboot

PATCH Superseded Sinan Kaya 3 participants 9 messages Dec 8, 2018 – Dec 11, 2018

Subsystems (4)

x86 ACPI PCI ACPI component architecture (acpica)

lore

Share & tools

lore

Sinan Kaya

author

2018-12-08 21:46 UTC #0 lore raw

PATCH v2 1/3 ACPI: Allow PCI to be disabled for reboot

Make PCI reboot conditional on PCI support being present on the kernel configuration.

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/acpi/reboot.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

drivers/acpi/reboot.c +9 -3

index 6fa9c2a4cfe9..9aa367f098a6 100644
@@ -7,8 +7,6 @@
 void acpi_reboot(void)
 {
 	struct acpi_generic_address *rr;
-	struct pci_bus *bus0;
-	unsigned int devfn;
 	u8 reset_value;
 
 	if (acpi_disabled)

@@ -33,6 +31,11 @@ void acpi_reboot(void)
 	 * on a device on bus 0. */
 	switch (rr->space_id) {
 	case ACPI_ADR_SPACE_PCI_CONFIG:
+	{
+#ifdef CONFIG_PCI
+		unsigned int devfn;
+		struct pci_bus *bus0;
+
 		/* The reset register can only live on bus 0. */
 		bus0 = pci_find_bus(0, 0);
 		if (!bus0)

@@ -45,7 +48,10 @@ void acpi_reboot(void)
 		pci_bus_write_config_byte(bus0, devfn,
 				(rr->address & 0xffff), reset_value);
 		break;
-
+#else
+		return;
+#endif
+	}
 	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		printk(KERN_DEBUG "ACPI MEMORY or I/O RESET_REG.\n");

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-08 21:46 UTC #1 lore raw

PATCH v2 2/3 ACPI / OSL: Allow PCI to be disabled

Getting ready to allow PCI to be disabled with ACPI enabled. Stub out calls that depend on PCI.

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/acpi/osl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/acpi/osl.c +8 -0

index b48874b8e1ea..0d3ed659cfe8 100644
@@ -773,6 +773,7 @@ acpi_status
 acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 			       u64 *value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 	u32 value32;

@@ -799,12 +800,16 @@ acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 	*value = value32;
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	return AE_SUPPORT;
+#endif
 }
 
 acpi_status
 acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				u64 value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 
 	switch (width) {

@@ -826,6 +831,9 @@ acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				reg, size, value);
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	return AE_SUPPORT;
+#endif
 }
 
 static void acpi_os_execute_deferred(struct work_struct *work)

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-08 21:46 UTC #2 lore raw

PATCH v2 3/3 PCI/ACPI: allow ACPI to be built without PCI support

In reply to Sinan Kaya

We are compiling PCI code today for systems with ACPI and no PCI device present. Remove the useless code and reduce the tight dependency.

Signed-off-by Sinan Kaya <[email protected]>

---
 arch/x86/include/asm/pci_x86.h | 7 +++++++
 drivers/acpi/Kconfig           | 1 -
 drivers/acpi/Makefile          | 2 +-
 drivers/acpi/internal.h        | 5 +++++
 drivers/pci/Makefile           | 2 +-
 include/acpi/acpi_drivers.h    | 7 +++++++
 include/linux/acpi.h           | 7 +++++++
 include/linux/pci.h            | 4 ++++
 8 files changed, 32 insertions(+), 3 deletions(-)

arch/x86/include/asm/pci_x86.h +7 -0

index 959d618dbb17..73bb404f4d2a 100644
@@ -121,7 +121,14 @@ extern void __init dmi_check_pciprobe(void);
 extern void __init dmi_check_skip_isa_align(void);
 
 /* some common used subsys_initcalls */
+#ifdef CONFIG_PCI
 extern int __init pci_acpi_init(void);
+#else
+static inline int  __init pci_acpi_init(void)
+{
+	return -EINVAL;
+}
+#endif
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
 extern int pci_legacy_init(void);

drivers/acpi/Kconfig +0 -1

index 7cea769c37df..a0abcb3bd673 100644
@@ -9,7 +9,6 @@ config ARCH_SUPPORTS_ACPI
 menuconfig ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support"
 	depends on ARCH_SUPPORTS_ACPI
-	depends on PCI
 	select PNP
 	default y if X86
 	help

drivers/acpi/Makefile +1 -1

index edc039313cd6..7c6afc111d76 100644
@@ -39,7 +39,7 @@ acpi-y				+= processor_core.o
 acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.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
 obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
 acpi-y				+= acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o

drivers/acpi/internal.h +5 -0

index 530a3f675490..b7060dae2789 100644
@@ -25,8 +25,13 @@ int acpi_osi_init(void);
 acpi_status acpi_os_initialize1(void);
 void 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
 void acpi_processor_init(void);
 void acpi_platform_init(void);
 void acpi_pnp_init(void);

drivers/pci/Makefile +1 -1

index f2bda77a2df1..657d642fcc67 100644
@@ -11,6 +11,7 @@ ifdef CONFIG_PCI
 obj-$(CONFIG_PROC_FS)		+= proc.o
 obj-$(CONFIG_SYSFS)		+= slot.o
 obj-$(CONFIG_OF)		+= of.o
+obj-$(CONFIG_ACPI)		+= pci-acpi.o
 endif
 
 obj-$(CONFIG_PCI_QUIRKS)	+= quirks.o

@@ -20,7 +21,6 @@ obj-$(CONFIG_PCI_MSI)		+= msi.o
 obj-$(CONFIG_PCI_ATS)		+= ats.o
 obj-$(CONFIG_PCI_IOV)		+= iov.o
 obj-$(CONFIG_PCI_BRIDGE_EMUL)	+= pci-bridge-emul.o
-obj-$(CONFIG_ACPI)		+= pci-acpi.o
 obj-$(CONFIG_PCI_LABEL)		+= pci-label.o
 obj-$(CONFIG_X86_INTEL_MID)	+= pci-mid.o
 obj-$(CONFIG_PCI_SYSCALL)	+= syscall.o

include/acpi/acpi_drivers.h +7 -0

index 14499757338f..9771373dd55e 100644
@@ -88,7 +88,14 @@ int acpi_pci_link_free_irq(acpi_handle handle);
 
 struct pci_bus;
 
+#ifdef CONFIG_PCI
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
+#else
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle hndl)
+{
+	return NULL;
+}
+#endif
 
 /* Arch-defined function to add a bus to the system */

include/linux/acpi.h +7 -0

index ed80f147bd50..eb1fdf4c196a 100644
@@ -340,7 +340,14 @@ struct pci_dev;
 int acpi_pci_irq_enable (struct pci_dev *dev);
 void acpi_penalize_isa_irq(int irq, int active);
 bool acpi_isa_irq_available(int irq);
+#ifdef CONFIG_PCI
 void acpi_penalize_sci_irq(int irq, int trigger, int polarity);
+#else
+static inline void acpi_penalize_sci_irq(int irq, int trigger,
+					int polarity)
+{
+}
+#endif
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 extern int ec_read(u8 addr, u8 *val);

include/linux/pci.h +4 -0

index 11c71c4ecf75..51a5a5217667 100644
@@ -1960,7 +1960,11 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 				 enum pcie_reset_state state);
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
+#ifdef CONFIG_PCI
 void pcibios_penalize_isa_irq(int irq, int active);
+#else
+static inline void pcibios_penalize_isa_irq(int irq, int active) {}
+#endif
 int pcibios_alloc_irq(struct pci_dev *dev);
 void pcibios_free_irq(struct pci_dev *dev);
 resource_size_t pcibios_default_alignment(void);

Signature · 1 line

2.19.0

Andy Shevchenko

2018-12-09 06:20 UTC #3 lore raw

Re: [PATCH v2 3/3] PCI/ACPI: allow ACPI to be built without PCI support

include/acpi/acpi_drivers.h int acpi_pci_link_free_irq(acpi_handle handle); · line 94

Quoted 9 lines · Sinan Kaya

We are compiling PCI code today for systems with ACPI and no PCI device present. Remove the useless code and reduce the tight dependency.
include/acpi/acpi_drivers.h +3 -0
+#ifdef CONFIG_PCI
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
+#else
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle hndl)

Can you spell handle fully?

Quoted 4 lines

include/acpi/acpi_drivers.h +4 -0
+{
+       return NULL;
+}
+#endif

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Sinan Kaya

author

2018-12-09 16:03 UTC #4 lore raw

Re: [PATCH v2 3/3] PCI/ACPI: allow ACPI to be built without PCI support

Quoted 6 lines · Andy Shevchenko

include/acpi/acpi_drivers.h int acpi_pci_link_free_irq(acpi_handle handle); · line 94
Quoted 4 lines
include/acpi/acpi_drivers.h +3 -0
+#ifdef CONFIG_PCI
  struct pci_dev *acpi_get_pci_dev(acpi_handle);
+#else
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle hndl)
Can you spell handle fully?

Yeah, I can do that.

Rafael J. Wysocki

maintainer

2018-12-11 10:12 UTC #5 lore raw

In reply to Sinan Kaya

Quoted 5 lines · Sinan Kaya

Make PCI reboot conditional on PCI support being present on the kernel configuration.
Signed-off-by Sinan Kaya <[email protected]>

Same comment as for patch [2/3]: make the subject say clearly that this is about CONFIG_PCI.

Quoted 30 lines

---
 drivers/acpi/reboot.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

drivers/acpi/reboot.c +5 -2

index 6fa9c2a4cfe9..9aa367f098a6 100644
@@ -7,8 +7,6 @@
 void acpi_reboot(void)
 {
        struct acpi_generic_address *rr;
-       struct pci_bus *bus0;
-       unsigned int devfn;
        u8 reset_value;
 
        if (acpi_disabled)

@@ -33,6 +31,11 @@ void acpi_reboot(void)
         * on a device on bus 0. */
        switch (rr->space_id) {
        case ACPI_ADR_SPACE_PCI_CONFIG:
+       {
+#ifdef CONFIG_PCI
+               unsigned int devfn;
+               struct pci_bus *bus0;
+
                /* The reset register can only live on bus 0. */
                bus0 = pci_find_bus(0, 0);
                if (!bus0)

drivers/acpi/reboot.c void acpi_reboot(void) · line 52

Quoted 6 lines

drivers/acpi/reboot.c +2 -1

@@ -45,7 +48,10 @@ void acpi_reboot(void)
                pci_bus_write_config_byte(bus0, devfn,
                                (rr->address & 0xffff), reset_value);
                break;
-
+#else
+               return;

Why not "break"?

Quoted 6 lines

drivers/acpi/reboot.c +2 -1

+#endif
+       }
        case ACPI_ADR_SPACE_SYSTEM_MEMORY:
        case ACPI_ADR_SPACE_SYSTEM_IO:
                printk(KERN_DEBUG "ACPI MEMORY or I/O RESET_REG.\n");
--

I struggled between break and return. Existing code seems to return on failure when bus0 is NULL. I assumed it would be more logical to return as someone could put some code after here that assumes everything is in order.

Rafael J. Wysocki

maintainer

2018-12-11 21:49 UTC #7 lore raw

Quoted 34 lines · Sinan Kaya

Quoted 9 lines · Rafael J. Wysocki
Quoted 5 lines · Sinan Kaya
Make PCI reboot conditional on PCI support being present on the kernel configuration.
Signed-off-by Sinan Kaya <[email protected]>
Same comment as for patch [2/3]: make the subject say clearly that this is about CONFIG_PCI.
Sure
Quoted 10 lines
Quoted 10 lines
case ACPI_ADR_SPACE_PCI_CONFIG:
drivers/acpi/reboot.c +5 -0
+       {
+#ifdef CONFIG_PCI
+               unsigned int devfn;
+               struct pci_bus *bus0;
+
                 /* The reset register can only live on bus 0. */
                 bus0 = pci_find_bus(0, 0);
                 if (!bus0)
@@ -45,7 +48,10 @@ void acpi_reboot(void)
Quoted 9 lines
drivers/acpi/reboot.c void acpi_reboot(void) · line 52
Quoted 6 lines
drivers/acpi/reboot.c +2 -1
                 pci_bus_write_config_byte(bus0, devfn,
                                 (rr->address & 0xffff), reset_value);
                 break;
-
+#else
+               return;
Why not "break"?

Quoted 3 lines

I struggled between break and return. Existing code seems to return on failure when bus0 is NULL. I assumed it would be more logical to return as someone could put some code after here that assumes everything is in order.

Well, there's no such code ATM, so there's no practical difference between the two and you don't really need the #else branch at all, do you?

Sinan Kaya

author

2018-12-11 22:00 UTC #8 lore raw

Quoted 7 lines · Rafael J. Wysocki

Quoted 3 lines
I struggled between break and return. Existing code seems to return on failure when bus0 is NULL. I assumed it would be more logical to return as someone could put some code after here that assumes everything is in order.
Well, there's no such code ATM, so there's no practical difference between the two and you don't really need the #else branch at all, do you?

OK. I'll go with break.