Skip to contentSkip to conversation

ACPI / LPSS: Add guards against CONFIG_PCI

PATCH In review Sinan Kaya 4 participants 20 messages Dec 22, 2018 – Dec 24, 2018 1 message not in the archive

Subsystems (13)
DRM drivers x86 Sound DRM misc +9 more subsystems
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Sinan Kaya

author
#1 lore raw

PATCH v2 1/11 ACPI / LPSS: Add guards against CONFIG_PCI

We can now compile ACPI without PCI support. If code depends on CONFIG_PCI, it needs to explicitly guard that piece.

---
 drivers/acpi/acpi_lpss.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/acpi_lpss.c +4 -1
index 5f94c35d165f..4148abcdf9ef 100644
@@ -534,8 +534,11 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid)
 	dev = bus_find_device(&platform_bus_type, NULL, &data, match_hid_uid);
 	if (dev)
 		return dev;
-
+#ifdef CONFIG_PCI
 	return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid);
+#else
+	return NULL;
+#endif
 }
 
 static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle)
Signature · 1 line
2.19.0
#2 lore raw
Quoted 14 lines · Sinan Kaya

We can now compile ACPI without PCI support. If code depends on CONFIG_PCI, it needs to explicitly guard that piece.

---
 drivers/acpi/acpi_lpss.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/acpi_lpss.c +0 -0
index 5f94c35d165f..4148abcdf9ef 100644

drivers/acpi/acpi_lpss.c static struct device *acpi_lpss_find_device(const char *hid, const char *uid) · line 542

Quoted 10 lines
drivers/acpi/acpi_lpss.c +4 -1
@@ -534,8 +534,11 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid)
        dev = bus_find_device(&platform_bus_type, NULL, &data, match_hid_uid);
        if (dev)
                return dev;
-
+#ifdef CONFIG_PCI
        return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid);
+#else
+       return NULL;
+#endif
 }

No.

Please make this driver depend on PCI instead.

Quoted 2 lines
drivers/acpi/acpi_lpss.c +0 -0
 static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle)
author
#3 lore raw
Quoted 3 lines · Rafael J. Wysocki

No.

Please make this driver depend on PCI instead.

Sure.

author
#4 lore raw

PATCH v2 2/11 ata: make PCI dependency explicit for PATA_ACPI

In reply to a message not in the archive

PATA_ACPI is a PCI device driver but the PCI dependency has not been explicitly called out.

---
 drivers/ata/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ata/Kconfig +1 -1
index 4ca7a6b4eaae..8218db17ebdb 100644
@@ -1091,7 +1091,7 @@ comment "Generic fallback / legacy drivers"
 
 config PATA_ACPI
 	tristate "ACPI firmware driver for PATA"
-	depends on ATA_ACPI && ATA_BMDMA
+	depends on ATA_ACPI && ATA_BMDMA && PCI
 	help
 	  This option enables an ACPI method driver which drives
 	  motherboard PATA controller interfaces through the ACPI
Signature · 1 line
2.19.0
author
#5 lore raw

PATCH v2 3/11 vga-switcheroo: make PCI dependency explicit

In reply to a message not in the archive

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

---
 drivers/gpu/vga/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/vga/Kconfig +1 -0
index b677e5d524e6..d5f1d8e1c6f8 100644
@@ -21,6 +21,7 @@ config VGA_SWITCHEROO
 	bool "Laptop Hybrid Graphics - GPU switching support"
 	depends on X86
 	depends on ACPI
+	depends on PCI
 	select VGA_ARB
 	help
 	  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
Signature · 6 lines
2.19.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw

Re: [PATCH v2 03/11] vga-switcheroo: make PCI dependency explicit

Quoted 5 lines · Sinan Kaya

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

This series doesn't have a cover letter so it's unclear to me through which tree it's supposed to go in? Each patch through the individual subsystem tree or all through the same tree? If the former I guess I could push this to drm-misc...

Thanks,

Lukas

Quoted 18 lines
---
 drivers/gpu/vga/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/vga/Kconfig +1 -0
