From 56ae75d2b240da9d13cbb945e4d93f99b25669d0 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Tue, 17 May 2022 20:26:17 +0200 Subject: [PATCH] device: Fully re-evaluate suspend/resume logic when delayed If we delayed the suspend(/resume) call, then the circumstances may have changed. In particular, an active action may have completed already which means that the driver handler should not be called anymore. --- libfprint/fp-device-private.h | 3 + libfprint/fp-device.c | 89 +--------------------------- libfprint/fpi-device.c | 105 +++++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 89 deletions(-) diff --git a/libfprint/fp-device-private.h b/libfprint/fp-device-private.h index 99eba410..9b2ea27c 100644 --- a/libfprint/fp-device-private.h +++ b/libfprint/fp-device-private.h @@ -130,6 +130,9 @@ typedef struct void match_data_free (FpMatchData *match_data); +void fpi_device_suspend (FpDevice *device); +void fpi_device_resume (FpDevice *device); + void fpi_device_configure_wakeup (FpDevice *device, gboolean enabled); void fpi_device_update_temp (FpDevice *device, diff --git a/libfprint/fp-device.c b/libfprint/fp-device.c index 23c7aaee..b94f7d80 100644 --- a/libfprint/fp-device.c +++ b/libfprint/fp-device.c @@ -949,16 +949,6 @@ fp_device_close_finish (FpDevice *device, return g_task_propagate_boolean (G_TASK (result), error); } -static void -complete_suspend_resume_task (FpDevice *device) -{ - FpDevicePrivate *priv = fp_device_get_instance_private (device); - - g_assert (priv->suspend_resume_task); - - g_task_return_boolean (g_steal_pointer (&priv->suspend_resume_task), TRUE); -} - /** * fp_device_suspend: * @device: a #FpDevice @@ -1009,48 +999,7 @@ fp_device_suspend (FpDevice *device, priv->suspend_resume_task = g_steal_pointer (&task); - /* If the device is currently idle, just complete immediately. - * For long running tasks, call the driver handler right away, for short - * tasks, wait for completion and then return the task. - */ - switch (priv->current_action) - { - case FPI_DEVICE_ACTION_NONE: - fpi_device_suspend_complete (device, NULL); - break; - - case FPI_DEVICE_ACTION_ENROLL: - case FPI_DEVICE_ACTION_VERIFY: - case FPI_DEVICE_ACTION_IDENTIFY: - case FPI_DEVICE_ACTION_CAPTURE: - if (FP_DEVICE_GET_CLASS (device)->suspend) - { - if (priv->critical_section) - priv->suspend_queued = TRUE; - else - FP_DEVICE_GET_CLASS (device)->suspend (device); - } - else - { - fpi_device_suspend_complete (device, fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED)); - } - break; - - default: - case FPI_DEVICE_ACTION_PROBE: - case FPI_DEVICE_ACTION_OPEN: - case FPI_DEVICE_ACTION_CLOSE: - case FPI_DEVICE_ACTION_DELETE: - case FPI_DEVICE_ACTION_LIST: - case FPI_DEVICE_ACTION_CLEAR_STORAGE: - g_signal_connect_object (priv->current_task, - "notify::completed", - G_CALLBACK (complete_suspend_resume_task), - device, - G_CONNECT_SWAPPED); - - break; - } + fpi_device_suspend (device); } /** @@ -1115,41 +1064,7 @@ fp_device_resume (FpDevice *device, priv->suspend_resume_task = g_steal_pointer (&task); - switch (priv->current_action) - { - case FPI_DEVICE_ACTION_NONE: - fpi_device_resume_complete (device, NULL); - break; - - case FPI_DEVICE_ACTION_ENROLL: - case FPI_DEVICE_ACTION_VERIFY: - case FPI_DEVICE_ACTION_IDENTIFY: - case FPI_DEVICE_ACTION_CAPTURE: - if (FP_DEVICE_GET_CLASS (device)->resume) - { - if (priv->critical_section) - priv->resume_queued = TRUE; - else - FP_DEVICE_GET_CLASS (device)->resume (device); - } - else - { - fpi_device_resume_complete (device, fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED)); - } - break; - - default: - case FPI_DEVICE_ACTION_PROBE: - case FPI_DEVICE_ACTION_OPEN: - case FPI_DEVICE_ACTION_CLOSE: - case FPI_DEVICE_ACTION_DELETE: - case FPI_DEVICE_ACTION_LIST: - case FPI_DEVICE_ACTION_CLEAR_STORAGE: - /* cannot happen as we make sure these tasks complete before suspend */ - g_assert_not_reached (); - complete_suspend_resume_task (device); - break; - } + fpi_device_resume (device); } /** diff --git a/libfprint/fpi-device.c b/libfprint/fpi-device.c index 4fe89080..af775542 100644 --- a/libfprint/fpi-device.c +++ b/libfprint/fpi-device.c @@ -866,16 +866,16 @@ fpi_device_critical_section_flush_idle_cb (FpDevice *device) if (priv->suspend_queued) { - cls->suspend (device); priv->suspend_queued = FALSE; + fpi_device_suspend (device); return G_SOURCE_CONTINUE; } if (priv->resume_queued) { - cls->resume (device); priv->resume_queued = FALSE; + fpi_device_resume (device); return G_SOURCE_CONTINUE; } @@ -1592,6 +1592,107 @@ update_attr (const char *attr, const char *value) return 0; } +static void +complete_suspend_resume_task (FpDevice *device) +{ + FpDevicePrivate *priv = fp_device_get_instance_private (device); + + g_assert (priv->suspend_resume_task); + + g_task_return_boolean (g_steal_pointer (&priv->suspend_resume_task), TRUE); +} + +void +fpi_device_suspend (FpDevice *device) +{ + FpDevicePrivate *priv = fp_device_get_instance_private (device); + + /* If the device is currently idle, just complete immediately. + * For long running tasks, call the driver handler right away, for short + * tasks, wait for completion and then return the task. + */ + switch (priv->current_action) + { + case FPI_DEVICE_ACTION_NONE: + fpi_device_suspend_complete (device, NULL); + break; + + case FPI_DEVICE_ACTION_ENROLL: + case FPI_DEVICE_ACTION_VERIFY: + case FPI_DEVICE_ACTION_IDENTIFY: + case FPI_DEVICE_ACTION_CAPTURE: + if (FP_DEVICE_GET_CLASS (device)->suspend) + { + if (priv->critical_section) + priv->suspend_queued = TRUE; + else + FP_DEVICE_GET_CLASS (device)->suspend (device); + } + else + { + fpi_device_suspend_complete (device, fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED)); + } + break; + + default: + case FPI_DEVICE_ACTION_PROBE: + case FPI_DEVICE_ACTION_OPEN: + case FPI_DEVICE_ACTION_CLOSE: + case FPI_DEVICE_ACTION_DELETE: + case FPI_DEVICE_ACTION_LIST: + case FPI_DEVICE_ACTION_CLEAR_STORAGE: + g_signal_connect_object (priv->current_task, + "notify::completed", + G_CALLBACK (complete_suspend_resume_task), + device, + G_CONNECT_SWAPPED); + + break; + } +} + +void +fpi_device_resume (FpDevice *device) +{ + FpDevicePrivate *priv = fp_device_get_instance_private (device); + + switch (priv->current_action) + { + case FPI_DEVICE_ACTION_NONE: + fpi_device_resume_complete (device, NULL); + break; + + case FPI_DEVICE_ACTION_ENROLL: + case FPI_DEVICE_ACTION_VERIFY: + case FPI_DEVICE_ACTION_IDENTIFY: + case FPI_DEVICE_ACTION_CAPTURE: + if (FP_DEVICE_GET_CLASS (device)->resume) + { + if (priv->critical_section) + priv->resume_queued = TRUE; + else + FP_DEVICE_GET_CLASS (device)->resume (device); + } + else + { + fpi_device_resume_complete (device, fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED)); + } + break; + + default: + case FPI_DEVICE_ACTION_PROBE: + case FPI_DEVICE_ACTION_OPEN: + case FPI_DEVICE_ACTION_CLOSE: + case FPI_DEVICE_ACTION_DELETE: + case FPI_DEVICE_ACTION_LIST: + case FPI_DEVICE_ACTION_CLEAR_STORAGE: + /* cannot happen as we make sure these tasks complete before suspend */ + g_assert_not_reached (); + complete_suspend_resume_task (device); + break; + } +} + void fpi_device_configure_wakeup (FpDevice *device, gboolean enabled) {