Commit 2bf427b25b79eb7cea27963a66c3d4684cae0e0c

Authored by Bartlomiej Zolnierkiewicz
Committed by David S. Miller
1 parent 9c72ebef5a

ide: fix resume for CONFIG_BLK_DEV_IDEACPI=y

commit 2f0d0fd2a605666d38e290c5c0d2907484352dc4 ("ide-acpi: cleanup
do_drive_get_GTF()") didn't account for the lack of hwif->acpidata
check in generic_ide_suspend() [ indirect user of do_drive_get_GTF()
through ide_acpi_exec_tfs() ] resulting in broken resume when ACPI
support is enabled but ACPI data is unavailable.

Fix it by adding ide_port_acpi() helper for checking if port needs
ACPI handling and cleaning generic_ide_{suspend,resume}() to use it
instead of hiding hwif->acpidata and ide_noacpi checks in IDE ACPI
helpers (this should help in preventing similar bugs in the future).

While at it:
- kill superfluous debugging printks in ide_acpi_{get,push}_timing()

Reported-and-tested-by: Etienne Basset <etienne.basset@numericable.fr>
Also-reported-and-tested-by: Jeff Chua <jeff.chua.linux@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 3 changed files with 27 additions and 42 deletions Side-by-side Diff

drivers/ide/ide-acpi.c
... ... @@ -92,6 +92,11 @@
92 92 return 0;
93 93 }
94 94  
  95 +bool ide_port_acpi(ide_hwif_t *hwif)
  96 +{
  97 + return ide_noacpi == 0 && hwif->acpidata;
  98 +}
  99 +
95 100 /**
96 101 * ide_get_dev_handle - finds acpi_handle and PCI device.function
97 102 * @dev: device to locate
... ... @@ -352,9 +357,6 @@
352 357 unsigned long gtf_address;
353 358 unsigned long obj_loc;
354 359  
355   - if (ide_noacpi)
356   - return 0;
357   -
358 360 DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn);
359 361  
360 362 ret = do_drive_get_GTF(drive, &gtf_length, &gtf_address, &obj_loc);
... ... @@ -389,16 +391,6 @@
389 391 struct acpi_buffer output;
390 392 union acpi_object *out_obj;
391 393  
392   - if (ide_noacpi)
393   - return;
394   -
395   - DEBPRINT("ENTER:\n");
396   -
397   - if (!hwif->acpidata) {
398   - DEBPRINT("no ACPI data for %s\n", hwif->name);
399   - return;
400   - }
401   -
402 394 /* Setting up output buffer for _GTM */
403 395 output.length = ACPI_ALLOCATE_BUFFER;
404 396 output.pointer = NULL; /* ACPI-CA sets this; save/free it later */
... ... @@ -479,16 +471,6 @@
479 471 struct ide_acpi_drive_link *master = &hwif->acpidata->master;
480 472 struct ide_acpi_drive_link *slave = &hwif->acpidata->slave;
481 473  
482   - if (ide_noacpi)
483   - return;
484   -
485   - DEBPRINT("ENTER:\n");
486   -
487   - if (!hwif->acpidata) {
488   - DEBPRINT("no ACPI data for %s\n", hwif->name);
489   - return;
490   - }
491   -
492 474 /* Give the GTM buffer + drive Identify data to the channel via the
493 475 * _STM method: */
494 476 /* setup input parameters buffer for _STM */
495 477  
... ... @@ -527,16 +509,11 @@
527 509 ide_drive_t *drive;
528 510 int i;
529 511  
530   - if (ide_noacpi || ide_noacpi_psx)
  512 + if (ide_noacpi_psx)
531 513 return;
532 514  
533 515 DEBPRINT("ENTER:\n");
534 516  
535   - if (!hwif->acpidata) {
536   - DEBPRINT("no ACPI data for %s\n", hwif->name);
537   - return;
538   - }
539   -
540 517 /* channel first and then drives for power on and verse versa for power off */
541 518 if (on)
542 519 acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0);
... ... @@ -616,7 +593,7 @@
616 593 drive->name, err);
617 594 }
618 595  
619   - if (!ide_acpionboot) {
  596 + if (ide_noacpi || ide_acpionboot == 0) {
620 597 DEBPRINT("ACPI methods disabled on boot\n");
621 598 return;
622 599 }
drivers/ide/ide-pm.c
... ... @@ -10,9 +10,11 @@
10 10 struct request_pm_state rqpm;
11 11 int ret;
12 12  
13   - /* call ACPI _GTM only once */
14   - if ((drive->dn & 1) == 0 || pair == NULL)
15   - ide_acpi_get_timing(hwif);
  13 + if (ide_port_acpi(hwif)) {
  14 + /* call ACPI _GTM only once */
  15 + if ((drive->dn & 1) == 0 || pair == NULL)
  16 + ide_acpi_get_timing(hwif);
  17 + }
16 18  
17 19 memset(&rqpm, 0, sizeof(rqpm));
18 20 rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
... ... @@ -26,9 +28,11 @@
26 28 ret = blk_execute_rq(drive->queue, NULL, rq, 0);
27 29 blk_put_request(rq);
28 30  
29   - /* call ACPI _PS3 only after both devices are suspended */
30   - if (ret == 0 && ((drive->dn & 1) || pair == NULL))
31   - ide_acpi_set_state(hwif, 0);
  31 + if (ret == 0 && ide_port_acpi(hwif)) {
  32 + /* call ACPI _PS3 only after both devices are suspended */
  33 + if ((drive->dn & 1) || pair == NULL)
  34 + ide_acpi_set_state(hwif, 0);
  35 + }
32 36  
33 37 return ret;
34 38 }
35 39  
... ... @@ -42,13 +46,15 @@
42 46 struct request_pm_state rqpm;
43 47 int err;
44 48  
45   - /* call ACPI _PS0 / _STM only once */
46   - if ((drive->dn & 1) == 0 || pair == NULL) {
47   - ide_acpi_set_state(hwif, 1);
48   - ide_acpi_push_timing(hwif);
49   - }
  49 + if (ide_port_acpi(hwif)) {
  50 + /* call ACPI _PS0 / _STM only once */
  51 + if ((drive->dn & 1) == 0 || pair == NULL) {
  52 + ide_acpi_set_state(hwif, 1);
  53 + ide_acpi_push_timing(hwif);
  54 + }
50 55  
51   - ide_acpi_exec_tfs(drive);
  56 + ide_acpi_exec_tfs(drive);
  57 + }
52 58  
53 59 memset(&rqpm, 0, sizeof(rqpm));
54 60 rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
... ... @@ -1419,6 +1419,7 @@
1419 1419  
1420 1420 #ifdef CONFIG_BLK_DEV_IDEACPI
1421 1421 int ide_acpi_init(void);
  1422 +bool ide_port_acpi(ide_hwif_t *hwif);
1422 1423 extern int ide_acpi_exec_tfs(ide_drive_t *drive);
1423 1424 extern void ide_acpi_get_timing(ide_hwif_t *hwif);
1424 1425 extern void ide_acpi_push_timing(ide_hwif_t *hwif);
... ... @@ -1427,6 +1428,7 @@
1427 1428 extern void ide_acpi_set_state(ide_hwif_t *hwif, int on);
1428 1429 #else
1429 1430 static inline int ide_acpi_init(void) { return 0; }
  1431 +static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; }
1430 1432 static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; }
1431 1433 static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; }
1432 1434 static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; }