index b677e5d524e6..d5f1d8e1c6f8 100644
@@ -21,6 +21,7 @@ config VGA_SWITCHEROO
 	bool "Laptop Hybrid Graphics - GPU switching support"
 	depends on X86
 	depends on ACPI
+	depends on PCI
 	select VGA_ARB
 	help
 	  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
Signature · 1 line
2.19.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#7 lore raw

Re: [PATCH v2 03/11] vga-switcheroo: make PCI dependency explicit

Quoted 5 lines · Lukas Wunner
Quoted 3 lines · Sinan Kaya

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

Quoted 8 lines
Quoted 2 lines

This series doesn't have a cover letter so it's unclear to me through which tree it's supposed to go in? Each patch through the individual subsystem tree or all through the same tree? If the former I guess I could push this to drm-misc...

Feel free to apply individual patches independently. Let me know which one you applied so that I can drop them on the next rev.

Quoted 8 lines

Thanks,

Lukas

Quoted 4 lines
---
 drivers/gpu/vga/Kconfig | 1 +
 1 file changed, 1 insertion(+)
Quoted 14 lines
Quoted 13 lines
drivers/gpu/vga/Kconfig +1 -1
index b677e5d524e6..d5f1d8e1c6f8 100644
@@ -21,6 +21,7 @@ config VGA_SWITCHEROO
      bool "Laptop Hybrid Graphics - GPU switching support"
      depends on X86
      depends on ACPI
+     depends on PCI
      select VGA_ARB
      help
        Many laptops released in 2008/9/10 have two GPUs with a multiplexer
--
Quoted 1 line

2.19.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#8 lore raw

Re: [PATCH v2 03/11] vga-switcheroo: make PCI dependency explicit

Quoted 8 lines · Sinan Kaya
Quoted 7 lines · Lukas Wunner
Quoted 5 lines · Sinan Kaya

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

Quoted 7 lines
Quoted 4 lines

This series doesn't have a cover letter so it's unclear to me through which tree it's supposed to go in? Each patch through the individual subsystem tree or all through the same tree? If the former I guess I could push this to drm-misc...

Feel free to apply individual patches independently. Let me know which one you applied so that I can drop them on the next rev.

I've realized only now that this patch fixes fallout of commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set") which is part of Rafael's acpi pull for v4.21.

Usually such fixes go through the same tree as the offending commit, so I'd expect Raphael to pick up the whole series and forward it to Linus in the second half of the merge window. I'm not sure if that's how it'll play out though given the holidays.

Additionally, drm-misc is currently in a somewhat confusing state:
Normally fixes targeting v4.21 should have been applied to drm-misc-next-fixes for the last 2 weeks, yet no new commits have been applied to that branch since December 5, but plenty of stuff that should have been applied (various fixes and a MAINTAINERS change) went to drm-misc-next instead. So it looks like a lot of folks picked the wrong branch and drm-misc-next is now a mix of v4.22 features and v4.21 fixes. I think I should rather not add to this mess before a drm-misc maintainer has had a chance to sort it out.

I'll be sure to keep an eye on this though. The fallout addressed by the patch is mostly confined to randconfigs I think, so waiting a few more days probably won't hurt much.

Thanks,

Lukas
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#9 lore raw

Re: [PATCH v2 03/11] vga-switcheroo: make PCI dependency explicit

Quoted 17 lines · Lukas Wunner
Quoted 8 lines · Sinan Kaya
Quoted 7 lines · Lukas Wunner
Quoted 5 lines · Sinan Kaya

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

Quoted 7 lines
Quoted 4 lines

This series doesn't have a cover letter so it's unclear to me through which tree it's supposed to go in? Each patch through the individual subsystem tree or all through the same tree? If the former I guess I could push this to drm-misc...

Feel free to apply individual patches independently. Let me know which one you applied so that I can drop them on the next rev.

Quoted 8 lines

I've realized only now that this patch fixes fallout of commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set") which is part of Rafael's acpi pull for v4.21.

Usually such fixes go through the same tree as the offending commit, so I'd expect Raphael to pick up the whole series and forward it to Linus in the second half of the merge window. I'm not sure if that's how it'll play out though given the holidays.

