Hi Marcel,

On 17/04/2012 11:36, Marcel Holtmann wrote:
Hi Guillaume,

Some modems use SIM notification to check SIM state.
If not do some poll using atutil helper.
---
  drivers/atmodem/sim.c |   28 ++++++++++++++++++++++++++++
  1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index c51b1d4..269079a 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
        GAtChat *chat;
        unsigned int vendor;
        guint ready_id;
+       struct at_util_sim_state_query *sim_state_query;
  };

  static const char *crsm_prefix[] = { "+CRSM:", NULL };
@@ -972,6 +973,23 @@ static void at_epev_notify(GAtResult *result, gpointer 
user_data)
        sd->ready_id = 0;
  }

+static void sim_state_cb(gboolean present, gpointer user_data)
+{
+       struct cb_data *cbd = user_data;
+       struct sim_data *sd = cbd->user;
+       ofono_sim_lock_unlock_cb_t cb = cbd->cb;
+
+       at_util_sim_state_query_free(sd->sim_state_query);
+       sd->sim_state_query = NULL;
+
+       if (present == 1)
+               CALLBACK_WITH_SUCCESS(cb, cbd->data);
+       else
+               CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+       g_free(cbd);
+}
+
  static void at_pin_send_cb(gboolean ok, GAtResult *result,
                                gpointer user_data)
  {
@@ -1005,6 +1023,13 @@ static void at_pin_send_cb(gboolean ok, GAtResult 
*result,
                sd->ready_id = g_at_chat_register(sd->chat, "*EPEV",
                                                        at_epev_notify,
                                                        FALSE, cbd, g_free);
+               /*
+                * After pin is entered, SIM state is check by doing
+                * some polling if modem doesn't use notification.
+                */
+       default:
+               sd->sim_state_query = at_util_sim_state_query_new(sd->chat,
+                                               2, 20, sim_state_cb, cbd);
                return;
        }
this part just looks wrong. You are now also doing CPIN polling for the
Ericsson and ST-Ericsson cards.


Ok I can fix it easily.


@@ -1246,6 +1271,9 @@ static void at_sim_remove(struct ofono_sim *sim)
  {
        struct sim_data *sd = ofono_sim_get_data(sim);

+       /* Cleanup potential SIM state polling */
+       at_util_sim_state_query_free(sd->sim_state_query);
+
Who owns the cbd data in this case? Are we not leaking that here?


Yes, we are. I see 2 options here:
a- Tweaking current at_util_sim_state_query_new(GAtChat *chat,
                        guint interval, guint num_times,
                        at_util_sim_inserted_cb_t cb,
void *userdata, GDestroyNotify data_destroy); in adding a destroyer for user data

b- Implementing new gpointer at_util_sim_state_query_get_data(struct at_util_sim_state_query *req); and free user_data during remove

Or maybe you have another solution?

        ofono_sim_set_data(sim, NULL);

        g_at_chat_unref(sd->chat);

Kind regards,
Guillaume

_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono

Reply via email to