PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI

rafaeljw · rafaeljw · commit d97c5d4c622f · 2021-09-29T21:17:20.000+02:00
Using struct pci_platform_pm_ops for ACPI adds unnecessary
indirection to the interactions between the PCI core and ACPI PM,
which is also subject to retpolines.

Moreover, it is not particularly clear from the current code that,
as far as PCI PM is concerned, "platform" really means just ACPI
except for the special casess when Intel MID PCI PM is used or when
ACPI support is disabled (through the kernel config or command line,
or because there are no usable ACPI tables on the system).

To address the above, rework the PCI PM code to invoke ACPI PM
functions directly as needed and drop the acpi_pci_platform_pm
object that is not necessary any more.

Accordingly, update some of the ACPI PM functions in question to do
extra checks in case the ACPI support is disabled (which previously
was taken care of by avoiding to set the pci_platform_ops pointer
in those cases).

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
Tested-by: Ferry Toth &lt;fntoth@gmail.com&gt;
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
@@ -906,10 +906,13 @@ acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
  *	choose highest power _SxD or any lower power
  */
 
-static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 {
 	int acpi_state, d_max;
 
+	if (acpi_pci_disabled)
+		return PCI_POWER_ERROR;
+
 	if (pdev->no_d3cold)
 		d_max = ACPI_STATE_D3_HOT;
 	else
@@ -965,7 +968,7 @@ int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
 	return 0;
 }
 
-static bool acpi_pci_power_manageable(struct pci_dev *dev)
+bool acpi_pci_power_manageable(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 
@@ -974,13 +977,13 @@ static bool acpi_pci_power_manageable(struct pci_dev *dev)
 	return acpi_device_power_manageable(adev);
 }
 
-static bool acpi_pci_bridge_d3(struct pci_dev *dev)
+bool acpi_pci_bridge_d3(struct pci_dev *dev)
 {
 	const union acpi_object *obj;
 	struct acpi_device *adev;
 	struct pci_dev *rpdev;
 
-	if (!dev->is_hotplug_bridge)
+	if (acpi_pci_disabled || !dev->is_hotplug_bridge)
 		return false;
 
 	/* Assume D3 support if the bridge is power-manageable by ACPI. */
@@ -1008,7 +1011,7 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
 	return obj->integer.value == 1;
 }
 
-static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 	static const u8 state_conv[] = {
@@ -1046,7 +1049,7 @@ static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 	return error;
 }
 
-static pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 	static const pci_power_t state_conv[] = {
@@ -1068,7 +1071,7 @@ static pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
 	return state_conv[state];
 }
 
-static void acpi_pci_refresh_power_state(struct pci_dev *dev)
+void acpi_pci_refresh_power_state(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 
@@ -1093,17 +1096,23 @@ static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable)
 	return 0;
 }
 
-static int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
 {
+	if (acpi_pci_disabled)
+		return 0;
+
 	if (acpi_pm_device_can_wakeup(&dev->dev))
 		return acpi_pm_set_device_wakeup(&dev->dev, enable);
 
 	return acpi_pci_propagate_wakeup(dev->bus, enable);
 }
 
