PNP / ACPI: Use DEVICE_ACPI_HANDLE() for device ACPI handle access

The PNP ACPI driver squirrels the ACPI handles of PNP devices' ACPI companions, but this isn't correct, because those handles should be accessed using the DEVICE_ACPI_HANDLE() macro operating on struct device objects. Using DEVICE_ACPI_HANDLE() in the PNP ACPI driver instead of the driver's own copies of the ACPI handles allows us to avoid a problem with docking stations where a machine docked before suspend to RAM and undocked while suspended crashes during the subsequent resume (in that case the ACPI companion of the PNP device in question doesn't exist any more while the device is being resumed). It also allows us to avoid the problem where suspend to RAM fails when the machine was undocked while suspended before (again, the ACPI companion of the PNP device is not present any more while it is being suspended). This change doesn't fix all of the the PNP ACPI driver's problems with PNP devices in docking stations (generally speaking, the driver has no idea that devices can come and go and doesn't even attempt to handle such events), but at least it makes suspend work for the users of docking stations who don't use the PNP devices located in there. References: https://bugzilla.kernel.org/show_bug.cgi?id=15100 Reported-and-tested-by: Toralf Förster <toralf.foerster@gmx.de> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Acked-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Signed-off-by: Len Brown <len.brown@intel.com>

PNP / ACPI: Use DEVICE_ACPI_HANDLE() for device ACPI handle access
The PNP ACPI driver squirrels the ACPI handles of PNP devices' ACPI companions, but this isn't correct, because those handles should be accessed using the DEVICE_ACPI_HANDLE() macro operating on struct device objects. Using DEVICE_ACPI_HANDLE() in the PNP ACPI driver instead of the driver's own copies of the ACPI handles allows us to avoid a problem with docking stations where a machine docked before suspend to RAM and undocked while suspended crashes during the subsequent resume (in that case the ACPI companion of the PNP device in question doesn't exist any more while the device is being resumed). It also allows us to avoid the problem where suspend to RAM fails when the machine was undocked while suspended before (again, the ACPI companion of the PNP device is not present any more while it is being suspended). This change doesn't fix all of the the PNP ACPI driver's problems with PNP devices in docking stations (generally speaking, the driver has no idea that devices can come and go and doesn't even attempt to handle such events), but at least it makes suspend work for the users of docking stations who don't use the PNP devices located in there. References: https://bugzilla.kernel.org/show_bug.cgi?id=15100 Reported-and-tested-by: Toralf Förster <toralf.foerster@gmx.de> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Acked-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Signed-off-by: Len Brown <len.brown@intel.com>
Rafael J. Wysocki · Len Brown
1 parent 3c0eee3fe6
Showing 2 changed files with 72 additions and 28 deletions Side-by-side Diff
drivers/pnp/driver.c
drivers/pnp/pnpacpi/core.c
@@ -189,8 +189,11 @@
 	if (!pnp_drv)
 		return 0;
  
-	if (pnp_dev->protocol->resume)
-		pnp_dev->protocol->resume(pnp_dev);
+	if (pnp_dev->protocol->resume) {
+		error = pnp_dev->protocol->resume(pnp_dev);
+		if (error)
+			return error;
+	}
  
 	if (pnp_can_write(pnp_dev)) {
 		error = pnp_start_dev(pnp_dev);
@@ -81,12 +81,19 @@
  
 static int pnpacpi_set_resources(struct pnp_dev *dev)
 {
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
 	struct acpi_buffer buffer;
 	int ret;
  
 	pnp_dbg(&dev->dev, "set resources\n");
+
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+		dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
+		return -ENODEV;
+	}
+
 	ret = pnpacpi_build_resource_template(dev, &buffer);
 	if (ret)
 		return ret;
  
@@ -105,12 +112,18 @@
  
 static int pnpacpi_disable_resources(struct pnp_dev *dev)
 {
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
 	int ret;
  
 	dev_dbg(&dev->dev, "disable resources\n");
  
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+		dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
+		return 0;
+	}
+
 	/* acpi_unregister_gsi(pnp_irq(dev, 0)); */
 	ret = 0;
 	if (acpi_bus_power_manageable(handle))
  
  
  
  
  
  
  
  
  
  
  
@@ -124,46 +137,74 @@
 #ifdef CONFIG_ACPI_SLEEP
 static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
 {
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
  
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+		dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
+		return false;
+	}
+
 	return acpi_bus_can_wakeup(handle);
 }
  
 static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)
 {
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
-	int power_state;
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
+	int error = 0;
  
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+		dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
+		return 0;
+	}
+
 	if (device_can_wakeup(&dev->dev)) {
-		int rc = acpi_pm_device_sleep_wake(&dev->dev,
+		error = acpi_pm_device_sleep_wake(&dev->dev,
 				device_may_wakeup(&dev->dev));
+		if (error)
+			return error;
+	}
  
-		if (rc)
-			return rc;
+	if (acpi_bus_power_manageable(handle)) {
+		int power_state = acpi_pm_device_sleep_state(&dev->dev, NULL);
+
+		if (power_state < 0)
+			power_state = (state.event == PM_EVENT_ON) ?
+					ACPI_STATE_D0 : ACPI_STATE_D3;
+
+		/*
+		 * acpi_bus_set_power() often fails (keyboard port can't be
+		 * powered-down?), and in any case, our return value is ignored
+		 * by pnp_bus_suspend().  Hence we don't revert the wakeup
+		 * setting if the set_power fails.
+		 */
+		error = acpi_bus_set_power(handle, power_state);
 	}
-	power_state = acpi_pm_device_sleep_state(&dev->dev, NULL);
-	if (power_state < 0)
-		power_state = (state.event == PM_EVENT_ON) ?
-				ACPI_STATE_D0 : ACPI_STATE_D3;
  
-	/* acpi_bus_set_power() often fails (keyboard port can't be
-	 * powered-down?), and in any case, our return value is ignored
-	 * by pnp_bus_suspend().  Hence we don't revert the wakeup
-	 * setting if the set_power fails.
-	 */
-	return acpi_bus_set_power(handle, power_state);
+	return error;
 }
  
 static int pnpacpi_resume(struct pnp_dev *dev)
 {
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
+	struct acpi_device *acpi_dev;
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	int error = 0;
  
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+		dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
+		return -ENODEV;
+	}
+
 	if (device_may_wakeup(&dev->dev))
 		acpi_pm_device_sleep_wake(&dev->dev, false);
