[PATCH] telit: lock/unlock CSIM operations by default

[Penalva, Salvador]

Thank you all,

You have all been really helpful and quick and the patch seems to do its job perfectly!

Salvador

-----Original Message-----
From: Aleksander Morgado [mailto:aleksander at aleksander.es] 
Sent: jueves, 16 de marzo de 2017 22:35
To: Carlo Lobrano; Penalva, Salvador; Daniele Palmas
Cc: ModemManager (development); Aleksander Morgado
Subject: Re: [PATCH] telit: lock/unlock CSIM operations by default

On Wed, Mar 15, 2017 at 12:19 AM, Aleksander Morgado <aleksander at aleksander.es> wrote:
> Wrap the AT+CSIM=XX commands between lock (CSIM=1) and unlock (CSIM=0) 
> operations.
>
> This seems to avoid the TTY lockup seen in several different Telit 
> modules.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=100205
>
> Reported-by: Penalva, Salvador <Salvador.Penalva at digi.com>
> ---
>
> Hey Salvador, Carlo and Daniele,
>
> Can you guys review and test this patch? It looks like it solved the issue I saw myself.
>
> The AT command reference for the Telit LE866 does say that the AT+CSIM commands need to be enclosed within lock/unlock operations like these.
>

Pushed to git master and mm-1-6.

> ---
>  plugins/telit/mm-broadband-modem-telit.c | 60 
> ++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/plugins/telit/mm-broadband-modem-telit.c 
> b/plugins/telit/mm-broadband-modem-telit.c
> index b1679ae4..8bc9d1e5 100644
> --- a/plugins/telit/mm-broadband-modem-telit.c
> +++ b/plugins/telit/mm-broadband-modem-telit.c
> @@ -452,6 +452,8 @@ modem_load_supported_bands (MMIfaceModem *self,  
> /*********************************************************************
> ********/
>  /* Load unlock retries (Modem interface) */
>
> +#define CSIM_LOCK_STR               "+CSIM=1"
> +#define CSIM_UNLOCK_STR             "+CSIM=0"
>  #define CSIM_QUERY_PIN_RETRIES_STR  "+CSIM=10,0020000100"
>  #define CSIM_QUERY_PUK_RETRIES_STR  "+CSIM=10,002C000100"
>  #define CSIM_QUERY_PIN2_RETRIES_STR "+CSIM=10,0020008100"
> @@ -460,10 +462,12 @@ modem_load_supported_bands (MMIfaceModem *self,
>
>  typedef enum {
>      LOAD_UNLOCK_RETRIES_STEP_FIRST,
> +    LOAD_UNLOCK_RETRIES_STEP_LOCK,
>      LOAD_UNLOCK_RETRIES_STEP_PIN,
>      LOAD_UNLOCK_RETRIES_STEP_PUK,
>      LOAD_UNLOCK_RETRIES_STEP_PIN2,
>      LOAD_UNLOCK_RETRIES_STEP_PUK2,
> +    LOAD_UNLOCK_RETRIES_STEP_UNLOCK,
>      LOAD_UNLOCK_RETRIES_STEP_LAST
>  } LoadUnlockRetriesStep;
>
> @@ -500,6 +504,25 @@ modem_load_unlock_retries_finish (MMIfaceModem 
> *self,  }
>
>  static void
> +csim_unlock_ready (MMBaseModem              *self,
> +                   GAsyncResult             *res,
> +                   LoadUnlockRetriesContext *ctx) {
> +    const gchar *response;
> +    GError      *error = NULL;
> +
> +    /* Ignore errors */
> +    response = mm_base_modem_at_command_finish (self, res, &error);
> +    if (!response) {
> +        mm_warn ("Couldn't unlock SIM card: %s", error->message);
> +        g_error_free (error);
> +    }
> +
> +    ctx->step++;
> +    load_unlock_retries_step (ctx);
> +}
> +
> +static void
>  csim_query_ready (MMBaseModem *self,
>                    GAsyncResult *res,
>                    LoadUnlockRetriesContext *ctx) @@ -542,6 +565,7 @@ 
> csim_query_ready (MMBaseModem *self,
>              mm_unlock_retries_set (ctx->retries, MM_MODEM_LOCK_SIM_PUK2, unlock_retries);
>              break;
>          default:
> +            g_assert_not_reached ();
>              break;
>      }
>
> @@ -551,12 +575,40 @@ next_step:
>  }
>
>  static void
> +csim_lock_ready (MMBaseModem              *self,
> +                 GAsyncResult             *res,
> +                 LoadUnlockRetriesContext *ctx) {
> +    const gchar *response;
> +    GError      *error = NULL;
> +
> +    response = mm_base_modem_at_command_finish (self, res, &error);
> +    if (!response) {
> +        g_prefix_error (&error, "Couldn't lock SIM card: ");
> +        g_simple_async_result_take_error (ctx->result, error);
> +        load_unlock_retries_context_complete_and_free (ctx);
> +        return;
> +    }
> +
> +    ctx->step++;
> +    load_unlock_retries_step (ctx);
> +}
> +
> +static void
>  load_unlock_retries_step (LoadUnlockRetriesContext *ctx)  {
>      switch (ctx->step) {
>          case LOAD_UNLOCK_RETRIES_STEP_FIRST:
>              /* Fall back on next step */
>              ctx->step++;
> +        case LOAD_UNLOCK_RETRIES_STEP_LOCK:
> +            mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
> +                                      CSIM_LOCK_STR,
> +                                      CSIM_QUERY_TIMEOUT,
> +                                      FALSE,
> +                                      (GAsyncReadyCallback) csim_lock_ready,
> +                                      ctx);
> +            break;
>          case LOAD_UNLOCK_RETRIES_STEP_PIN:
>              mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
>                                        CSIM_QUERY_PIN_RETRIES_STR, @@ 
> -589,6 +641,14 @@ load_unlock_retries_step (LoadUnlockRetriesContext *ctx)
>                                        (GAsyncReadyCallback) csim_query_ready,
>                                        ctx);
>              break;
> +        case LOAD_UNLOCK_RETRIES_STEP_UNLOCK:
> +            mm_base_modem_at_command (MM_BASE_MODEM (ctx->self),
> +                                      CSIM_UNLOCK_STR,
> +                                      CSIM_QUERY_TIMEOUT,
> +                                      FALSE,
> +                                      (GAsyncReadyCallback) csim_unlock_ready,
> +                                      ctx);
> +            break;
>          case LOAD_UNLOCK_RETRIES_STEP_LAST:
>              if (ctx->succeded_requests == 0) {
>                  g_simple_async_result_set_error (ctx->result,
> --
> 2.12.0



--
Aleksander
https://aleksander.es