I'll do follow-up with Rafael. Apologize for not giving the full story. Trying to get pieces out in the middle of vacation as there was a breakage. I didn't have time to polish.

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#10 lore raw

Re: [PATCH v2 03/11] vga-switcheroo: make PCI dependency explicit

In reply to Lukas Wunner
Quoted 18 lines · Lukas Wunner
Quoted 8 lines · Sinan Kaya
Quoted 7 lines · Lukas Wunner
Quoted 5 lines · Sinan Kaya

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

Quoted 7 lines
Quoted 4 lines

This series doesn't have a cover letter so it's unclear to me through which tree it's supposed to go in? Each patch through the individual subsystem tree or all through the same tree? If the former I guess I could push this to drm-misc...

Feel free to apply individual patches independently. Let me know which one you applied so that I can drop them on the next rev.

Quoted 8 lines

I've realized only now that this patch fixes fallout of commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set") which is part of Rafael's acpi pull for v4.21.

Usually such fixes go through the same tree as the offending commit, so I'd expect Raphael to pick up the whole series and forward it to Linus in the second half of the merge window. I'm not sure if that's how it'll play out though given the holidays.

Yeah, best to get this merged through the tree that broke things.
Acked-by: Daniel Vetter <daniel.ffwll.ch> for that.

Quoted 9 lines

Additionally, drm-misc is currently in a somewhat confusing state:
Normally fixes targeting v4.21 should have been applied to drm-misc-next-fixes for the last 2 weeks, yet no new commits have been applied to that branch since December 5, but plenty of stuff that should have been applied (various fixes and a MAINTAINERS change) went to drm-misc-next instead. So it looks like a lot of folks picked the wrong branch and drm-misc-next is now a mix of v4.22 features and v4.21 fixes. I think I should rather not add to this mess before a drm-misc maintainer has had a chance to sort it out.

If a patch has been misplaced the only way to fix it is to cherry-pick it over. Hence no need to wait for maintainers to sort out anything if you have patches for drm-misc-next-fixes, you can't make it worse :-)

Quoted 3 lines

I'll be sure to keep an eye on this though. The fallout addressed by the patch is mostly confined to randconfigs I think, so waiting a few more days probably won't hurt much.

Agreed.
-Daniel

Quoted 8 lines

Thanks,

Lukas
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#11 lore raw

PATCH v2 4/11 platform/x86: make PCI dependency explicit

In reply to a message not in the archive

ipss driver is a PCI device driver but this has not been mentioned anywhere in Kconfig.

---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/platform/x86/Kconfig +1 -1
index e3b62c2ee8d1..b36ea14b41ad 100644
@@ -1009,7 +1009,7 @@ config INTEL_MFLD_THERMAL
 
 config INTEL_IPS
 	tristate "Intel Intelligent Power Sharing"
-	depends on ACPI
+	depends on ACPI && PCI
 	---help---
 	  Intel Calpella platforms support dynamic power sharing between the
 	  CPU and GPU, maximizing performance in a given TDP.  This driver,
Signature · 1 line
2.19.0
author
#12 lore raw

PATCH v2 5/11 platform/x86: intel_pmc: Hide PCI specific pieces behind CONFIG_PCI

In reply to a message not in the archive

In the configuration where CONFIG_PCI is unset, this driver is failing to compile due to PCI framework dependencies. Hide these behind CONFIG_PCI ifdef.

---
 drivers/platform/x86/intel_pmc_ipc.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/platform/x86/intel_pmc_ipc.c +6 -0
index 7964ba22ef8d..d85dfed3bf9c 100644
@@ -504,6 +504,7 @@ static irqreturn_t ioc(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_PCI
 static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct intel_pmc_ipc_dev *pmc = &ipcdev;
@@ -556,6 +557,7 @@ static struct pci_driver ipc_pci_driver = {
 	.id_table = ipc_pci_ids,
 	.probe = ipc_pci_probe,
 };
+#endif
 
 static ssize_t intel_pmc_ipc_simple_cmd_store(struct device *dev,
 					      struct device_attribute *attr,
@@ -1007,18 +1009,22 @@ static int __init intel_pmc_ipc_init(void)
 		pr_err("Failed to register PMC ipc platform driver\n");
 		return ret;
 	}
+#ifdef CONFIG_PCI
 	ret = pci_register_driver(&ipc_pci_driver);
 	if (ret) {
 		pr_err("Failed to register PMC ipc pci driver\n");
 		platform_driver_unregister(&ipc_plat_driver);
 		return ret;
 	}
+#endif
 	return ret;
 }
 
 static void __exit intel_pmc_ipc_exit(void)
 {
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&ipc_pci_driver);
+#endif
 	platform_driver_unregister(&ipc_plat_driver);
 }
 