-	return acpi_bus_set_power(handle, ACPI_STATE_D0);
+
+	if (acpi_bus_power_manageable(handle))
+		error = acpi_bus_set_power(handle, ACPI_STATE_D0);
+
+	return error;
 }
 #endif
...	...	@@ -189,8 +189,11 @@
189	189	if (!pnp_drv)
190	190	return 0;
191	191
192		- if (pnp_dev->protocol->resume)
193		- pnp_dev->protocol->resume(pnp_dev);
	192	+ if (pnp_dev->protocol->resume) {
	193	+ error = pnp_dev->protocol->resume(pnp_dev);
	194	+ if (error)
	195	+ return error;
	196	+ }
194	197
195	198	if (pnp_can_write(pnp_dev)) {
196	199	error = pnp_start_dev(pnp_dev);
...	...	@@ -81,12 +81,19 @@
81	81
82	82	static int pnpacpi_set_resources(struct pnp_dev *dev)
83	83	{
84		- struct acpi_device *acpi_dev = dev->data;
85		- acpi_handle handle = acpi_dev->handle;
	84	+ struct acpi_device *acpi_dev;
	85	+ acpi_handle handle;
86	86	struct acpi_buffer buffer;
87	87	int ret;
88	88
89	89	pnp_dbg(&dev->dev, "set resources\n");
	90	+
	91	+ handle = DEVICE_ACPI_HANDLE(&dev->dev);
	92	+ if (!handle \|\| ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
	93	+ dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
	94	+ return -ENODEV;
	95	+ }
	96	+
90	97	ret = pnpacpi_build_resource_template(dev, &buffer);
91	98	if (ret)
92	99	return ret;
93	100
...	...	@@ -105,12 +112,18 @@
105	112
106	113	static int pnpacpi_disable_resources(struct pnp_dev *dev)
107	114	{
108		- struct acpi_device *acpi_dev = dev->data;
109		- acpi_handle handle = acpi_dev->handle;
	115	+ struct acpi_device *acpi_dev;
	116	+ acpi_handle handle;
110	117	int ret;
111	118
112	119	dev_dbg(&dev->dev, "disable resources\n");
113	120
	121	+ handle = DEVICE_ACPI_HANDLE(&dev->dev);
	122	+ if (!handle \|\| ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
	123	+ dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
	124	+ return 0;
	125	+ }
	126	+
114	127	/* acpi_unregister_gsi(pnp_irq(dev, 0)); */
115	128	ret = 0;
116	129	if (acpi_bus_power_manageable(handle))
117	130
118	131
119	132
120	133
121	134
122	135
123	136
124	137
125	138
126	139
127	140
...	...	@@ -124,46 +137,74 @@
124	137	#ifdef CONFIG_ACPI_SLEEP
125	138	static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
126	139	{
127		- struct acpi_device *acpi_dev = dev->data;
128		- acpi_handle handle = acpi_dev->handle;
	140	+ struct acpi_device *acpi_dev;
	141	+ acpi_handle handle;
129	142
	143	+ handle = DEVICE_ACPI_HANDLE(&dev->dev);
	144	+ if (!handle \|\| ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
	145	+ dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
	146	+ return false;
	147	+ }
	148	+
130	149	return acpi_bus_can_wakeup(handle);
131	150	}
132	151
133	152	static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)
134	153	{
135		- struct acpi_device *acpi_dev = dev->data;
136		- acpi_handle handle = acpi_dev->handle;
137		- int power_state;
	154	+ struct acpi_device *acpi_dev;
	155	+ acpi_handle handle;
	156	+ int error = 0;
138	157
	158	+ handle = DEVICE_ACPI_HANDLE(&dev->dev);
	159	+ if (!handle \|\| ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
	160	+ dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
	161	+ return 0;
	162	+ }
	163	+
139	164	if (device_can_wakeup(&dev->dev)) {
140		- int rc = acpi_pm_device_sleep_wake(&dev->dev,
	165	+ error = acpi_pm_device_sleep_wake(&dev->dev,
141	166	device_may_wakeup(&dev->dev));
	167	+ if (error)
	168	+ return error;
	169	+ }
142	170
143		- if (rc)
144		- return rc;
	171	+ if (acpi_bus_power_manageable(handle)) {
	172	+ int power_state = acpi_pm_device_sleep_state(&dev->dev, NULL);
	173	+
	174	+ if (power_state < 0)
	175	+ power_state = (state.event == PM_EVENT_ON) ?
	176	+ ACPI_STATE_D0 : ACPI_STATE_D3;
	177	+
	178	+ /*
	179	+ * acpi_bus_set_power() often fails (keyboard port can't be
	180	+ * powered-down?), and in any case, our return value is ignored
	181	+ * by pnp_bus_suspend(). Hence we don't revert the wakeup
	182	+ * setting if the set_power fails.
	183	+ */
	184	+ error = acpi_bus_set_power(handle, power_state);
145	185	}
146		- power_state = acpi_pm_device_sleep_state(&dev->dev, NULL);
147		- if (power_state < 0)
148		- power_state = (state.event == PM_EVENT_ON) ?
149		- ACPI_STATE_D0 : ACPI_STATE_D3;
150	186
151		- /* acpi_bus_set_power() often fails (keyboard port can't be
152		- * powered-down?), and in any case, our return value is ignored
153		- * by pnp_bus_suspend(). Hence we don't revert the wakeup
154		- * setting if the set_power fails.
155		- */
156		- return acpi_bus_set_power(handle, power_state);
	187	+ return error;
157	188	}
158	189
159	190	static int pnpacpi_resume(struct pnp_dev *dev)
160	191	{
161		- struct acpi_device *acpi_dev = dev->data;
162		- acpi_handle handle = acpi_dev->handle;
	192	+ struct acpi_device *acpi_dev;
	193	+ acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
	194	+ int error = 0;
163	195
	196	+ if (!handle \|\| ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
	197	+ dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
	198	+ return -ENODEV;
	199	+ }
	200	+
164	201	if (device_may_wakeup(&dev->dev))
165	202	acpi_pm_device_sleep_wake(&dev->dev, false);
166		- return acpi_bus_set_power(handle, ACPI_STATE_D0);
	203	+
	204	+ if (acpi_bus_power_manageable(handle))
	205	+ error = acpi_bus_set_power(handle, ACPI_STATE_D0);
	206	+
	207	+ return error;
167	208	}
168	209	#endif
169	210