Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

[Aleksander Morgado <aleksander lanedo com>]

On 03/01/2013 09:06 AM, Ben Chan wrote:
> On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado
> <aleksander lanedo com <mailto:aleksander lanedo com>> wrote:
>
>     On 02/28/2013 08:11 PM, Ben Chan wrote:
>     > ---
>     >  src/mm-iface-modem-3gpp.c | 15 +++++++++++++++
>     >  1 file changed, 15 insertions(+)
>     >
>
>     I pushed it after modifying it to include the
>     clear_deferred_registration_state_update() call in the first
>     DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said previously, we
>     don't really mind if we get an unsolicited registration event during the
>     disabling process, as we're disabling anyway.
>
>
> I'm a bit confused. If an unsolicited registration event is received
> after clearing deferred_update_id, deferred_update_id is set to a new
> value, which will be scheduled after the modem is disabled. That will
> cause the modem state to become enabled due to the following code in
> mm-iface-modem-3gpp.c:
>
>     mm_iface_modem_update_subsystem_state (
>         MM_IFACE_MODEM (self),
>         SUBSYSTEM_3GPP,
>         (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
>          MM_MODEM_STATE_SEARCHING :
>          MM_MODEM_STATE_ENABLED),
>         MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);

Ah, good point; didn't consider that the deferred update could get
re-scheduled... In that case we should actually clear it *after*
DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, once we know we
will not process those unsolicited events. How does this look?

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 1d3fdce..6c8bb14 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1347,6 +1347,7 @@ typedef enum {
     DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS,
     DISABLING_STEP_DISABLE_UNSOLICITED_REGISTRATION_EVENTS,
     DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS,
+    DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE,
     DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS,
     DISABLING_STEP_DISABLE_UNSOLICITED_EVENTS,
     DISABLING_STEP_REGISTRATION_STATE,
@@ -1419,8 +1420,6 @@ interface_disabling_step (DisablingContext *ctx)
     case DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS:
         /* Disable periodic registration checks, if they were set */
         periodic_registration_check_disable (ctx->self);
-        /* Prevent any deferred registration state update from
happening after the modem is disabled */
-        clear_deferred_registration_state_update (ctx->self);
         /* Fall down to next step */
         ctx->step++;

@@ -1462,6 +1461,12 @@ interface_disabling_step (DisablingContext *ctx)
         /* Fall down to next step */
         ctx->step++;

+    case DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE:
+        /* Prevent any deferred registration state update from
happening after the modem is disabled */
+        clear_deferred_registration_state_update (ctx->self);
+        /* Fall down to next step */
+        ctx->step++;
+
     case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
         if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_events &&
             MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_events_finish) {

-- 
Aleksander