-static bool acpi_pci_need_resume(struct pci_dev *dev)
+bool acpi_pci_need_resume(struct pci_dev *dev)
 {
-	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
+	struct acpi_device *adev;
+
+	if (acpi_pci_disabled)
+		return false;
 
 	/*
 	 * In some cases (eg. Samsung 305V4A) leaving a bridge in suspend over
@@ -1115,6 +1124,7 @@ static bool acpi_pci_need_resume(struct pci_dev *dev)
 	if (pci_is_bridge(dev) && acpi_target_system_state() != ACPI_STATE_S0)
 		return true;
 
+	adev = ACPI_COMPANION(&dev->dev);
 	if (!adev || !acpi_device_power_manageable(adev))
 		return false;
 
@@ -1128,17 +1138,6 @@ static bool acpi_pci_need_resume(struct pci_dev *dev)
 	return !!adev->power.flags.dsw_present;
 }
 
-static const struct pci_platform_pm_ops acpi_pci_platform_pm = {
-	.bridge_d3 = acpi_pci_bridge_d3,
-	.is_manageable = acpi_pci_power_manageable,
-	.set_state = acpi_pci_set_power_state,
-	.get_state = acpi_pci_get_power_state,
-	.refresh_state = acpi_pci_refresh_power_state,
-	.choose_state = acpi_pci_choose_state,
-	.set_wakeup = acpi_pci_wakeup,
-	.need_resume = acpi_pci_need_resume,
-};
-
 void acpi_pci_add_bus(struct pci_bus *bus)
 {
 	union acpi_object *obj;
@@ -1448,7 +1447,6 @@ static int __init acpi_pci_init(void)
 	if (acpi_pci_disabled)
 		return 0;
 
-	pci_set_platform_pm(&acpi_pci_platform_pm);
 	acpi_pci_slot_init();
 	acpiphp_init();
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
@@ -988,7 +988,7 @@ static inline bool platform_pci_power_manageable(struct pci_dev *dev)
 	if (pci_use_mid_pm())
 		return true;
 
-	return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;
+	return acpi_pci_power_manageable(dev);
 }
 
 static inline int platform_pci_set_power_state(struct pci_dev *dev,
@@ -997,57 +997,53 @@ static inline int platform_pci_set_power_state(struct pci_dev *dev,
 	if (pci_use_mid_pm())
 		return mid_pci_set_power_state(dev, t);
 
-	return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;
+	return acpi_pci_set_power_state(dev, t);
 }
 
 static inline pci_power_t platform_pci_get_power_state(struct pci_dev *dev)
 {
 	if (pci_use_mid_pm())
 		return mid_pci_get_power_state(dev);
 
-	return pci_platform_pm ? pci_platform_pm->get_state(dev) : PCI_UNKNOWN;
+	return acpi_pci_get_power_state(dev);
 }
 
 static inline void platform_pci_refresh_power_state(struct pci_dev *dev)
 {
-	if (!pci_use_mid_pm() && pci_platform_pm && pci_platform_pm->refresh_state)
-		pci_platform_pm->refresh_state(dev);
+	if (!pci_use_mid_pm())
+		acpi_pci_refresh_power_state(dev);
 }
 
 static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)
 {
 	if (pci_use_mid_pm())
 		return PCI_POWER_ERROR;
 
-	return pci_platform_pm ?
-			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
+	return acpi_pci_choose_state(dev);
 }
 
 static inline int platform_pci_set_wakeup(struct pci_dev *dev, bool enable)
 {
 	if (pci_use_mid_pm())
 		return PCI_POWER_ERROR;
 
-	return pci_platform_pm ?
-			pci_platform_pm->set_wakeup(dev, enable) : -ENODEV;
+	return acpi_pci_wakeup(dev, enable);
 }
 
 static inline bool platform_pci_need_resume(struct pci_dev *dev)
 {
 	if (pci_use_mid_pm())
 		return false;
 
-	return pci_platform_pm ? pci_platform_pm->need_resume(dev) : false;
+	return acpi_pci_need_resume(dev);
 }
 
 static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
 {
 	if (pci_use_mid_pm())
 		return false;
 
-	if (pci_platform_pm && pci_platform_pm->bridge_d3)
-		return pci_platform_pm->bridge_d3(dev);
-	return false;
+	return acpi_pci_bridge_d3(dev);
 }
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
@@ -725,17 +725,53 @@ int pci_acpi_program_hp_params(struct pci_dev *dev);
 extern const struct attribute_group pci_dev_acpi_attr_group;
 void pci_set_acpi_fwnode(struct pci_dev *dev);
 int pci_dev_acpi_reset(struct pci_dev *dev, bool probe);
+bool acpi_pci_power_manageable(struct pci_dev *dev);
+bool acpi_pci_bridge_d3(struct pci_dev *dev);
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state);
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev);
+void acpi_pci_refresh_power_state(struct pci_dev *dev);
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable);
+bool acpi_pci_need_resume(struct pci_dev *dev);
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev);
 #else
 static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
 {
 	return -ENOTTY;
 }
-
 static inline void pci_set_acpi_fwnode(struct pci_dev *dev) {}
 static inline int pci_acpi_program_hp_params(struct pci_dev *dev)
 {
 	return -ENODEV;
 }
+static inline bool acpi_pci_power_manageable(struct pci_dev *dev)
+{
+	return false;
+}
+static inline bool acpi_pci_bridge_d3(struct pci_dev *dev)
+{
+	return false;
+}
+static inline int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+	return -ENODEV;
+}
+static inline pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+{
+	return PCI_UNKNOWN;
+}
+static inline void acpi_pci_refresh_power_state(struct pci_dev *dev) {}
+static inline int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+{
+	return -ENODEV;
+}
+static inline bool acpi_pci_need_resume(struct pci_dev *dev)
+{
+	return false;
+}
+static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+{
+	return PCI_POWER_ERROR;
+}
 #endif
 
 #ifdef CONFIG_PCIEASPM

Original file line number	Diff line number	Diff line change
`@@ -988,7 +988,7 @@ static inline bool platform_pci_power_manageable(struct pci_dev *dev)`
`988`	`988`	`if (pci_use_mid_pm())`
`989`	`989`	`return true;`
`990`	`990`
`991`		`- return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;`
	`991`	`+ return acpi_pci_power_manageable(dev);`
`992`	`992`	`}`
`993`	`993`
`994`	`994`	`static inline int platform_pci_set_power_state(struct pci_dev *dev,`
`@@ -997,57 +997,53 @@ static inline int platform_pci_set_power_state(struct pci_dev *dev,`
`997`	`997`	`if (pci_use_mid_pm())`
`998`	`998`	`return mid_pci_set_power_state(dev, t);`
`999`	`999`
`1000`		`- return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;`
	`1000`	`+ return acpi_pci_set_power_state(dev, t);`
`1001`	`1001`	`}`
`1002`	`1002`
`1003`	`1003`	`static inline pci_power_t platform_pci_get_power_state(struct pci_dev *dev)`
`1004`	`1004`	`{`
`1005`	`1005`	`if (pci_use_mid_pm())`
`1006`	`1006`	`return mid_pci_get_power_state(dev);`
`1007`	`1007`
`1008`		`- return pci_platform_pm ? pci_platform_pm->get_state(dev) : PCI_UNKNOWN;`
	`1008`	`+ return acpi_pci_get_power_state(dev);`
`1009`	`1009`	`}`
`1010`	`1010`
`1011`	`1011`	`static inline void platform_pci_refresh_power_state(struct pci_dev *dev)`
`1012`	`1012`	`{`
`1013`		`- if (!pci_use_mid_pm() && pci_platform_pm && pci_platform_pm->refresh_state)`
`1014`		`- pci_platform_pm->refresh_state(dev);`
	`1013`	`+ if (!pci_use_mid_pm())`
	`1014`	`+ acpi_pci_refresh_power_state(dev);`
`1015`	`1015`	`}`
`1016`	`1016`
`1017`	`1017`	`static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)`
`1018`	`1018`	`{`
`1019`	`1019`	`if (pci_use_mid_pm())`
`1020`	`1020`	`return PCI_POWER_ERROR;`
`1021`	`1021`
`1022`		`- return pci_platform_pm ?`
`1023`		`- pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;`
	`1022`	`+ return acpi_pci_choose_state(dev);`
`1024`	`1023`	`}`
`1025`	`1024`
`1026`	`1025`	`static inline int platform_pci_set_wakeup(struct pci_dev *dev, bool enable)`
`1027`	`1026`	`{`
`1028`	`1027`	`if (pci_use_mid_pm())`
`1029`	`1028`	`return PCI_POWER_ERROR;`
`1030`	`1029`
`1031`		`- return pci_platform_pm ?`
`1032`		`- pci_platform_pm->set_wakeup(dev, enable) : -ENODEV;`
	`1030`	`+ return acpi_pci_wakeup(dev, enable);`
`1033`	`1031`	`}`
`1034`	`1032`
`1035`	`1033`	`static inline bool platform_pci_need_resume(struct pci_dev *dev)`
`1036`	`1034`	`{`
`1037`	`1035`	`if (pci_use_mid_pm())`
`1038`	`1036`	`return false;`
`1039`	`1037`
`1040`		`- return pci_platform_pm ? pci_platform_pm->need_resume(dev) : false;`
	`1038`	`+ return acpi_pci_need_resume(dev);`
`1041`	`1039`	`}`
`1042`	`1040`
`1043`	`1041`	`static inline bool platform_pci_bridge_d3(struct pci_dev *dev)`
`1044`	`1042`	`{`
`1045`	`1043`	`if (pci_use_mid_pm())`
`1046`	`1044`	`return false;`
`1047`	`1045`
`1048`		`- if (pci_platform_pm && pci_platform_pm->bridge_d3)`
`1049`		`- return pci_platform_pm->bridge_d3(dev);`
`1050`		`- return false;`
	`1046`	`+ return acpi_pci_bridge_d3(dev);`
`1051`	`1047`	`}`
`1052`	`1048`
`1053`	`1049`	`/**`