Signature · 1 line
2.19.0
author
#13 lore raw

PATCH v2 6/11 apple-gmux: Make PCI dependency explicit

In reply to a message not in the archive

This driver depends on the PCI infrastructure but the dependency has not been explicitly called out.

---
 drivers/platform/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/platform/x86/Kconfig +1 -0
index b36ea14b41ad..86b75fa55dbc 100644
@@ -1136,6 +1136,7 @@ config SAMSUNG_Q10
 config APPLE_GMUX
 	tristate "Apple Gmux Driver"
 	depends on ACPI
+	depends on PCI
 	depends on PNP
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on BACKLIGHT_APPLE=n || BACKLIGHT_APPLE
Signature · 1 line
2.19.0
author
#14 lore raw

PATCH v2 7/11 drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

In reply to a message not in the archive

This driver is both a platform and PCI driver. Hide PCI specific pieces when CONFIG_PCI is unset.

---
 .../intel/int340x_thermal/processor_thermal_device.c      | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c +6 -2
index 284cf2c5a8fd..b84a475a1162 100644
@@ -374,6 +374,7 @@ static int int3401_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PCI
 static irqreturn_t proc_thermal_pci_msi_irq(int irq, void *devid)
 {
 	struct proc_thermal_device *proc_priv;
@@ -482,6 +483,7 @@ static struct pci_driver proc_thermal_pci_driver = {
 	.remove		= proc_thermal_pci_remove,
 	.id_table	= proc_thermal_pci_ids,
 };
+#endif
 
 static const struct acpi_device_id int3401_device_ids[] = {
 	{"INT3401", 0},
@@ -505,16 +507,18 @@ static int __init proc_thermal_init(void)
 	ret = platform_driver_register(&int3401_driver);
 	if (ret)
 		return ret;
-
+#ifdef CONFIG_PCI
 	ret = pci_register_driver(&proc_thermal_pci_driver);
-
+#endif
 	return ret;
 }
 
 static void __exit proc_thermal_exit(void)
 {
 	platform_driver_unregister(&int3401_driver);
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&proc_thermal_pci_driver);
+#endif
 }
 
 module_init(proc_thermal_init);
Signature · 1 line
2.19.0
#15 lore raw

Re: [PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

Quoted 40 lines · Sinan Kaya

This driver is both a platform and PCI driver. Hide PCI specific pieces when CONFIG_PCI is unset.

---
 .../intel/int340x_thermal/processor_thermal_device.c      | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c +4 -2
index 284cf2c5a8fd..b84a475a1162 100644
@@ -374,6 +374,7 @@ static int int3401_remove(struct platform_device *pdev)
        return 0;
 }
 
