Re: [MM] [PATCH] sim: retry SIM operations during initialization



Hey Ben,

>>> Do you mean that you still need the timeout even if the PIN request is
>>> not enabled?
>>>
>>> If the code tries to load Sim properties before unlocking the PIN and
>>> those fail, they will anyway get retried after the successful PIN unlock.
>>
>> Yes, the issue happens on a SIM without PIN locked. One scenario is
>> when the modem is powered off during suspend. After the system
>> resumes, the modem boots up and ModemManager sees a new device.
>> ModemManager gets a CPIN response from the modem indicating that no
>> PIN unlock is required. If ModemManager immediately issues AT+CRSM to
>> access ICCID or IMSI, the command either fails with a +CRSM,110,0
>> response or sometimes a bogus value (which could be a problem with the
>> modem firmware, but we somehow need to work around that in software).
>> It becomes more reliable if there is a delay between getting CPIN
>> ready and initializing MMSim.  Similar problem could happen on a
>> system that finishes booting (and launching ModemManager) before the
>> SIM interface is fully ready.
>>
> 
> I see; so yes, we should probably run the after-sim-unlock waiting step
> also after getting the READY reply without having inserted the PIN
> ourselves.
> 
> Then, the after-sim-unlock step may be implemented either as:
>  * just sleeping for some ms; like the MBM plugin does.
>  * or issuing a command known to need access to the SIM (e.g. +CPMS?);
> like the ZTE plugin does,
>  * or any other method to (try to) ensure the SIM is ready.
> 
> I'll look into moving the after-sim-unlock step.
> 

Attached is a patch which makes the after-sim-unlock step get run also
when PIN request is disabled. Could you give it a try with your use case
and a proper implementation of the modem_after_sim_unlock() callback? I
would actually try the ZTE way (running +CPMS? retries), as IIRC you
were having the same issue with +CPMS? in the Samsung modem. Just copy &
paste the implementation from the ZTE plugin, and if it works we can
create some helper source files with common interface implementations
that are not used by the generic broadband modem but may be used by
plugins (so that we don't rewrite exactly the same implementation in
different plugins).

-- 
Aleksander
>From 4065606a694d55d03bee5b4c7e6fd431d8f8e9ec Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksander lanedo com>
Date: Tue, 28 Aug 2012 18:34:07 +0200
Subject: [PATCH] iface-modem: run after-sim-unlock also when PIN request is
 not enabled

Whenever we query current unlock required status and we get that we're unlocked,
we'll launch the after-sim-unlock step so that we try to ensure that the SIM is
ready.
---
 src/mm-iface-modem.c | 88 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index da2371a..4c40836 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -2001,6 +2001,7 @@ struct _UnlockCheckContext {
     guint pin_check_timeout_id;
     GSimpleAsyncResult *result;
     MmGdbusModem *skeleton;
+    MMModemLock lock;
 };
 
 static UnlockCheckContext *
@@ -2045,41 +2046,13 @@ reinitialize_ready (MMBaseModem *self,
     }
 }
 
-static void
-modem_after_sim_unlock_ready (MMIfaceModem *self,
-                              GAsyncResult *res)
+static gboolean
+restart_initialize_idle (MMIfaceModem *self)
 {
-    GError *error = NULL;
-
-    if (!MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock_finish (self, res, &error)) {
-        mm_warn ("After SIM unlock failed setup: '%s'", error->message);
-        g_error_free (error);
-    }
-
-    /* Go on */
+    /* If no wait needed, just go on */
     mm_base_modem_initialize (MM_BASE_MODEM (self),
                               (GAsyncReadyCallback) reinitialize_ready,
                               NULL);
-}
-
-static gboolean
-restart_initialize_idle (MMIfaceModem *self)
-{
-    /* If we were asked to run something after having sent the PIN unlock,
-     * do it now. This may be just a timeout or some other command that gives us
-     * the real SIM state */
-    if (MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock != NULL &&
-        MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock_finish != NULL) {
-        MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock(
-            self,
-            (GAsyncReadyCallback)modem_after_sim_unlock_ready,
-            NULL);
-    } else {
-        /* If no wait needed, just go on */
-        mm_base_modem_initialize (MM_BASE_MODEM (self),
-                                  (GAsyncReadyCallback) reinitialize_ready,
-                                  NULL);
-    }
     return FALSE;
 }
 
@@ -2146,16 +2119,36 @@ unlock_check_again  (UnlockCheckContext *ctx)
 }
 
 static void
+modem_after_sim_unlock_ready (MMIfaceModem *self,
+                              GAsyncResult *res,
+                              UnlockCheckContext *ctx)
+{
+    GError *error = NULL;
+
+    if (!MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock_finish (self, res, &error)) {
+        /* Complete anyway */
+        mm_warn ("After SIM unlock failed setup: '%s'", error->message);
+        g_error_free (error);
+    }
+
+    /* Update lock status and modem status if needed */
+    set_lock_status (self, ctx->skeleton, ctx->lock);
+
+    g_simple_async_result_set_op_res_gpointer (ctx->result,
+                                               GUINT_TO_POINTER (ctx->lock),
+                                               NULL);
+    g_simple_async_result_complete (ctx->result);
+    unlock_check_context_free (ctx);
+}
+
+static void
 unlock_check_ready (MMIfaceModem *self,
                     GAsyncResult *res,
                     UnlockCheckContext *ctx)
 {
     GError *error = NULL;
-    MMModemLock lock;
 
-    lock = MM_IFACE_MODEM_GET_INTERFACE (self)->load_unlock_required_finish (self,
-                                                                             res,
-                                                                             &error);
+    ctx->lock = MM_IFACE_MODEM_GET_INTERFACE (self)->load_unlock_required_finish (self, res, &error);
     if (error) {
         /* Treat several SIM related, serial and other core errors as critical
          * and abort the checks. These will end up moving the modem to a FAILED
@@ -2197,14 +2190,33 @@ unlock_check_ready (MMIfaceModem *self,
         }
 
         /* If reached max retries and still reporting error, set UNKNOWN */
-        lock = MM_MODEM_LOCK_UNKNOWN;
+        ctx->lock = MM_MODEM_LOCK_UNKNOWN;
+    }
+
+    /* If we get that no lock is required, run the after SIM unlock step
+     * in order to wait for the SIM to get ready */
+    if (ctx->lock == MM_MODEM_LOCK_NONE ||
+        ctx->lock == MM_MODEM_LOCK_SIM_PIN2 ||
+        ctx->lock == MM_MODEM_LOCK_SIM_PUK2) {
+        if (MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock != NULL &&
+            MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock_finish != NULL) {
+            mm_dbg ("SIM is ready, running after SIM unlock step...");
+            MM_IFACE_MODEM_GET_INTERFACE (self)->modem_after_sim_unlock(
+                self,
+                (GAsyncReadyCallback)modem_after_sim_unlock_ready,
+                NULL);
+            return;
+        }
+
+        /* If no way to run after SIM unlock step, we're done */
+        mm_dbg ("SIM is ready, and no need for the after SIM unlock step...");
     }
 
     /* Update lock status and modem status if needed */
-    set_lock_status (self, ctx->skeleton, lock);
+    set_lock_status (self, ctx->skeleton, ctx->lock);
 
     g_simple_async_result_set_op_res_gpointer (ctx->result,
-                                               GUINT_TO_POINTER (lock),
+                                               GUINT_TO_POINTER (ctx->lock),
                                                NULL);
     g_simple_async_result_complete (ctx->result);
     unlock_check_context_free (ctx);
-- 
1.7.11.4



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]