diff options
| author | Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> | 2009-06-29 22:20:42 -0400 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2009-06-29 22:20:42 -0400 |
| commit | 2bf427b25b79eb7cea27963a66c3d4684cae0e0c (patch) | |
| tree | 23d8da7a647b91eaaf3b63d7c09ae3f115a88428 | |
| parent | 9c72ebef5aabf3532469d602a9d87beceea268b1 (diff) | |
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>
| -rw-r--r-- | drivers/ide/ide-acpi.c | 37 | ||||
| -rw-r--r-- | drivers/ide/ide-pm.c | 30 | ||||
| -rw-r--r-- | include/linux/ide.h | 2 |
3 files changed, 27 insertions, 42 deletions
diff --git a/drivers/ide/ide-acpi.c b/drivers/ide/ide-acpi.c index 77f79d26b264..c509c9916464 100644 --- a/drivers/ide/ide-acpi.c +++ b/drivers/ide/ide-acpi.c | |||
| @@ -92,6 +92,11 @@ int ide_acpi_init(void) | |||
| 92 | return 0; | 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 | * ide_get_dev_handle - finds acpi_handle and PCI device.function | 101 | * ide_get_dev_handle - finds acpi_handle and PCI device.function |
| 97 | * @dev: device to locate | 102 | * @dev: device to locate |
| @@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive) | |||
| 352 | unsigned long gtf_address; | 357 | unsigned long gtf_address; |
| 353 | unsigned long obj_loc; | 358 | unsigned long obj_loc; |
| 354 | 359 | ||
| 355 | if (ide_noacpi) | ||
| 356 | return 0; | ||
| 357 | |||
| 358 | DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn); | 360 | DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn); |
| 359 | 361 | ||
| 360 | ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc); | 362 | ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc); |
| @@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwif) | |||
| 389 | struct acpi_buffer output; | 391 | struct acpi_buffer output; |
| 390 | union acpi_object *out_obj; | 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 | /* Setting up output buffer for _GTM */ | 394 | /* Setting up output buffer for _GTM */ |
| 403 | output.length = ACPI_ALLOCATE_BUFFER; | 395 | output.length = ACPI_ALLOCATE_BUFFER; |
| 404 | output.pointer = NULL; /* ACPI-CA sets this; save/free it later */ | 396 | output.pointer = NULL; /* ACPI-CA sets this; save/free it later */ |
| @@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hwif) | |||
| 479 | struct ide_acpi_drive_link *master = &hwif->acpidata->master; | 471 | struct ide_acpi_drive_link *master = &hwif->acpidata->master; |
| 480 | struct ide_acpi_drive_link *slave = &hwif->acpidata->slave; | 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 | /* Give the GTM buffer + drive Identify data to the channel via the | 474 | /* Give the GTM buffer + drive Identify data to the channel via the |
| 493 | * _STM method: */ | 475 | * _STM method: */ |
| 494 | /* setup input parameters buffer for _STM */ | 476 | /* setup input parameters buffer for _STM */ |
| @@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif, int on) | |||
| 527 | ide_drive_t *drive; | 509 | ide_drive_t *drive; |
| 528 | int i; | 510 | int i; |
| 529 | 511 | ||
| 530 | if (ide_noacpi || ide_noacpi_psx) | 512 | if (ide_noacpi_psx) |
| 531 | return; | 513 | return; |
| 532 | 514 | ||
| 533 | DEBPRINT("ENTER:\n"); | 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 | /* channel first and then drives for power on and verse versa for power off */ | 517 | /* channel first and then drives for power on and verse versa for power off */ |
| 541 | if (on) | 518 | if (on) |
| 542 | acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0); | 519 | acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0); |
| @@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif_t *hwif) | |||
| 616 | drive->name, err); | 593 | drive->name, err); |
| 617 | } | 594 | } |
| 618 | 595 | ||
| 619 | if (!ide_acpionboot) { | 596 | if (ide_noacpi || ide_acpionboot == 0) { |
| 620 | DEBPRINT("ACPI methods disabled on boot\n"); | 597 | DEBPRINT("ACPI methods disabled on boot\n"); |
| 621 | return; | 598 | return; |
| 622 | } | 599 | } |
diff --git a/drivers/ide/ide-pm.c b/drivers/ide/ide-pm.c index c14ca144cffe..ad7be2669dcb 100644 --- a/drivers/ide/ide-pm.c +++ b/drivers/ide/ide-pm.c | |||
| @@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg) | |||
| 10 | struct request_pm_state rqpm; | 10 | struct request_pm_state rqpm; |
| 11 | int ret; | 11 | int ret; |
| 12 | 12 | ||
| 13 | /* call ACPI _GTM only once */ | 13 | if (ide_port_acpi(hwif)) { |
| 14 | if ((drive->dn & 1) == 0 || pair == NULL) | 14 | /* call ACPI _GTM only once */ |
| 15 | ide_acpi_get_timing(hwif); | 15 | if ((drive->dn & 1) == 0 || pair == NULL) |
| 16 | ide_acpi_get_timing(hwif); | ||
| 17 | } | ||
| 16 | 18 | ||
| 17 | memset(&rqpm, 0, sizeof(rqpm)); | 19 | memset(&rqpm, 0, sizeof(rqpm)); |
| 18 | rq = blk_get_request(drive->queue, READ, __GFP_WAIT); | 20 | rq = blk_get_request(drive->queue, READ, __GFP_WAIT); |
| @@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg) | |||
| 26 | ret = blk_execute_rq(drive->queue, NULL, rq, 0); | 28 | ret = blk_execute_rq(drive->queue, NULL, rq, 0); |
| 27 | blk_put_request(rq); | 29 | blk_put_request(rq); |
| 28 | 30 | ||
| 29 | /* call ACPI _PS3 only after both devices are suspended */ | 31 | if (ret == 0 && ide_port_acpi(hwif)) { |
| 30 | if (ret == 0 && ((drive->dn & 1) || pair == NULL)) | 32 | /* call ACPI _PS3 only after both devices are suspended */ |
| 31 | ide_acpi_set_state(hwif, 0); | 33 | if ((drive->dn & 1) || pair == NULL) |
| 34 | ide_acpi_set_state(hwif, 0); | ||
| 35 | } | ||
| 32 | 36 | ||
| 33 | return ret; | 37 | return ret; |
| 34 | } | 38 | } |
| @@ -42,13 +46,15 @@ int generic_ide_resume(struct device *dev) | |||
| 42 | struct request_pm_state rqpm; | 46 | struct request_pm_state rqpm; |
| 43 | int err; | 47 | int err; |
| 44 | 48 | ||
| 45 | /* call ACPI _PS0 / _STM only once */ | 49 | if (ide_port_acpi(hwif)) { |
| 46 | if ((drive->dn & 1) == 0 || pair == NULL) { | 50 | /* call ACPI _PS0 / _STM only once */ |
| 47 | ide_acpi_set_state(hwif, 1); | 51 | if ((drive->dn & 1) == 0 || pair == NULL) { |
| 48 | ide_acpi_push_timing(hwif); | 52 | ide_acpi_set_state(hwif, 1); |
| 49 | } | 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 | memset(&rqpm, 0, sizeof(rqpm)); | 59 | memset(&rqpm, 0, sizeof(rqpm)); |
| 54 | rq = blk_get_request(drive->queue, READ, __GFP_WAIT); | 60 | rq = blk_get_request(drive->queue, READ, __GFP_WAIT); |
diff --git a/include/linux/ide.h b/include/linux/ide.h index c6af7c44d46c..edc93a6d931d 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h | |||
| @@ -1419,6 +1419,7 @@ static inline void ide_dma_unmap_sg(ide_drive_t *drive, | |||
| 1419 | 1419 | ||
| 1420 | #ifdef CONFIG_BLK_DEV_IDEACPI | 1420 | #ifdef CONFIG_BLK_DEV_IDEACPI |
| 1421 | int ide_acpi_init(void); | 1421 | int ide_acpi_init(void); |
| 1422 | bool ide_port_acpi(ide_hwif_t *hwif); | ||
| 1422 | extern int ide_acpi_exec_tfs(ide_drive_t *drive); | 1423 | extern int ide_acpi_exec_tfs(ide_drive_t *drive); |
| 1423 | extern void ide_acpi_get_timing(ide_hwif_t *hwif); | 1424 | extern void ide_acpi_get_timing(ide_hwif_t *hwif); |
| 1424 | extern void ide_acpi_push_timing(ide_hwif_t *hwif); | 1425 | extern void ide_acpi_push_timing(ide_hwif_t *hwif); |
| @@ -1427,6 +1428,7 @@ void ide_acpi_port_init_devices(ide_hwif_t *); | |||
| 1427 | extern void ide_acpi_set_state(ide_hwif_t *hwif, int on); | 1428 | extern void ide_acpi_set_state(ide_hwif_t *hwif, int on); |
| 1428 | #else | 1429 | #else |
| 1429 | static inline int ide_acpi_init(void) { return 0; } | 1430 | static inline int ide_acpi_init(void) { return 0; } |
| 1431 | static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; } | ||
| 1430 | static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; } | 1432 | static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; } |
| 1431 | static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; } | 1433 | static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; } |
| 1432 | static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; } | 1434 | static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; } |