+#ifdef CONFIG_PCI
 static irqreturn_t proc_thermal_pci_msi_irq(int irq, void *devid)
 {
        struct proc_thermal_device *proc_priv;
@@ -482,6 +483,7 @@ static struct pci_driver proc_thermal_pci_driver = {
        .remove         = proc_thermal_pci_remove,
        .id_table       = proc_thermal_pci_ids,
 };
+#endif
 
 static const struct acpi_device_id int3401_device_ids[] = {
        {"INT3401", 0},
@@ -505,16 +507,18 @@ static int __init proc_thermal_init(void)
        ret = platform_driver_register(&int3401_driver);
        if (ret)
                return ret;
-
+#ifdef CONFIG_PCI
        ret = pci_register_driver(&proc_thermal_pci_driver);
-
+#endif
        return ret;
 }

drivers/thermal/intel/int340x_thermal/processor_thermal_device.c static int __init proc_thermal_init(void) · line 524

Quoted 10 lines
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c +2 -0
 
 static void __exit proc_thermal_exit(void)
 {
        platform_driver_unregister(&int3401_driver);
+#ifdef CONFIG_PCI
        pci_unregister_driver(&proc_thermal_pci_driver);
+#endif
 }
 
 module_init(proc_thermal_init);

Can you actually test this driver with your new #ifdef stuff and with CONFIG_PCI unset?

Have you really test it in that configuration?

If not, you shouldn't even post this patch.

Please make the driver depend on PCI.

author
#16 lore raw

Re: [PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

Quoted 5 lines · Rafael J. Wysocki

Can you actually test this driver with your new #ifdef stuff and with CONFIG_PCI unset?

Have you really test it in that configuration?

No, I haven't. Code was compile tested only.

Quoted 2 lines

If not, you shouldn't even post this patch.

I was hoping the maintainer to chime in. By looking at the code, it was registering a platform and PCI driver. I assumed that it was generic code that can be used with and without PCI.

Quoted 1 line

Please make the driver depend on PCI.

I can do that.

author
#17 lore raw

PATCH v2 8/11 ASoC: Intel: Make PCI dependency explicit

In reply to a message not in the archive

Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on CONFIG_PCI set but this is not specified anywhere.

---
 sound/soc/intel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/intel/Kconfig +1 -1
index 2fd1b61e8331..b0764b2fe001 100644
@@ -91,7 +91,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_PCI
 config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
 	tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
 	default ACPI
-	depends on X86 && ACPI
+	depends on X86 && ACPI && PCI
 	select SND_SST_IPC_ACPI
 	select SND_SST_ATOM_HIFI2_PLATFORM
 	select SND_SOC_ACPI_INTEL_MATCH
Signature · 1 line
2.19.0
author
#18 lore raw

PATCH v2 9/11 mmc: add PCI dependency into IOSF_MBI

In reply to a message not in the archive

Select IOSF_MBI only when PCI is set.

---
 drivers/mmc/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/Kconfig +1 -1
index e26b8145efb3..4c5f037b246d 100644
@@ -117,7 +117,7 @@ config MMC_RICOH_MMC
 config MMC_SDHCI_ACPI
 	tristate "SDHCI support for ACPI enumerated SDHCI controllers"
 	depends on MMC_SDHCI && ACPI
-	select IOSF_MBI if X86
+	select IOSF_MBI if (X86 && PCI)
 	help
 	  This selects support for ACPI enumerated SDHCI controllers,
 	  identified by ACPI Compatibility ID PNP0D40 or specific
Signature · 1 line
2.19.0
author
#19 lore raw

PATCH v2 10/11 x86: select IOSF_MBI only when CONFIG_PCI is set

In reply to a message not in the archive

Need CONFIG_PCI to be set in order to be able to use IOSF_MBI functionality.

---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/Kconfig +1 -1
index cc4f0c6ee1ed..322a58ababb0 100644
@@ -617,7 +617,7 @@ config X86_INTEL_QUARK
 
 config X86_INTEL_LPSS
 	bool "Intel Low Power Subsystem Support"
-	depends on X86 && ACPI
+	depends on X86 && ACPI && PCI
 	select COMMON_CLK
 	select PINCTRL
 	select IOSF_MBI
Signature · 1 line
2.19.0
author
#20 lore raw

PATCH v2 11/11 drivers: thermal: Make PCI dependency explicit

In reply to a message not in the archive

IOSF_CORE depends on PCI. This was never mentioned.

---
 drivers/thermal/intel/int340x_thermal/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/thermal/intel/int340x_thermal/Kconfig +1 -1
index 0582bd12a239..0ca908d12750 100644
@@ -4,7 +4,7 @@
 
 config INT340X_THERMAL
 	tristate "ACPI INT340X thermal drivers"
-	depends on X86 && ACPI
+	depends on X86 && ACPI && PCI
 	select THERMAL_GOV_USER_SPACE
 	select ACPI_THERMAL_REL
 	select ACPI_FAN
Signature · 1 line
2.19.0